Thread overview
[dmd-internals] 2.062 regression: Broken AST with mixins and fwd refs
Mar 17, 2013
David Nadlinger
Mar 18, 2013
kenji hara
Mar 18, 2013
David Nadlinger
March 17, 2013
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
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals
March 18, 2013
How about this?

https://github.com/D-Programming-Language/dmd/pull/1760

Kenji Hara


2013/3/17 David Nadlinger <code@klickverbot.at>

> 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
> _______________________________________________
> dmd-internals mailing list
> dmd-internals@puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals


March 18, 2013
On Mon, Mar 18, 2013 at 8:30 AM, kenji hara <k.hara.pg@gmail.com> wrote:
> How about this?
>
> https://github.com/D-Programming-Language/dmd/pull/1760

That was fast! The change seems to fix the issues I'm seeing.

Thanks,
David
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals