Menu
▾
▴
Re: [TCLCORE] TIP 676 C implementation
|
From: Colin M. <col...@ya...> - 2025-12-06 12:43:27
|
Hello All,
I have now fixed this crash and updated the patch file. I had assumed
that if EnvHasLVT() was true then AnonymousLocal() would always succeed,
which is not the case. Thanks very much to Sergey for picking this up,
even though he doesn't like the proposal.
Colin.
On 05/12/2025 18:17, Colin Macleod via Tcl-Core wrote:
>
> Hi Sergey,
>
> Thanks very much for this report. I'll dig into it tomorrow.
>
> Colin.
>
> On 05/12/2025 15:09, Dipl. Ing. Sergey G. Brester wrote:
>>
>> I already said everything I want about the proposal.
>> And I still don't think it'd be good idea at all to introduce such
>> sugar without to modify tcl parser
>> (e. g. to behave different on certain things like it was with args
>> expansion {*}, or without to consider [= ...] as an expression
>> without braces or something similar).
>> But...
>>
>> Just to point to an issue in case of compilation like concat with
>> invokeStk for [=] at end:
>> neither it looks like good idea to me, nor it is safe, because could
>> segfault in certain constellations.
>>
>> Simplified PoC (thereby it is irrelevant what [] contains, it'd be
>> anything else, e. g. [set s], and used here to avoid pure literal
>> compilation):
>>
>> % proc test {} { set s 0; timerate { = s + [] } }; test
>>
>> Thread 1 received signal SIGSEGV, Segmentation fault.
>> 0x00007ff9e48120f8 in FollowLinks (varPtr=0x100074a210) at
>> ./generic/tclExecute.c:2067
>> 2067 while (TclIsVarLink(varPtr)) {
>> (gdb) where
>> #0 0x00007ff9e48120f8 in FollowLinks (varPtr=0x100074a210) at
>> ./generic/tclExecute.c:2067
>> #1 0x00007ff9e48170da in TEBCresume (data=0x798178, interp=0x7460c0,
>> result=0) at ./generic/tclExecute.c:3340
>> #2 0x00007ff9e4775f15 in TclNRRunCallbacks (interp=0x7460c0,
>> result=0, rootPtr=0x797270) at ./generic/tclBasic.c:4668
>> #3 0x00007ff9e47b4c60 in Tcl_TimeRateObjCmd (dummy4169=0x0,
>> interp=0x7460c0, objc=2, objv=0x74a2b0) at ./generic/tclCmdMZ.c:4453
>> #...
>>
>> It is surely TEBC, because It'd work as expected if would fallback to
>> direct invocation (variable `s` doesn't compiled in the frame yet):
>>
>> % proc test {} { timerate { = s + [] } }; test
>> = Expected start of expression but found ''
>>
>> Regards,
>> Serg.
>>
>> Am 03.12.2025 16:49, schrieb Colin Macleod via Tcl-Core:
>>
>>> Hi again,
>>>
>>> I decided to treat "NaN", "Inf" & "Infinity" (any capitalisation) as
>>> variable names, eliminating that special case. Does anyone actually
>>> write expressions, other than test cases, using these values? If so
>>> they will need to stick to `expr`. I have updated the code, tests
>>> and man page to reflect this, and updated the patch at
>>> https://cmacleod.me.uk/tcl/tip676.patch.txt.
>>>
>>> I have also added an equals.test file of tests. Some of these I
>>> have adapted from the existing tests for `expr`, but more than half
>>> were contributed by Eric, which is extremely helpful.
>>>
>>> I now need to update the TIP to match the implementation. Once that
>>> is done I think it will be ready for formal consideration.
>>>
>>> Colin.
>>>
>>> On 02/12/2025 17:10, Colin Macleod wrote:
>>>>
>>>> Hi again all,
>>>>
>>>> I'm now working on creating tests for the `=` command, by adapting
>>>> the tests for `expr` which are relevant. This has highlighted a
>>>> few edge cases which I'm not sure how best to handle, so I would
>>>> like to ask what other people think.
>>>>
>>>> When given a single number as the expression, `expr` always
>>>> rewrites it to decimal form. At present `=` leaves it unchanged
>>>> unless it's used in a calculation:
>>>>
>>>>> % expr 0xff
>>>>> 255
>>>>> % = 0xff
>>>>> 0xff
>>>>> % = 0xff + 0
>>>>> 255
>>>>>
>>>> So in this case `=` behaves differently from `expr` - I don't think
>>>> this is really a problem and I would rather not introduce an
>>>> unnecessary conversion here. Is this acceptable?
>>>>
>>>> I'm not allowing the non-numeric representations of boolean values:
>>>> true, false, yes, no, etc. The `expr` code also accepts any
>>>> abbreviations of these, doing this in `=` would make it impossible
>>>> to use variables called t, f, y or n, which I think is
>>>> unacceptable. However the current `=` does accept Inf and NaN
>>>> (with any capitalisation, but not abbreviated) as numeric
>>>> constants, which makes it impossible to use variables with those
>>>> names. I'm not very happy about this being a special case, so I'm
>>>> wondering about disallowing those also. If so, someone who really
>>>> wanted to use such a value in a computation could still do so by
>>>> assigning it to a variable first and using that variable in the `=`
>>>> command: *set nan NaN; = nan + 0 *. What do others think?
>>>>
>>>> Combining these two issues, we get the following:
>>>>
>>>>> % expr nan
>>>>> domain error: argument not in valid range
>>>>> % = nan
>>>>> nan
>>>>> % = nan + 0
>>>>> cannot use non-numeric floating-point value "nan" as left operand
>>>>> of "+"
>>>>>
>>>> Acceptable or not?
>>>>
>>>> If anyone would like to experiment, I think the code at
>>>> https://cmacleod.me.uk/tcl/tip676.patch.txt is now fairly solid.
>>>> It would be good to get feedback, not just on these specific issues
>>>> but on the whole approach.
>>>>
>>>> Best regards,
>>>> Colin.
>>>>
>>>> On 30/11/2025 12:45, Colin Macleod via Tcl-Core wrote:
>>>>>
>>>>> Hello again,
>>>>>
>>>>> I have updated the patch now, adding a man page for `=` and fixing
>>>>> some bugs which Eric reported. I had to make a little tweak to
>>>>> installManPage to prevent it mangling the name.
>>>>>
>>>>> Colin.
>>>>>
>>>>> On 29/11/2025 13:57, Colin Macleod via Tcl-Core wrote:
>>>>>>
>>>>>> Hello All,
>>>>>>
>>>>>> I have now implemented compilation of the case where
>>>>>> pre-substitutions are purely numeric, falling back to the
>>>>>> non-compiled code where this turns out not to be the case. This
>>>>>> added about 100 lines more code. I also added an optimisation for
>>>>>> accessing local variables, which Eric pointed out. Both these
>>>>>> improvements are only possible when we have a local variable
>>>>>> table, which may not be the case for one-off code, but is always
>>>>>> the case within a proc. Again the updated code can be found in
>>>>>> patch form at https://cmacleod.me.uk/tcl/tip676.patch.txt.
>>>>>>
>>>>>> Timings of simple expressions:
>>>>>>
>>>>>> Proc using [expr]:
>>>>>>
>>>>>>> % proc fred1 {a b} {expr {$a*2-$b}}
>>>>>>> % fred1 4 6
>>>>>>> 2
>>>>>>> % timerate {fred1 [incr c] 7}
>>>>>>> 0.292607 µs/# 3417549 # 3417549 #/sec 1000.000 net-ms
>>>>>>>
>>>>>> Same proc using [=]:
>>>>>>
>>>>>>> % proc fred2 {a b} {= a*2-b}
>>>>>>> % fred2 4 6
>>>>>>> 2
>>>>>>> % timerate {fred2 [incr c] 7}
>>>>>>> 0.291969 µs/# 3425016 # 3425016 #/sec 1000.000 net-ms
>>>>>>>
>>>>>> Timings of expressions including an array reference:
>>>>>>
>>>>>> Proc using [expr]:
>>>>>>
>>>>>>> % array set nums {one 1 two 2 three 3 four IIII}
>>>>>>> %
>>>>>>> % proc ada1 name {expr {sin($::nums($name))+1}}
>>>>>>> % ada1 two
>>>>>>> 1.9092974268256817
>>>>>>> % timerate {ada1 three}
>>>>>>> 0.411508 µs/# 2430088 # 2430088 #/sec 1000.000 net-ms
>>>>>>>
>>>>>> Proc using [=], array reference not a separate argument, so
>>>>>> compilation fails:
>>>>>>
>>>>>>> % proc ada2 name {= sin($::nums($name))+1}
>>>>>>> % ada2 two
>>>>>>> 1.9092974268256817
>>>>>>> % timerate {ada2 three}
>>>>>>> 1.346676 µs/# 742569 # 742569 #/sec 1000.000 net-ms
>>>>>>>
>>>>>> Proc using [=], array reference is a separate argument, so
>>>>>> compilation succeeds:
>>>>>>
>>>>>>> % proc ada2a name {= sin( $::nums($name) )+1}
>>>>>>> % ada2a two
>>>>>>> 1.9092974268256817
>>>>>>> % timerate {ada2a three}
>>>>>>> 0.468546 µs/# 2134260 # 2134260 #/sec 1000.000 net-ms
>>>>>>>
>>>>>> But note of course that [expr] and [=] do not fail in the same
>>>>>> way here:
>>>>>>
>>>>>>> % ada1 four
>>>>>>> expected floating-point number but got "IIII"
>>>>>>> % ada2 four
>>>>>>> can't read "IIII": no such variable
>>>>>>> % ada2a four
>>>>>>> can't read "IIII": no such variable
>>>>>>>
>>>>>> I think the code is now pretty much complete. I have not worked
>>>>>> in the Tcl core before, apart from a few little fixes a very long
>>>>>> time ago, so I would be glad if other people could take a look at
>>>>>> my code.
>>>>>>
>>>>>> I still need to create a man page and some tests. Eric has
>>>>>> contributed a set of tests but I haven't found time to look at
>>>>>> them yet.
>>>>>>
>>>>>> Colin.
>>>>>>
>>>>>> On 25/11/2025 03:19, Colin Macleod via Tcl-Core wrote:
>>>>>>>
>>>>>>> Thanks Eric,
>>>>>>>
>>>>>>> I think it may be possible to take compilation one step further,
>>>>>>> to handle the common case where pre-substitutions are purely
>>>>>>> numeric. This would work similarly to the extract_numbers step
>>>>>>> I added in the Tcl prototype. The compilation step would
>>>>>>> generate code like:
>>>>>>>
>>>>>>> For any pre-substituted words,
>>>>>>> generate runtime code to check that the actual value is
>>>>>>> numeric (how?)
>>>>>>> and stash each value somewhere.
>>>>>>> If any of these tests fail,
>>>>>>> branch to code which pushes all the arguments on the stack
>>>>>>> and invokes the non-compiled `=` implementation.
>>>>>>> If all tests ok,
>>>>>>> run the compiled code,
>>>>>>> which can be generated assuming that the pre-substituted
>>>>>>> values are numeric,
>>>>>>> and will pull in the stashed values where the corresponding
>>>>>>> arguments are required.
>>>>>>>
>>>>>>> I'm not sure about the details of this, it may turn out not to
>>>>>>> be practical.
>>>>>>>
>>>>>>> Colin.
>>>>>>>
>>>>>>> On 24/11/2025 21:18, EricT wrote:
>>>>>>>> Hi Colin,
>>>>>>>>
>>>>>>>> Congratulations on getting the = command to this level of
>>>>>>>> quality! The compilation optimization is particularly
>>>>>>>> impressive - you've integrated it cleanly into Tcl's proc
>>>>>>>> compilation system.
>>>>>>>>
>>>>>>>> I added a few more proc tests with variable substitutions to
>>>>>>>> round out the test coverage. They all passed. These can be
>>>>>>>> added to the end of calc.test:
>>>>>>>>
>>>>>>>> # ---------- Pre-Substitution in Procs ----------
>>>>>>>>
>>>>>>>> test calc-39.1 {variable substitution in proc} -constraints
>>>>>>>> hasCalcCmd -body {
>>>>>>>> proc test_var_subst {a b} {
>>>>>>>> = $a + $b
>>>>>>>> }
>>>>>>>> test_var_subst 10 20
>>>>>>>> } -result 30 -cleanup {
>>>>>>>> rename test_var_subst {}
>>>>>>>> }
>>>>>>>>
>>>>>>>> test calc-39.2 {mixed literal and substitution} -constraints
>>>>>>>> hasCalcCmd -body {
>>>>>>>> proc test_mixed {n} {
>>>>>>>> = $n * 2 + 5
>>>>>>>> }
>>>>>>>> test_mixed 10
>>>>>>>> } -result 25 -cleanup {
>>>>>>>> rename test_mixed {}
>>>>>>>> }
>>>>>>>>
>>>>>>>> test calc-39.3 {command substitution in proc} -constraints
>>>>>>>> hasCalcCmd -body {
>>>>>>>> proc test_cmd_subst {x} {
>>>>>>>> = [expr {$x * 2}] + 10
>>>>>>>> }
>>>>>>>> test_cmd_subst 5
>>>>>>>> } -result 20 -cleanup {
>>>>>>>> rename test_cmd_subst {}
>>>>>>>> }
>>>>>>>>
>>>>>>>> I also verified an important safety advantage of your
>>>>>>>> implementation - it doesn't suffer from expr's
>>>>>>>> double-evaluation security issue:
>>>>>>>>
>>>>>>>> set xxx {[cd a:/]}
>>>>>>>> expr $xxx + 1
>>>>>>>> # Changes directory! expr evaluates the string as code
>>>>>>>>
>>>>>>>> = $xxx + 1
>>>>>>>> # Error: "Expected start of expression but found '['"
>>>>>>>> # Does NOT execute the command - safer!
>>>>>>>>
>>>>>>>> Your parser treats the value as data, not code, which is the
>>>>>>>> correct behavior.
>>>>>>>>
>>>>>>>> Great work!
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> Eric
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Tcl-Core mailing list
>>>>>> Tcl...@li...
>>>>>> https://lists.sourceforge.net/lists/listinfo/tcl-core
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Tcl-Core mailing list
>>>>> Tcl...@li...
>>>>> https://lists.sourceforge.net/lists/listinfo/tcl-core
>>>
>>> _______________________________________________
>>> Tcl-Core mailing list
>>> Tcl...@li...
>>> https://lists.sourceforge.net/lists/listinfo/tcl-core
>
>
> _______________________________________________
> Tcl-Core mailing list
> Tcl...@li...
> https://lists.sourceforge.net/lists/listinfo/tcl-core |
