Menu
▾
▴
Re: [TCLCORE] TIP 676 C implementation
|
From: Dipl. I. S. G. B. <se...@us...> - 2025-12-07 01:12:04
|
My pleasure...
The issue with "don't like" is - it is not tclish enough to me.
Or rather it is a bit strange sugar in my opinion, because:
* it doesn't support many things what expression does, like comparison
with strings/literals or usage of functions;
* does weird subst of literal parameters as variables without dollar
at every offset (commands like set, (l)append etc, do that with single
variable only, and it is absolutely justified);
* therefore confusing or else wrong interpreting of the tcl code by
reading, for instance what happens here?
[= a < "b"]
or rather although "b" and b are the same literals in tcl, it is
completely weird here to handle "b" as variable name IMHO (I know why,
but it is confusing for reading);
* or even here (double subst of var, but if $b "resolves" a literal,
not number, otherwise not at all)?
[= a < $b] * all of this objectively makes this sugar in context of TCL
to Malbolge (esoteric programming language that humans hardly
understand);
* the way how it is compiled (and it cannot be compiled differently
and optimal) quasi forces the devs to write not effective code;
* etc (I can continue here with further arguments).
As already said, I am also for some sugar to simplify [expr {...}]
writing, but it must be well thought-out, clearly discernible as such
and ideally parse-able in that form at level of tcl-parser.
Even if basic rules of tcl-parser get modified for that purposes, e. g.
like it was done with args expansion {*}.
All the attempts I've seen so far looked more like a workaround to me,
sorry guys.
Regards,
Serg.
06.12.2025 13:43, Colin Macleod wrote:
> 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 [1].
>
> 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 [1] 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 [1].
>
> 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 [2]
_______________________________________________
Tcl-Core mailing list
Tcl...@li...
https://lists.sourceforge.net/lists/listinfo/tcl-core [2]
_______________________________________________
Tcl-Core mailing list
Tcl...@li...
https://lists.sourceforge.net/lists/listinfo/tcl-core [2]
_______________________________________________
Tcl-Core mailing list
Tcl...@li...
https://lists.sourceforge.net/lists/listinfo/tcl-core [2]
Links:
------
[1] https://cmacleod.me.uk/tcl/tip676.patch.txt
[2] https://lists.sourceforge.net/lists/listinfo/tcl-core
|
