tcl-core

[TCLCORE] Tcl 9 `expr` bug, int.rep typing

[Andreas K. <and...@gm...>]

Hi all.

I believe that I have found a bug in the `expr` command of

	Tcl 9 (commit <1f19a433ec>).

This issue is, IMHO, __not__ a release blocker.

It should not affect the Tcl level at all.

Note however that it can trip/break scripts using extensions
performing type-checks while not doing or able to do shimmering.

IOW extensions expecting expect specific int-reps. Among the affected
will be extensions providing objtypes do not have a string-rep, making
shimmering impossible.

Other extensions will simply shimmer the value back to the int-rep
they need. This is why the script level is not affected, except for
a performance loss I did not bother to try and measure.


Test code:
```
proc show-type {x} { puts <>=[::tcl::unsupported::representation $x] }

set i xx
show-type $i
show-type [expr {0 ? $i : $i}]
show-type [expr {$i}]
```

For Tcl 8.6 the results are:

```
<>=value is a pure string with a refcount of 4, object pointer at 0x1ec87b0, 
string representation "xx"
<>=value is a pure string with a refcount of 3, object pointer at 0x1ec9440, 
string representation "xx"
<>=value is a pure string with a refcount of 3, object pointer at 0x1ecacd0, 
string representation "xx"
```

For Tcl 9 <commit 1f19a433ec> the results for the `expr` command change:

```
<>=value is a pure string with a refcount of 4, object pointer at 
0x55dc2b5776d0, string representation "xx"
<>=value is a list with a refcount of 3, object pointer at 0x55dc2b52aab0, 
internal representation 0x55dc2b5a30a0:0x0, string representation "xx"
<>=value is a list with a refcount of 3, object pointer at 0x55dc2b578c00, 
internal representation 0x55dc2b5a30a0:0x0, string representation "xx"
```

It looks as if the $x going into `expr` is wrapped into a single-element list.
Something I truly see no reason for.

-- 
Happy Tcling,
	Andreas Kupries <and...@gm...>
			<https://core.tcl-lang.org/akupries/>
			<https://akupries.tclers.tk/>
	Developer @	SUSE Software Solutions Germany GmbH
-------------------------------------------------------------------------------

Re: [TCLCORE] Tcl 9 `expr` bug, int.rep typing

[Donald G P. <don...@ni...>]

On 9/24/24 12:53, Andreas Kupries wrote:
> I believe that I have found a bug in the `expr` command of
>
> 	Tcl 9 (commit <1f19a433ec>).

This is a bad joke.

That commit is made up entirely of edits to comments plus a single
whitespace change in non-comment code.  I cannot imagine how
that checkin can be the source of any behavior change.

-- 
| Don Porter            Applied and Computational Mathematics Division |
| don...@ni...             Information Technology Laboratory |
| http://math.nist.gov/~DPorter/                                  NIST |
|______________________________________________________________________|

Re: [TCLCORE] Tcl 9 `expr` bug, int.rep typing

[Jan N. <jan...@gm...>]

Op di 24 sep 2024 om 19:57 schreef Donald G Porter via Tcl-Core <
tcl...@li...>:

> On 9/24/24 12:53, Andreas Kupries wrote:
> > I believe that I have found a bug in the `expr` command of
> >
> >       Tcl 9 (commit <1f19a433ec>).
>
> This is a bad joke.
>
>
This behavior is caused by this commit:
     <https://core.tcl-lang.org/tcl/info/a197811f135d262b>
and it's done on purpose. Read the ticket for the
reason behind it. It only happens when the value doesn't
match any other known type, so I don't see how it
can cause problems.

>From the ticket:

    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.

Hope this helps,

     Jan Nijtmans

Re: [TCLCORE] Tcl 9 `expr` bug, int.rep typing

[Andreas K. <and...@gm...>]

> Op di 24 sep 2024 om 19:57 schreef Donald G Porter via Tcl-Core <
> tcl...@li...>:
> 
> > On 9/24/24 12:53, Andreas Kupries wrote:
> > > I believe that I have found a bug in the `expr` command of
> > >
> > >       Tcl 9 (commit <1f19a433ec>).
> >
> > This is a bad joke.
> >
> >
> This behavior is caused by this commit:

>      <https://core.tcl-lang.org/tcl/info/a197811f135d262b>

> and it's done on purpose. Read the ticket for the
> reason behind it. It only happens when the value doesn't
> match any other known type, so I don't see how it
> can cause problems.

Well, as I said in my initial mail, for extensions providing
full-fledged Tcl_ObjTypes, i.e. full conversion to/from a string rep
this is indeed not a problem. The next extension command will shimmer
the value back to the type it wants. Just a bit of a performance-loss
due to the additional shimmer.

It is an issue for extensions using Tcl_ObjTypes without a string
rep. These extensions usually do not try to shimmer from other types
into theirs, and simply expect that the type coming in is as they
expected.

One of mine is such an extension (*, **), which is why I ran into the
issue when I executed the test suite against a near-current Tcl 9
commit (3 days old) and got a load of test failures I knew had worked
before with a commit from near the beginning of the year.

I am willing to a accept the change, given the reasoning from the
ticket, and given that I can change my code to detect the
single-element list int.rep and then undo the wrapping to get at the
actual and desired value.

Given how I stumbled into this I would recommend and ask to place a
note about this into the compatibility notes as something extension
writers may have to be aware of.

(*) https://core.tcl-lang.org/akupries/aktive/doc/trunk/README.md
    uses Tcl_Obj values to carry images. Which may be large. So I
    decided against dealing with a string rep which will not only
    materialize the entire block of pixels into memory, but worse, as
    some form of text.

(**) The problematic `expr` is at
     https://core.tcl-lang.org/akupries/aktive/file?ci=trunk&name=op/embed.tcl&
ln=58
     and caught out at
     https://core.tcl-lang.org/akupries/aktive/file?ci=trunk&name=runtime/objty
pe_image.c&ln=57

> From the ticket:
> 
>     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.
> 
> Hope this helps,

Yes, very much, and thank you.

-- 
Happy Tcling,
	Andreas Kupries <and...@gm...>
			<https://core.tcl-lang.org/akupries/>
			<https://akupries.tclers.tk/>
	Developer @	SUSE Software Solutions Germany GmbH
-------------------------------------------------------------------------------

Re: [TCLCORE] Tcl 9 `expr` bug, int.rep typing

[Jan N. <jan...@gm...>]

Op di 24 sep 2024 om 21:02 schreef Andreas Kupries <
and...@gm...>:

> Well, as I said in my initial mail, for extensions providing
> full-fledged Tcl_ObjTypes, i.e. full conversion to/from a string rep
> this is indeed not a problem. The next extension command will shimmer
> the value back to the type it wants. Just a bit of a performance-loss
> due to the additional shimmer.
>

Thinking about it, there is a way out. The only reason for converting to
a Tcl list because I want to know whether it can be represented as
a list with length > 1. We can also check that by verifying the
string representation for spaces, tabs \n or \r (not counting the ones
at the beginning or end). A complication is that we should
not count spaces preceded by '\'. So, a little string trickery,
but this is cheaper than converting to a Tcl list, so worth
the trouble.

I'll work on that after 9.0.0 is out (since this isn't blocking)

Thanks!
       Jan Nijtmans

Re: [TCLCORE] Tcl 9 `expr` bug, int.rep typing

[Donald G P. <don...@ni...>]

On 9/24/24 15:21, Jan Nijtmans wrote:
> Op di 24 sep 2024 om 21:02 schreef Andreas Kupries <and...@gm...>:
>
>     Well, as I said in my initial mail, for extensions providing
>     full-fledged Tcl_ObjTypes, i.e. full conversion to/from a string rep
>     this is indeed not a problem. The next extension command will shimmer
>     the value back to the type it wants. Just a bit of a performance-loss
>     due to the additional shimmer.
>
>
> Thinking about it, there is a way out. The only reason for converting to
> a Tcl list because I want to know whether it can be represented as
> a list with length > 1. We can also check that by verifying the
> string representation for spaces, tabs \n or \r (not counting the ones
> at the beginning or end). A complication is that we should
> not count spaces preceded by '\'. So, a little string trickery,
> but this is cheaper than converting to a Tcl list, so worth
> the trouble.

Take note of the existing internal utility routine TclMaxListLength().

-- 
| Don Porter            Applied and Computational Mathematics Division |
| don...@ni...             Information Technology Laboratory |
| http://math.nist.gov/~DPorter/                                  NIST |
|______________________________________________________________________|

Re: [TCLCORE] Tcl 9 `expr` bug, int.rep typing

[Andreas K. <and...@gm...>]

Jan wrote:
> Thinking about it, there is a way out. The only reason for
> converting to a Tcl list because I want to know whether it can be
> represented as a list with length > 1. We can also check that by
> verifying the string representation for spaces, tabs \n or \r (not
> counting the ones at the beginning or end). A complication is that
> we should not count spaces preceded by '\'. So, a little string
> trickery, but this is cheaper than converting to a Tcl list, so
> worth the trouble.

> I'll work on that after 9.0.0 is out (since this isn't blocking)

+1, and good luck.

-- 
Happy Tcling,
	Andreas Kupries <and...@gm...>
			<https://core.tcl-lang.org/akupries/>
			<https://akupries.tclers.tk/>
	Developer @	SUSE Software Solutions Germany GmbH
-------------------------------------------------------------------------------

Re: [TCLCORE] Tcl 9 `expr` bug, int.rep typing

[Donald G P. <don...@ni...>]

On 9/24/24 16:44, Andreas Kupries wrote:
> Jan wrote:
>> Thinking about it, there is a way out. The only reason for
>> converting to a Tcl list because I want to know whether it can be
>> represented as a list with length > 1. We can also check that by
>> verifying the string representation for spaces, tabs \n or \r (not
>> counting the ones at the beginning or end). A complication is that
>> we should not count spaces preceded by '\'. So, a little string
>> trickery, but this is cheaper than converting to a Tcl list, so
>> worth the trouble.
>> I'll work on that after 9.0.0 is out (since this isn't blocking)
> +1, and good luck.
>

A few thoughts.

I was not involved in the changes under review, and my first reaction
is that it seems weird for [expr] to convert a value to a list. Can't
say for sure, but I don't think I would pursued this approach.  If it
can be reformed as described, that seems good.

That said, I am very strongly of the opinion that Tcl 9 should be out
of the business of protecting the intreps of extensions and apps. This
was never part of the interface contract, and if we can't escape it in
a new major version we never will.  The movement from Tcl 8 to Tcl 9
is the right moment for extensions to pull up their pants and implement
their own intrep preservation properly.

-- 
| Don Porter            Applied and Computational Mathematics Division |
| don...@ni...             Information Technology Laboratory |
| http://math.nist.gov/~DPorter/                                  NIST |
|______________________________________________________________________|

Re: [TCLCORE] Tcl 9 `expr` bug, int.rep typing

[Jan N. <jan...@gm...>]

Op di 24 sep 2024 om 22:44 schreef Andreas Kupries <
and...@gm...>:

> > I'll work on that after 9.0.0 is out (since this isn't blocking)
>
> +1, and good luck.


Proposed solution here:
     <https://core.tcl-lang.org/tcl/info/1870ed5dd9662b93>

Appears to work fine, but needs more testing.     ;-)

@Don, thanks for the hint to use TclMaxListLength()!

Regards,
   Jan Nijtmans