Tcl Source Code

Further along the lines of stub cleanup:

I see no good reason why the various stubs tables should not be 'static const' in both tcl and tk.

'static' because the only access outside of *StubInit.c that we need is a pointer to the main stub table for the Tcl_PkgProvideEx call (and for the interp init in the case of tcl); that pointer can be exported separately, no need for all the stub tables to be global because of that.

'const' because there is no need for the stubs tables to be modifiable, they can be in read-only memory.
The only user of writable stubs tables is the gross Tcl_SetNotifier hack that relies on overwriting the notifier proc stub entries.
As we cannot guarantee that the notifier procs are only called via stubs anyway, the builtin notifiers already all have to check whether they been hooked by comparing their stub entry against the default, so we might as well do away with the stubs table modification and check for hooked notifier via a global set by Tcl_SetNotifier.

Please review the attached patches that make those changes.

File Deleted - 273841:

File Added - 274796: tk-conststubs.diff

Logged In: YES 
user_id=90580
Originator: YES

attached updated patch for tk const stubs change & committed to HEAD
File Added: tk-conststubs.diff

File Deleted - 273840:

File Deleted - 274792: 



File Added - 274795: tcl-conststubs.diff

Logged In: YES 
user_id=90580
Originator: YES

attached updated patch for tcl const stubs change & committed to HEAD
File Added: tcl-conststubs.diff

File Added - 274793: tcl-setnotifier.diff

Logged In: YES 
user_id=90580
Originator: YES

attached separate patch for the Tcl_SetNotifier change & committed to HEAD
File Added: tcl-setnotifier.diff

File Added - 274792: tcl-setnotifier.diff

Logged In: YES 
user_id=90580
Originator: YES

ok, will commit tomorrow unless I hear objections.

any suggestions on the naming of the new MODULE_SCOPE pointers to the static stubs tables? I don't particularly like the current choices but couldn't come up with anything better:

    MODULE_SCOPE const TclStubs * const tclConstStubsPtr;
    MODULE_SCOPE const TclTomMathStubs * const tclTomMathConstStubsPtr;
    MODULE_SCOPE const TkStubs * const tkConstStubsPtr;

cannot use tclStubsPtr et al obviously (already in use by *StubLib.c)

Logged In: YES 
user_id=80530
Originator: NO


ok, I think that reduces my concerns to
just those about breaking stuff, and the
best way to discover whether the change
breaks stuff is to make it and see who
complains.

Logged In: YES 
user_id=90580
Originator: YES

note that the extra Tcl_SetNotifier hooks and the buggy checks for them were introduced by the macosx branch merge and were probably never used by anything besides the old TkAqua notifier:
    http://fisheye.categorifiedcoder.info/browse/Tcl/tcl/generic/tclNotify.c?r1=1.8&r2=1.9
    http://fisheye.categorifiedcoder.info/changelog/Tcl?cs=MAIN:das:20020831060945

Logged In: YES 
user_id=90580
Originator: YES

The notifier part of this patch is independent of the stubs changes and should be applied regardless of the decision on const stubs as the current code is buggy IMO.

There should be no change in functionality for the notifier, whether hooked or not, the main difference after the patch is that in a few cases the check for hooks has moved from before the call to a notifier proc to inside the call, which will lead to an extra function call when the notifier is hooked. Only doing the check before the call was arguably a bug anyway as it was not going to catch all cases of calls to the function...

Tcl_SetNotifier as a whole is a bit of a hack anyway, it is really only safe to use before Tcl is initialized, TkAqua was using it at Tk load time before I implemented the CoreFoundation notifier and had to call the internals TclFinalizeNotifier() and TclInitNotifier() to get things to work (and even then starting and immediately killing the threading unix notifier never worked all that well):
    http://fisheye.categorifiedcoder.info/changelog/Tk?cs=MAIN:das:20050514204814

As far as const stubs go, IMO the only way a modifiable stubs table could ever become reliably useful would be if we could assure that all calls from inside tcl (resp tk) as well as through the normal shared library function calls go through the stubs table (if this work is ever done inside the core, the stubs table can easily be made non-const again at that time...)
Given that this is not the case now, writable stubs tables only give the incorrect impression that modifying them might be a useful thing to do.

IMO now is an opportune time to make this change, alongside the other recent stubs changes (in particular stubsPtrs constification), if it causes any problems we will hopefully find out before 8.6.0 final...

Logged In: YES 
user_id=80530
Originator: NO


Before taking such a step, I need review of
the impact by somebody who has a clue just
what the Notifier is doing.

Although I don't have a reason to encourage
folks modifying the stubs table, I'm also
a bit shy about taking that possibility away.
Concerned in part about what folks might
already be doing, and in part about what
future development avenues such a change
might close off.

(I suppose with a read-only stubs table,
code could still make a copy of it and
modify the copy)

Is there some way in which this change fixes
something that is broken, or is it entirely
a code hygiene matter?

Logged In: YES 
user_id=90580
Originator: YES

Alexandre,
the stubs mechanism is precisely equivalent to a shared library mechanism a la dlsym, except that "symbol lookup" does not rely on symbol names (nor OS functionality to resolve them) but on a fixed slot number in a table of function pointers. The compatibility guarantee is that the slot numbers do not change from release to release.

I am unaware of any statements that changing the entries in the stubs table at runtime is a supported mechanism to override tcl functions (and doing that would not work anyway in all cases, as mentioned not all functions calls go through the stubs tables, in particular calls from inside the core itself are just normal functions calls unaware of the stubs table...)

Logged In: YES 
user_id=496139
Originator: NO

Sorry if I'm only a distant observer, but my grasp of the Stubs idea was precisely to allow for a compatible "override-and-extend" of the vectored functions. If you make them const, don't they become strictly equivalent to vanilla global functions back-linkable through ld.so or dlsym() ?

File Added - 273841: tk-conststubs.diff

Logged In: YES 
user_id=90580
Originator: YES

File Added: tk-conststubs.diff

File Added - 273840: tcl-conststubs.diff