Ticket Hash: | e251a4f77f5dfb59fa3bc81fdf047296c6e170ce | |||
Title: | ItclMemberFunc flags confusion | |||
Status: | Closed | Type: | Code_Defect | |
Severity: | Critical | Priority: | Immediate | |
Subsystem: | Resolution: | Fixed | ||
Last Modified: | 2015-05-21 14:03:26 | |||
Version Found In: | trunk | |||
User Comments: | ||||
dgp added on 2015-05-08 12:31:52:
(text/x-fossil-plain)
In ItclClassBaseCmd(), the flags field of an ItclMemberFunc is tested for whether the flag ITCL_IMPLEMENT_NONE is set. http://core.tcl.tk/itcl/info/07f70b5b42?ln=779 Trouble is that this flag value is only ever set in the flags field of an ItclMemberCode struct, *not* an ItclMemberFunc struct. This turns the conditional test into one that is always true, which appears to be against the coded intent. Suspect that what is meant here is to test this instead: if (!(imPtr->codePtr->flags & ITCL_IMPLEMENT_NONE)) { so that an additional dereference gets us a struct of the right type. Note that would be better written with use of the macro: http://core.tcl.tk/itcl/info/a6c3f38589?ln=779 which improves the clarity. So why bother with a ticket? Because making this "obvious" fix produces failures and segfaults in the test suite. dgp added on 2015-05-20 20:00:05: (text/x-fossil-plain) The segfault comes from something apparently totally broken. the "functions" hash table stores values of (ItclMemberFunc *). However the routine ItclBiObjectUnknownCmd can pull values from that hash table, and then start acting on them as if they were (ItclDelegatedFunction *) values. The two structs do not share a common structure. Hilarity ensues. It seems that without my flags experimenting the test suite just doesn't cover cases broken by this error. dgp added on 2015-05-21 14:03:26: (text/x-fossil-plain) Appears the attempt to keep unimplemented methods secret from the TclOO machinery was just the wrong idea to begin with, so the buggy failure to do so was accidentally making things work. And masking the broken bit of the unknown method handler, that is now fixed. |