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

Evalulation order during concat changed #16597

Open
p5pRT opened this issue Jun 25, 2018 · 21 comments
Open

Evalulation order during concat changed #16597

p5pRT opened this issue Jun 25, 2018 · 21 comments

Comments

@p5pRT
Copy link

p5pRT commented Jun 25, 2018

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

Searchable as RT133301$

@p5pRT
Copy link
Author

p5pRT commented Jun 25, 2018

From @JRaspass

Created by @JRaspass

This is a bug report for perl from jraspass@​gmail.com,
generated with the help of perlbug 1.40 running under perl 5.26.1.

-----------------------------------------------------------------
I presume this is down to the new multiconcat op, but the following
line has changed behaviour and I couldn't see any mention of it in
the docs.

cat poc.pl; perl poc.pl; perl2 poc.pl
$n=1;print$^V.' '.$n.' '.--$n."\n";
v5.26.1 1 0
v5.28.0 0 0

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.26.1:

Configured by jraspass at Thu Feb 22 12:41:06 UTC 2018.

Summary of my perl5 (revision 5 version 26 subversion 1) configuration:

  Platform:
    osname=linux
    osvers=4.15.4-1-arch
    archname=x86_64-linux
    uname='linux ux-015 4.15.4-1-arch #1 smp preempt sat feb 17 16:01:38
utc 2018 x86_64 gnulinux '
    config_args='-de
-Dprefix=/home/jraspass/perl5/perlbrew/perls/perl-5.26.1 -Dman1dir=none
-Dman3dir=none -Accflags= -DNO_MATHOMS    -DPERL_DISABLE_PMC
 -DSILENT_NO_TAINT_SUPPORT        -march=native -O2 -Aldflags=-march=native
-O2 -Aeval:scriptdir=/home/jraspass/perl5/perlbrew/perls/perl-5.26.1/bin'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=undef
    usemultiplicity=undef
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
    bincompat5005=undef
  Compiler:
    cc='cc'
    ccflags ='-DNO_MATHOMS -DPERL_DISABLE_PMC -DSILENT_NO_TAINT_SUPPORT
-march=native -O2 -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='-DNO_MATHOMS -DPERL_DISABLE_PMC -DSILENT_NO_TAINT_SUPPORT
-march=native -O2 -fwrapv -fno-strict-aliasing -pipe
-fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='7.3.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='double'
    nvsize=8
    Off_t='off_t'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='cc'
    ldflags =' -march=native -O2 -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib
/usr/lib/gcc/x86_64-pc-linux-gnu/7.3.0/include-fixed /usr/lib /lib/../lib
/usr/lib/../lib /lib /lib64 /usr/lib64
    libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc
-lgdbm_compat
    perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.26.so
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.26'
  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'

Locally applied patches:
    Devel::PatchPerl 1.48


@INC for perl 5.26.1:

/home/jraspass/perl5/perlbrew/perls/perl-5.26.1/lib/site_perl/5.26.1/x86_64-linux
    /home/jraspass/perl5/perlbrew/perls/perl-5.26.1/lib/site_perl/5.26.1
    /home/jraspass/perl5/perlbrew/perls/perl-5.26.1/lib/5.26.1/x86_64-linux
    /home/jraspass/perl5/perlbrew/perls/perl-5.26.1/lib/5.26.1


Environment for perl 5.26.1:
    HOME=/home/jraspass
    LANG=en_GB.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)

PATH=/home/jraspass/perl5/perlbrew/bin:/home/jraspass/perl5/perlbrew/perls/perl-5.26.1/bin:/home/jraspass/go/bin:/home/jraspass/.rakudup/install/bin:/home/jraspass/.rakudup/site/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl
    PERLBREW_HOME=/home/jraspass/.perlbrew
    PERLBREW_MANPATH=/home/jraspass/perl5/perlbrew/perls/perl-5.26.1/man

PERLBREW_PATH=/home/jraspass/perl5/perlbrew/bin:/home/jraspass/perl5/perlbrew/perls/perl-5.26.1/bin
    PERLBREW_PERL=perl-5.26.1
    PERLBREW_ROOT=/home/jraspass/perl5/perlbrew
    PERLBREW_SHELLRC_VERSION=0.82
    PERLBREW_VERSION=0.82
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Jun 25, 2018

From @xenu

On Mon, 25 Jun 2018 04​:46​:07 -0700
" \(via RT\)" <perlbug-followup@​perl.org> wrote​:

I presume this is down to the new multiconcat op, but the following
line has changed behaviour and I couldn't see any mention of it in
the docs.

cat poc.pl; perl poc.pl; perl2 poc.pl
$n=1;print$^V.' '.$n.' '.--$n."\n";
v5.26.1 1 0
v5.28.0 0 0

I'm not 100% sure if it's relevant, but perlop says​:

Note that just as in C, Perl doesn't define when the variable is incremented or decremented. You just know it will be done sometime before or after the value is returned. This also means that modifying a variable twice in the same statement will lead to undefined behavior.

@p5pRT
Copy link
Author

p5pRT commented Jun 25, 2018

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

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2018

From @Abigail

On Tue, Jun 26, 2018 at 01​:54​:27AM +0200, Tomasz Konojacki wrote​:

On Mon, 25 Jun 2018 04​:46​:07 -0700
" \(via RT\)" <perlbug-followup@​perl.org> wrote​:

I presume this is down to the new multiconcat op, but the following
line has changed behaviour and I couldn't see any mention of it in
the docs.

cat poc.pl; perl poc.pl; perl2 poc.pl
$n=1;print$^V.' '.$n.' '.--$n."\n";
v5.26.1 1 0
v5.28.0 0 0

I'm not 100% sure if it's relevant, but perlop says​:

Note that just as in C, Perl doesn't define when the variable is incremented or decremented. You just know it will be done sometime before or after the value is returned. This also means that modifying a variable twice in the same statement will lead to undefined behavior.

I'd argue that the 5.28.0 version is more correct than the 5.26.1 one.
While it's undefined what the value of '$n' should be, '--$n' should
be 0, not 1. After all, it should return 1 less than the original value
of $n, which starts out as 1.

Note also that I cannot reproduce this. Running this with various versions
of Perl, I get​:

  #!/opt/perl/bin/perl

  use 5.010;

  use strict;
  use warnings;
  no warnings 'syntax';

  my $n = 1;
  say $^V, " ", $n, " ", -- $n;

  __END__
  v5.18.4 0 0
  v5.20.3 0 0
  v5.22.4 0 0
  v5.24.0 0 0
  v5.26.2 0 0
  v5.28.0 0 0

Abigail

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2018

From @JRaspass

Looks like you used commas rather than periods. But yes I agree, the docs
mention it's undefined behaviour, so anything's fair game. This isn't a bug
:-)

On Tue, 26 Jun 2018, 18​:21 Abigail, <abigail@​abigail.be> wrote​:

On Tue, Jun 26, 2018 at 01​:54​:27AM +0200, Tomasz Konojacki wrote​:

On Mon, 25 Jun 2018 04​:46​:07 -0700
" \(via RT\)" <perlbug-followup@​perl.org> wrote​:

I presume this is down to the new multiconcat op, but the following
line has changed behaviour and I couldn't see any mention of it in
the docs.

cat poc.pl; perl poc.pl; perl2 poc.pl
$n=1;print$^V.' '.$n.' '.--$n."\n";
v5.26.1 1 0
v5.28.0 0 0

I'm not 100% sure if it's relevant, but perlop says​:

Note that just as in C, Perl doesn't define when the variable is
incremented or decremented. You just know it will be done sometime before
or after the value is returned. This also means that modifying a variable
twice in the same statement will lead to undefined behavior.

I'd argue that the 5.28.0 version is more correct than the 5.26.1 one.
While it's undefined what the value of '$n' should be, '--$n' should
be 0, not 1. After all, it should return 1 less than the original value
of $n, which starts out as 1.

Note also that I cannot reproduce this. Running this with various versions
of Perl, I get​:

\#\!/opt/perl/bin/perl

use 5\.010;

use strict;
use warnings;
no  warnings 'syntax';

my $n = 1;
say $^V\, " "\, $n\, " "\, \-\- $n;

\_\_END\_\_
v5\.18\.4 0 0
v5\.20\.3 0 0
v5\.22\.4 0 0
v5\.24\.0 0 0
v5\.26\.2 0 0
v5\.28\.0 0 0

Abigail

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2018

From @Abigail

On Tue, Jun 26, 2018 at 07​:27​:42PM +0200, Abigail wrote​:

On Tue, Jun 26, 2018 at 01​:54​:27AM +0200, Tomasz Konojacki wrote​:

On Mon, 25 Jun 2018 04​:46​:07 -0700
" \(via RT\)" <perlbug-followup@​perl.org> wrote​:

I presume this is down to the new multiconcat op, but the following
line has changed behaviour and I couldn't see any mention of it in
the docs.

cat poc.pl; perl poc.pl; perl2 poc.pl
$n=1;print$^V.' '.$n.' '.--$n."\n";
v5.26.1 1 0
v5.28.0 0 0

I'm not 100% sure if it's relevant, but perlop says​:

Note that just as in C, Perl doesn't define when the variable is incremented or decremented. You just know it will be done sometime before or after the value is returned. This also means that modifying a variable twice in the same statement will lead to undefined behavior.

I'd argue that the 5.28.0 version is more correct than the 5.26.1 one.
While it's undefined what the value of '$n' should be, '--$n' should
be 0, not 1. After all, it should return 1 less than the original value
of $n, which starts out as 1.

And it does. It seems I cannot read.

Note also that I cannot reproduce this. Running this with various versions
of Perl, I get​:

\#\!/opt/perl/bin/perl

use 5\.010;

use strict;
use warnings;
no  warnings 'syntax';

my $n = 1;
say $^V\, " "\, $n\, " "\, \-\- $n;

\_\_END\_\_
v5\.18\.4 0 0
v5\.20\.3 0 0
v5\.22\.4 0 0
v5\.24\.0 0 0
v5\.26\.2 0 0
v5\.28\.0 0 0

And again, I cannot read. The original used '.', not ','. And
that makes all the difference. Replacing the ',' with '.', I do
get

  v5.18.4 1 0
  v5.20.3 1 0
  v5.22.4 1 0
  v5.24.0 1 0
  v5.26.2 1 0
  v5.28.0 0 0

which is a change of behaviour -- but as Tomasz indicated, it's a
change of behaviour which has been defined as being undefined
behaviour.

Abigail

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2018

From @Abigail

On Tue, Jun 26, 2018 at 06​:25​:29PM +0100, James Raspass wrote​:

Looks like you used commas rather than periods.

Yeah, I noticed that half a second after I hit send.
Not half a second before :(

Abigail

@p5pRT
Copy link
Author

p5pRT commented Aug 10, 2018

From wolf-dietrich_moeller@t-online.de

Hi,
I think James got the correct point, that the new OP_MULTICONCAT
changes the order of execution.
Unfortunately he used a in-/decrement to change his variable $n,
so I agree with the comments of others with regard to the
order of storing of in-/decremeted variables.
But this has hidden the real issue.

Please see the following​:

#### test program ####
$e = 'ab'; $f = 'cd'; $x = $e.$f.($f = 'FF');print '$x = '.$x;
#### end test program ####

#### output test program ####
$x = abcdFF #### up to Perl 5.26
$x = abFFFF #### in Perl 5.28.0
#### end output ####

As 'concat' operation is left-associative (see perlop-documentation),
first $e.$f is evaluated. As second step the following bracket
is evaluated, assigning 'FF' to $f. As third step the second
concat is evaluated.

Now in Perl 5.28 with the MULTI_CONCAT this execution model
is no longer valid. Now all inputs to the multiconcat are collected
before the execution of any concatenation, as if the series of concat
operations would be one list operator expecting a list
(but still giving scalar context to the elements in the list).
This means that all concatenations are executed after evaluation of
all arguments (as it is correct for functions in list context).

For me the test program above does not violate any requirements
in the perl documentation, and the result abcdFF should be the
correct result (as up to Perl 5.26).
Thus for me this is an incompatible change to a
series of single concat '.' operations.

Wolf

@p5pRT
Copy link
Author

p5pRT commented Aug 10, 2018

From @iabyn

On Fri, Aug 10, 2018 at 06​:24​:56PM +0200, Wolf-Dietrich Moeller (Munchen) wrote​:

$e = 'ab'; $f = 'cd'; $x = $e.$f.($f = 'FF');print '$x = '.$x;

$x = abcdFF #### up to Perl 5.26
$x = abFFFF #### in Perl 5.28.0

As 'concat' operation is left-associative (see perlop-documentation),
first $e.$f is evaluated. As second step the following bracket
is evaluated, assigning 'FF' to $f. As third step the second
concat is evaluated.

The issue here is what order (if any) does perl guarantee sub-expressions
to be evaluated? For example in

  ($a+$b) * ($c*$d)

does perl guarantee that the first addition will take place before the
second addition? I think the answer is no, although I'm willing to be
proved wrong. If not, then multiconcat has only changed undefined
behaviour.

--
Diplomacy is telling someone to go to hell in such a way that they'll
look forward to the trip

@p5pRT
Copy link
Author

p5pRT commented Aug 10, 2018

From @davidnicol

I don't think it's possible to interpret perldoc perlop's

*Operator precedence* means some operators are evaluated before others. For
example, in 2 + 4 * 5 , the multiplication has higher precedence so 4 * 5 is
evaluated first yielding 2 + 20 == 22 and not 6 * 5 == 30 .

*Operator associativity* defines what happens if a sequence of the same
operators is used one after another​: whether the evaluator will evaluate
the left operations first, or the right first. For example, in 8 - 4 - 2 ,
subtraction is left associative so Perl evaluates the expression left to
right. 8 - 4 is evaluated first making the expression 4 - 2 == 2and not 8 -
2 == 6 .

in a way that allows the right side of a left-associative operation to be
evaluated first and still conform with the documentation.

Dot has always been documented as left associative, making Moeller's result
a bug.

(the rest of this e-mail is half-baked)

How heavy would the modification to the multiconcat code be to allow
"snapshots" that would upgrade to copies of the before state when a thing
gets modified later on, instead of simple aliases? I'm guessing just using
copies everywhere would defeat the purpose of the initiative.

How big of a change is recategorizing Dot as non-associative instead of
left-associative? I think huge, in that any code written to rely on the
associativity will not work after upgrade.

Here's a reasonable (contrived for this message, but reasonable) use which
relies on documented evaluation order, and would woefully break (or
possibly not, as postincrement already returns a copy not a lvalue)

my $line_number = 1;

for my $thing (@​ListOfThingsThatTakeFourLinesToOutput){

  # all the line methods return lines ending with newlines.

  print $line_number++.' '.$thing->first_line()

  .$line_number++.' '.$thing->second_line()

  .$line_number++.' '.$thing->third_line()

  .$line_number++.' '.$thing->fourth_line();

}

On Fri, Aug 10, 2018 at 12​:56 PM Dave Mitchell <davem@​iabyn.com> wrote​:

The issue here is what order (if any) does perl guarantee sub-expressions
to be evaluated? For example in

\($a\+$b\) \* \($c\+$d\)

does perl guarantee that the first addition will take place before the
second addition? I think the answer is no, although I'm willing to be
proved wrong. If not, then multiconcat has only changed undefined
behaviour.

--
Diplomacy is telling someone to go to hell in such a way that they'll
look forward to the trip

--
"At this point, given the limited available data, certainty about only a
very small number of things can be achieved." -- Plato, and others

@p5pRT
Copy link
Author

p5pRT commented Aug 10, 2018

From @Grinnz

On Fri, Aug 10, 2018 at 4​:08 PM David Nicol <davidnicol@​gmail.com> wrote​:

How heavy would the modification to the multiconcat code be to allow
"snapshots" that would upgrade to copies of the before state when a thing
gets modified later on, instead of simple aliases? I'm guessing just using
copies everywhere would defeat the purpose of the initiative.

How big of a change is recategorizing Dot as non-associative instead of
left-associative? I think huge, in that any code written to rely on the
associativity will not work after upgrade.

There's a third option, which is to not apply the optimization (or apply
the copying behavior) when operands of the concatenation are not simple
scalars.

-Dan

@p5pRT
Copy link
Author

p5pRT commented Aug 10, 2018

From Eirik-Berg.Hanssen@allverden.no

On Fri, Aug 10, 2018 at 10​:08 PM David Nicol <davidnicol@​gmail.com> wrote​:

I don't think it's possible to interpret perldoc perlop's

*Operator precedence* means some operators are evaluated before others.
For example, in 2 + 4 * 5 , the multiplication has higher precedence so 4
* 5 is evaluated first yielding 2 + 20 == 22 and not 6 * 5 == 30 .

*Operator associativity* defines what happens if a sequence of the same
operators is used one after another​: whether the evaluator will evaluate
the left operations first, or the right first. For example, in 8 - 4 - 2 ,
subtraction is left associative so Perl evaluates the expression left to
right. 8 - 4 is evaluated first making the expression 4 - 2 == 2and not 8
- 2 == 6 .

in a way that allows the right side of a left-associative operation to be
evaluated first and still conform with the documentation.

  Oh, yes it is​: The docs for associativity here speak of the order in
which the ("same-operator") _operations_ will be evaluated – not the order
in which their _operands_ will be evaluated.

  In C<< $e.$f.($f = 'FF') >>, the left-most concatenation is, per the
associativity docs, evaluated before the right-most concatenation. But the
evaluation order of the _operands_ of the right-most concatenation – that
is, whether or not the left-most concatenation is evaluated before the
assignment – is not specified in the quoted part of the docs. And that is
the change in question; it may be fair to criticise it, but I don't think
it is fair to suggest it doesn't conform with this documentation.

  If I recall correctly (and it hasn't changed since last this topic was
raised), the assignment, short-cutting (logical), and comma are the only
operators that specify an (operand) evaluation order. Every other operator
leaves the (operand) evaluation order unspecified.

Eirik

@p5pRT
Copy link
Author

p5pRT commented Aug 11, 2018

From @sisyphus

On Fri, 10 Aug 2018 14​:21​:02 -0700, Eirik-Berg.Hanssen@​allverden.no wrote​:

Oh, yes it is​: The docs for associativity here speak of the order in
which the ("same-operator") _operations_ will be evaluated – not the order
in which their _operands_ will be evaluated.

Which would mean that for $x=8, $y=4, the expression "$x - $y - ($y=2)" could acceptably evaluate to 4 ?

If the docs don't already specify the order in which the operands will be evaluated, then perhaps they ought to - even if it's just to state that the order is undefined.

Cheers,
Rob

@p5pRT
Copy link
Author

p5pRT commented Aug 11, 2018

From wolf-dietrich_moeller@t-online.de

Resent because of problems. Sorry

-----Original Message-----
From​: Wolf-Dietrich Moeller (München) [mailto​:wolf-dietrich_moeller@​t-online.de]
Sent​: Samstag, 11. August 2018 10​:34
To​: 'perlbug-followup@​perl.org'
Cc​: 'Dave Mitchell'; 'Eirik Berg Hanssen'; 'David Nicol'; 'sisyphus@​cpan.org'; 'Dan Book'
Subject​: RE​: [perl #133301] Evalulation order during concat changed

David cited the old perlop doc (up to Perl 5.26). This documentation
was changed in Perl 5.28 and the section on operator precedence and
associativity was extended quite a lot (see [perl #127391]).

One sentence there reads​:
"In fact Perl has a general rule that the operands of an operator
are evaluated in left-to-right order."

Eirik, did you consider this new text?

Wolf

@p5pRT
Copy link
Author

p5pRT commented Aug 11, 2018

From Eirik-Berg.Hanssen@allverden.no

On Sat, Aug 11, 2018 at 12​:58 PM Wolf-Dietrich Moeller (München) <
wolf-dietrich_moeller@​t-online.de> wrote​:

Resent because of problems. Sorry

-----Original Message-----
From​: Wolf-Dietrich Moeller (München) [mailto​:
wolf-dietrich_moeller@​t-online.de]
Sent​: Samstag, 11. August 2018 10​:34
To​: 'perlbug-followup@​perl.org'
Cc​: 'Dave Mitchell'; 'Eirik Berg Hanssen'; 'David Nicol'; '
sisyphus@​cpan.org'; 'Dan Book'
Subject​: RE​: [perl #133301] Evalulation order during concat changed

David cited the old perlop doc (up to Perl 5.26). This documentation
was changed in Perl 5.28 and the section on operator precedence and
associativity was extended quite a lot (see [perl #127391]).

One sentence there reads​:
"In fact Perl has a general rule that the operands of an operator
are evaluated in left-to-right order."

Eirik, did you consider this new text?

  No, sorry; I was unaware of it.

  That might be a game changer.

  Or not; the new text is still pretty vague, and don't strictly match the
behaviour of perl. I also don't see any discussion of this part of the
patch in the RT ticket ...

  Reading that ticket, I get the impression of a failure to properly
distinguish between (operand) evaluation order and the order in which
_operations_ are evaluated. I'm sorry I didn't catch that at the time​: I
surely would have quibbled.

  This example demonstrates that the left-hand operand is evaluated _after_
the right-hand operand, in violation of this "general rule"​:

eirik@​greencat[15​:26​:12]$ perl -E 'say for ($x=1 + say "LHS") = ($x=2 +
say "RHS")'
RHS
LHS
2
eirik@​greencat[15​:27​:56]
$

  ... assuming that did not also change? (I don't have a new perl here;
currently stuck on vanilla Ubuntu 18.04.1 LTS and it system perl v5.26.1.)

Eirik

@p5pRT
Copy link
Author

p5pRT commented Aug 11, 2018

From @iabyn

On Sat, Aug 11, 2018 at 03​:36​:15PM +0200, Eirik Berg Hanssen wrote​:

This example demonstrates that the left-hand operand is evaluated _after_
the right-hand operand, in violation of this "general rule"​:

There's also an example just with a concat mutator, where the order of
FETCH()es of tied arg changes depending on what the args are. This
ordering is unaffected by the introduction of multiconcat​:

  sub TIESCALAR { bless [ $_[1] ]; }
  sub FETCH { my $s = $_[0][0]; print "FETCH($s)\n"; $s }
  sub STORE { print "STORE($_[0][0] => $_[1])\n"; $_[0][0] = $_[1] }

  my ($a,$b,$c);
  tie $a, 'main', 'A';
  tie $b, 'main', 'B';
  tie $c, 'main', 'C';

  $a .= $b;
  #$a .= $b . $c;

with $a .= $b, you get​:

  FETCH(A)
  FETCH(B)
  STORE(A => AB)

with $a .= $b . $c you get

  FETCH(B)
  FETCH(C)
  FETCH(A)
  STORE(A => ABC)

Note how it changes from FETCHing the LHS first to the RHS first.

This isn't an example of ordering of operand execution inconsistency,
but of fetching the results of such executions. It none the less
demonstrates the danger of relying on an assumed order.

--
"You may not work around any technical limitations in the software"
  -- Windows Vista license

@p5pRT
Copy link
Author

p5pRT commented Aug 23, 2018

From @davidnicol

On Sat, Aug 11, 2018 at 10​:40 AM Dave Mitchell <davem@​iabyn.com> wrote​:

This isn't an example of ordering of operand execution inconsistency,
but of fetching the results of such executions. It none the less
demonstrates the danger of relying on an assumed order.

Attached, and here in-line, is a revised proposed patch to the
documentation, about the behavior change. It might be too long, as it seems
to state everything twice.

Inline Patch
diff -r 67f5ed179113 pod/perlop.pod
--- a/pod/perlop.pod    Mon Aug 13 11:57:05 2018 -0500
+++ b/pod/perlop.pod    Thu Aug 23 13:18:07 2018 -0500
@@ -389,6 +389,16 @@
 X<->

 Binary C<"."> concatenates two strings.
+
+As of version 5.28, a series of terms joined by  C<"."> operators are
+evaluated prior to all getting concatenated, rather than the first
+two getting joined, followed by each additional in its own binary
+operation. This affects the result when the terms return aliases that
+get modified in later terms, such as preincremented scalars. Prior to
+version 5.28, aliasing issues only affected the first term in a series
+of concatenations, as the left side of the second and later operations
+was the unaliased result of the previous binary operation.
+
 X<string, concatenation> X<concatenation>
 X<cat> X<concat> X<concatenate> X<.>


-- 

"At this point, given the limited available data, certainty about only a
very small number of things can be achieved." -- Plato, and others

@p5pRT
Copy link
Author

p5pRT commented Aug 23, 2018

From @davidnicol

multiconcatdoc_dln_20180823A.patch
diff -r 67f5ed179113 pod/perlop.pod
--- a/pod/perlop.pod	Mon Aug 13 11:57:05 2018 -0500
+++ b/pod/perlop.pod	Thu Aug 23 13:19:01 2018 -0500
@@ -389,6 +389,16 @@
 X<->
 
 Binary C<"."> concatenates two strings.
+
+As of version 5.28, a series of terms joined by  C<"."> operators are
+evaluated prior to all getting concatenated, rather than the first
+two getting joined, followed by each additional in its own binary
+operation. This affects the result when the terms return aliases that
+get modified in later terms, such as preincremented scalars. Prior to
+version 5.28, aliasing issues only affected the first term in a series
+of concatenations, as the left side of the second and later operations
+was the unaliased result of the previous binary operation.
+
 X<string, concatenation> X<concatenation>
 X<cat> X<concat> X<concatenate> X<.>
 

@p5pRT
Copy link
Author

p5pRT commented Aug 23, 2018

From @davidnicol

Made it shorter​:

Inline Patch
--- a/pod/perlop.pod    Mon Aug 13 11:57:05 2018 -0500
+++ b/pod/perlop.pod    Thu Aug 23 14:35:46 2018 -0500
@@ -389,6 +389,13 @@
 X<->

 Binary C<"."> concatenates two strings.
+
+A series of terms joined by  C<"."> operators are evaluated prior to
+all getting concatenated in a single operation.  Prior to version 5.28,
+aliasing issues only affected the value of the first term in a series
+of concatenations, as the left side of the second and later operations
+was the intermediate result of the previous binary operation.
+
 X<string, concatenation> X<concatenation>
 X<cat> X<concat> X<concatenate> X<.>


On Thu, Aug 23, 2018 at 1:29 PM David Nicol wrote:

It might be too long, as it seems to state everything twice.

@p5pRT
Copy link
Author

p5pRT commented Aug 24, 2018

From @iabyn

On Thu, Aug 23, 2018 at 02​:39​:08PM -0500, David Nicol wrote​:

Binary C<"."> concatenates two strings.
+
+A series of terms joined by C<"."> operators are evaluated prior to
+all getting concatenated in a single operation. Prior to version 5.28,
+aliasing issues only affected the value of the first term in a series
+of concatenations, as the left side of the second and later operations
+was the intermediate result of the previous binary operation.

What this documentation patch does, is change what was undefined
behaviour (or at least that was my feeling of where the discussion ended
up) into defined behaviour, which we are now committed to support.

I don't want the internal (and possibly changeable) details of an
optimisation to become a straight jacket.

But the actual details of the ordering are much more complicated than
implied by the text of your patch. For example in 5.28.0 onwards, which
order do you think these functions are called and the concatenations done​:

  ( f1() . f2() ) . ( f3() . f4() )

The order is actually equivalent to

  $tmp1 = f1();
  $tmp2 = f2();
  $tmp3 = f3();
  $tmp4 = f4();
  $tmp5 = $tmp3 . $tmp4;
  $result = $tmp1 . $tmp2 . $tmp5;

Would you have predicted that? Does your text describe that? Could you
come up with some text that could always describe what happens.

In conclusion, I don't think the pod for the concatenation op should
specify anything about order. I think that we should however fix the text
recently added to perlop which implied a guarantee about the ordering of
arg evaluation in general which we shouldn't have given.

--
"I used to be with it, but then they changed what ‘it’ was, and now what
I’m with isn’t it. And what’s ‘it’ seems weird and scary to me."
  -- Grandpa Simpson
(It will happen to you too.)

@p5pRT
Copy link
Author

p5pRT commented Aug 24, 2018

From @xsawyerx

On 08/24/2018 11​:38 AM, Dave Mitchell wrote​:

On Thu, Aug 23, 2018 at 02​:39​:08PM -0500, David Nicol wrote​:

Binary C<"."> concatenates two strings.
+
+A series of terms joined by C<"."> operators are evaluated prior to
+all getting concatenated in a single operation. Prior to version 5.28,
+aliasing issues only affected the value of the first term in a series
+of concatenations, as the left side of the second and later operations
+was the intermediate result of the previous binary operation.
What this documentation patch does, is change what was undefined
behaviour (or at least that was my feeling of where the discussion ended
up) into defined behaviour, which we are now committed to support.

I don't want the internal (and possibly changeable) details of an
optimisation to become a straight jacket.

[...]

In conclusion, I don't think the pod for the concatenation op should
specify anything about order. I think that we should however fix the text
recently added to perlop which implied a guarantee about the ordering of
arg evaluation in general which we shouldn't have given.

The documentation could specify the lack of guarantee. That would remove
any possible ambiguity about whether it's defined or not.

"The order in which concatenated expressions is undefined and should not
be relied upon."

(Or some better phrasing.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants