Ticket UUID: | 707104 | |||
Title: | [interp alias] overwrites [rename]d alias | |||
Type: | Bug | Version: | obsolete: 8.4.2 | |
Submitter: | vincentdarley | Created on: | 2003-03-20 20:05:39 | |
Subsystem: | 20. [interp] | Assigned To: | msofer | |
Priority: | 8 | Severity: | ||
Status: | Closed | Last Modified: | 2004-09-15 00:46:31 | |
Resolution: | Fixed | Closed By: | msofer | |
Closed on: | 2004-09-14 17:46:31 | |||
Description: |
I see some very peculiar behaviour with aliased commands, which is at the very least not documented, and possible a bug: % proc foo {} { puts "orig" } % interp alias {} bar {} foo % bar orig % rename bar bar2 % bar2 orig % bar orig # (So now both 'bar' and 'bar2' are aliases, i.e. 'rename' actually copies the alias rather than renaming it) % proc foo2 {} { puts "new" } % interp alias {} bar {} foo2 % bar new % bar2 invalid command name "bar2" # (So the act of creating a new alias "bar" deletes both the old aliases "bar" and "bar2") | |||
User Comments: |
msofer added on 2004-09-15 00:46:31:
Logged In: YES user_id=148712 Committed to HEAD; not sure about backporting - decided not to unless someone complains, putting this ticket on "Pending". dgp added on 2004-09-13 22:07:44: Logged In: YES user_id=80530 modified patch looks good to me. msofer added on 2004-09-12 09:05:27: File Added - 101184: 707104.patch2 msofer added on 2004-09-12 09:05:26: Logged In: YES user_id=148712 I now got on friendlier terms with the patch, after getting a clue ;) I attach a modified patch: it changes variable names to underline the distinction between the token and the command name, corrects a couple of tests and modifies doc/interp.n This patch also fixes Bug 1026493. Don: could you please review this? msofer added on 2004-09-12 04:11:52: Logged In: YES user_id=148712 Just to record dgp's clarification in the chat (thx!): set token [interp alias slave alias {} target] slave eval $script interp alias slave $token {} without a token, that operation can't work reliably msofer added on 2004-09-11 22:06:07: Logged In: YES user_id=148712 I really do not like this patch, possibly because I am not yet sure that I understand all the reqs. It fixes this particular bug, but renders introspection even more buggy than what it already was. * interp-19.3: the new behaviour seems correct interp create a interp alias a foo a bar interp eval a {rename foo zop} interp alias a foo a zop catch {interp eval a foo} msg interp delete a set msg should indeed return {invalid command name "bar"} IMHO. In other words,I do expect that we have a redirection foo->zop->bar * interp-19.6: the new behaviour makes no sense to me, I would expect that interp alias a foo a bar interp eval a {rename foo zop} interp alias a foo a zop interp aliases a returns {foo zop}: foo is an alias for zop, zop is an alias for bar. The new return {::foo foo} is weird, and not helpful at all. Alias introspection is broken wrt to renaming. The patch fails to fix this, and introduces a new breakage IMO. OTOH, I confess that I seem to find the whole alias introspection rather unsatisfactory: why would I be interested in the *creation* name of an alias as opposed to its current name, as documented to be introspectable by [interp alias]? Looks to me like we turned some bugs into features. It seems to be related to the general confusion (in the docs, and parts of the code) between commands and their names. I need to think some more about all this - looks like a more fundamental redesign may be in order. dgp added on 2004-09-11 03:50:59: File Added - 101071: 707104.patch dgp added on 2004-09-11 03:50:58: Logged In: YES user_id=80530 Here's a patch to try. Causes tests interp-19.3,6 to fail, but they're testing for continued existence of this particular (mis-)feature. dgp added on 2004-09-10 06:18:54: Logged In: YES user_id=80530 made summary more precise. Note that the flaw in adopting "current names" everywhere is that it leave the original creator no way to delete its creation. That capability requires some kind of stable "token" value such as now provided. This may be a feature that has no real value, but it was the intent behind the existing code as evidences by several comments in the source code. dgp added on 2004-09-10 01:21:40: Logged In: YES user_id=80530 Note an error in the original report. It purports to demonstrate that at one point both "bar" and "bar2" are active as the same alias. This is false. "bar2" is an alias, and "bar" is auto-expanded to "bar2" because the submitted demo is in an interactive tclsh. At any given time, any given alias has only one name. It is true, though, that aliases remember their original name, and get destroyed when that original name is re-used as another alias. Given that, it appears that a re-engineering of aliases so that they do not remember their original names, but [interp aliases] reports their current names should be possible. It seems that the code that implements [namespace ensemble] maps should be close to the right thing, needing only the added ability of command rewriting to go into another interp. vincentdarley added on 2003-10-06 16:33:05: Logged In: YES user_id=32170 Any chance of doing something about this in 8.4.5? dgp added on 2003-03-27 01:26:58: Logged In: YES user_id=80530 One thing to consider if we revise what gets returned by [interp aliases]: over what lifetime should the caller trust the result? The current implementation, nasty as it is, lets one store the [interp aliases] result, then iterate over it later, even after [rename]s. If we change [interp aliases] to return the current names of aliases, then it should not be stored for later use. Analogies with [info procs] suggest that storing for later use is a bogus idea anyway, but it is something we would lose. Anyone have actual use cases here? vincentdarley added on 2003-03-21 17:29:58: Logged In: YES user_id=32170 Yup, I'm now using this, which copes with command traces too: proc hook::_safeRename {origName newName} { set alias [interp alias {} $origName] if {[string length $alias]} { set trace [trace info command $origName] rename $origName $newName # Must re-create the alias, and the trace interp alias {} $newName {} $alias foreach t $trace { eval [list [list trace add command $newName]] $t } } else { rename $origName $newName } } I'm glad Donal agrees --- using command traces on aliases with this limitation is pretty hopeless (fortunately there seems to be the above workaround, but I've not used it enough to know whether I'll see further problems too). Since there seems to be some consensus, I'm moving the priority back to normal. beric added on 2003-03-21 16:56:55: Logged In: YES user_id=493507 Does this help ? It takes care of renaming an alias (restricted in one interp) proc rename_alias {oldCmd newCmd} { set alias [interp alias {} $oldCmd] if {[llength $alias]} { if {$newCmd ne ""} { eval [concat [list interp alias {} $newCmd {}] $alias] } interp alias {} $oldCmd {} } else { rename $oldCmd $newCmd } } dkf added on 2003-03-21 16:45:39: Logged In: YES user_id=79902 I agree with Vince here, especially now we've got command traces. vincentdarley added on 2003-03-21 15:48:24: Logged In: YES user_id=32170 The 'rename' man page needs updating then to reflect this limitation. It's a shame that aliases don't work in this way, since they are otherwise a very powerful part of Tcl. However, they completely break the semantics of a Tcl command, which is bad! Is there any reason 'interp aliases' can't return the *current* name of the alias, which users can quite happily iterate over. I wouldn't even see this as a back-compatibility issue, but as a bug fix. After all: proc foo {} {} rename foo bar info procs foo # should obviously not return 'foo' So, similarly interp alias {} foo {} implementation rename foo bar interp aliases # should not return foo either msofer added on 2003-03-21 03:55:07: Logged In: YES user_id=148712 As noted in http://groups.google.com/groups?hl=en&safe=off&th=1784032ddc241340%2C4&seekm=8mf2uq%24jkq%241%40bob.news.rcn.net this behaviour is ancient (since 7.6). Don's summary of the issue (from the chat): The issue is the [interp aliases] introspection. It always returns the original names of the aliases. So even if there is a [rename], the alias still keeps a connection to its original name, so it can be reached by folks iterating of [interp aliases]. This was a bad choice. If we need a permanent name for [interp aliases] to return, we should have a special token just for that purpose. For now, the practical advice is "you can't [rename] an alias" Low priority to signify "not much can be done about it for now". msofer added on 2003-03-21 03:26:28: Logged In: YES user_id=148712 Yup, this is weird ... It is not a new issue in 8.4 (pheww!), it fails already in 8.3.4 % info patch 8.3.4 % proc foo {} { puts "orig" } % interp alias {} bar {} foo bar % bar orig % rename bar bar2 % bar orig % bar2 orig % rename bar {} can't delete "bar": command doesn't exist % bar orig % bar2 orig % rename bar2 {} % bar invalid command name "bar" % bar2 invalid command name "bar2" dgp added on 2003-03-21 03:26:07: Logged In: YES user_id=80530 This is a known issue, and can't really be corrected unless/until [interp aliases] is changed to return a token. See FR 524140. |