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

cpan/Scalar-List-Utils/t/uniq.t fails on perl built with -Duselongdouble #17125

Closed
p5pRT opened this issue Aug 13, 2019 · 16 comments
Closed

cpan/Scalar-List-Utils/t/uniq.t fails on perl built with -Duselongdouble #17125

p5pRT opened this issue Aug 13, 2019 · 16 comments

Comments

@p5pRT
Copy link

p5pRT commented Aug 13, 2019

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

Searchable as RT134358$

@p5pRT
Copy link
Author

p5pRT commented Aug 13, 2019

From @jkeenan

Test output​:

#####
ok 17 - uniqnum preserves uniqness of full integer range
not ok 18 - uniqnum preserves uniqness of full integer range (stringified)

# Failed test 'uniqnum preserves uniqness of full integer range
(stringified)'
# at t/uniq.t line 115.
# Structures begin differing at​:
# $got->[1] = '-1'
# $expected->[1] = '18446744073709551614'
ok 19 - uniqnum considers undef and zero equivalent
#####

This breakage occurred upstream on CPAN
https://rt.cpan.org/Ticket/Display.html?id=130277 and is reflected in
core as of the following synch-with-CPAN action​:

#####
commit 96684a7
Author​: Chris 'BinGOs' Williams <chris@​bingosnet.co.uk>
Date​: Mon Aug 12 10​:27​:18 2019 +0100

  Update Scalar-List-Utils to CPAN version 1.51
#####

Occurs when perl is built with -Dlongdouble. See
http​://perl5.test-smoke.org/submatrix?test=../cpan/Scalar-List-Utils/t/uniq.t&pversion=5.31.3
for list of failing smoker runs.

Reporting here because of importance of this module within core, but fix
will have to be made upstream first.

Thank you very much.
Jim Keenan

#####
perl perl perl

@p5pRT
Copy link
Author

p5pRT commented Aug 13, 2019

From @andk

See also
https://www.nntp.perl.org/group/perl.daily-build.reports/2019/08/msg236198.html
plus circa 18 more FAIL reports for v5.31.2-99-gdefad34a5a

See also
https://rt.cpan.org/Ticket/Display.html?id=130277

Perl Info

Flags:
    category=install
    severity=none
    ack=no

Site configuration information for perl 5.31.3:

Configured by sand at Tue Aug 13 18:40:21 UTC 2019.

Summary of my perl5 (revision 5 version 31 subversion 3) configuration:
  Commit id: defad34a5a721604191590bcfd4fff6873310609
  Platform:
    osname=linux
    osvers=4.18.0-2-amd64
    archname=x86_64-linux-thread-multi-ld
    uname='linux k93msid 4.18.0-2-amd64 #1 smp debian 4.18.10-2 (2018-10-07) x86_64 gnulinux '
    config_args='-Dprefix=/home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.31.2-99-gdefad34a5a/8854 -Dmyhostname=k93msid -Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db -Dlibswanted=cl pthread socket inet nsl gdbm dbm malloc dl ld sun m crypt sec util c cposix posix ucb BSD gdbm_compat -Duseithreads -Duselongdouble -DEBUGGING=-g'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=define
    usemultiplicity=define
    use64bitint=define
    use64bitall=define
    uselongdouble=define
    usemymalloc=n
    default_inc_excludes_dot=define
    bincompat5005=undef
  Compiler:
    cc='cc'
    ccflags ='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
    optimize='-O2 -g'
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='8.2.0'
    gccosandvers=''
    intsize=4
    longsize=8
    ptrsize=8
    doublesize=8
    byteorder=12345678
    doublekind=3
    d_longlong=define
    longlongsize=8
    d_longdbl=define
    longdblsize=16
    longdblkind=3
    ivtype='long'
    ivsize=8
    nvtype='long double'
    nvsize=16
    Off_t='off_t'
    lseeksize=8
    alignbytes=16
    prototype=define
  Linker and Libraries:
    ld='cc'
    ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/8/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
    libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.27.so
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.27'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector-strong'



@INC for perl 5.31.3:
    lib
    .
    /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.31.2-99-gdefad34a5a/8854/lib/site_perl/5.31.3/x86_64-linux-thread-multi-ld
    /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.31.2-99-gdefad34a5a/8854/lib/site_perl/5.31.3
    /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.31.2-99-gdefad34a5a/8854/lib/5.31.3/x86_64-linux-thread-multi-ld
    /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.31.2-99-gdefad34a5a/8854/lib/5.31.3


Environment for perl 5.31.3:
    HOME=/home/sand
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/sand/bin:/usr/local/bin:/usr/bin:/bin:/usr/games:/usr/local/perl/bin:/usr/X11/bin:/sbin:/usr/sbin
    PERL_BADLANG (unset)
    SHELL=/usr/bin/zsh

@p5pRT
Copy link
Author

p5pRT commented Aug 13, 2019

From @jkeenan

Most of the failures are in t/uniq.t from Scalar-List-Utils's test suite. This has already been reported for -Dlongdouble builds by Slaven in the cpan bug ticket referenced, as well as 134358. On IRC #p5p, ilmari has diagnosed this as affecting -Dusequadmath builds as well. Ilmari has prepared a pull request for PEVANS, the upstream maintainer of Scalar-List-Utils. Pull request looked good to me; awaiting new CPAN release of Scalar-List-Utils.

So we can merge this RT into the earlier one.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Aug 14, 2019

From @sisyphus

On Tue, 13 Aug 2019 12​:25​:00 -0700, jkeenan wrote​:
....

Ilmari has prepared a pull request for PEVANS, the upstream
maintainer of Scalar-List-Utils.

There are, however, also issues with uniqnum() on double builds, too.

For example, the strings '1.73205080756888' and '1.7320508075688772' numify to different values​:

C​:\>perl -le "print 'ok' if '1.73205080756888' != '1.7320508075688772';"
ok

but uniqnum() regards them as the same value​:

C​:\>perl -MList​::Util -le "@​x = List​::Util​::uniqnum('1.73205080756888', '1.7320508075688772'); print \"@​x\"; "
1.73205080756888

This happens because of this line in ListUtil.xs​:

sv_setpvf(keysv, "%.15" NVgf, SvNV(arg));

On 'double' builds it should be "%.17" (not "%.15").
On extended precision 'long double' builds it should be "%.21".
On 'quadmath' and IEEE 'long double' builds it should be "%.36".

At least that's the way it looks to me - though I haven't had time to fully evaluate this. (I think that "%.36" should probably be ok for *all* builds of perl - though that may not be optimal, and it's also untested at this stage.)

I can say that replacing "%.15" with "%.21" on the long double build fixed the failing test.
And when I replace the "%.15" with "%.17", then uniqnum() behaves correctly on the double builds, too​:

C​:\>perl -MList​::Util -le "@​x = List​::Util​::uniqnum('1.73205080756888', '1.7320508075688772'); print \"@​x\"; "
1.73205080756888 1.7320508075688772

I also think that such a fix renders ilmari's pull request unnecessary.

However, I'm out of time for tonight, and further testing will have to wait until tomorrow.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Aug 14, 2019

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

@p5pRT
Copy link
Author

p5pRT commented Aug 14, 2019

From @jkeenan

On Wed, 14 Aug 2019 15​:18​:24 GMT, sisyphus@​cpan.org wrote​:

On Tue, 13 Aug 2019 12​:25​:00 -0700, jkeenan wrote​:
....

Ilmari has prepared a pull request for PEVANS, the upstream
maintainer of Scalar-List-Utils.

There are, however, also issues with uniqnum() on double builds, too.

For example, the strings '1.73205080756888' and '1.7320508075688772'
numify to different values​:

C​:\>perl -le "print 'ok' if '1.73205080756888' !=
'1.7320508075688772';"
ok

but uniqnum() regards them as the same value​:

C​:\>perl -MList​::Util -le "@​x =
List​::Util​::uniqnum('1.73205080756888', '1.7320508075688772'); print
\"@​x\"; "
1.73205080756888

This happens because of this line in ListUtil.xs​:

sv_setpvf(keysv, "%.15" NVgf, SvNV(arg));

On 'double' builds it should be "%.17" (not "%.15").
On extended precision 'long double' builds it should be "%.21".
On 'quadmath' and IEEE 'long double' builds it should be "%.36".

At least that's the way it looks to me - though I haven't had time to
fully evaluate this. (I think that "%.36" should probably be ok for
*all* builds of perl - though that may not be optimal, and it's also
untested at this stage.)

I can say that replacing "%.15" with "%.21" on the long double build
fixed the failing test.
And when I replace the "%.15" with "%.17", then uniqnum() behaves
correctly on the double builds, too​:

C​:\>perl -MList​::Util -le "@​x =
List​::Util​::uniqnum('1.73205080756888', '1.7320508075688772'); print
\"@​x\"; "
1.73205080756888 1.7320508075688772

I also think that such a fix renders ilmari's pull request
unnecessary.

However, I'm out of time for tonight, and further testing will have to
wait until tomorrow.

When you get back, could you make sure that that gets reported upstream in https://rt.cpan.org/Ticket/Display.html?id=130277 ? That's where this has to be resolved first.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Aug 14, 2019

From @leonerd

On Wed, 14 Aug 2019 08​:18​:24 -0700
"sisyphus@​cpan.org via RT" <perlbug-followup@​perl.org> wrote​:

On 'double' builds it should be "%.17" (not "%.15").
On extended precision 'long double' builds it should be "%.21".
On 'quadmath' and IEEE 'long double' builds it should be "%.36".

This feels the sort of thing crying out for an abstracting macro

--
Paul "LeoNerd" Evans

leonerd@​leonerd.org.uk | https://metacpan.org/author/PEVANS
http​://www.leonerd.org.uk/ | https://www.tindie.com/stores/leonerd/

@p5pRT
Copy link
Author

p5pRT commented Aug 14, 2019

From @jkeenan

On Wed, 14 Aug 2019 15​:18​:24 GMT, sisyphus@​cpan.org wrote​:

On Tue, 13 Aug 2019 12​:25​:00 -0700, jkeenan wrote​:
....

Ilmari has prepared a pull request for PEVANS, the upstream
maintainer of Scalar-List-Utils.

There are, however, also issues with uniqnum() on double builds, too.

For example, the strings '1.73205080756888' and '1.7320508075688772'
numify to different values​:

C​:\>perl -le "print 'ok' if '1.73205080756888' !=
'1.7320508075688772';"
ok

but uniqnum() regards them as the same value​:

C​:\>perl -MList​::Util -le "@​x =
List​::Util​::uniqnum('1.73205080756888', '1.7320508075688772'); print
\"@​x\"; "
1.73205080756888

This happens because of this line in ListUtil.xs​:

sv_setpvf(keysv, "%.15" NVgf, SvNV(arg));

On 'double' builds it should be "%.17" (not "%.15").

Can you clarify what you mean by "'double' builds"? In INSTALL, for instance, I see '-Duselongdouble' but I don't see '-Dusedouble'.

Perhaps provide a 'perl -V' output for such a build?

On extended precision 'long double' builds it should be "%.21".
On 'quadmath' and IEEE 'long double' builds it should be "%.36".

[snip]

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Aug 14, 2019

From @leonerd

On Wed, 14 Aug 2019 21​:36​:41 +0200
"Tomasz Konojacki" <me@​xenu.pl> wrote​:

On Wed, 14 Aug 2019, at 18​:21, Paul "LeoNerd" Evans wrote​:

This feels the sort of thing crying out for an abstracting macro

They exist in standard C since C11​: FLT_DECIMAL_DIG for floats,
DBL_DECIMAL_DIG for doubles and LDBL_DECIMAL_DIG for long doubles.

Oh right - but I meant more having a singly-named macro provided by
Perl that redirects to whatever is the appropriate one for the size
that an NV is; such that you can

  printf("My NV value is " NVgf_ALL_DIGITS "\n", (NV)1.23);

and get whatever is correct for your configured size of NV.

--
Paul "LeoNerd" Evans

leonerd@​leonerd.org.uk | https://metacpan.org/author/PEVANS
http​://www.leonerd.org.uk/ | https://www.tindie.com/stores/leonerd/

@p5pRT
Copy link
Author

p5pRT commented Aug 15, 2019

From @sisyphus

Can you clarify what you mean by "'double' builds"?

I mean any build of perl where the nvtype is double.

Cheers,
Rob


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

@p5pRT
Copy link
Author

p5pRT commented Aug 15, 2019

From @sisyphus

On Thu, Aug 15, 2019 at 2​:14 AM James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

When you get back, could you make sure that that gets reported upstream
in https://rt.cpan.org/Ticket/Display.html?id=130277 ? That's where this
has to be resolved first.

Seems that the test failure has already been fixed on github by enacting
the pull request.
So I've created a separate bug report for this additional shortcoming of
uniq()​:

https://rt.cpan.org/Ticket/Display.html?id=130302

Cheers,
Rob


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

@khwilliamson
Copy link
Contributor

shouldn't this be in the cpan queue for this module?

@hvds
Copy link
Contributor

hvds commented Apr 17, 2022

shouldn't this be in the cpan queue for this module?

The OP does specifically say "Reporting here because of importance of this module within core, but fix
will have to be made upstream first."

I don't know what the current status is though: @jkeenan reported at least one PR related to https://rt-archive.perl.org/perl5/Ticket/Display.html?id=134358 was applied on github, but it isn't clear whether there has been a new release, or if it has been pulled into blead; and @sisyphus appears to mention further test failures not addressed by that PR.

If this module fails its tests with -Duselongdouble, it would be good to get that fixed before 5.36; but I assume our own smokes should be able to tell us that. I guess @leonerd should be the one that decides whether to close this, and perhaps whether it should be a blocker for 5.36.

@jkeenan
Copy link
Contributor

jkeenan commented Apr 17, 2022 via email

@sisyphus
Copy link
Contributor

I think that this issue was fixed long ago.
AFAIK, there's no current issue with t/uniq.t and -Duselongdouble builds.
Is there some evidence to the contrary ?

Cheers,
Rob

@jkeenan
Copy link
Contributor

jkeenan commented Jul 1, 2022

I think that this issue was fixed long ago. AFAIK, there's no current issue with t/uniq.t and -Duselongdouble builds. Is there some evidence to the contrary ?

Cheers, Rob

The matrix isn't currently reporting anything for t/uniq.t. CPANtesters isn't reporting any failures. I can build with -Duselongdouble on FreeBSD and not get any test failures. So this ticket appears closable.

@leonerd, can you review the upstream ticket queue for Scalar-List-Utils and close tickets that have been resolved?

@jkeenan jkeenan closed this as completed Jul 1, 2022
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

6 participants