Menu
▾
▴
Re: [TCLCORE] TIP 676 C implementation
|
From: Colin M. <col...@ya...> - 2025-12-05 18:17:18
|
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 |
