Tcl Source Code

View Ticket
Login
Ticket UUID: 2969488
Title: trace variable doesn't fire if you upvar to an array element
Type: Bug Version: obsolete: 8.6b1.1
Submitter: twylite Created on: 2010-03-12 16:04:58
Subsystem: 07. Variables Assigned To: ferrieux
Priority: 5 Medium Severity: Minor
Status: Open Last Modified: 2022-08-26 18:59:29
Resolution: None Closed By: nobody
    Closed on:
Description:
(This is FR 572889 from June 2002 converted to a bug)

Tcl Versions: all 8.4 and onwards; confirmed on 8.4.11 and 8.6b1.1
OS platform and version: probably all, confirmed on Windows XP + SP3

Problem behaviour:
If you set a trace on an array, then link to a specific element of that array using [upvar], any changes made via the linked variable will not cause traces to fire.

Expected behaviour:
If a trace has been placed on a variable, any access to that variable (even indirectly) should invoke the trace.

Additional detail / Use case
I have a "fileentry" megawidget that combines an entry, a button with an icon, and a tk_getOpenFile / tk_getSaveFile.  If the button is clicked the appropriate file dialog is displayed, and the selected filename written back into the -textvariable.  The obvious implementation is to link to the -textvariable using [upvar].  If the -textvariable is an array element then traces on the element will not fire.  In this case I am using variable traces to maintain consistency between various UI fields, so I lose consistency if the trace doesn't fire.

Workarounds
(1) Change the UI to use a scalar rather than an array element for this field
(2) set and dereference the varname given by -textvariable directly rather than using [upvar]
(3) have the megawidget use the entry's get/insert/delete methods rather than accessing -textvariable.

Code sample
---
proc tracer {args} { puts "<trace> $args" }
set ui(field1) "alpha"
trace add variable ui write "tracer write"
trace add variable ui array "tracer array"

set ui(field1) "beta" ;# trace write fires
array set ui {field1 "gamma"} ;# trace write & array both fire

proc alter {varname} {
  upvar #0 $varname v
  set v "delta"
}

alter ui(field1) ;# trace doesn't fire
---
User Comments: chw added on 2022-08-26 18:59:29: (text/x-fossil-plain)
Additionally to TIP#634/635 I propose to make a function similar to

 Var *TclObjLookupVar(Tcl_Interp *interp,
                      Tcl_Obj *part1Ptr, const char *part2,
                      int flags, const char *msg,
                      const int createPart1, const int createPart2,
                      Var **arrayPtrPtr);

a public (aka tclDecls.h) function in order to enable extension
writers to deal with upvar'ed array elements as you did in tclEnv.c.
The sole objective is to be able to find out the real names of the
variable referenced in the trace callback.

chw added on 2022-08-26 18:42:15: (text/x-fossil-plain)
Schelte, please consider the changes in

 https://www.androwish.org/home/info/1982e49611f2a9f3

which try to fix the case of an unknown array element
tracked by a trace on an entire array. In the current
implementation, this does not fire the trace, which
makes ::env inconsistent.

sbron added on 2022-08-25 08:13:20: (text/x-fossil-wiki)
The fact that an [upvar] to an array element won't trigger a trace on the entire array is a documented feature (in the upvar manual page). So, in theory, people may use it on purpose to change an array without firing the trace. This means that this is a breaking change. Those can only go into 9.0.

chw added on 2022-08-25 07:53:58:
Schelte, since your patch can be applied to core-8-branch as well,
why not target TIP#634 for Tcl 8.7? Yet should it be feasible to
backport it for 8.6.

oehhar added on 2022-08-24 12:23:06: (text/x-fossil-wiki)
Schelte,
perhaps, you may ask for commit right to TCL core & friends and provide it in a branch ?

Thanks,
Harald

sbron added on 2022-08-24 12:17:54: (text/x-fossil-wiki)
I attached an updated version of ferrieux' patch that will apply against Tcl 9.0a4. This also addresses the failing tests in trace-16.* and ::env traces.

Tests for the new functionality are included.

oehhar added on 2022-08-18 07:55:17: (text/x-fossil-wiki)
Schelte,

thanks for the great effort !

Maybe, you may drive the process:

   *   Provide the patch in a branch and add tests
   *   If it is a general discussion, perhaps write a TIP for 9.0

Take care,
Harald

sbron added on 2022-08-17 11:06:39: (text/x-fossil-wiki)
I just ran into this problem again today. Considering the multiple bug reports of similar issues over the last 20 years, I'm clearly not the only one.

Tcl 9.0 seems like the perfect opportunity to rectify the situation. Can the fix attached to this ticket please make it into 9.0?

ferrieux added on 2010-03-13 07:18:30:

File Added - 366551: upvartraces.patch

ferrieux added on 2010-03-13 07:15:08:
Attaching a patch doing 99% of the job :/
(The 1% is failing tests in trace-16.* by giving a misleading error message "no such variable" vs "no such element in array". Will look into this later.)

Method:
 (1) Enrich TclVarHashTable with an arrayPtr field, backlinking to the parent array when it exists, NULL otherwise.
 (2) Enrich the various TclIsVarDirectXXXable with proper tests on the parent array
 (3) At the beginning of the TclPtr(Set,Get,Incr,Unset) functions, when the arrayPtr passed is NULL, check again, and overwrite it with the parent array from the table.

As a result, array elements now know their parent, *and* the superficial arrayPtr estimated from the var's syntax (an upvar may look like a scalar) is no longer  enough to hide the truth.

Doc update is just the removal of that ugly paragraph describing the old behavior.

Tests need a specific case checking for this (apparently no one bothered to test that ugly documented behavior !)

dgp added on 2010-03-13 01:08:52:
In the realm of workarounds, see also
what the tcltest package does to work around
the problem (search for 572889).

ferrieux added on 2010-03-13 00:22:21:
Hmm, I think it's not as unsolvable as it seems.
The test TclIsVarDirectWritable(varPtr) is currently:

    !((varPtr)->flags & (VAR_ARRAY|VAR_LINK|VAR_TRACED_WRITE|VAR_DEAD_HASH))

and in this case it returns 'true' because the linked array element's flags are only

   VAR_ARRAY_ELEMENT | VAR_IN_HASHTABLE

Now, I believe we can still reach the containing array with moderate additional complexity , because the  VAR_IN_HASHTABLE shows that the (Var *) is in fact a (VarInHash *), whose entry->tablePtr is the array's backing hashtable. Dunno exactly how easily we could climb back from hashtable to array, but we could even add a field in the hashtable struct for that. The important thing is that we don't need a per-element field.

Miguel, do you agree with this view ?

msofer added on 2010-03-12 23:46:05:
IIRC, the "unsolvable problem" (without some redesign) is that an array element does not know to which array it belongs. If it is accessed as a scalar, that info is just not available. Will look again.

twylite added on 2010-03-12 23:35:55:
I don't know what the impact is on ::env.  IMHO ::env should be changed to a command [env] or similar and not managed as a variable, but that's a different discussion.

If there are conditions under which a traced variable can change without the trace being invoked the whole value of trace is undermined.  I can't rely on it, and must change the application's design not use this behaviour.

I can use a number of useful tricks if my UI widgets link to elements in an array (e.g. setting up defaults, loading/saving records, reverting changes all become trivial).  With certain UIs I may have to rethink that strategy as I cannot use [trace] to protect the consistency of the variables or the UI as a whole.

dkf added on 2010-03-12 23:20:31:
This has an impact on ::env traces, though things are even worse there.

Attachments: