Tcl Source Code

View Ticket
Login
Ticket UUID: 0439e1e1a37ff9c634280bc2490981f3e3c7e985
Title: Slow detection of illegal expr argument
Type: Bug Version: 8.7/9.0
Submitter: jan.nijtmans Created on: 2024-07-05 09:03:41
Subsystem: 48. Number Handling Assigned To: jan.nijtmans
Priority: 5 Medium Severity: Minor
Status: Closed Last Modified: 2024-10-08 22:40:56
Resolution: Fixed Closed By: jan.nijtmans
    Closed on: 2024-10-08 22:40:56
Description: (text/x-fossil-wiki)
Example:
<pre>
$ tclsh9.0
% set x [lseq 1 11111111111111];set y 5
5
% expr {$x+$y}
</pre>

Doing this hangs a long time, because a string representation is generated for the lseq. But we know already that an lseq with 0 or  >1 entry can never be handled by '+'. Would be nice to detect that early, without taking that much time.
User Comments: jan.nijtmans added on 2024-10-08 22:40:56: (text/x-fossil-wiki)
Should be [a618dcb27007fabd|better] now. Also (partly) backported to 8.7

jan.nijtmans added on 2024-09-26 19:45:45: (text/x-fossil-wiki)
> Calling something "a list" because TMLL() returns a value bigger than 1
is not an accurate claim.

Indeed! There's still time until 9.0.1 to make it better. Current use was meant as a quick fix, in order to - at least - prevent conversion to a list.

dgp added on 2024-09-26 17:01:10:
Beware that TclMaxListLength() is not written to be accurate.
It is written to be reasonably quick, and to produce an over-estimate
of how many elements the list might have -- if it is a list at all.

Calling something "a list" because TMLL() returns a value bigger than 1
is not an accurate claim.

jan.nijtmans added on 2024-07-09 13:38:38: (text/x-fossil-wiki)
More examples handled now (integers and doubles):
<pre>
$ tclsh9.0
%  set x [lseq 1 11111111111111]
5
% binary encode base64 -maxlen $x data
expected integer but got a list
% binary format d $x
expected floating-point number but got a list
</pre>

See: [5d333307578b6a00]

jan.nijtmans added on 2024-07-09 09:32:08: (text/x-fossil-wiki)
While testing:
<pre>
$ tclsh9.0
% set x [lseq 1 11111111111111];set y 5
5
% if {$x} {puts stdout abc}
</pre>

This also hangs, and could be handled the same way.

jan.nijtmans added on 2024-07-08 20:29:20: (text/x-fossil-wiki)
Christian Werner wrote:
<pre>
While I fully support elimination of single single quotes in error
messages regardless of formalism or conversation I wonder how many
omelettes we'll be able to make from the potentially many broken eggs.
Alternatively, we could conserve these eggs for later.

Best,
Christian
<pre>

jan.nijtmans added on 2024-07-08 20:27:32: (text/x-fossil-wiki)
Remark by Brian Griffin:
<pre>
The only change I would recommend is to not use a contraction in the error message. In other words, use “cannot” instead of “can’t”. IMO, messages should be formal, not conversational.

-Brian
</pre>

jan.nijtmans added on 2024-07-07 14:10:38: (text/x-fossil-wiki)
I think [2290847d23a494e4|this] is how it should be:

If the obj being handled by "expr" can be represented as a list and has more than 1 element (so a dict as well), it now gives a slightly different error-message:

<pre>
% set x [lseq 1 11111111111111];set y 5
5
% expr {$x+$y}
can't use a list as operand of "+"
</pre>

A side-effect of this is that Tcl_GetNumberFromObj() now tries to
convert the object to a list, if it isn't already one of the
other known types (like dict, lseq, or index). That's very useful in
further error-handling, since the typePtr or lengthProc then
can be inspected for this case without trying to do more parsing.

It might also be very useful for the implementation of a future
Tcl_GetValueFromObj() ....

jan.nijtmans added on 2024-07-07 14:10:12:
I think [2290847d23a494e4|this] is how it should be:

If the obj being handled by "expr" can be represented as a list and has more than 1 element (so a dict as well), it now gives a slightly different error-message:

<pre>
% set x [lseq 1 11111111111111];set y 5
5
% expr {$x+$y}
can't use a list as operand of "+"
</pre>

A side-effect of this is that Tcl_GetNumberFromObj() now tries to
convert the object to a list, if it isn't already one of the
other known types (like dict, lseq, or index). That's very useful in
further error-handling, since the typePtr or lengthProc then
can be inspected for this case without trying to do more parsing.

It might also be very useful for the implementation of a future
Tcl_GetValueFromObj() ....