search

gisle / tcl.pm

Tcl extension module for Perl
http://search.cpan.org/dist/Tcl
9 stars 8 forks source link

SvFromTclObj(): unreachable code uses invalid `internalRep` #47

Closed chrstphrchvz closed 1 year ago

chrstphrchvz commented 2 years ago

In this code which appeared in 323f8e1bf48894c854dc0b5cd1135d38e95dc42a: https://github.com/gisle/tcl.pm/blob/4067e75618de28f6f35affcef6c1774e660da1af/Tcl.xs#L566-L579

The statement 

sv = newSVsv(boolSV(objPtr->internalRep.longValue != 0));

is unreachable (assuming tclBooleanTypePtr != NULL). It also does not make sense for the statement to be using objPtr->internalRep because it is invalid when objPtr->typePtr == NULL.

I am still trying to understand how this code is intended to work. I may propose changes to it anyway in #42 due to various other issues in SvFromTclObj() which I encounter in Tcl 9.0.

chrstphrchvz commented 1 year ago

I am not sure I will ever figure out what the original intention was…

At this point, I would be inclined to only ever return 0 or 1 even for “string” booleans. I don’t see how returning them as strings in Perl is useful, as I imagine doing so would only needlessly complicate things for users; why make the user check for "true"/"false"/"yes"/"no"/"on"/"off" when Tcl_GetBooleanFromObj() can do so for them?

Compare to how Tkinter converts any value Tcl says is already boolean (including string booleans) to a Python boolean—False or True.