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

[Data-Dumper] fix string comparisons with $] to use numeric compariso… #22891

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

book
Copy link
Contributor

@book book commented Jan 7, 2025

…n instead

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 set of changes does not require a perldelta entry.

…n instead

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";
@book book force-pushed the book/fix-version-string-comparisons branch from d64c0f2 to 77a58c0 Compare January 7, 2025 00:52
@book
Copy link
Contributor Author

book commented Jan 7, 2025

This is ported from the psc/ppc0025 branch. The fix is useful even if we don't change the version scheme for Perl.

@book book requested a review from haarg January 7, 2025 01:25
@jkeenan
Copy link
Contributor

jkeenan commented Jan 7, 2025

Data-Dumper is one of those blead-first, dual-life CPAN distributions which people actually do try to install against older perls. Do we have a way of testing the modifications in this p.r. against older perls (before merging this into blead)?

@haarg
Copy link
Contributor

haarg commented Jan 7, 2025

The GitHub actions check these against older perl versions, and those checks are passing.

@book
Copy link
Contributor Author

book commented Jan 7, 2025

It seem the cygwin-related failures can be ignored.

Excerpt from IRC #p5p on 2025-01-07:

12:49 <mauke> some kind of cygwin bug
12:51 <mauke> I was trying to test the changes in older perls,
              but somehow they fail to load the XS parts of the module,
              and several tests are broken without XS
12:51 <mauke> some have been for years (decades?)
12:51 <mauke> shows you how often people try to run the test suite
              against pure perl Data::Dumper :-)

@mauke mauke merged commit 61bacb9 into blead Jan 8, 2025
65 of 67 checks passed
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.

4 participants