Tcl Source Code

Kudos to Joe English for catching this one,
and Donal Fellows for analyzing the right
solution.

list(n) says:
     list ?arg arg ...?

Braces and backslashes get added as necessary,...
so that eval may be used to execute the resulting 
list, with arg1 comprising the command's name and
the other args comprising its arguments.

However, in an interacive tclsh:

% proc # {} {puts !}
% set cmd [list #]
#
% eval $cmd
%

A patch is attached that offers one fix with
changes to Tcl_{Scan|Convert}CountedElement.
It is overkill in that it quotes leading '#'
in all list elements when it is only needed
for the first list element, but in those
routines, the ordinal of the element is not
known.  Perhaps an additional flag value could
be added if more sophistication is wanted, limiting
the quoting to the first element.

Two part.test tests were updated, and tests
based on the demo above was added to util.test.

IP - Comment Removed: 130.88.1.31

data_type - 210894

Logged In: YES 
user_id=80530


TIP 148 Accepted.  Patch committed
to HEAD (8.5a0).

Logged In: YES 
user_id=80530


The attached file 489537-fix.patch
is the prototype implementation
of TIP 148.

File Deleted - 56746:

File Added - 58219: 489537-fix.patch

Logged In: YES 
user_id=80530


Updated fix patch that implements
draft TIP.

File Added - 56747: 489537-test.patch

Logged In: YES 
user_id=80530

...and here's the test patch,
that I am committing to HEAD.

File Deleted - 56681: 



File Added - 56746: 489537-fix.patch

Logged In: YES 
user_id=80530


updated to address those
comments.  Now in the form
of two patches, one that adds
tests to the test suite that demo
the bug.  And a second that
contains the fix.

Here's the fix patch...

Logged In: YES 
user_id=79902

I'd feel happier if the addition of TCL_DONT_QUOTE_HASH
didn't rearrange the positions of the internal
Tcl_ConvertElement flags.  No reason why that value has to
occupy a contiguous bit with TCL_DONT_USE_BRACES...

Apart from that, there ought to be an extra dict test like
this (or maybe with the result the other way round):
test dict-2.8 {dict create command - #-quoting in string
rep} {
    dict create # #comment ## ##comment
} {{#} #comment ## ##comment}

Minor issues aside, looks excellent to me.

File Deleted - 56550: 



File Added - 56681: 489537.patch

Logged In: YES 
user_id=80530


Here's a completely new patch
(replaces previous 489537.patch)
that I think is better than all.

1) Does not change the public API (no TIP needed!)
2) Does not disturb the Tk test suite.
3) By default, chooses safe quoting.
4) Leaves open door for a TIP exposing to
     extensions the power to ask for less 
     quoting of hashes when it is known safe.
5) Corrects several more callers of
    Tcl_Convert*Element()
6) Adds many more tests for bugs when 
     those callers are not patched.

Please review.

File Added - 56550: 489537.patch

Logged In: YES 
user_id=80530


New patch 489537.patch is
same as liststring2.patch
except it adds two more tests
to util.test that force string rep
generation to illustrate the
shimmering problem.

Logged In: YES 
user_id=80530


Because [eval] has pure-list optimizations,
this bug now appears as a shimmering
inconsistency.  Consider this demo script

# FILE: demo.tcl
set cmdName [lindex $argv 0]
proc $cmdName {} {puts Success!}
set command [list $cmdName]
puts "Eval \[$command]" ;# comment this out
eval $command

Correct behavior is for the script
to write Success! to stdout.  As written
above, this fails for an argument that
begins with the # character.   However,
if you comment out the indicated [puts],
then we avoid string-rep generation,
and the script works correctly for all
arguments.

Seems we should be able to correct
a shimmering inconsistency without
a TIP.  Anyone relying on the 
buggy behavior should have been
caught by the pure-list optimizations
in eval by now.

Logged In: YES 
user_id=72656

I think it requires a TIP.

Logged In: YES 
user_id=80530


opinion on making this fix in 8.5a0 ?

Logged In: YES 
user_id=72656

I would like to make sure that this is logically worked 
over before commiting.  It may be that the tests that would 
break don't exist, as it seems like a subtle change that 
might have ramifications.

File Deleted - 18583: 



File Added - 18584: liststring2.patch

File Deleted - 18577: 



File Added - 18583: liststring2.patch

Logged In: YES 
user_id=148712

Adding a comment to the patch (thanks don!)

Logged In: YES 
user_id=80530

The new patch works and passes both Tcl and Tk
test suites.  Haven't gone looking for ways to
break it yet.

File Added - 18577: liststring2.patch

Logged In: YES 
user_id=148712

A second patch: this will only quote '#' if it is the first
element of the list: A new flag TCL_QUOTE_HASH is added to
the interface of Tcl_Convert(Counted)Element.

Logged In: YES 
user_id=80530

If the current patch is accepted, many tests in the
Tk test suite will fail.  The first example
is test button-7.2.  It expects the result to be:

.b1 #543210 {} {}

but after the patch the result will actually be:

.b1 {#543210} {} {}

Possible remedies:

  1) Just update all the relevant tests to expect the new
     string rep.  (Create expected value with [list])
  2) Make a more sohpisticated patch that will only
     quote leading '#' in the first element of a list.
  3) Make these tests compare their results as lists,
     rather than as strings, adding a new type of
     comparison to tcltest if necessary.

Logged In: YES 
user_id=79902

This is a *vevry* long-standing bug, since it was there in
Tcl 7.4 (which is the oldest version I've got hanging about
here) and it definitely should be resolved the way described
as the alternative is to put a special case into the list(n)
manpage. Which would be deeply blecherous.

Logged In: YES 
user_id=80530

With the patch:

% list #foo a b c
{#foo} a b c

Does that answer the question?  No error thrown,
but yes the string rep changes. That's the point.

Each list has multiple valid string reps.  The
guarantees of list(n) are met by the string rep
after the patch, but not before.

Logged In: YES 
user_id=72656

The patch changes semantics to the following:
   [list #foo a b c] == {\#foo a b c}
by placing the \ in front of the #.  This means
that #foo will now be seen as a command, but when
it is not (in most cases # will be for comments),
then you will throw an error, no?

Logged In: YES 
user_id=80530

Example?

Logged In: YES 
user_id=72656

This is not correct for most cases, where the # is really 
meant to be a quote and has no command associated with it.  
This is only valid for the rare cases where the initial #-
starting element has a command of the same name.

File Added - 14214: liststring.patch