Bug 19479 – Garbage .init in string mixins in static foreach in mixin templates

Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2018-12-12T06:59:36Z
Last change time
2020-02-06T06:41:41Z
Keywords
pull, wrong-code
Assigned to
No Owner
Creator
[email protected]

Comments

Comment #0 by johannes.riecken — 2018-12-12T06:59:36Z
Code: import std.conv; import std.stdio; mixin template genInts() { enum arr = [0,1]; static foreach (t; arr) { mixin("int i" ~ to!string(t) ~ " = 5;"); } } void main() { mixin genInts!(); writeln(i0); writeln(i1); } Expected output: 5 5 Actual output is two garbage integer values. $ dmd --version DMD64 D Compiler v2.083.0 Copyright (C) 1999-2018 by The D Language Foundation, All Rights Reserved written by Walter Bright $ uname -a Linux jo-FX80 4.19.4-arch1-1-ARCH #1 SMP PREEMPT Fri Nov 23 09:06:58 UTC 2018 x86_64 GNU/Linux
Comment #1 by simen.kjaras — 2018-12-12T08:31:13Z
This issue is far stranger than one might expect. All of these simplifications give correct results: unittest { mixin("int i = 5;"); assert(i == 5); } template genInts1() { int i = 5; } unittest { mixin genInts1!(); assert(i == 5); } template genInts2() { mixin("int i = 5;"); } unittest { mixin genInts2!(); assert(i == 5); } unittest { static foreach (t; 0..1) { mixin("int i = 5;"); } assert(i == 5); } While this one fails: mixin template genInts3() { static foreach (t; 0..1) { mixin("int i = 5;"); } } unittest{ mixin genInts3!(); assert(i == 5); } I also tested this by replacing int with string. Same issue. So, a correct description is: initial values are not correctly set for variables defined in string mixins inside static foreach inside mixin templates. run.dlang.io says that this has never worked (static foreach was introduced in 2.076, so it doesn't even compile before then).
Comment #2 by ibuclaw — 2020-02-04T08:59:19Z
--- mixin template genInts3() { static foreach (t; 0..1) { mixin("int i = 5;"); } } unittest{ mixin genInts3!(); assert(i == 5); } --- This is a correct reduced test case of the original issue. The (sanitized) generated code looks like: --- unittest { int t = 0; assert(i == 5); } --- The variable `i` is never declared!
Comment #3 by dlang-bot — 2020-02-06T00:18:57Z
@ibuclaw created dlang/dmd pull request #10767 "fix Issue 19479 - Garbage .init in string mixins in static foreach in mixin templates" fixing this issue: - fix Issue 19479 - Garbage .init in string mixins in static foreach in 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. https://github.com/dlang/dmd/pull/10767
Comment #4 by dlang-bot — 2020-02-06T06:41:41Z
dlang/dmd pull request #10767 "fix Issue 19479 - Garbage .init in string mixins in static foreach in mixin templates" was merged into master: - 02ddfe07acc0c0cee4cc8f5aa30210d9ad235e92 by Iain Buclaw: fix Issue 19479 - Garbage .init in string mixins in static foreach in 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. https://github.com/dlang/dmd/pull/10767