-
-
Notifications
You must be signed in to change notification settings - Fork 609
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
fix Issue 16408 - [REG2.065] left-to-right order of evaluation of fun… #6705
Conversation
|
2965cfd to
e6a4812
Compare
…ction arguments not consistently followed
|
This PR makes left-to-right evaluation of function arguments much more consistent than before. While this is in more conformance with the D spec, it may break some existing code that inadvertently had a dependency of r-t-l evaluation. It is impractical for the compiler to detect this (good 'ole halting problem). |
|
Can we then close the order of evaluation matter, or there are a few stragglers left? |
| @@ -182,10 +182,10 @@ elem *callfunc(Loc loc, | |||
| ty = ec.Ety; | |||
| if (fd) | |||
| ty = toSymbol(fd).Stype.Tty; | |||
| reverse = tyrevfunc(ty); | |||
| reverse = tyrevfunc(ty); // left-to-right parameter evaluation (TYnpfunc, TYjfunc, TYfpfunc, TYf16func) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
long comment
| elem*[5] elems_array = void; | ||
| import core.stdc.stdlib : malloc, free; | ||
| auto elems = (n <= 5) ? elems_array.ptr : cast(elem**)malloc(arguments.dim * (elem*).sizeof); | ||
| assert(elems); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nicer to use this instead of relying on code downstream:
scope(exit) if (n > 5) free(elems);There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've avoided that form because the intervening code is not nothrow, meaning it sets up an EH frame even though the intervening code will never throw.
| * D presumes left-to-right argument evaluation, but we're evaluating things | ||
| * right-to-left here. | ||
| * 1. determine if this matters | ||
| * 2. fix it if it does |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does matter... best to keep to the spec and keep the spec simple
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant that foo(3,4) does not have order of evaluation issues.
|
There may be a couple other OOE issues, but this should fix the biggie. |
|
Auto-merge toggled on |
|
This apparently introduced a regression Issue 17356 – [Reg 2.075] __simd_sto no longer executed @WalterBright, yet another backend issue sabotaging the arrayops rewrite :/. |
|
@WalterBright r-t-l order makes more sense for function calls. |
…ction arguments not consistently followed