Tcl Source Code

When the following script is evaluated by trunk build of tcltest, p1 produces the expected value, 2, whereas p2 produces an incorrect value, 1.

proc p1 {} {
	string length [testbytestring \xc0][testbytestring \x80]
}
proc p2 {} {
	string length [list [testbytestring \xc0]][testbytestring \x80]
}

puts [p1]
puts [p2]

For p1, TclStringCat() uses the tclStringType internal representation where \xc0 is treated as a single character and so is \x80, and therefore character concatenation is performed producing the correct result.

For p2, tclStringCat does not use tclStringType because it detects that there is another internal representation already in place. Instead, it simply concatenates the "bytes" fields of the values, and therefore byte concatenation is performed, so the two invalid byte sequences become a single valid byte sequence encoding one character.

If the body of p2 is evaluated directly instead of in the body of a procedure, the result is 2 instead of 1. This happens because there is no bytecode to execute, and instead TclEvalEx calls TclSubstTokens(), which does not call TclStringCat, but instead calls Tcl_AppendObjToObj, which replaces the list internal representation with a tclSringType internal representation, and eventually calls AppendUtfToUnicodeRep().

> Fixed in [b699959d62]. I looked into trying to make the ....

> Fixed in [2ddc0c3fa9] by allowing TclStringCat() to assume that all Tcl_Obj.bytes values can be safely catenated as-is.

Since TclStringCat() concatenates Unicode code-points - not bytes - this is a nun-bug. Your latest 'fix' was merged to the 'unchained' branch. Please keep it there and all is well.

I agree that Tcl should be able to retain efficient functions such as TclStringCat(). I think that in this case it is the test that is wrong, not TclStringCat(). Tcl never stores improperly-encoded data in Tcl_Obj.bytes, which is documented to contain data encoded in utf-8, with each nul character being represented in its overlong utf-8 form, \xC0\x80. Thus, a function like TclStringCat() should be able to assume that in the catenation of Tcl_Obj.bytes values there is no change to the interepretation of any byte in any of the catenated values.

TclStringCat was competently written with that in mind to achieve the following goals:

• Avoid shimmering & string rep generation.
• Produce pure bytearray when possible.
• Error on overflow.

In 2021 the commits [fd66e6e20e] and [a7df881994] modified TclStringCat() to force the generation of a tclStringType internal representation in order to avoid the effect of combining two otherwise unrelated incomplete byte sequences into one complete sequence. This change degraded TclStringCat() with respect to its stated goals. The current issue report could probably be resolved by further degrading TclStringCat() in a similar way, but that would move Tcl further in the wrong direction of supporting incomplete values in Tcl_Obj.bytes.

As I stated a couple of years ago in [7f1162a867], any third party that stores improperly-encoded data in Tcl_Obj.bytes is creating a garbage-in/garbage-out situation, and should not expect the results of any Tcl functions thereafter to make sense. Third parties must use Tcl_ExternalTo* functions to produce properly-encoded values. If a third party directly modifies Tcl_Obj.bytes such that the last byte sequence for an encoded code point isn't complete, then it should not expect Tcl to interpret that sequence as complete.

Either third parties pay a relatively small penalty of properly-encoding values at the boundary of Tcl, or Tcl pays a relatively large penalty throughout its internal routines, and at a cost of greater code complexity.

Fixed in [2ddc0c3fa9] by allowing TclStringCat() to assume that all Tcl_Obj.bytes values can be safely catenated as-is.

I share Ashok's concern. In addition, the build with TCL_UTF+MAX=3 was broken by this 'fix'. Therefore I moved it away from trunk.

% set b [testbigdata bytearray 100000]; string length $b
100000
% timerate -calibrate {}
0.03403821298619367 µs/#-overhead 0.038996 µs/# 50056827 # 25643866 #/sec
% timerate {string cat $b $b $b $b $b $b $b $b $b $b}
866.686 µs/# 1154 # 1153.8 #/sec 1000.156 net-ms
% set b [testbigdata string 100000]; string length $b
100000
% timerate {string cat $b $b $b $b $b $b $b $b $b $b}
730.523 µs/# 1369 # 1368.9 #/sec 1000.086 net-ms

% set b [testbigdata bytearray 100000]; string length $b
100000
% timerate -calibrate {}
0.0332670332449772 µs/#-overhead 0.036267 µs/# 53823298 # 27573410 #/sec
% timerate {string cat $b $b $b $b $b $b $b $b $b $b}
28.1215 µs/# 35519 # 35560.0 #/sec 998.846 net-ms
% set b [testbigdata string 100000]; string length $b
100000
% timerate {string cat $b $b $b $b $b $b $b $b $b $b}
31.6985 µs/# 31515 # 31547.2 #/sec 998.979 net-ms

Fixed in [b699959d62]. I looked into trying to make the existing code for TclStringCat work, but it eventually became clear that it would have to mostly become a clone of Tcl_AppendObjToObj() in order to work correctly. There are a couple added benefits to this fix: First, it reduces the number of code paths that perform concatenation. Second, all callers of Tcl_AppendObjToObj will benefit from the reduction in generation of string representations.