| Ticket UUID: | 489537 | ||
| Title: | string rep of list: should quote '#' | ||
| Type: | Support | Created on: | 2001-12-05 20:56:52 |
| Submitter: | dgp | Assigned to: | nobody |
| Subsystem: | None | Severity: | |
| Priority: | 4 | Last modified: | 2009-07-29 19:51:30 |
| Status: | Closed | Closed by: | dgp |
| Resolution: | Closed on: | 2003-09-04 16:43:41 | |
| Version: | None | ||
| Description: | ||||
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.
| ||||
| User Comments: | ||||
dgp added on 2001-12-06 03:56:54:
File Added - 14214: liststring.patch hobbs added on 2001-12-06 04:10:51:
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. dgp added on 2001-12-06 04:13:04:
Logged In: YES user_id=80530 Example? hobbs added on 2001-12-06 05:07:59:
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?
dgp added on 2001-12-06 05:21:22:
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.
dkf added on 2001-12-06 17:00:37:
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. dgp added on 2001-12-08 00:05:26:
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.
msofer added on 2002-03-01 00:50:44:
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. dgp added on 2002-03-01 02:28:53:
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. msofer added on 2002-03-01 02:42:17:
File Deleted - 18577: File Added - 18583: liststring2.patch Logged In: YES user_id=148712 Adding a comment to the patch (thanks don!) msofer added on 2002-03-01 02:46:40:
File Deleted - 18583: File Added - 18584: liststring2.patch hobbs added on 2002-03-01 02:52:44:
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. dgp added on 2003-03-05 13:41:57:
Logged In: YES user_id=80530 opinion on making this fix in 8.5a0 ? hobbs added on 2003-03-06 00:40:46:
Logged In: YES user_id=72656 I think it requires a TIP. dgp added on 2003-07-23 00:00:33:
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.
dgp added on 2003-07-23 04:24:02:
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. dgp added on 2003-07-24 07:35:34:
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.
dgp added on 2003-07-24 07:35:35:
File Deleted - 56550: File Added - 56681: 489537.patch dkf added on 2003-07-24 21:12:33:
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.
dgp added on 2003-07-24 22:59:59:
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... dgp added on 2003-07-24 23:00:00:
File Deleted - 56681: File Added - 56746: 489537-fix.patch dgp added on 2003-07-24 23:01:26:
Logged In: YES user_id=80530 ...and here's the test patch, that I am committing to HEAD. dgp added on 2003-07-24 23:01:27:
File Added - 56747: 489537-test.patch dgp added on 2003-08-09 02:57:59:
File Added - 58219: 489537-fix.patch Logged In: YES user_id=80530 Updated fix patch that implements draft TIP. dgp added on 2003-08-09 02:58:00:
File Deleted - 56746: dgp added on 2003-08-10 05:44:18:
Logged In: YES user_id=80530 The attached file 489537-fix.patch is the prototype implementation of TIP 148. dgp added on 2003-09-04 23:43:41:
Logged In: YES user_id=80530 TIP 148 Accepted. Patch committed to HEAD (8.5a0). dkf added on 2008-11-21 21:26:58:
data_type - 210894 dkf added on 2009-07-29 19:51:30:
IP - Comment Removed: 130.88.1.31 | ||||
Attachments:
- 489537-fix.patch [download] added by dgp on 2003-08-09 02:57:59. [details]
- 489537-test.patch [download] added by dgp on 2003-07-24 23:01:26. [details]
- liststring2.patch [download] added by msofer on 2002-03-01 02:46:40. [details]
- liststring.patch [download] added by dgp on 2001-12-06 03:56:53. [details]
