Evalulation order during concat changed

p5pRT commented 6 years ago

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

Searchable as RT133301$

p5pRT commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

p5pRT commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 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 X X X X 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 commented 6 years ago

From @davidnicol

multiconcatdoc_dln_20180823A.patch

```diff 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 X X X X X<.> ```

p5pRT commented 6 years ago

From @davidnicol

Made it shorter:

Inline Patch

```diff --- 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 X X X X X<.> ```

On Thu, Aug 23, 2018 at 1:29 PM David Nicol davidnicol@gmail.com wrote:

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

p5pRT commented 6 years ago

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 commented 6 years ago

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.)

Perl / perl5