Merge pull request #4635 from 9rnsr/fix14549

Issue 14549 - isVirtualMethod does not work well with Github DMD

src/attrib.c

@@ -1166,7 +1166,7 @@ StaticIfDeclaration::StaticIfDeclaration(Condition *condition,
         : ConditionalDeclaration(condition, decl, elsedecl)
 {
     //printf("StaticIfDeclaration::StaticIfDeclaration()\n");
-    sds = NULL;
+    scopesym = NULL;
     addisdone = 0;
 }
 
@@ -1178,43 +1178,52 @@ Dsymbol *StaticIfDeclaration::syntaxCopy(Dsymbol *s)
         Dsymbol::arraySyntaxCopy(elsedecl));
 }
 
+/****************************************
+ * Different from other AttribDeclaration subclasses, include() call requires
+ * the completion of addMember and setScope phases.
+ */
 Dsymbols *StaticIfDeclaration::include(Scope *sc, ScopeDsymbol *sds)
 {
     //printf("StaticIfDeclaration::include(sc = %p) scope = %p\n", sc, scope);
 
     if (condition->inc == 0)
     {
-        /* Bugzilla 10101: Condition evaluation may cause self-recursive
-         * condition evaluation. To resolve it, temporarily save sc into scope.
-         */
-        bool x = !scope && sc;
-        if (x) scope = sc;
-        Dsymbols *d = ConditionalDeclaration::include(sc, sds);
-        if (x) scope = NULL;
+        assert(scopesym);   // addMember is already done
+        assert(scope);      // setScope is already done
 
-        // Set the scopes lazily.
-        if (scope && d)
+        Dsymbols *d = ConditionalDeclaration::include(scope, scopesym);
+
+        if (d && !addisdone)
         {
-           for (size_t i = 0; i < d->dim; i++)
-           {
-               Dsymbol *s = (*d)[i];
+            // Add members lazily.
+            for (size_t i = 0; i < d->dim; i++)
+            {
+                Dsymbol *s = (*d)[i];
+                s->addMember(scope, scopesym, 1);
+            }
 
-               s->setScope(scope);
-           }
+            // Set the member scopes lazily.
+            for (size_t i = 0; i < d->dim; i++)
+            {
+                Dsymbol *s = (*d)[i];
+                s->setScope(scope);
+            }
+
+            addisdone = 1;
         }
         return d;
     }
     else
     {
-        return ConditionalDeclaration::include(sc, sds);
+        return ConditionalDeclaration::include(sc, scopesym);
     }
 }
 
 int StaticIfDeclaration::addMember(Scope *sc, ScopeDsymbol *sds, int memnum)
 {
-    //printf("StaticIfDeclaration::addMember() '%s'\n",toChars());
-    /* This is deferred until semantic(), so that
-     * expressions in the condition can refer to declarations
+    //printf("StaticIfDeclaration::addMember() '%s'\n", toChars());
+    /* This is deferred until the condition evaluated later (by the include() call),
+     * so that expressions in the condition can refer to declarations
      * in the same scope, such as:
      *
      * template Foo(int i)
@@ -1224,15 +1233,9 @@ int StaticIfDeclaration::addMember(Scope *sc, ScopeDsymbol *sds, int memnum)
      *         const int k;
      * }
      */
-    this->sds = sds;
-    int m = 0;
+    this->scopesym = sds;
 
-    if (0 && memnum == 0)
-    {
-        m = AttribDeclaration::addMember(sc, sds, memnum);
-        addisdone = 1;
-    }
-    return m;
+    return 0;
 }
 
 void StaticIfDeclaration::importAll(Scope *sc)
@@ -1250,23 +1253,7 @@ void StaticIfDeclaration::setScope(Scope *sc)
 
 void StaticIfDeclaration::semantic(Scope *sc)
 {
-    Dsymbols *d = include(sc, sds);
-
-    //printf("\tStaticIfDeclaration::semantic '%s', d = %p\n",toChars(), d);
-    if (d)
-    {
-        if (!addisdone)
-        {
-            AttribDeclaration::addMember(sc, sds, 1);
-            addisdone = 1;
-        }
-
-        for (size_t i = 0; i < d->dim; i++)
-        {
-            Dsymbol *s = (*d)[i];
-            s->semantic(sc);
-        }
-    }
+    AttribDeclaration::semantic(sc);
 }
 
 const char *StaticIfDeclaration::kind()
@@ -1284,7 +1271,7 @@ CompileDeclaration::CompileDeclaration(Loc loc, Expression *exp)
     //printf("CompileDeclaration(loc = %d)\n", loc.linnum);
     this->loc = loc;
     this->exp = exp;
-    this->sds = NULL;
+    this->scopesym = NULL;
     this->compiled = 0;
 }
 
@@ -1297,10 +1284,12 @@ Dsymbol *CompileDeclaration::syntaxCopy(Dsymbol *s)
 int CompileDeclaration::addMember(Scope *sc, ScopeDsymbol *sds, int memnum)
 {
     //printf("CompileDeclaration::addMember(sc = %p, sds = %p, memnum = %d)\n", sc, sds, memnum);
+#if 0
     if (compiled)
         return 1;
-
-    this->sds = sds;
+#endif
+    this->scopesym = sds;
+#if 0
     if (memnum == 0)
     {
         /* No members yet, so parse the mixin now
@@ -1309,6 +1298,7 @@ int CompileDeclaration::addMember(Scope *sc, ScopeDsymbol *sds, int memnum)
         memnum |= AttribDeclaration::addMember(sc, sds, memnum);
         compiled = 1;
     }
+#endif
     return memnum;
 }
 
@@ -1357,7 +1347,7 @@ void CompileDeclaration::semantic(Scope *sc)
     if (!compiled)
     {
         compileIt(sc);
-        AttribDeclaration::addMember(sc, sds, 0);
+        AttribDeclaration::addMember(sc, scopesym, 0);
         compiled = 1;
 
         if (scope && decl)

src/attrib.h

@@ -170,7 +170,7 @@ class ConditionalDeclaration : public AttribDeclaration
 class StaticIfDeclaration : public ConditionalDeclaration
 {
 public:
-    ScopeDsymbol *sds;
+    ScopeDsymbol *scopesym;
     int addisdone;
 
     StaticIfDeclaration(Condition *condition, Dsymbols *decl, Dsymbols *elsedecl);
@@ -191,7 +191,7 @@ class CompileDeclaration : public AttribDeclaration
 public:
     Expression *exp;
 
-    ScopeDsymbol *sds;
+    ScopeDsymbol *scopesym;
     int compiled;
 
     CompileDeclaration(Loc loc, Expression *exp);

src/cond.c

@@ -354,6 +354,11 @@ int StaticIfCondition::include(Scope *sc, ScopeDsymbol *sds)
         sc->pop();
         --nest;
 
+        // Prevent repeated condition evaluation.
+        // See: fail_compilation/fail7815.d
+        if (inc != 0)
+            return (inc == 1);
+
         if (!e->type->isBoolean())
         {
             if (e->type->toBasetype() != Type::terror)

test/compilable/testfwdref.d

@@ -434,3 +434,46 @@ static assert(
 
     return true;
 }());
+
+/***************************************************/
+// 14549
+
+string foo14549(T)()
+{
+    static if (T.tupleof.length >= 0)
+        return "";
+}
+
+class Frop14549
+{
+    mixin(foo14549!(typeof(this)));
+
+    static if (__traits(compiles, undefined))
+    {
+    }
+    else
+    {
+        int bar = 0;
+    }
+
+    static if (!__traits(isVirtualMethod, this.bar)) {}
+}
+
+// ----
+
+template Mix14549()
+{
+    mixin(code14549!(typeof(this)));
+}
+
+template code14549(T)
+{
+    enum string code14549 =
+        q{ static if (!__traits(isVirtualMethod, "boo")) {} };
+}
+
+class Bar14549
+{
+    mixin Mix14549;
+    int boo;
+}

test/fail_compilation/fail7815.d

@@ -0,0 +1,65 @@
+// REQUIRED_ARGS: -o-
+/*
+TEST_OUTPUT:
+---
+X: tuple("x")
+fail_compilation/fail7815.d(47): Error: no property 'flags' for type 'Move'
+---
+*/
+
+mixin template Helpers()
+{
+    static if (is(Flags!Move))
+    {
+        Flags!Move flags;
+    }
+    else
+    {
+        pragma(msg, "X: ", __traits(derivedMembers, Flags!Move));
+    }
+}
+
+template Flags(T)
+{
+    mixin({
+        int defs = 1;
+        foreach (name; __traits(derivedMembers, Move))
+        {
+            defs++;
+        }
+        if (defs)
+        {
+            return "struct Flags { bool x; }";
+        }
+        else
+        {
+            return "";
+        }
+    }());
+}
+
+struct Move
+{
+    int a;
+    mixin Helpers!();
+}
+
+enum a7815 = Move.init.flags;
+
+/+
+This is an invalid case.
+When the Move struct member is analyzed:
+1. mixin Helpers!() is instantiated.
+2. In Helpers!(), static if and its condition is(Flags!Move)) evaluated.
+3. In Flags!Move, string mixin evaluates and CTFE lambda.
+4. __traits(derivedMembers, Move) tries to see the member of Move.
+   4a. mixin Helpers!() member is analyzed.
+   4b. `static if (is(Flags!Move))` in Helpers!() is evaluated
+   4c. The Flags!Move instantiation is already in progress, so it cannot be resolved.
+   4d. `static if` fails because Flags!Move cannot be determined as a type.
+5. __traits(derivedMembers, Move) returns a 1-length tuple("a").
+6. The lambda in Flags!Move returns a string "struct Flags {...}", then
+   Flags!Move is instantiated to a new struct Flags.
+7. Finally Move struct does not have flags member, then the `enum a7815`
+   definition will fail in its initializer.
++/

test/runnable/xtest46.d

@@ -5899,41 +5899,6 @@ void test7735()
     a7735([]);
 }
 
-/***************************************************/
-// 7815
-
-mixin template Helpers() {
-
-  static if (is(Flags!Move)) {
-    Flags!Move flags;
-  } else {
-    // DMD will happily instantiate the allegedly
-    // non-existent Flags!This here. (!)
-    pragma(msg, __traits(derivedMembers, Flags!Move));
-  }
-}
-
-template Flags(T) {
-  mixin({
-    int defs = 1;
-    foreach (name; __traits(derivedMembers, Move)) {
-        defs++;
-    }
-    if (defs) {
-      return "struct Flags { bool a; }";
-    } else {
-      return "";
-    }
-  }());
-}
-
-struct Move {
-  int a;
-  mixin Helpers!();
-}
-
-enum a7815 = Move.init.flags;
-
 /***************************************************/
 
 struct A7823 {