Tcl Source Code

The NRE commit causes two new errors in the testsuite. They seem to be actually the same error due to a change in the output of 'testcmdtrace tracetest'. It may be a bug, it may be an improvement ...

==== parse-8.5 Tcl_EvalObjv procedure, command traces FAILED
==== Contents of test case:

    testevalobjv 0 set x 123
    testcmdtrace tracetest {testevalobjv 0 set x $x}

---- Result was:
{testevalobjv 0 set x $x} {testevalobjv 0 set x 123} {testevalobjv 0 set x $x} {set x 123}
---- Result should have been (exact matching):
{testevalobjv 0 set x $x} {testevalobjv 0 set x 123} {set x 123} {set x 123}
==== parse-8.5 FAILED

==== trace-29.2 Tcl_CreateTrace, correct command and argc/argv arguments of trace proc FAILED
==== Contents of test case:

    testcmdtrace tracetest {set stuff [info tclversion]}

---- Result was:
{info tclversion} {info tclversion} {info tclversion} {::tcl::info::tclversion} {set stuff [info tclversion]} {set stuff 8.6}
---- Result should have been (exact matching):
{info tclversion} {info tclversion} ::tcl::info::tclversion {::tcl::info::tclversion} {set stuff [info tclversion]} {set stuff 8.6}
==== trace-29.2 FAILED

Logged In: YES 
user_id=1312539
Originator: NO

This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).

Logged In: YES 
user_id=80530
Originator: NO


I confirm it's effective at making
the tests pass.  I confirm that
all the cases I know about involve
getting the command string from 
the objv and ignoring the other
"out of date/scope/whatever" values.

I can't confirm that all the new
cases that might exist with the
new Tcl_NR* routines an extension
may (soon) call are covered.  Trusting
miguel on that.

Related to this bug is a need to
at least briefly review what
changes to Tcl_CreateTrace() might
be appropriate to add to TIP 322.

Tcl_CreateTrace explicitly traces
command procedure invocation.  Now
that the Tcl_NR* interfaces permit
other command-procedure-like things
to be invoked too, it's worth asking
whether Tcl_CreateTrace function should
extend to tracing them too.

Or possibly I just don't grok NRE yet.

File Added - 285588: 2017146.patch

Logged In: YES 
user_id=148712
Originator: YES

The attached patch solves these two issues and has been committed. Leaving open for review if necessary, afaic the bug is solved.
File Added: 2017146.patch

Logged In: YES 
user_id=148712
Originator: YES

(Rough idea, so that I do not forget)
If GetCommandSource would detect when that happened, would it generating the string from objc/objv make things right again?

I think it might be possible to detect that situation if pushing a cmdFrame would record eg the numLevels when it happened. Or it might be possible to detect the situation from the TEOV_record stack (nre-specific)?

Logged In: YES 
user_id=80530
Originator: NO

dgpthese failures are just indicators of a deeper problem
dgpWhen the CmdFrame gets re-used for a new command....
dgp...but the tip 280 data stored in the CmdFrame is left unmodified
dgpThis is going to bite any Tcl_ObjCmdProc that calls Tcl_EvalObjv()

Logged In: YES 
user_id=80530
Originator: NO


In these failing test scenarios, it seems
that GetCommandSource() has until now not
been able to pass in the pre-substitution
command string.  It had to fallback on
generating a command string as a list
constructed from the objv[] array.

On the HEAD, in these scenarios, something
claiming to be the pre-substitution command
string is now getting pulled from the
iPtr->cmdFramePtr struct.  In theory, this
should be better, so long as the value
stored in that struct is actually correct.

Logged In: YES 
user_id=80530
Originator: NO


...which appears to trace back to
changes to GetCommandSource()
in tclBasic.c

Logged In: YES 
user_id=80530
Originator: NO


The failing tests indicate a difference in
the values of "command" passed to
TclCheckInterpTraces().

Logged In: YES 
user_id=148712
Originator: YES

Don: do you have an opinion on this?