Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.9k stars 540 forks source link

BBC: Blead Breaks PDL #22354

Open cjg-cguevara opened 2 months ago

cjg-cguevara commented 2 months ago

This is a bug report for perl from "Carlos Guevara" carlos@carlosguevara.com, generated with the help of perlbug 1.43 running under perl 5.41.1.


BBC: Blead Breaks PDL

Please see http://fast-matrix.cpantesters.org/?dist=PDL%202.089


Flags

Configured by cpan at Fri Jun 28 23:41:57 EDT 2024.

Summary of my perl5 (revision 5 version 41 subversion 1) configuration: Commit id: 99e7291a160c07384076cfdcf5b3126143155452 Platform: osname=linux osvers=5.14.0-427.22.1.el9_4.x86_64 archname=x86_64-linux-thread-multi uname='linux cjg-rhel9 5.14.0-427.22.1.el9_4.x86_64 #1 smp preempt_dynamic mon jun 10 09:23:36 edt 2024 x86_64 x86_64 x86_64 gnulinux ' config_args='-des -Dprefix=/home/cpan/bin/perl -Dscriptdir=/home/cpan/bin/perl/bin -Dusedevel -Duse64bitall -Duseithreads' hint=recommended useposix=true d_sigaction=define useithreads=define usemultiplicity=define use64bitint=define use64bitall=define uselongdouble=undef usemymalloc=n default_inc_excludes_dot=define 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' cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include' ccversion='' gccversion='11.4.1 20231218 (Red Hat 11.4.1-3)' 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='double' nvsize=8 Off_t='off_t' lseeksize=8 alignbytes=8 prototype=define Linker and Libraries: ld='cc' ldflags =' -fstack-protector-strong -L/usr/local/lib' libpth=/usr/local/lib /usr/lib /usr/lib64 /usr/local/lib64 libs=-lpthread -ldb -ldl -lm -lcrypt -lutil -lc perllibs=-lpthread -ldl -lm -lcrypt -lutil -lc libc=/lib/../lib64/libc.so.6 so=so useshrplib=false libperl=libperl.a gnulibc_version='2.34' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=so d_dlsymun=undef ccdlflags='-Wl,-E' cccdlflags='-fPIC' lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'


@INC for perl 5.41.1: /home/cpan/bin/perl/lib/site_perl/5.41.1/x86_64-linux-thread-multi /home/cpan/bin/perl/lib/site_perl/5.41.1 /home/cpan/bin/perl/lib/5.41.1/x86_64-linux-thread-multi /home/cpan/bin/perl/lib/5.41.1


Environment for perl 5.41.1: HOME=/home/cpan LANG=en_US.UTF-8 LANGUAGE (unset) LC_ALL=C LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/cpan/bin/perl/bin:/home/cpan/bin:/usr/share/Modules/bin:/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin PERL_BADLANG (unset) SHELL=/bin/bash

jkeenan commented 2 months ago

Bisecting (which took exactly 1 hour on my fastest machine) with the following invocation:

$ perl Porting/bisect.pl --start=56d5b6ef820802cd4c57fc2cfd8301a3f5d7af04 --end=99e7291a160c07384076cfdcf5b3126143155452 --module=PDL

... pointed to 42cf75238c

42cf75238c3687c1b1fad9263614036f7059c2dc is the first bad commit
commit 42cf75238c3687c1b1fad9263614036f7059c2dc
Author: Leon Timmermans <fawaka@gmail.com>
Date:   Fri Jun 14 01:38:48 2024 +0200

    Simplify global typemap

@Leont, can you take a look? Thanks.

Leont commented 2 months ago

That is quite a surprise.

It seems PDL has its own typemap evaluator that will need to be adapted to this change.

jkeenan commented 2 months ago

That is quite a surprise.

It seems PDL has its own typemap evaluator that will need to be adapted to this change.

Notified upstream of problem: https://github.com/PDLPorters/pdl/issues/487

jkeenan commented 2 months ago

That is quite a surprise.

It seems PDL has its own typemap evaluator that will need to be adapted to this change.

Fortunately, that code in PDL's Basic::Gen::PP was developed relatively recently ...

commit 3cfe4bafeee5fea3582d9d39b38984b805b37d60
Author:     Ed J <mohawk2@users.noreply.github.com>
AuthorDate: Fri Aug 5 03:25:45 2022 +0100
Commit:     Ed J <mohawk2@users.noreply.github.com>
CommitDate: Fri Aug 5 04:39:44 2022 +0100

... so hopefully @mohawk2 will be able to help you out on this.

mohawk2 commented 2 months ago

@Leont Can you guide me on what this change means for PDL? Are you asking me to change the code? Will it work on older EU:PXS? Whose life is better as a result of this "alignment"? When is the new EU:PXS going to be released on CPAN?

I added the "" into that evaluation function because it was necessary to be compatible with EU:PXS typemaps, not out of preference. I couldn't find a method/function I can call in EU:PXS to replace the PDL code. In case it's not obvious, I do not think that this incompatible change is something that PDL should have to adapt for. EDIT Also, even if I did change PDL, older versions of PDL would no longer work on Perl 5.42, which doesn't seem like a great situation to knowingly allow.

mohawk2 commented 2 months ago

@Leont To cut to the chase: please revert your change, and find another way if you must to achieve what you were trying to do. Thanks.

Leont commented 2 months ago

Are you asking me to change the code?

Absolutely. Even if this change is reverted you should change the code (but in a different way); it's broken for output maps because those are already using \a quotes, and hence there are output templates (even in the core set) that use double-quotes freely.

Will it work on older EU:PXS?

I'm not sure exactly what you're asking here, but yes. It would break any typemap abusing quotes to get out of the string, but there's only one typemap on all of CPAN that does such a thing and it's easily fixed.

Whose life is better as a result of this "alignment"?

People who actually have to write typemaps.

Previously, unescaped double quotes would break in highly confusing ways in input maps if you ever used them. So one would have to write stuff like:

if (SvIV($arg) < 0))
  croak(\"Something is wrong\");

It's one of those things that makes writing typemaps really confusing and counter-intuitive. After this change you can just do:

if (SvIV($arg) < 0))
  croak("Something is wrong");

It's extra confusing because output templates have always used the latter syntax.

When is the new EU:PXS going to be released on CPAN?

Not sure. Would you want me to do a trial release now?

mohawk2 commented 2 months ago

@Leont Are you happy to make the PR to PDL that is backwards-compatible with both currently-released and your updated version of EU:PXS? If there's backward compatibility, then EU:PXS's sometimes-rather-slow release cycle stops being a problem. I don't see that a trial release per se will help with this overall issue, but am open to being wrong.

Leont commented 2 months ago

Yeah I will do that.

HaraldJoerg commented 1 month ago

I ran into this issue today and can confirm that with the fix @Leont suggested in https://github.com/PDLPorters/pdl/issues/487#issuecomment-2198754946, PDL tests pass with Perl 5.41.3, and also with my system Perl (5.34). Shall I prepare a PR with that patch and/or run tests with some old Perl version? The change looks harmless enough.

mohawk2 commented 4 weeks ago

I ran into this issue today and can confirm that with the fix @Leont suggested in PDLPorters/pdl#487 (comment), PDL tests pass with Perl 5.41.3, and also with my system Perl (5.34). Shall I prepare a PR with that patch and/or run tests with some old Perl version? The change looks harmless enough.

If it works with existing Perls and existing EU:PXS (which the CI will show), yes, please do.

mohawk2 commented 1 week ago

PDL 2.090 has been released, with @HaraldJoerg's update within it. Please check this against the latest EU:PXS.

jkeenan commented 1 week ago

PDL 2.090 has been released, with @HaraldJoerg's update within it. Please check this against the latest EU:PXS.

PDL 2.090 is doing well on CPANtesters: http://fast-matrix.cpantesters.org/?dist=PDL. I believe we should be able to close this ticket after a couple more days of testing.