improve definition of assign expression behaviour

[Alexibu]

The spec appears to not specify that an assign expression must be evaluated after it's operands.
It also helps to define that it must do the same thing as an asignment of the binary operator.

I believe this is the intention of the language, so that it has the same operator behaviour as C++.

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97843

[dlang-bot]

Thanks for your pull request and interest in making D better, @Alexibu! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

[PetarKirov]

@ibuclaw @kinke @WalterBright do you guys agree?

[thewilsonator]

what if a has side-effects?

[PetarKirov]

Isn't that the point? Whether or not a has side effects, it would be evaluated once:

exp1 op= exp2;

=>

{
    auto tmp1 = exp1;
    auto tmp2 = exp2;
    tmp1 = tmp1 op tmp2;
}

That is, unless exp1 resolves to a type with custom opAssign, or opOpAssign.

[ibuclaw]

what if a has side-effects?

Then in most cases, a can't be an lvalue in an op= expression, unless the result of a is a stable reference (iirc, *a op= b)

[PetarKirov]

@thewilsonator I suppose this would be a more accurate wording:

Suggested change

	is exactly the same as:
	is rewritten to:

Which would address the question of side effects.

[thewilsonator]

Is it though? I thought it would be rewritten to auto [ref] __tmp = a; __tmp = __tmp op b;, the number of evaluation is important.

[PetarKirov]

Ah, I thought that was clear. Yeah, I guess it makes sense to specify this more accurately.

[ibuclaw]

One exception to that rewrite though is array concatenation.

a ~= b
// a = a ~ b;

Because evaluating a changes its underlying size, so it would instead be conceptually

auto __tmp = b
extend(&a)[oldlen .. newlen] = __tmp;

Unless I'm failing to see a disconnect between the evaluation of a and extending its size for the concatenation.

[Alexibu]

Why does evaluating a change its size ? Isn't the size change part of the execution of ~=, not the evaluation of operands ?

[ibuclaw]

Because in a op= b, operand a can only be evaluated once. Separating the size change from the evaluation means you'll have to compute it twice.

[kinke]

Is it though?

Nope, this is definitely wrong, see e.g. https://github.com/dlang/dmd/blob/a9e5b0db723f8edaf80df28f93257edf06d3f17b/test/runnable/evalorder.d#L59-L61.

[Alexibu]

I translated that test for eval order to the ~= operator:

auto mul11ret3(T)(ref T s)
{
    s ~= 11;
    return [3];
}

void main()
{
	static auto test3(int[] val) { (val ~= 7) ~= mul11ret3(val); return val; }
	assert(test3([2]) == [2,7,11,3]);
}

DMD fails with [2,11,7]
gdc 10.2 fails with core.exception.OutOfMemoryError.

[ibuclaw]

I translated that test for eval order to the ~= operator:
gdc 10.2 fails with core.exception.OutOfMemoryError.

Excellent, that means garbage is passed as the destination slot in append(&dst, mul11ret3(val)) instead of the address to val.

[thewilsonator]

cc @JohanEngelen I know we had a couple of bugs w.r.t. this or sometime similar in LDC.

[ibuclaw]

So a little further down the page, you already have a section on assignment operator expressions, would it be better just to clarify the wording of that passage to avoid redundancy?

[Alexibu]

The part about a op= b being exactly equal to a = a op b was both incorrect and redundant because of 10.5,2.
I've modified the PR to what I think is missing.
Is it possible that this is super obvious but omittied from the spec ?
I had thought it was a basic assumption, but in discussing why a ~= a.sum didn't work I realise it is a topic of debate, and not clarified by the spec.
There is probably a lot more not documented. For example see https://en.cppreference.com/w/cpp/language/eval_order

[JohanEngelen]

See: #1429

[WalterBright]

See: #1429

Yes. In particular, see @andralex 's comment:

I'd say a correct formulation goes as follows:
The expression a @= b, where @ stands for any of the binary operators,
is evaluated as if it were lowered as follows:

(function ref(ref x, y) { return x = cast(typeof(x)) (x @ y); })(a, b)

This is a much clearer specification, as the text in this PR was rather baffling to me, and I had to study the examples to see what was going on.

[WalterBright]

See my comment.

[Alexibu]

Closed PR because #1429 will clarify.