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

perlvar - recommend numeric comparisons for $] #17087

Closed
p5pRT opened this issue Jul 9, 2019 · 10 comments · Fixed by #17378
Closed

perlvar - recommend numeric comparisons for $] #17087

p5pRT opened this issue Jul 9, 2019 · 10 comments · Fixed by #17378

Comments

@p5pRT
Copy link

p5pRT commented Jul 9, 2019

Migrated from rt.perl.org#134273 (status was 'open')

Searchable as RT134273$

@p5pRT
Copy link
Author

p5pRT commented Jul 9, 2019

From @Grinnz

The $] variable is a numeric version and so numeric comparisons should be
used to avoid weirdness due to trailing zeroes; these comparisons work fine
in most cases, stringifying $] before the comparison fixes issues on old
Perls where its numeric value is calculated differently. The attached patch
will update the documentation for $] to recommend this.

-Dan

@p5pRT
Copy link
Author

p5pRT commented Jul 9, 2019

From @Grinnz

Patch is attached.

@p5pRT
Copy link
Author

p5pRT commented Jul 9, 2019

From @Grinnz

0001-recommend-numeric-comparisons-for.patch
From ca07388701b8a02eea0470e23ff9ad05488d0d3e Mon Sep 17 00:00:00 2001
From: Dan Book <grinnz@grinnz.com>
Date: Tue, 9 Jul 2019 01:36:32 -0400
Subject: [PATCH] recommend numeric comparisons for $]

---
 pod/perlvar.pod | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/pod/perlvar.pod b/pod/perlvar.pod
index 1f9c08ce36..38111559fc 100644
--- a/pod/perlvar.pod
+++ b/pod/perlvar.pod
@@ -437,12 +437,11 @@ is the subversion / 1e6.  For example, Perl v5.10.1 would be "5.010001".
 This variable can be used to determine whether the Perl interpreter
 executing a script is in the right range of versions:
 
-    warn "No PerlIO!\n" if $] lt '5.008';
+    warn "No PerlIO!\n" if "$]" < 5.008;
 
-When comparing C<$]>, string comparison operators are B<highly
-recommended>.  The inherent limitations of binary floating point
-representation can sometimes lead to incorrect comparisons for some
-numbers on some architectures.
+When comparing C<$]>, numeric comparison operators should be used, but the
+variable should be stringified first to avoid issues where its original
+numeric value is inaccurate.
 
 See also the documentation of C<use VERSION> and C<require VERSION>
 for a convenient way to fail if the running Perl interpreter is too old.
@@ -453,9 +452,10 @@ object, which allows more flexible string comparisons.
 The main advantage of C<$]> over C<$^V> is that it works the same on any
 version of Perl.  The disadvantages are that it can't easily be compared
 to versions in other formats (e.g. literal v-strings, "v1.2.3" or
-version objects) and numeric comparisons can occasionally fail; it's good
-for string literal version checks and bad for comparing to a variable
-that hasn't been sanity-checked.
+version objects) and numeric comparisons are subject to the binary
+floating point representation; it's good for numeric literal version
+checks and bad for comparing to a variable that hasn't been
+sanity-checked.
 
 The C<$OLD_PERL_VERSION> form was added in Perl v5.20.0 for historical
 reasons but its use is discouraged. (If your reason to use C<$]> is to
-- 
2.20.1

@p5pRT
Copy link
Author

p5pRT commented Jul 15, 2019

From @xdg

That's not a good idea. While it seems like $] is numeric, it's not​:

$ peekperl -wE 'Dump($])'
SV = PV(0x7fa06a804cd0) at 0x7fa06a8213a0
  REFCNT = 1
  FLAGS = (POK,READONLY,pPOK)
  PV = 0x7fa06a5001b0 "5.028001"\0
  CUR = 8
  LEN = 10

Given that the data is string data, it should be compared
lexicographically, not via floating point comparison.

Even older Perls have a string representation, so old and new comparisons
will be consistent if they are done with strings and not numbers​:

$ peekperl -wle 'Dump($])'
SV = PVNV(0x55ce034cb8e0) at 0x55ce034e1040
  REFCNT = 1
  FLAGS = (NOK,POK,READONLY,pNOK,pPOK)
  IV = 0
  NV = 5.008008
  PV = 0x55ce034d6fc0 "5.008008"\0
  CUR = 8
  LEN = 16

On Tue, Jul 9, 2019 at 1​:40 AM Dan Book via RT <perlbug-followup@​perl.org>
wrote​:

Patch is attached.

---
via perlbug​: queue​: perl5 status​: new
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=134273

--
David Golden <xdg@​xdg.me> • https://xdg.me/ • Twitter/GitHub​: @​xdg

@p5pRT
Copy link
Author

p5pRT commented Jul 15, 2019

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Jul 15, 2019

From @Grinnz

On Sun, 14 Jul 2019 18​:37​:18 -0700, xdg@​xdg.me wrote​:

That's not a good idea. While it seems like $] is numeric, it's not​:

$ peekperl -wE 'Dump($])'
SV = PV(0x7fa06a804cd0) at 0x7fa06a8213a0
REFCNT = 1
FLAGS = (POK,READONLY,pPOK)
PV = 0x7fa06a5001b0 "5.028001"\0
CUR = 8
LEN = 10

Given that the data is string data, it should be compared
lexicographically, not via floating point comparison.

Even older Perls have a string representation, so old and new comparisons
will be consistent if they are done with strings and not numbers​:

$ peekperl -wle 'Dump($])'
SV = PVNV(0x55ce034cb8e0) at 0x55ce034e1040
REFCNT = 1
FLAGS = (NOK,POK,READONLY,pNOK,pPOK)
IV = 0
NV = 5.008008
PV = 0x55ce034d6fc0 "5.008008"\0
CUR = 8
LEN = 16

Yes, it is internally a string, but it is a numeric comparison and such should be recommended. This avoids confusion where '5.006' and '5.006000' are different in a lexicographic comparison. As long as it is first stringified the numeric comparison is consistent.

-Dan

@p5pRT
Copy link
Author

p5pRT commented Jul 15, 2019

From @xdg

I suggest researching why the numeric representation was removed and
numeric comparisons in perlvar changed to string comparisons in the first
place. (If you look at 5.8 documentation. you'll see that perlvar docs use
numeric.) If want to revert a prior change, you should offer substantial
evidence for why that prior change was ill-considered, rather than just
claiming "these comparisons work fine in most cases".

Additionally, this part of the patch makes no sense to me​:

When comparing C<$]>, numeric comparison operators should be used, but the
+variable should be stringified first to avoid issues where its original
+numeric value is inaccurate.

In what way does creation of a PV representation change how NV comparison
occurs? Please provide evidence of how this works for current and
historical versions of Perl across supported architectures. (Please
include unusual, high-precision floating point architectures like s390x.)

If you want to improve version comparison, marking $] "discouraged" in
favor $^V is a better documentation change.

On Sun, Jul 14, 2019 at 10​:35 PM Dan Book via RT <perlbug-followup@​perl.org>
wrote​:

On Sun, 14 Jul 2019 18​:37​:18 -0700, xdg@​xdg.me wrote​:

That's not a good idea. While it seems like $] is numeric, it's not​:

$ peekperl -wE 'Dump($])'
SV = PV(0x7fa06a804cd0) at 0x7fa06a8213a0
REFCNT = 1
FLAGS = (POK,READONLY,pPOK)
PV = 0x7fa06a5001b0 "5.028001"\0
CUR = 8
LEN = 10

Given that the data is string data, it should be compared
lexicographically, not via floating point comparison.

Even older Perls have a string representation, so old and new comparisons
will be consistent if they are done with strings and not numbers​:

$ peekperl -wle 'Dump($])'
SV = PVNV(0x55ce034cb8e0) at 0x55ce034e1040
REFCNT = 1
FLAGS = (NOK,POK,READONLY,pNOK,pPOK)
IV = 0
NV = 5.008008
PV = 0x55ce034d6fc0 "5.008008"\0
CUR = 8
LEN = 16

Yes, it is internally a string, but it is a numeric comparison and such
should be recommended. This avoids confusion where '5.006' and '5.006000'
are different in a lexicographic comparison. As long as it is first
stringified the numeric comparison is consistent.

-Dan

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=134273

--
David Golden <xdg@​xdg.me> • https://xdg.me/ • Twitter/GitHub​: @​xdg

@p5pRT
Copy link
Author

p5pRT commented Jul 15, 2019

From @sisyphus

On Mon, 15 Jul 2019 04​:04​:16 -0700, xdg@​xdg.me wrote​:

Additionally, this part of the patch makes no sense to me​:

+When comparing C<$]>, numeric comparison operators should be used,
but the
+variable should be stringified first to avoid issues where its
original
+numeric value is inaccurate.
In what way does creation of a PV representation change how NV
comparison

David, you omitted the opening "+", which I've inserted above.
But I think you're right. I can see no reason that that section of the patch needs to be anything more than​:

+When comparing C<$]>, numeric comparison operators should be used.

If you want to improve version comparison, marking $] "discouraged" in
favor $^V is a better documentation change.

Not so sure about that suggestion, however.
$],in its current state, is fine for version comparison if used in numeric context.
Personally, I would much rather use $] than $^V.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Jul 15, 2019

From @Grinnz

On Mon, 15 Jul 2019 04​:04​:16 -0700, xdg@​xdg.me wrote​:

In what way does creation of a PV representation change how NV
comparison
occurs? Please provide evidence of how this works for current and
historical versions of Perl across supported architectures. (Please
include unusual, high-precision floating point architectures like
s390x.)

I will be happy to do this when I have time, though I of course do not have access to such exotic architectures. In my opinion the likelihood of someone comparing $] incorrectly with lexicographical operators is higher than the likelihood they are using such an architecture. But that is a judgment for the porters to make. I'll also add that while I already thought this was the recommended approach, the impetus for this patch was an instance where the lexicographical comparison fails but the numeric comparison succeeds on some ancient version (5.00403?), but use of such is also very unlikely.

If you want to improve version comparison, marking $] "discouraged" in
favor $^V is a better documentation change.

I do not think this will ever be a good idea, at least for a decade or two. $] is uniquely purposed in that it is used to guard against use of ancient Perl versions, where the behavior of $^V is radically different, or $^V does not exist.

@p5pRT
Copy link
Author

p5pRT commented Jul 15, 2019

From @khwilliamson

On 7/15/19 5​:03 AM, David Golden wrote​:

I suggest researching why the numeric representation was removed and
numeric comparisons in perlvar changed to string comparisons in the
first place.  (If you look at 5.8 documentation. you'll see that perlvar
docs use numeric.)  If want to revert a prior change, you should offer
substantial evidence for why that prior change was ill-considered,
rather than just claiming "these comparisons work fine in most cases".

Additionally, this part of the patch makes no sense to me​:

When comparing C<$]>, numeric comparison operators should be used,
but the
+variable should be stringified first to avoid issues where its original
+numeric value is inaccurate.

In what way does creation of a PV representation change how NV
comparison occurs?  Please provide evidence of how this works for
current and historical versions of Perl across supported architectures.
(Please include unusual, high-precision floating point architectures
like s390x.)

If you want to improve version comparison, marking $] "discouraged" in
favor $^V is a better documentation change.

The reason this topic has surfaced is that I've been working on
Devel​::PPPort, and found that the string comparisons failed in at least
one early perl. When I asked on #irc, I was told that the conventional
wisdom for best results across most versions is to use numeric
comparisons instead, which is contradicted by our curent pod. I
encouraged Dan to write a patch, and then David to contribute his knowledge.

One argument on #irc was that the same architecture is doing all the
floating point conversion, so shouldn't that yield consistent results?

It would be good if someone has a concrete counterexample of how this is
wrong.

On Sun, Jul 14, 2019 at 10​:35 PM Dan Book via RT
<perlbug-followup@​perl.org <mailto​:perlbug-followup@​perl.org>> wrote​:

On Sun\, 14 Jul 2019 18&#8203;:37&#8203;:18 \-0700\, xdg@&#8203;xdg\.me \<mailto&#8203;:xdg@&#8203;xdg\.me>
wrote&#8203;:
 > That's not a good idea\.  While it seems like $\] is numeric\, it's not&#8203;:
 >
 > $ peekperl \-wE 'Dump\($\]\)'
 > SV = PV\(0x7fa06a804cd0\) at 0x7fa06a8213a0
 >   REFCNT = 1
 >   FLAGS = \(POK\,READONLY\,pPOK\)
 >   PV = 0x7fa06a5001b0 "5\.028001"\\0
 >   CUR = 8
 >   LEN = 10
 >
 > Given that the data is string data\, it should be compared
 > lexicographically\, not via floating point comparison\.
 >
 > Even older Perls have a string representation\, so old and new
comparisons
 > will be consistent if they are done with strings and not numbers&#8203;:
 >
 > $ peekperl \-wle 'Dump\($\]\)'
 > SV = PVNV\(0x55ce034cb8e0\) at 0x55ce034e1040
 >   REFCNT = 1
 >   FLAGS = \(NOK\,POK\,READONLY\,pNOK\,pPOK\)
 >   IV = 0
 >   NV = 5\.008008
 >   PV = 0x55ce034d6fc0 "5\.008008"\\0
 >   CUR = 8
 >   LEN = 16
 >

Yes\, it is internally a string\, but it is a numeric comparison and
such should be recommended\. This avoids confusion where '5\.006' and
'5\.006000' are different in a lexicographic comparison\. As long as
it is first stringified the numeric comparison is consistent\.

\-Dan



\-\-\-
via perlbug&#8203;:  queue&#8203;: perl5 status&#8203;: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=134273

--
David Golden <xdg@​xdg.me <mailto​:xdg@​xdg.me>> • https://xdg.me/
Twitter/GitHub​: @​xdg

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 a pull request may close this issue.

1 participant