Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SvFromTclObj(): unreachable code uses invalid internalRep #47

Closed
chrstphrchvz opened this issue Apr 23, 2022 · 1 comment
Closed

SvFromTclObj(): unreachable code uses invalid internalRep #47

chrstphrchvz opened this issue Apr 23, 2022 · 1 comment

Comments

@chrstphrchvz
Copy link
Contributor

In this code which appeared in 323f8e1:

tcl.pm/Tcl.xs

Lines 566 to 579 in 4067e75

else if (objPtr->typePtr == tclBooleanTypePtr) {
/*
* Booleans can originate as words (yes/true/...), so if there is a
* string rep, use it instead. We could check if the first byte
* isdigit(). No need to check utf-8 as the all valid boolean words
* are ascii-7.
*/
if (objPtr->typePtr == NULL) {
sv = newSVsv(boolSV(objPtr->internalRep.longValue != 0));
} else {
str = Tcl_GetStringFromObj(objPtr, &len);
sv = newSVpvn(str, len);
}
}

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
Copy link
Contributor Author

chrstphrchvz commented Sep 10, 2022

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.

chrstphrchvz added a commit to chrstphrchvz/tcl.pm that referenced this issue Mar 4, 2023
@vadrer vadrer closed this as completed in 7c38046 Mar 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant