My understanding is that, as per spec, a += k is rewriten as :

a = cast(typeof(a)) (a + k);

Which make our example looks like :

a = cast(typeof(a)) (a + a++);
a = cast(typeof(a)) (a + { auto olda = a; a++; return olda; }());

The whole damn thing should end up with newa == 2 * olda . This is what SDC does. DMD tells me newa == 2 * olda + 1, as it evaluate a++ before that the value of a for the sum.

I though we decided to do everything LTR, but discussing with Andrei, it seems that Walter oppose this in some special cases. I think we should keep it consistent and do LTR all the way down.

Relevant bug report: https://issues.dlang.org/show_bug.cgi?id=14364

So in SDC there's no difference between a += a and a += a++ ? Damn SDC!

I think DMD does it right. This is commented behavior. And nobody will change that: as there are many cries in bugzilla for example for phobos fixes « no, we cannot change it, it will break existing code ! », so like this nobody will change evaluation order.

On Monday, 30 March 2015 at 20:31:10 UTC, Temtaime wrote:
> I think DMD does it right. This is commented behavior.

This is certainly not. SDC act as per spec. DMD act as per DMD.

But maybe random evaluation order with incorrect documentation is what we want.

I'll say +1 for making everything LTR, as you say.

On 3/30/15 4:12 PM, deadalnix wrote:
> On Monday, 30 March 2015 at 20:31:10 UTC, Temtaime wrote:
>> I think DMD does it right. This is commented behavior.
>
> This is certainly not. SDC act as per spec. DMD act as per DMD.
>
> But maybe random evaluation order with incorrect documentation is what
> we want.

As an aside, we can change the reference to do what we think is right. The principled way is to go by two rules:

1. Use left-to-right wherever there's a choice

2. Express complex operations in terms of lowerings.

So now we have to figure e1 += e2. By rule (2) we should go with this lowering:

e1 += e2

-\>

(ref a, b) { a = cast(typeof(a)) (a + b); }(e1, e2)

By rule (1) we should evaluate e1 before e2 when passing into the lambda. To apply that to i += i++, recall this lowering:

i++

-\>

(ref x) { auto y = x; ++x; return y; }(i)

which takes as to this lowering:

i = i++

-\>

(ref a, b) { a = cast(typeof(a)) (a + b); }( // lambda1
  i,
  (ref x) { auto y = x; ++x; return y; }(i) // lambda2
)

So first we evaluate the address of i (trivial). Then we evaluate lambda2, which increments i before the += lambda1.

So assuming i starts at 5, by the time lambda1 is called i is 6 and b has value 5. Then lambda1 executes, bringing i to 11.

It seems that dmd is correct and we should fix the spec and sdc.


Andrei

On Tuesday, 31 March 2015 at 00:38:25 UTC, Andrei Alexandrescu wrote:
> On 3/30/15 4:12 PM, deadalnix wrote:
>> On Monday, 30 March 2015 at 20:31:10 UTC, Temtaime wrote:
>>> I think DMD does it right. This is commented behavior.
>>
>> This is certainly not. SDC act as per spec. DMD act as per DMD.
>>
>> But maybe random evaluation order with incorrect documentation is what
>> we want.
>
> As an aside, we can change the reference to do what we think is right. The principled way is to go by two rules:
>
> 1. Use left-to-right wherever there's a choice
>
> 2. Express complex operations in terms of lowerings.
>
> So now we have to figure e1 += e2. By rule (2) we should go with this lowering:
>
> e1 += e2
>
> -\>
>
> (ref a, b) { a = cast(typeof(a)) (a + b); }(e1, e2)
>
> By rule (1) we should evaluate e1 before e2 when passing into the lambda. To apply that to i += i++, recall this lowering:
>
> i++
>
> -\>
>
> (ref x) { auto y = x; ++x; return y; }(i)
>
> which takes as to this lowering:
>
> i = i++
>
> -\>
>
> (ref a, b) { a = cast(typeof(a)) (a + b); }( // lambda1
>   i,
>   (ref x) { auto y = x; ++x; return y; }(i) // lambda2
> )
>
> So first we evaluate the address of i (trivial). Then we evaluate lambda2, which increments i before the += lambda1.
>
> So assuming i starts at 5, by the time lambda1 is called i is 6 and b has value 5. Then lambda1 executes, bringing i to 11.
>
> It seems that dmd is correct and we should fix the spec and sdc.
>
>
> Andrei

Why are you doing the replacement top/down rather than bottom up ?

On 3/30/15 5:49 PM, deadalnix wrote:
> Why are you doing the replacement top/down rather than bottom up ?

What would be the alternative? -- Andrei

On Tuesday, 31 March 2015 at 01:01:24 UTC, Andrei Alexandrescu wrote:
> On 3/30/15 5:49 PM, deadalnix wrote:
>> Why are you doing the replacement top/down rather than bottom up ?
>
> What would be the alternative? -- Andrei

Doing the replacement bottom up :

a = a++;

a += { auto olda = a; a = a + 1; return olda; }();

a = cast(typeof(a)) (a + { auto olda = a; a = a + 1; return olda; }());

Other code transformation do not require a function call to take place, so why would this require one (which force eager evaluation of parameters) ?

That is also inconsistent with LTR evaluation. On the other hand, that would make behavior consistent with what opOpAssign would give, so why not.

On 3/30/15 8:49 PM, deadalnix wrote:
> On Tuesday, 31 March 2015 at 01:01:24 UTC, Andrei Alexandrescu wrote:
>> On 3/30/15 5:49 PM, deadalnix wrote:
>>> Why are you doing the replacement top/down rather than bottom up ?
>>
>> What would be the alternative? -- Andrei
>
> Doing the replacement bottom up :
>
> a = a++;
>
> a += { auto olda = a; a = a + 1; return olda; }();
>
> a = cast(typeof(a)) (a + { auto olda = a; a = a + 1; return olda; }());

You need another lowering here because you evaluate a twice. Consider:

import std.stdio;
int a;
ref int fun() { writeln("meh"); return a; }
void main() {
  fun() = i++;
}

> Other code transformation do not require a function call to take place,
> so why would this require one (which force eager evaluation of
> parameters) ?

Everything needs to be evaluated (and only once).

> That is also inconsistent with LTR evaluation.

I don't see how.

> On the other hand, that
> would make behavior consistent with what opOpAssign would give, so why not.

Whatever makes all sleep at night. Could you please submit a PR for the spec with the lowerings discussed?


Andrei

On Tuesday, 31 March 2015 at 06:20:22 UTC, Andrei Alexandrescu wrote:
> On 3/30/15 8:49 PM, deadalnix wrote:
>> On Tuesday, 31 March 2015 at 01:01:24 UTC, Andrei Alexandrescu wrote:
>>> On 3/30/15 5:49 PM, deadalnix wrote:
>>>> Why are you doing the replacement top/down rather than bottom up ?
>>>
>>> What would be the alternative? -- Andrei
>>
>> Doing the replacement bottom up :
>>
>> a = a++;
>>
>> a += { auto olda = a; a = a + 1; return olda; }();
>>
>> a = cast(typeof(a)) (a + { auto olda = a; a = a + 1; return olda; }());
>
> You need another lowering here because you evaluate a twice. Consider:
>
> import std.stdio;
> int a;
> ref int fun() { writeln("meh"); return a; }
> void main() {
>   fun() = i++;
> }
>

Yeah, I had an int in mind. Make it

{
    aPtr = &a;
    *aPtr = cast(typeof(a)) (*aPtr + { auto olda = *aPtr; *aPtr = a + 1; return olda; }());
    return *aPtr;
}()

>> Other code transformation do not require a function call to take place,
>> so why would this require one (which force eager evaluation of
>> parameters) ?
>
> Everything needs to be evaluated (and only once).
>
>> That is also inconsistent with LTR evaluation.
>
> I don't see how.
>

a appear on the left of a++. a should be evaluated before a++. You propose that it isn't.

>> On the other hand, that
>> would make behavior consistent with what opOpAssign would give, so why not.
>
> Whatever makes all sleep at night. Could you please submit a PR for the spec with the lowerings discussed?
>