Ticket Hash: | 5ea6971709bc83c50a5edd1fd184769c958f8c17 | |||
Title: | FinalizeCreateObject WTF? | |||
Status: | Closed | Type: | Code_Defect | |
Severity: | Critical | Priority: | Immediate | |
Subsystem: | Resolution: | Fixed | ||
Last Modified: | 2015-05-29 18:13:00 | |||
Version Found In: | ||||
User Comments: | ||||
dgp added on 2015-05-22 17:00:28:
(text/x-fossil-plain)
FinalizeCreateObject converts all TCL_ERROR codes into reraised [return -level 2] which is implicitly [return -level 2 -code ok] which makes no sense at all. It goes to the trouble of calling Tcl_GetReturnOptions() then throws it away. It has some bizarre dance where the value of "result" is only captured from Tcl_SetReturnOptions() for certain kinds of classes. The whole exercise is just a giant red flag that YOU'RE DOING IT WRONG. Then up in clazzUnknownBody we have to [catch] the TCL_RETURN is order to properly convert it back to a TCL_ERROR. But the whole clazzUnknownBody invocation of handleClass is also broken -- at least in the case where we're using unknown in the TclOO style of reacting to an unknown method. We react to a missing method by creating a new instance of the class with the name of that method? WTF? Broken broken broken broken. dgp added on 2015-05-22 17:17:04: (text/x-fossil-plain) Commited to branch bug-5ea6971709 a disabling of this mess. Running the test suite with the change reveals only changes to stack trace details and error messages -- no functional difference at all. Whatever problem(s) were believed to be addressed by this ugliness are not demonstrated by the test suite. I think it's likely they do not exist. If they do exist, then for goodness sake, lets solve the real problems and not reach for this again. dgp added on 2015-05-26 12:16:07: (text/x-fossil-plain) A TclOO object is driven by an access command. Often we think that command "is" the object. The access command operates exclusively via subcommands, and the valid subcommands is determined by all the components in the definition of the object, its class, inheritance, etc. etc. $object $sub {*}$args A TclOO class, including oo::class, is a TclOO object, so it's driven by the same sort of command. It follows that instance creation takes on one of these forms: $class create $instanceName $class new In contrast, Itcl casts its instance creation commands in one of the forms: $iClass $instanceName $iClass #auto Since Itcl 4 is implemented so that an Itcl class is a TclOO class, this syntax mismatch has to be addressed. This is the reason the clazzUnknownBody does what it does. To take $iClass $instanceName as an encounter with an undefined method and convert it into instance generation. It's fine that this is done; the problem is that it is also done in other circumstances like my $unknownMethod The unknown body to accommodate Itcl instance creation syntax should be more selective in applying its adaptation. dgp added on 2015-05-26 18:23:26: (text/x-fossil-plain) Fix the "unknown" method of Itcl classes so that it is more selective, enabling the C++ declaration inspired syntax [$class $instance] without taking lots of missteps in the presence of unknown methods as well. Patch takes significant effort to preserve error messages, even when in my judgment they are sub-par, and better ones would be simpler, just for sake of compatibility. This patch leaves test import-2.5 failing due to another bug [f3a2e7407c]. dgp added on 2015-05-29 18:13:00: (text/x-fossil-plain) That other bug is now fixed enough to permit merge of this bug fix. |