You can also reproduce this by pulling the latest automake code, building it and running automake.

curl -O https://ftp.gnu.org/gnu/automake/automake-1.17.tar.gz
cd automake*
./configure
make -j10
make install
automake

If it is intended for !!$val to be used as the integer 0 or 1 to be compared to $neg, this warning could be avoided by making that explicit: 0+!!$val == $neg

The purpose of the warning is to call out a common mistake where !$x == $y is written instead of !($x == $y) - but warnings just produce STDERR output, is it causing some other issue?

If it is intended for !!$val to be used as the integer 0 or 1 to be compared to $neg, this warning could be avoided by making that explicit: 0+!!$val == $neg

The purpose of the warning is to call out a common mistake where !$x == $y is written instead of !($x == $y) - but warnings just produce STDERR output, is it causing some other issue?

Yes, in automake case it's causing automake to exit with rc of 255

 ./bin/automake-1.17 ; echo $?
Possible precedence problem between ! and numeric eq (==) at ./bin/automake-1.17 line 6875.
255

Ah they have: use warnings FATAL => 'all';

Using fatal warnings is an assumption of risk of breakage: https://perldoc.perl.org/warnings#Fatal-Warnings

my $neg = ($1 eq '!') ? 1 : 0;
my $val = transform_token($token, $transform, $2);
return (!!$val == $neg) ? '##%' : '';

This code is bizarre. It uses ? 1 : 0 in the initialization of $neg where it is unnecessary (since eq already returns a boolean), but does not use it for $val where it would make sense (since transform_token returns a string).

That is,

my $neg = $1 eq '!';
my $val = transform_token($token, $transform, $2) ? 1 : 0;
return ($val == $neg) ? '##%' : '';

would do the right thing and not trigger a warning.

(Alternatively, parenthesizing the last expression as return (!!$val) == $neg ? '##%' : '' would also avoid the warning.)

That said, the warning already has a list of exceptions where it doesn't trigger:

• (!...) == ... (LHS parenthesized) does not warn
• !... == !... (negation on both LHS and RHS) does not warn

It would be fairly easy to add !!... == ... (double negation on LHS) to the list of exceptions. Is this something we want?

I think that would risk failing to catch instances where !!($x == $y) was accidentally written without the parentheses, as that is rather similar to the original case it is meant to catch, and that exception should only be added if automake isn't reasonably able to work around the warning; and if that is the case, they should be strongly recommended not to use fatal warnings as this issue will occur again.

I'm also seeing a similar case in texinfo (another GNU package):

makeinfo
Possible precedence problem between ! and string eq at /home/itodoro/zopen/usr/local/zopen/texinfo/texinfo/share/texinfo/Texinfo/Convert/Converter.pm line 1092.
Possible precedence problem between ! and string eq at /home/itodoro/zopen/usr/local/zopen/texinfo/texinfo/share/texinfo/Texinfo/Convert/Info.pm line 92.
makeinfo: missing file argument.
Try `makeinfo --help' for more information.

/home/itodoro/zopen/usr/local/zopen/texinfo/texinfo/share/texinfo/Texinfo/Convert/Converter.pm line 1092.

1091   my $fh;
1092   if (! $self->{'output_file'} eq '') {
1093     $fh = $self->Texinfo::Common::open_out($self->{'output_file'});
1094     if (!$fh) {
1095       $self->document_error(sprintf(__("could not open %s for writing: %s"),
1096                                     $self->{'output_file'}, $!));
1097       return undef;
1098     }
1099   }

/home/itodoro/zopen/usr/local/zopen/texinfo/texinfo/share/texinfo/Texinfo/Convert/Info.pm line 92.

 90
 91   my $fh;
 92   if (! $self->{'output_file'} eq '') {
 93     if ($self->get_conf('VERBOSE')) {
 94       print STDERR "Output file $self->{'output_file'}\n";
 95     }
 96     $fh = _open_info_file($self, $self->{'output_file'});
 97     if (!$fh) {
 98       return undef;
 99     }
100   }

Curl fails at make check with Perl 5.41.7 due to this as well:

make[2]: Leaving directory '/jenkins/workspace/Port-Build/curl/curl-8.11.1/tests'
srcdir=. /jenkins/zopen/usr/local/zopen/perl/perl5-heads.v5.41.7.20250123_102326.zos/bin/perl -I. ./runtests.pl   -a -s 
Possible precedence problem between ! and string eq at ./runtests.pl line 1691.
make[1]: *** [Makefile:898: quiet-test] Error 129
make[1]: Leaving directory '/jenkins/workspace/Port-Build/curl/curl-8.11.1/tests'

I have sent an email to [email protected] <[email protected]> to report the problem.

I'm also seeing a similar case in texinfo (another GNU package):

makeinfo
Possible precedence problem between ! and string eq at /home/itodoro/zopen/usr/local/zopen/texinfo/texinfo/share/texinfo/Texinfo/Convert/Converter.pm line 1092.
Possible precedence problem between ! and string eq at /home/itodoro/zopen/usr/local/zopen/texinfo/texinfo/share/texinfo/Texinfo/Convert/Info.pm line 92.
makeinfo: missing file argument.
Try `makeinfo --help' for more information.

/home/itodoro/zopen/usr/local/zopen/texinfo/texinfo/share/texinfo/Texinfo/Convert/Converter.pm line 1092.

1091   my $fh;
1092   if (! $self->{'output_file'} eq '') {
1093     $fh = $self->Texinfo::Common::open_out($self->{'output_file'});
1094     if (!$fh) {
1095       $self->document_error(sprintf(__("could not open %s for writing: %s"),
1096                                     $self->{'output_file'}, $!));
1097       return undef;
1098     }
1099   }

/home/itodoro/zopen/usr/local/zopen/texinfo/texinfo/share/texinfo/Texinfo/Convert/Info.pm line 92.

 90
 91   my $fh;
 92   if (! $self->{'output_file'} eq '') {
 93     if ($self->get_conf('VERBOSE')) {
 94       print STDERR "Output file $self->{'output_file'}\n";
 95     }
 96     $fh = _open_info_file($self, $self->{'output_file'});
 97     if (!$fh) {
 98       return undef;
 99     }
100   }

These are actual logical errors which the warning has pointed out; the code is attempting to check whether $self->{output_file} is non-empty, but instead checks whether $self->{output_file} is true (via checking whether its boolean negation is equal to the empty string).

I'm also seeing a similar case in texinfo (another GNU package):

makeinfo
Possible precedence problem between ! and string eq at /home/itodoro/zopen/usr/local/zopen/texinfo/texinfo/share/texinfo/Texinfo/Convert/Converter.pm line 1092.
Possible precedence problem between ! and string eq at /home/itodoro/zopen/usr/local/zopen/texinfo/texinfo/share/texinfo/Texinfo/Convert/Info.pm line 92.
makeinfo: missing file argument.
Try `makeinfo --help' for more information.

/home/itodoro/zopen/usr/local/zopen/texinfo/texinfo/share/texinfo/Texinfo/Convert/Converter.pm line 1092.

1091   my $fh;
1092   if (! $self->{'output_file'} eq '') {
1093     $fh = $self->Texinfo::Common::open_out($self->{'output_file'});
1094     if (!$fh) {
1095       $self->document_error(sprintf(__("could not open %s for writing: %s"),
1096                                     $self->{'output_file'}, $!));
1097       return undef;
1098     }
1099   }

/home/itodoro/zopen/usr/local/zopen/texinfo/texinfo/share/texinfo/Texinfo/Convert/Info.pm line 92.

 90
 91   my $fh;
 92   if (! $self->{'output_file'} eq '') {
 93     if ($self->get_conf('VERBOSE')) {
 94       print STDERR "Output file $self->{'output_file'}\n";
 95     }
 96     $fh = _open_info_file($self, $self->{'output_file'});
 97     if (!$fh) {
 98       return undef;
 99     }
100   }

These are actual logical errors which the warning has pointed out; the code is attempting to check whether $self->{output_file} is non-empty, but instead checks whether $self->{output_file} is true (via checking whether its boolean negation is equal to the empty string).

Should these warnings have been introduced in a minor or major release rather than a patch release? 5.41.3 did not emit these warnings. I've seen 3 popular open source tools break as a result of these changes.

As mentioned, these warnings emit only STDERR in supported operation. If these utilities are using fatal warnings, they have accepted the contract that they must update their code whenever new warnings are introduced. This allows new warnings to be introduced, as otherwise Perl's devotion to backwards compatibility would make it near impossible to do so, major version or not.

In addition, this is not a patch release, this is a development release. These warnings will not be introduced to a stable version of Perl until 5.42.0.

Ok, thanks I have reported the issue to the curl community: curl/curl#16128

On Thu, 30 Jan 2025, 07:13 mauke, @.***> wrote:
my $neg = ($1 eq '!') ? 1 : 0;my $val = transform_token($token, $transform, $2);return (!!$val == $neg) ? '##%' : '';

This code is bizarre.
I dont agree it's bizarre. The ternary is superluous, but it's not that
weird or unusual for people to write code like this in the interest of
clarity.

It would be fairly easy to add !!... == ... (double negation on LHS) to the
list of exceptions. Is this something we want?
Imo yes. !!$x is a standard "operator" to booleanify a var. (Book published
a huge list of such "operators" years ago.) Using it with == should not be
a warning.

So is !, and adding that exception would make this warning do nothing, so I don't agree with this logic.

I dont agree it's bizarre. The ternary is superluous, but it's not that
weird or unusual for people to write code like this in the interest of
clarity.

My issue is not that the ternary is superfluous. It's that they go out of their way to manually convert a boolean to an integer in one case ($1 eq '!') ? 1 : 0, but in the other case they simply slap a !! on the variable and call it a day.

If they had used ? 1 : 0 consistently on both sides (using == to compare two integers), they wouldn't get a warning. If they had used !! consistently on both sides (using == to implicitly numify the booleans), they wouldn't get a warning.

To put it another way, one side of the equation is natively a boolean ($1 eq '!'), the other a string (transform_token(...)). Those are different types and some people like to be explicit about type conversions, sure. But here the first value is explicitly converted from a boolean to an integer, and the second value from a string to a boolean. In the end, we still have different types and must rely on == to perform an implicit conversion. This feels bizarre to me and I don't see how doing things this way makes anything clearer.

(Also, I'm not familiar with the context of the code, but boolifying a string treats '0' and '' as the same value. I'm not sure if this is intentional.)

Content Warning: Light hearted pedantry

The word "regression" in the title of this ticket is not appropriate to the situation. This is a new (fatal) warning from a new version of perl. So, gression-wise, I'd say it's progression, not regression.

I'd agree the pure idiomatic form is clearer. But that isn't really
relevant, the code is still functionally correct , false is silently
equivalent to 0 in numeric context.

I agree. Generally speaking false positives we really want to avoid in warnings. It doesn't take that many for people to turn that warning off, and then we're in an even worse position.

I have some thoughts.

• Fatalizing all warnings is a bad idea; doing so in production code is just asking for trouble. That's automake's problem.

• However, there is a more general issue here. I agree that false positives are bad in general and should be avoided where reasonably possible.

• Only the automake case is a false positive. The texinfo and curl cases are true positives: The warning has found real bugs.

• I tried to search CPAN for instances of !! being used in suspicious ways. I only found two results:

  • Alien::Build::Plugin::Fetch::Git:

    $can_minus_c = !! $? == 0 && -d $tmp->child('.git');

    I'm not sure what's going on here. You could call this a false positive because the code produces correct results, but !! $? == 0 leaves me scratching my head. I can't tell what the author's intent was and I think the !! should just be removed.
  • Device::BusPirate::Mode::SPI:

    if( any { defined $args{$_} and !!$args{$_} != $self->$_ } qw( open_drain ckp cke sample ) ) {

    This is a genuine false positive. Avoiding the warning would be simple (by writing $args{$_} ^^ $self->$_ or !!$args{$_} != !!$self->$_ or (!!$args{$_}) != $self->$_), but I count it as a point towards making !! exempt from the warning.

  If you have more examples, please let me know.

• As for the automake code not being bizarre: I don't follow Yves' reasoning. Sure, I can see how code might end up in this shape due to inattention or multiple developers of different experience levels working on it, but that wasn't the point. The point is that the end result is incoherent code.

• As for treating '0' and '' as the same value being intentional because otherwise people wouldn't write !! in their code: I don't buy it. People write sloppy code all the time. That doesn't mean they fully understand the ramifications of writing a check a particular way. (For example, the texinfo case above doesn't use !!, but it is a bug that doesn't let you use 0 as the output filename.)

• This kind of "change your code because we don't approve of your coding
  style" is something we long avoided, I dont see why that should change
  now.

  That ... is not something I would agree with. Consider this warning from perl 4.0.33:

  $ perl -wE 'say (":-)");'
  say (...) interpreted as function at -e line 1.
  :-)
  

  Or this one from perl 5.000:

  $ perl -we 'use Carp qw(croak); exec "echo ok"; croak "Could not exec: $!"'
  Statement unlikely to be reached at -e line 1.
          (Maybe you meant system() when you said exec()?)
  ok
  

  Or these from perl 5.003_08:

  $ perl -we 'my @c = qw( ! ? . , ; # );'
  Possible attempt to separate words with commas at -e line 1.
  Possible attempt to put comments in qw() list at -e line 1.
  

  These are all false positives, enforcing a particular "coding style". (In particular, the qw ones have always annoyed me. They've never identified a single bug in my code. Can we patch them out?)

On the whole, I'll err on the side of caution and prepare a patch that exempts !! from the warning because of code like the one in Device::BusPirate. (More examples welcome.)

The warning itself should stay, I think. It has identified real bugs in high-profile projects such as perl's Test2 suite, texinfo, and curl.

+1 to what @mauke said (and not just because he used one of my modules as a counterexample ;) )

The new warning is providing valuable feedback on genuinely suspect code. The false positives are an annoying distraction to it, which the PR fixes (and I have reviewed and given it a thumbs up).

Also agree that anyone running fatalized warnings in a production system is skating on very thin ice. They have asked for their code to randomly start crashing out on warnings from future versions of Perl, so that is what they shall receive.