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

fix string comparisons with $] to use numeric comparison instead #136

Merged
merged 1 commit into from
Jan 3, 2025

Conversation

book
Copy link
Contributor

@book book commented Dec 14, 2024

The fix follows Zefram's suggestion from
https://www.nntp.perl.org/group/perl.perl5.porters/2012/05/msg186846.html

On older perls, however, $] had a numeric value that was built up using
floating-point arithmetic, such as 5+0.006+0.000002. This would not
necessarily match the conversion of the complete value from string form
[perl #72210]. You can work around that by explicitly stringifying
$] (which produces a correct string) and having that numify (to a
correctly-converted floating point value) for comparison. I cultivate
the habit of always stringifying $] to work around this, regardless of
the threshold where the bug was fixed. So I'd write

use if "$]" >= 5.014, warnings => "non_unicode";

This ensures that the comparisons will still work when Perl's major version changes to anything greater than 9.

@sisyphus
Copy link
Contributor

The proposed patch interpolates and numifies both the left and right hand sides (eg .... unless "$]" >= "5.008000"), whereas Zefram has suggested .... unless "$]" >= 5.008.
I have a (nitpicky) preference for the latter, as it avoids the need to numify the right hand side.

I habitually do such comparisons without interpolating either side - ie .... unless $] >= 5.008. I find it to be more readily understandable - though the other two forms aren't exactly taxing ;-)
Such action also sets $]'s IV slot to 5, and its NV slot to its interpolated numeric value. It also sets the NOK, pIOK and pNOK flags.
For all subsequent such calls, the value of $] can then be accessed immediately from the NV slot.
If anyone has an example of how my "habitual" usage might break something/anything, then I'd be most interested to learn of it.

@haarg
Copy link
Member

haarg commented Dec 15, 2024

As mentioned in the commit message, older versions of perl had floating point values that couldn't be directly compared to the expected numbers. Converting to a string then back to a number forces the value to have the expected float value. This only impacts perl versions prior to 5.8 though, so it's no longer a real concern. But some people continue to use that form out of habit or an excess of caution.

Comparing to a string on the RHS is pointless, but the patch just maintained the values that were previously in the code. It would be better to change them to numeric values.

@sisyphus
Copy link
Contributor

Comparing to a string on the RHS is pointless, but the patch just maintained the values that were previously in the code.

Oh, yes - and it also won't matter whether the LHS is $] or "$]".
The condition "$]" == 5.006002 will be true if and only if $] == 5.006002 is also true.

The key thing (from which I was distracted) is that it has to be a numeric comparison, not a string comparison.

All good - and thank you for taking the time to clarify.

@book
Copy link
Contributor Author

book commented Dec 15, 2024

Comparing to a string on the RHS is pointless, but the patch just maintained the values that were previously in the code.

Yes, I've updated my core patches to do numeric comparisons, to avoid confusing things more.
I'll update my patch here too.

Oh, yes - and it also won't matter whether the LHS is $] or "$]". The condition "$]" == 5.006002 will be true if and only if $] == 5.006002 is also true.

Well, actually:

$ perl -E 'say 5 + 0.006 + 0.000002 == 5.006002 ? "true" : "false"'
false
$ perl -E 'say sprintf "%.16f", $_ for 5 + 0.006 + 0.000002, 5.006002'
5.0060020000000005
5.0060019999999996

The bug that "$]" is working around is that in the old days, $] was built as above (by adding the three version components as individual floats), and now it's built directly as a string.

@book book force-pushed the book/fix-version-string-comparisons branch from 4fd044e to 9544d51 Compare December 15, 2024 14:45
@sisyphus
Copy link
Contributor

Well, actually:

Duh .... oh, yes. Finally got it through my thick skull that, on those old perls, $] is an NV (not a PV), and needs to be interpolated for the numeric comparisons to work.
Thanks for your tolerance.

The fix follows Zefram's suggestion from
https://www.nntp.perl.org/group/perl.perl5.porters/2012/05/msg186846.html

> On older perls, however, $] had a numeric value that was built up using
> floating-point arithmetic, such as 5+0.006+0.000002.  This would not
> necessarily match the conversion of the complete value from string form
> [perl #72210].  You can work around that by explicitly stringifying
> $] (which produces a correct string) and having *that* numify (to a
> correctly-converted floating point value) for comparison.  I cultivate
> the habit of always stringifying $] to work around this, regardless of
> the threshold where the bug was fixed.  So I'd write
>
>     use if "$]" >= 5.014, warnings => "non_unicode";

This ensures that the comparisons will still work when Perl's major
version changes to anything greater than 9.
@book book force-pushed the book/fix-version-string-comparisons branch from 9544d51 to af1feb7 Compare January 2, 2025 17:34
@haarg haarg merged commit 3941392 into Dual-Life:master Jan 3, 2025
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

Successfully merging this pull request may close these issues.

3 participants