Tcl Source Code

View Ticket
Login
Ticket UUID: 1290457
Title: TEOVI refactoring
Type: Patch Version: None
Submitter: tallniel Created on: 2005-09-13 21:06:36
Subsystem: 45. Parsing and Eval Assigned To: tallniel
Priority: 5 Medium Severity:
Status: Closed Last Modified: 2009-03-03 00:28:58
Resolution: Out of Date Closed By: tallniel
    Closed on: 2009-03-02 17:28:58
Description:
This patch refactors TclEvalObjvInternal into a bunch
of distinct functions to handle command lookup, enter
traces, exit traces, unknown commands, and actual
invocation. I think the resultant code is clearer and
easier to maintain. Passes the test-suite, and should
behave identically to the previous code.
User Comments: tallniel added on 2009-03-03 00:28:58:

allow_comments - 1

I believe the NRE changes make this patch obsolete now.

tallniel added on 2006-07-08 21:42:37:

File Added - 184266: teovi-08July06.patch

Logged In: YES 
user_id=102050

I've added a new patch (teovi-08July06.patch) which converts
TclObjInvoke, InvokeImportedCmd and TclInvokeObjectCommand
(needed?) to use the new TclInvokeCommand. This passes the
test-suite and passes the original code for bug 582506.

I put the declaration for TclInvokeCommand in tclInt.h for now.

dgp added on 2006-07-07 00:12:42:
Logged In: YES 
user_id=80530


What's the right way for
TclObjInvoke() to use the
new routines instead of
calling cmdPtr->objProc
directly?  Can an updated
patch make that change?

Likewise for InvokeImportedCmd()
in tclNamespace.c

tallniel added on 2006-06-30 17:47:31:

File Added - 183405: teovi-29Jun06.patch

Logged In: YES 
user_id=102050

Uploading newer patch:

The new patch refactors things again to create a new
TclInvokeCommand function that can be used in place of the
various places where the core directly calls a
cmdPtr->objProc (e.g. 582506). The new function is not
declared "static", but I've not put a declaration in any
header file as I wasn't sure which would be appropriate
(tclCompile.h?). For instance, to fix 582506 you can change
InvokeImportedCmd (in tclNamesp.c) from:

    return (*realCmdPtr->objProc)(realCmdPtr->objClientData,
interp,
        objc, objv);

to:

    return TclInvokeCommand(interp, objc, objv, realCmdPtr, 0);

(Actually, this only fixes the original problem of 582506 --
miguel's added comment is not fixed as that requires further
changes to how execution traces are stored/invoked).

I've also added a TODO comment in the source for
TclEvalObjvInternalUnknownHandler. In the original code this
calls Tcl_GetCommandFromObj to check if the handler exists
and then calls TEOVI to invoke it (which then calls T_GCFO
again). I think this can be replaced by a call to the new
TclInvokeCommand, which would avoid the extra call to T_GCFO
as we could just pass in the Command from the previous call.
However, the call to TEOVI also passes in the *original*
command+args string when calling the unknown handler. I'm
not sure if this is ever actually used (it is passed to
further execution trace handlers, but I haven't dug
further), so I'm not sure if this replacement can be made
safely.

The new patch passes the test-suite, *EXCEPT* it causes a
segfault in one test in stack.test on my system, but only
when symbols are *disabled*. The segfault is from blowing
the C stack, and is obviously caused by the extra function
call added to the stack (TEOVI -> TEOVIInvokeCommand).
Adding an "inline" keyword to TEOVIIC fixes this for me, but
is C99/GCC-specific. Alternatives would be either to inline
this function manually again (still leaves some refactoring,
which may be worth it), or a further refactoring. The
further refactoring would split into before/after functions
for taking care of traces etc, but leave the actual calling
of cmdPtr->objProc to the caller (TEOVI or
TclInvokeCommand). I can produce a further patch for this if
you want.

dgp added on 2006-06-14 23:09:59:
Logged In: YES 
user_id=80530


This patch may contain the
seeds for a fix for 582506.

tallniel added on 2006-06-14 02:37:08:

File Added - 181538: teovi-13Jun06.patch

tallniel added on 2006-06-14 02:37:07:
Logged In: YES 
user_id=102050

Updated patch for latest CVS HEAD (*not* dgp-refactor).

tallniel added on 2005-09-14 04:40:00:

File Added - 148982: teovi-dgp-refactor.patch

tallniel added on 2005-09-14 04:06:38:

File Added - 148976: teovi.patch

Attachments: