[dmd-internals] 2.062 regression: Broken AST with mixins and fwd refs

[David Nadlinger]

Hi all,

I'm still working on tracking down the last couple of LDC test suite
regressions from 2.061 to 2.062. In one of the cases, the problem is
actually in the frontend, even if it might not cause a visible bug
for DMD.


Consider this test case (reduced from xtest46):

———
mixin template DefineCoreType() {
  struct Faulty {
    static int x;
    X164!() xxx;
  }
}

mixin DefineCoreType!();

mixin template A164() {}

struct X164() { mixin A164!(); }

———

The issue is that if sd is the/a 'Faulty' declaration,
((TypeStruct*)sd->type)->sym != sd – there seem to be two
StructDeclaration instances around that both correspond to 'Faulty'!

An easy way to see this is by adding an assertion for this to the end
of StructDeclaration::semantic:

———
--- a/src/struct.c
+++ b/src/struct.c
@@ -655,6 +655,8 @@ void StructDeclaration::semantic(Scope *sc)
         deferred->semantic2(sc);
         deferred->semantic3(sc);
     }
+
+    assert(static_cast<TypeStruct*>(type)->sym == this);
 }

 Dsymbol *StructDeclaration::search(Loc loc, Identifier *ident, int flags)
———

The commit in which this assertion first triggers on the above test
case is 14d3c7535, which is a fix for a 2.061 regression, issue 9276.
I'm not sure, though, if this piece of code is to blame, or if the
root cause lies in the way the declaration copying for template
mixins is handled.

I would be glad if somebody could have a look at this (Kenji, maybe
the issue is obvious to you?), as it wreaks havoc with LDC's codegeń
(LDC needs type information all over the place, as the LLVM IR is
typed, and was definitely written with the assumption of having
a consistent AST to work with.)


Similar issues have occured multiple times in the past; one almost
identical problem is http://d.puremagic.com/issues/show_bug.cgi?id=8626,
but there have also been problems that later turned out to actually
trigger DMD bugs as well (such as the inout nested struct mangling
issue).

As they are additionally quite time-consuming to track down, I have
been thinking about introducing a śet of »AST validation« checks
that are run before codegen and are activated by a compile time
switch (for development and on the CI bots). What do you think? Any
implementation suggestions?


Thanks,
David