Fix issue 18097 - unittest symbol names can be used before semantic

src/dmd/dmodule.d

@@ -323,12 +323,6 @@ extern (C++) final class Module : Package
     int isDocFile;              // if it is a documentation input file, not D source
     bool isPackageFile;         // if it is a package.d
     int needmoduleinfo;
-    /**
-       How many unit tests have been seen so far in this module. Makes it so the
-       unit test name is reproducible regardless of whether it's compiled
-       separately or all at once.
-     */
-    uint unitTestCounter;       // how many unittests have been seen so far
     int selfimports;            // 0: don't know, 1: does not, 2: does
 
     /*************************************

src/dmd/dsymbolsem.d

@@ -3598,12 +3598,6 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
 
     override void visit(UnitTestDeclaration utd)
     {
-        // The identifier has to be generated here in order for it to be possible
-        // to link regardless of whether the files were compiled separately
-        // or all at once. See:
-        // https://issues.dlang.org/show_bug.cgi?id=16995
-        utd.setIdentifier();
-
         if (utd.semanticRun >= PASS.semanticdone)
             return;
         if (utd._scope)

src/dmd/func.d

@@ -3462,10 +3462,7 @@ extern (C++) final class UnitTestDeclaration : FuncDeclaration
 
     extern (D) this(const ref Loc loc, const ref Loc endloc, StorageClass stc, char* codedoc)
     {
-        // Id.empty can cause certain things to fail, so we create a
-        // temporary one here that serves for most purposes with
-        // createIdentifier. There is no scope to pass so we pass null.
-        super(loc, endloc, createIdentifier(loc, null), stc, null);
+        super(loc, endloc, createIdentifier(loc), stc, null);
         this.codedoc = codedoc;
     }
 
@@ -3476,56 +3473,17 @@ extern (C++) final class UnitTestDeclaration : FuncDeclaration
         return FuncDeclaration.syntaxCopy(utd);
     }
 
-    /**
-       Sets the "real" identifier, replacing the one created in the contructor.
-       The reason for this is that the "real" identifier can only be generated
-       properly in the semantic pass. See:
-       https://issues.dlang.org/show_bug.cgi?id=16995
-     */
-    final void setIdentifier()
-    {
-        ident = createIdentifier(loc, _scope);
-    }
-
     /***********************************************************
      * Generate unique unittest function Id so we can have multiple
      * instances per module.
      */
-    private static Identifier createIdentifier(const ref Loc loc, Scope* sc)
+    private static Identifier createIdentifier(const ref Loc loc)
     {
         OutBuffer buf;
-        writeModuleNameOrFileName(buf, loc, sc);
-        buf.prependstring("__unittest_");
-        const index = sc ? sc._module.unitTestCounter++ : 0;
-        buf.printf("_%u_%d", loc.linnum, index);
-
-        // replace characters that demangle can't handle
-        auto str = buf.peekString;
-        for(int i = 0; str[i] != 0; ++i)
-            if(str[i] == '/' || str[i] == '\\' || str[i] == '.') str[i] = '_';
-
+        buf.printf("__unittest_L%u_C%u", loc.linnum, loc.charnum);
         return Identifier.idPool(buf.peekSlice());
     }
 
-    /*************************************************************************
-     * Writes a module name to name a unittest. Tries to use the fully
-     * qualified name if possible to avoid mismatches when compiling separately.
-     * Otherwise uses the file name.
-     * Params:
-     *    buf = The buffer to write to.
-     *    loc = The location of the unit test declaration.
-     *    scope = The scope of the unit test declaration.
-     */
-    private static void writeModuleNameOrFileName(ref OutBuffer buf, const ref Loc loc, Scope* scope_)
-    {
-        if (scope_ is null || scope_._module is null || scope_._module.ident is null)
-        {
-            buf.writestring(loc.filename);
-            return;
-        }
-        scope_._module.fullyQualifiedName(buf);
-    }
-
     override AggregateDeclaration isThis()
     {
         return null;

src/dmd/module.h

@@ -86,12 +86,6 @@ class Module : public Package
     int isDocFile;      // if it is a documentation input file, not D source
     bool isPackageFile; // if it is a package.d
     int needmoduleinfo;
-    /**
-       How many unit tests have been seen so far in this module. Makes it so the
-       unit test name is reproducible regardless of whether it's compiled
-       separately or all at once.
-     */
-    unsigned unitTestCounter;
     int selfimports;            // 0: don't know, 1: does not, 2: does
     bool selfImports();         // returns true if module imports itself
 

test/compilable/issue18097.d

@@ -0,0 +1,12 @@
+// REQUIRED_ARGS: -unittest
+module issue18097;
+
+unittest // this first unittest is needed to trigger the bug
+{
+}
+
+unittest // second unittest
+{
+    auto a = &mixin(__traits(identifier, __traits(parent, {  })));
+    auto b = &__traits(parent, {  });
+}

test/fail_compilation/fail7848.d

@@ -3,11 +3,11 @@
 /*
 TEST_OUTPUT:
 ---
-fail_compilation/fail7848.d(35): Error: `pure` function `fail7848.C.__unittest_fail7848_33_0` cannot call impure function `fail7848.func`
-fail_compilation/fail7848.d(35): Error: `@safe` function `fail7848.C.__unittest_fail7848_33_0` cannot call `@system` function `fail7848.func`
-fail_compilation/fail7848.d(35): Error: `@nogc` function `fail7848.C.__unittest_fail7848_33_0` cannot call non-@nogc function `fail7848.func`
+fail_compilation/fail7848.d(35): Error: `pure` function `fail7848.C.__unittest_L33_C30` cannot call impure function `fail7848.func`
+fail_compilation/fail7848.d(35): Error: `@safe` function `fail7848.C.__unittest_L33_C30` cannot call `@system` function `fail7848.func`
+fail_compilation/fail7848.d(35): Error: `@nogc` function `fail7848.C.__unittest_L33_C30` cannot call non-@nogc function `fail7848.func`
 fail_compilation/fail7848.d(35): Error: function `fail7848.func` is not `nothrow`
-fail_compilation/fail7848.d(33): Error: `nothrow` function `fail7848.C.__unittest_fail7848_33_0` may throw
+fail_compilation/fail7848.d(33): Error: `nothrow` function `fail7848.C.__unittest_L33_C30` may throw
 fail_compilation/fail7848.d(40): Error: `pure` function `fail7848.C.__invariant1` cannot call impure function `fail7848.func`
 fail_compilation/fail7848.d(40): Error: `@safe` function `fail7848.C.__invariant1` cannot call `@system` function `fail7848.func`
 fail_compilation/fail7848.d(40): Error: `@nogc` function `fail7848.C.__invariant1` cannot call non-@nogc function `fail7848.func`

test/fail_compilation/ice14424.d

@@ -2,7 +2,7 @@
 /*
 TEST_OUTPUT:
 ---
-fail_compilation/ice14424.d(12): Error: `tuple(__unittest_imports_a14424_3_0)` has no effect
+fail_compilation/ice14424.d(12): Error: `tuple(__unittest_L3_C1)` has no effect
 ---
 */
 

test/runnable/imports/module_with_tests.d

@@ -0,0 +1 @@
+unittest {} unittest { assert(false); }

test/runnable/issue16995.d

@@ -0,0 +1,28 @@
+// REQUIRED_ARGS: -unittest
+// COMPILE_SEPARATELY
+// EXTRA_SOURCES: imports/module_with_tests.d
+
+import imports.module_with_tests;
+import core.exception: AssertError;
+
+shared static this()
+{
+    import core.runtime: Runtime, UnitTestResult;
+    Runtime.extendedModuleUnitTester = () => UnitTestResult.pass;
+}
+
+void main()
+{
+    import module_with_tests;
+    foreach(i, ut; __traits(getUnitTests, module_with_tests)) {
+        try
+        {
+            ut();
+            assert(i == 0, "2nd unittest should fail");
+        }
+        catch(AssertError e)
+        {
+            assert(i == 1, "Only 2nd unittest should fail");
+        }
+    }
+}