Skip to content
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

[REG2.069.0-b1] Issue 15207 - Wrong codegen with -inline #5206

Merged
merged 6 commits into from Oct 21, 2015

Conversation

9rnsr
Copy link
Contributor

@9rnsr 9rnsr commented Oct 20, 2015

https://issues.dlang.org/show_bug.cgi?id=15207

When issue 14944 fixed, the ref variable content initializing form had been flagged by MemorySet.refValueInit.

However, inilining may replace AssignExp.e1 operand with a STCref temporary variable.
When it happens, the MemorySet.refValueInit flag will be ignored in AssignExp.toElem and wrong code had generated.

To avoid the issue, instead flag the case "ref varaible initialization" by MemorySet.referenceInit.
Its meaning will be kept beyond inlining stage, and the bug won't happen.

- Too squashed code blocks hard to understand.
- Too long condition lines are hard to read.
When issue 14944 fixed, the ref variable content initializing form had been flagged by `MemorySet.refValueInit`.

However, inilining may replace `AssignExp.e1` operand with a `STCref` temporary variable.
When it happens, the `MemorySet.refValueInit` flag will be ignored in `AssignExp.toElem` and wrong code had generated.

To avoid the issue, instead flag the case "ref varaible initialization" by `MemorySet.referenceInit`.
Its meaning will be kept beyond inlining stage, and the bug won't happen.
After this change, the frontend code for internal code generation would be simple:
- If value assignment (construction) may need postblit call, use `ConstructExp`.
- If bare memory copy is necessary, use `BlitExp`.
A referernce variable initialization is not normally generated from source code.
It's always introduced by internal AST generation or inlining.

So, the ref variables handling is necessary only when the new convenient constructos used for them.
@9rnsr 9rnsr changed the title [REG2.069-b2] Issue 15207 - Wrong codegen with -inline [REG2.069-b1] Issue 15207 - Wrong codegen with -inline Oct 20, 2015
@9rnsr 9rnsr changed the title [REG2.069-b1] Issue 15207 - Wrong codegen with -inline [REG2.069.0-b1] Issue 15207 - Wrong codegen with -inline Oct 20, 2015
@MartinNowak
Copy link
Member

Looks good.

MartinNowak added a commit that referenced this pull request Oct 21, 2015
[REG2.069.0-b1] Issue 15207 - Wrong codegen with -inline
@MartinNowak MartinNowak merged commit 77463d7 into dlang:stable Oct 21, 2015
@9rnsr
Copy link
Contributor Author

9rnsr commented Oct 21, 2015

Thanks!

@9rnsr 9rnsr deleted the fix15207 branch October 22, 2015 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants