#439 TEOVI refactoring

closed-out-of-date
5
2009-03-02
2005-09-13
Neil Madden
No

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.

Discussion

  • Neil Madden

    Neil Madden - 2005-09-13

    Patch to tclBasic.c to refactor TEOVI

     
  • Neil Madden

    Neil Madden - 2005-09-13
    • assigned_to: msofer --> dgp
     
  • Neil Madden

    Neil Madden - 2005-09-13

    Same patch updated for dgp-refactor branch

     
  • Neil Madden

    Neil Madden - 2006-06-13

    Logged In: YES
    user_id=102050

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

     
  • Neil Madden

    Neil Madden - 2006-06-13

    Updated version of teovi.patch

     
  • Don Porter

    Don Porter - 2006-06-14

    Logged In: YES
    user_id=80530

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

     
  • Neil Madden

    Neil Madden - 2006-06-30

    Updated patch, added TclInvokeCommand fn

     
  • Neil Madden

    Neil Madden - 2006-06-30

    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.

     
  • Don Porter

    Don Porter - 2006-07-06

    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

     
  • Don Porter

    Don Porter - 2006-07-06
    • assigned_to: dgp --> tallniel
     
  • Neil Madden

    Neil Madden - 2006-07-08

    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.

     
  • Neil Madden

    Neil Madden - 2006-07-08

    Updated patch converting TclObjInvoke etc to use TclInvokeCommand

     
  • Neil Madden

    Neil Madden - 2009-03-02
    • status: open --> closed-out-of-date
     
  • Neil Madden

    Neil Madden - 2009-03-02

    I believe the NRE changes make this patch obsolete now.

     

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

JavaScript is required for this form.





No, thanks