Tcl Source Code

View Ticket
Login
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.

Attachments: