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

fix Issue 19479 - Garbage .init in string mixins in static foreach in mixin templates #10767

Merged
merged 1 commit into from
Feb 6, 2020

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Feb 6, 2020

The problem was that the mixin was being omitted from the codegen during the conversion from StaticForeachDeclaration to a Statement.

The first issue with toStatement is that it is visiting all members in the StaticForeachDeclaration's decl, whereas the semantically compiled list of Dsymbols is instead found in cache. This is what caused the CompileDeclaration to be omitted.

The second issue with toStatement is that converting an already compiled StaticForeachDeclaration into a new StaticForeachStatement results in makeTupleForeach being called twice for the same
StaticForeach symbol. There is no need to create a new StaticForeachStatement, simply the unrolling of all members is enough.

@ibuclaw ibuclaw requested a review from Geod24 as a code owner February 6, 2020 00:18
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @ibuclaw!

Bugzilla references

Auto-close Bugzilla Severity Description
19479 normal Garbage .init in string mixins in static foreach in mixin templates

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#10767"

… mixin templates

The problem was that the mixin was being omitted from the codegen during
the conversion from StaticForeachDeclaration to a Statement.

The first issue with `toStatement` is that it is visiting all members in
the StaticForeachDeclaration's `decl`, whereas the semantically compiled
list of Dsymbols is instead found in `cache`.  This is what caused the
CompileDeclaration to be omitted.

The second issue with `toStatement` is that converting an already
compiled StaticForeachDeclaration into a new StaticForeachStatement
results in `makeTupleForeach` being called twice for the same
StaticForeach symbol.  There is no need to create a new
StaticForeachStatement, simply the unrolling of all members is enough.
Copy link
Member

@WalterBright WalterBright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work

@dlang-bot dlang-bot merged commit 979d8ef into dlang:master Feb 6, 2020
@ibuclaw ibuclaw deleted the issue19479 branch February 6, 2020 17:52
@PetarKirov
Copy link
Member

PetarKirov commented Feb 23, 2020

CC @tgehr

@ibuclaw
Copy link
Member Author

ibuclaw commented Feb 23, 2020

Did I break something, or just notifying the original author?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants