perl5 icon indicating copy to clipboard operation
perl5 copied to clipboard

Fetch operands of binary operators in left-to-right order

Open t-a-k opened this issue 9 months ago • 9 comments

I noticed that perl emits "Use of uninitialized value ..." warnings in right-to-left order if both operands of a binary operator are uninitialized:

% perl -wle 'my ($a, $b); print $a + $b'
Use of uninitialized value $b in addition (+) at -e line 1.
Use of uninitialized value $a in addition (+) at -e line 1.
0

I think this is not intuitive and inconsistent with Perl's general rule of left-to-right evaluation order (mentioned in perlop).

This change will fix the order of this warning messages by reordering operand fetch.


  • This set of changes requires a perldelta entry, and it is included.

t-a-k avatar Mar 14 '25 15:03 t-a-k

I think this is a reasonable change.

I recommend it be labeled 'defer-next-dev'.

I agree.

tonycoz avatar Mar 17 '25 00:03 tonycoz

I'm not necessarily opposed to this change, but note that it affects more than just the ordering of 'uninit' warnings. Since it changes the ordering of retrieving various ops' args, it will also affect things like the order in which FETCH() is called if both args are tied, and similarly for other magic types. Thus it could change the behaviour of existing code (which would arguably be relying on undefined behaviour).

iabyn avatar Mar 17 '25 09:03 iabyn

Since it changes the ordering of retrieving various ops' args, it will also affect things like the order in which FETCH() is called if both args are tied, and similarly for other magic types.

I think that this is not the case, as this change will only transpose SvXV_nomg() which do not handle magic types, and for these binary operators, magic types (including tied variables) are handled in Perl_try_amagic_bin() (via rpp_try_AMAGIC_2) which already calls FETCH() in left-to-right order.

So, I think that this change should not change the behavior of tied variables.

t-a-k avatar Mar 17 '25 16:03 t-a-k

On Mon, Mar 17, 2025 at 09:02:19AM -0700, TAKAI Kousuke wrote:

I think that this is not the case, as this change will only transpose SvXV_nomg() which do not handle magic types, and for these binary operators, magic types (including tied variables) are handled in Perl_try_amagic_bin() (via rpp_try_AMAGIC_2) which already calls FETCH() in left-to-right order.

So, I think that this change should not change the behavior of tied variables.

Oh yeah, silly me!

-- 31 Dec 1661: "I have newly taken a solemne oath about abstaining from plays". 1 Jan 1662: "And after ... we went by coach to the play". -- The Diary of Samuel Pepys

iabyn avatar Mar 18 '25 08:03 iabyn

So, I think that this change should not change the behavior of tied variables.

It can change the order of "" and 0+ overloading calls.

tonycoz avatar Mar 18 '25 23:03 tonycoz

It can change the order of "" and 0+ overloading calls.

Current implementation seems to handle 0+ overloading calls also in Perl_try_amagic_bin and already call in left-to-right order even before this change:

# test.pl
use v5.10;
use strict;
use warnings;

package Foo {
    use overload
	fallback => 1,
	'0+' => sub { warn "0+ called on ${$_[0]}"; 1 };
    sub new { bless \ (my $a = $_[1]) }
}

my $a = Foo->new('a');
my $b = Foo->new('b');

say '$a + $b = ', $a + $b;
$ perl -wle 'print $^V'
v5.32.1
$ perl test.pl
0+ called on a at test.pl line 9.
0+ called on b at test.pl line 9.
$a + $b = 2

And this patch changes only numeric operators which will not involve "" overloading calls.

t-a-k avatar Mar 21 '25 16:03 t-a-k

Current implementation seems to handle 0+ overloading calls also in Perl_try_amagic_bin and already call in left-to-right order even before this change:

For most cases the non-integer path, ie. when we can't see that both arguments are integers, uses left to right order and there's no change in evaluation ordering.

I did find some cases where it did change - the use integer mode ops:

tony@venus:.../git/perl6$ cat ../23108-over.pl
use warnings;
use strict;
use integer;

my $l = Foo->new(1);
my $r = Foo->new(2);

my $x = $l + $r;
$x = $l * $r;
$x = $l < $r;

package Foo;
use overload '0+' =>
  sub {
    print "over ", $_[0]->$*, "\n";
    $_[0]->$*;
  },
  fallback => 1;

sub new {
  bless \(my $x = $_[1]), $_[0];
}
tony@venus:.../git/perl6$ perl ../23108-over.pl
over 2
over 1
over 2
over 1
over 2
over 1
# built from this PR:
tony@venus:.../git/perl6$ ./perl -Ilib ../23108-over.pl
over 1
over 2
over 1
over 2
over 1
over 2

tonycoz avatar Mar 25 '25 04:03 tonycoz

I did find some cases where it did change - the use integer mode ops: ...

Oh, that makes sense.

I tested test.pl (from my previous comment) with perl v5.41.10 (before applying this patch) which shows that use integer changes the order to call 0+ overloading calls:

$ perl test.pl
0+ called on a at test.pl line 9.
0+ called on b at test.pl line 9.
$a + $b = 2
$ perl -Minteger test.pl
0+ called on b at test.pl line 9.
0+ called on a at test.pl line 9.
$a + $b = 2

I think that this behavior, changing the order of calling 0+ calls depending on use integer, is considered a bug to be fixed.

Anyway, this patch will (slightly) change the behavior of perl, not only changing the output ordering of warnings. I will modify the title and perldelta of this issue.

t-a-k avatar Mar 25 '25 17:03 t-a-k

If it wasn't clear, this is approved, but for 5.43.

tonycoz avatar Mar 25 '25 21:03 tonycoz

@t-a-k t-a-k. Please rebase and repush and it will get put into blead

khwilliamson avatar Jul 04 '25 02:07 khwilliamson

Sorry for delayed response, and thank you for rebasing and comments.

I've force-pushed the branch to:

  • correct typo in comments (s/ensue/ensure/ in pp.c, pp_hot.c)
  • correct typo in commit message (s/thange/change/)
  • reword the perldelta entry
  • rebase again to the current blead

t-a-k avatar Jul 09 '25 16:07 t-a-k