| Ticket UUID: | 0439e1e1a37ff9c634280bc2490981f3e3c7e985 | ||
| Title: | Slow detection of illegal expr argument | ||
| Type: | Bug | Created on: | 2024-07-05 09:03:41 |
| Submitter: | jan.nijtmans | Assigned to: | jan.nijtmans |
| Subsystem: | 48. Number Handling | Severity: | Minor |
| Priority: | 5 Medium | Last modified: | 2024-10-08 22:40:56 |
| Status: | Closed | Closed by: | jan.nijtmans |
| Resolution: | Fixed | Closed on: | 2024-10-08 22:40:56 |
| Version: | 8.7/9.0 | ||
| Description: | ||||
|
Example:
$ tclsh9.0
% set x [lseq 1 11111111111111];set y 5
5
% expr {$x+$y}
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-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() ....
jan.nijtmans added on 2024-07-07 14:10:38:
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:
% set x [lseq 1 11111111111111];set y 5
5
% expr {$x+$y}
can't use a list as operand of "+"
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-08 20:27:32:
Remark by Brian Griffin: 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. jan.nijtmans added on 2024-07-08 20:29:20:
Christian Werner wrote: 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. jan.nijtmans added on 2024-07-09 09:32:08:
While testing:
$ tclsh9.0
% set x [lseq 1 11111111111111];set y 5
5
% if {$x} {puts stdout abc}
This also hangs, and could be handled the same way. jan.nijtmans added on 2024-07-09 13:38:38:
More examples handled now (integers and doubles): $ 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 See: [5d333307578b6a00] 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-09-26 19:45:45:
> 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. jan.nijtmans added on 2024-10-08 22:40:56:
Should be [a618dcb27007fabd|better] now. Also (partly) backported to 8.7 | ||||
