CTFE: Fix binassign evaluation/load order #5966
Conversation
... and add long overdue tests for binops and binassigns. For binassigns, the lhs is firstly evaluated as lvalue, then the rhs is evaluated, and only then the lhs lvalue is loaded for the underlying binops' lhs. I.e., the rhs side effects are reflected in the binop's lhs. CTFE used to incorrectly load the lhs before evaluating the rhs, diverging from runtime.
|
Oh nice, so DMD seems to be plagued by ldc-developers/ldc#1327 too (edit: only when optimizing). Argh. |
|
I created an issue for the other optimizer bug: https://issues.dlang.org/show_bug.cgi?id=16317 |
|
Pinging @yebblies and @klickverbot can't hurt. |
|
This does introduce difference behavior between runtime and CTFE correct ? |
|
Nope this makes sure runtime and CTFE behave identically, see the tests. |
|
Auto-merge toggled on |
|
Auto-merge toggled off |
|
Auto-merge toggled on |
|
@klickverbot where is the rational as to what is the correct behavior ? |
|
See https://issues.dlang.org/show_bug.cgi?id=16317 for the FIXME comment, which is linked above. (It would have been better to include the bug id in the commit message or directly in the comment.) And as for what is correct, see the other links in the PR description – in particular the NG discussion, which is short and unequivocal. dlang/dlang.org#1429 (which clarifies the spec) sadly fizzled out over a discussion about the best way to write down the semantics (but not the behaviour itself). DMD has had a number of bugs in this area in the past, and this PR was on the tail end of a large change/fix of the behaviour. It is quite plausible that there are still inconsistencies, if that is why you are asking. |
... and add long overdue tests for binops and binassigns.
For binassigns, the lhs is firstly evaluated as lvalue, then the rhs is evaluated, and only then the lhs lvalue is loaded for the underlying binop's lhs. I.e., the rhs side effects are reflected in the binop's lhs.
CTFE used to incorrectly load the lhs before evaluating the rhs, diverging from runtime.
See ldc-developers/ldc#1617 and http://forum.dlang.org/thread/tpucyhjhiwkcxajuuzqg@forum.dlang.org, where @tgehr came up with this CTFE bug. Also pinging @ibuclaw.