Thread overview
[Issue 19479] Garbage .init in string mixins in static foreach in mixin templates
Dec 12, 2018
Simen Kjaeraas
Apr 01, 2019
ag0aep6g
Feb 04, 2020
Iain Buclaw
Feb 06, 2020
Dlang Bot
Feb 06, 2020
Dlang Bot
December 12, 2018
https://issues.dlang.org/show_bug.cgi?id=19479

Simen Kjaeraas <simen.kjaras@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |simen.kjaras@gmail.com
           Hardware|x86_64                      |All
            Summary|ints defined in template    |Garbage .init in string
                   |mixins have garbage values  |mixins in static foreach in
                   |                            |mixin templates
                 OS|Linux                       |All

--- Comment #1 from Simen Kjaeraas <simen.kjaras@gmail.com> ---
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).

--
April 01, 2019
https://issues.dlang.org/show_bug.cgi?id=19479

ag0aep6g <ag0aep6g@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
                 CC|                            |ag0aep6g@gmail.com

--
February 04, 2020
https://issues.dlang.org/show_bug.cgi?id=19479

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |ibuclaw@gdcproject.org

--- Comment #2 from Iain Buclaw <ibuclaw@gdcproject.org> ---
---
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!

--
February 06, 2020
https://issues.dlang.org/show_bug.cgi?id=19479

Dlang Bot <dlang-bot@dlang.rocks> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |pull

--- Comment #3 from Dlang Bot <dlang-bot@dlang.rocks> ---
@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

--
February 06, 2020
https://issues.dlang.org/show_bug.cgi?id=19479

Dlang Bot <dlang-bot@dlang.rocks> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Dlang Bot <dlang-bot@dlang.rocks> ---
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

--