Subroutine redefined warnings since Perl 5.35.9, dependent on module loading order

[jkeenan]

(Creating a GH issue from https://www.nntp.perl.org/group/perl.perl5.porters/2022/09/msg264699.html.)

Kenichi Ishigaki wrote to Perl 5 Porters on September 4, 2022:

I have received a bug report that says loading JSON::PP after Cpanel::JSON::XS has started issuing Subroutine redefined warnings (again) since Perl 5.36 (more precisely, 5.35.9).

The report suggests this should be fixed in JSON::PP, and it is possible to add a weird workaround such as this in Cpanel-JSON-XS to JSON-PP, but I am wondering if this is more like a regression in Perl as the warnings reappear only recently.

Any thoughts?

Kenichi, initial thoughts ... On the one hand, these new warnings can be attributed to a change in Perl. On the other hand, you may wish to add a workaround similar to what @rurban did in Cpanel-JSON-XS.

My investigation indicates that these warnings first appeared in this commit:

commit c6874a08f06d60ec8b3f9e21a538a38282910123
Author:     Paul Evans <[email protected]>
AuthorDate: Fri Jan 21 18:37:38 2022 +0000
Commit:     Paul Evans <[email protected]>
CommitDate: Tue Jan 25 15:02:58 2022 +0000

    Fix bundled .pm files for experimental::builtin warnings

...

diff --git a/lib/overload.pm b/lib/overload.pm
index 88e5ecc657..78b72567e4 100644
--- a/lib/overload.pm
+++ b/lib/overload.pm
@@ -2,8 +2,9 @@ package overload;
 
 use strict;
 no strict 'refs';
+no warnings 'experimental::builtin';
 
-our $VERSION = '1.34';
+our $VERSION = '1.35';
 
 our %ops = (
     with_assign         => "+ - * / % ** << >> x .",

This commit actually improved the warnings situation, because if we were to build perl at the previous commit, you would get even murkier warnings. Before and after on FreeBSD:

# c6874a08f06d60ec8b3f9e21a538a38282910123^ FreeBSD
[perlmonger: c6874a08f06d60ec8b3f9e21a538a38282910123^] $ ./bin/perl -Ilib -v | head -2 | tail -1;./bin/perl -Ilib -MCpanel::JSON::XS -E 'say q{hello world cpanel-only};';./bin/perl -Ilib -MJSON::PP -E 'say q{hello world pp-only};';./bin/perl -Ilib -w -e 'use Cpanel::JSON::XS(); use JSON::PP;'
This is perl 5, version 35, subversion 9 (v5.35.9 (v5.35.8-15-g8b1953ca0c)) built for amd64-freebsd-thread-multi
Built-in function 'builtin::blessed' is experimental at /home/jkeenan/testing/c6874a08f06d60ec8b3f9e21a538a38282910123^/lib/perl5/5.35.9/overload.pm line 104.
hello world cpanel-only
Built-in function 'builtin::blessed' is experimental at /home/jkeenan/testing/c6874a08f06d60ec8b3f9e21a538a38282910123^/lib/perl5/5.35.9/overload.pm line 104.
hello world pp-only
Built-in function 'builtin::blessed' is experimental at /home/jkeenan/testing/c6874a08f06d60ec8b3f9e21a538a38282910123^/lib/perl5/5.35.9/overload.pm line 104.

# c6874a08f06d60ec8b3f9e21a538a38282910123 FreeBSD
$ ./bin/perl -Ilib -v | head -2 | tail -1;./bin/perl -Ilib -MCpanel::JSON::XS -E 'say q{hello world cpanel-only};';./bin/perl -Ilib -MJSON::PP -E 'say q{hello world pp-only};';./bin/perl -Ilib -w -e 'use Cpanel::JSON::XS(); use JSON::PP;'
This is perl 5, version 35, subversion 9 (v5.35.9 (v5.35.8-16-gc6874a08f0)) built for amd64-freebsd-thread-multi
hello world cpanel-only
hello world pp-only
Subroutine JSON::PP::Boolean::(++ redefined at /home/jkeenan/testing/c6874a08f06d60ec8b3f9e21a538a38282910123/lib/perl5/5.35.9/overload.pm line 52.
Subroutine JSON::PP::Boolean::(0+ redefined at /home/jkeenan/testing/c6874a08f06d60ec8b3f9e21a538a38282910123/lib/perl5/5.35.9/overload.pm line 52.
Subroutine JSON::PP::Boolean::(-- redefined at /home/jkeenan/testing/c6874a08f06d60ec8b3f9e21a538a38282910123/lib/perl5/5.35.9/overload.pm line 52.

If you were to run a one-liner without warnings at the before and after commits, you would get:

[perlmonger: c6874a08f06d60ec8b3f9e21a538a38282910123^] $ ./bin/perl -Ilib -e 'use Cpanel::JSON::XS(); use JSON::PP;'
Built-in function 'builtin::blessed' is experimental at /home/jkeenan/testing/c6874a08f06d60ec8b3f9e21a538a38282910123^/lib/perl5/5.35.9/overload.pm line 104.
[perlmonger: c6874a08f06d60ec8b3f9e21a538a38282910123^] $ cd -
/home/jkeenan/testing/c6874a08f06d60ec8b3f9e21a538a38282910123
[perlmonger: c6874a08f06d60ec8b3f9e21a538a38282910123] $ ./bin/perl -Ilib -e 'use Cpanel::JSON::XS(); use JSON::PP;'
[perlmonger: c6874a08f06d60ec8b3f9e21a538a38282910123] $ 

So the crucial commit suppressed certain experimental warnings, but (I suspect) earlier commits in this time period created the potential for the warnings which you will only see if you ask for warnings.

@leonerd, can you investigate further?

Thank you very much. Jim Keenan

[jkeenan]

@leonerd, can you take a look at this ticket? The problem appears connected to one of your commits from the last dev cycle. Thanks.

[ntyni]

Hi, could I request some eyeballs on this? It's a clear regression that affects at least MetaCPAN-Client for us, making it warn on plain use MetaCPAN::Client. (This is probably very sensitive to the dependency tree so it may not be easily reproducible with the CPAN toolchain.)

See also https://bugs.debian.org/1019757

Thanks for your work as always!

[jkeenan]

Hi, could I request some eyeballs on this? It's a clear regression that affects at least MetaCPAN-Client for us, making it warn on plain use MetaCPAN::Client. (This is probably very sensitive to the dependency tree so it may not be easily reproducible with the CPAN toolchain.)

See also https://bugs.debian.org/1019757

Thanks for your work as always!

@leonerd, please take a look at this ticket. Thanks.

[haarg]

Previously, overload.pm did not enable or disable any warnings. This meant that it would emit redefined warnings based on the $^W variable or -w argument. Both JSON::PP and Cpanel::JSON::XS would do local $^W before calling overload.pm, silencing these warnings.

Now, overload.pm includes no warnings 'experimental::builtin';. Since it doesn't enable or disable any other warnings, this ends up taking the default warning set, disabling the experimental::builtin warning, and enforcing this across the entire module. This means changes to $^W after loading the module will have no impact. But the "default" warning set that gets applied depends on the current value of $^W. So when run under -w, having most warnings enabled is baked in at compile time, and can't be changed later.

It would be possible for overload.pm to only disable the experimental::builtin warnings in the immediate scope the builtin functions are used. This would restore the old behavior.

But overload.pm should probably apply a consistent set of warnings to itself. If we wanted to keep the old normal behavior (without -w), this would mean silencing the redefinition warnings. I'm not sure if this is sensible.

As for Cpanel::JSON::XS and JSON::PP, they should probably take advantage of overload's unimport method to remove the methods they intend to overwrite.

[charsbar]

@haarg , thank you. Shipped JSON::PP 4.12 with your suggested fix.

[jkeenan]

JSON-PP-4.12 synched into blead via commit 2edec0e0c82cf00befaffdbced3b22aef1eef8d7.

Will monitor for a few days before closing ticket.

[jkeenan]

JSON-PP-4.12 synched into blead via commit 2edec0e.

Will monitor for a few days before closing ticket.

Well, that was more than a few days. No problems reported; closing ticket.