[BLOCKED][WIP] Don't instantiate unittest templates in libraries

src/dmd/astbase.d

@@ -1441,6 +1441,21 @@ struct ASTBase
             srcfile = new File(srcfilename);
         }
 
+        /**
+        A root module is one that will be compiled to object code.
+        */
+        final bool isRoot() const
+        {
+            return true;
+        }
+
+        /**
+        Called by the parser after the module declaration has been determined.
+        */
+        final void afterModuleDeclaration(ModuleDeclaration* md)
+        {
+        }
+
         override void accept(Visitor v)
         {
             v.visit(this);

src/dmd/dmodule.d

@@ -292,10 +292,10 @@ extern (C++) final class Module : Package
     extern (C++) static __gshared Dsymbols deferred3;   // deferred Dsymbol's needing semantic3() run on them
     extern (C++) static __gshared uint dprogress;       // progress resolving the deferred list
     /**
-     * A callback function that is called once an imported module is
-     * parsed. If the callback returns true, then it tells the
-     * frontend that the driver intends on compiling the import.
-     */
+    A callback function that is called when a module is loaded as soon as its
+    module declaration has been parsed. If the callback returns true, then it tells the
+    frontend that the driver intends on compiling the import.
+    */
     extern (C++) static __gshared bool function(Module mod) onImport;
 
     static void _init()
@@ -370,12 +370,10 @@ extern (C++) final class Module : Package
     int searchCacheFlags;       // cached flags
 
     /**
-     * A root module is one that will be compiled all the way to
-     * object code.  This field holds the root module that caused
-     * this module to be loaded.  If this module is a root module,
-     * then it will be set to `this`.  This is used to determine
-     * ownership of template instantiation.
-     */
+    This field holds the root module that caused this module
+    to be loaded.  If this module is a root module, then it will
+    be set to `this`.
+    */
     Module importedFrom;
 
     Dsymbols* decldefs;         // top level declarations for this Module
@@ -515,41 +513,6 @@ extern (C++) final class Module : Package
             message("import    %s", buf.peekString());
         }
         m = m.parse();
-
-        // Call onImport here because if the module is going to be compiled then we
-        // need to determine it early because it affects semantic analysis. This is
-        // being done after parsing the module so the full module name can be taken
-        // from whatever was declared in the file.
-
-        //!!!!!!!!!!!!!!!!!!!!!!!
-        // Workaround for bug in dmd version 2.068.2 platform Darwin_64_32.
-        // This is the compiler version that the autotester uses, and this code
-        // has been carefully crafted using trial and error to prevent a seg fault
-        // bug that occurs with that version of the compiler.  Note, this segfault
-        // does not occur on the next version of dmd, namely, version 2.069.0. If
-        // the autotester upgrades to that version, then this workaround can be removed.
-        //!!!!!!!!!!!!!!!!!!!!!!!
-        version(OSX)
-        {
-            if (!m.isRoot() && onImport)
-            {
-                auto onImportResult = onImport(m);
-                if(onImportResult)
-                {
-                    m.importedFrom = m;
-                    assert(m.isRoot());
-                }
-            }
-        }
-        else
-        {
-            if (!m.isRoot() && onImport && onImport(m))
-            {
-                m.importedFrom = m;
-                assert(m.isRoot());
-            }
-        }
-
         Target.loadModule(m);
         return m;
     }
@@ -853,7 +816,6 @@ extern (C++) final class Module : Package
             scope p = new Parser!ASTCodegen(this, buf[0 .. buflen], docfile !is null);
             p.nextToken();
             members = p.parseModule();
-            md = p.md;
             numlines = p.scanloc.linnum;
             if (p.errors)
                 ++global.errors;
@@ -1288,11 +1250,35 @@ extern (C++) final class Module : Package
         return false;
     }
 
-    bool isRoot()
+    /**
+    A root module is one that will be compiled to object code.  This is used to
+    determine ownership of template instantiation and when to compile unittests.
+    Returns:
+        true if this is a root module
+    */
+    final bool isRoot() const
     {
         return this.importedFrom == this;
     }
 
+    /**
+    Called by the parser after the module declaration has been determined.
+    */
+    final void afterModuleDeclaration(ModuleDeclaration* md)
+    {
+        this.md = md;
+
+        // Call onImport here because if the module is going to be compiled then we
+        // need to determine it early because it affects whether unittests will be parsed
+        // and semantic analysis. This is being done right after parsing the module delclaration
+        // so the full module name can be taken from whatever was declared in the file.
+        if (!isRoot() && onImport && onImport(this))
+        {
+            importedFrom = this;
+            assert(isRoot());
+        }
+    }
+
     // true if the module source file is directly
     // listed in command line.
     bool isCoreModule(Identifier ident)

src/dmd/dtemplate.d

@@ -6360,8 +6360,7 @@ extern (C++) class TemplateInstance : ScopeDsymbol
          * is behind a conditional debug declaration.
          */
         // workaround for https://issues.dlang.org/show_bug.cgi?id=11239
-        if (global.params.useUnitTests ||
-            global.params.debuglevel)
+        if (global.params.debuglevel)
         {
             // Prefer instantiations from root modules, to maximize link-ability.
             if (minst.isRoot())
@@ -7354,7 +7353,7 @@ extern (C++) class TemplateInstance : ScopeDsymbol
     {
         Module mi = minst; // instantiated . inserted module
 
-        if (global.params.useUnitTests || global.params.debuglevel)
+        if (global.params.debuglevel)
         {
             // Turn all non-root instances to speculative
             if (mi && !mi.isRoot())

src/dmd/glue.d

@@ -752,8 +752,17 @@ void FuncDeclaration_toObjFile(FuncDeclaration fd, bool multiobj)
     if (!fd.fbody)
         return;
 
+    // Find module m for this function
+    Module m = null;
+    for (Dsymbol p = fd.parent; p; p = p.parent)
+    {
+        m = p.isModule();
+        if (m)
+            break;
+    }
+
     UnitTestDeclaration ud = fd.isUnitTestDeclaration();
-    if (ud && !global.params.useUnitTests)
+    if (ud && (!global.params.useUnitTests || !m.isRoot))
         return;
 
     if (multiobj && !fd.isStaticDtorDeclaration() && !fd.isStaticCtorDeclaration())
@@ -886,15 +895,6 @@ void FuncDeclaration_toObjFile(FuncDeclaration fd, bool multiobj)
     symtab_t *symtabsave = cstate.CSpsymtab;
     cstate.CSpsymtab = &f.Flocsym;
 
-    // Find module m for this function
-    Module m = null;
-    for (Dsymbol p = fd.parent; p; p = p.parent)
-    {
-        m = p.isModule();
-        if (m)
-            break;
-    }
-
     Dsymbols deferToObj;                   // write these to OBJ file later
     Array!(elem*) varsInScope;
     Label*[void*] labels = null;

src/dmd/module.h

@@ -144,7 +144,7 @@ class Module : public Package
     static void clearCache();
     int imports(Module *m);
 
-    bool isRoot() { return this->importedFrom == this; }
+    bool isRoot() const { return this->importedFrom == this; }
     // true if the module source file is directly
     // listed in command line.
     bool isCoreModule(Identifier *ident);

src/dmd/parse.d

@@ -377,6 +377,8 @@ final class Parser(AST) : Lexer
             }
         }
 
+        mod.afterModuleDeclaration(md);
+
         decldefs = parseDeclDefs(0, &lastDecl);
         if (token.value != TOK.endOfFile)
         {
@@ -547,7 +549,7 @@ final class Parser(AST) : Lexer
                     break;
                 }
             case TOK.unittest_:
-                if (global.params.useUnitTests || global.params.doDocComments || global.params.doHdrGeneration)
+                if ( (global.params.useUnitTests || global.params.doDocComments || global.params.doHdrGeneration) && mod.isRoot)
                 {
                     s = parseUnitTest(pAttrs);
                     if (*pLastDecl)

test/runnable/extra-files/check_bin.sh

@@ -0,0 +1,17 @@
+#!/usr/bin/env bash
+
+if [ "$2" == "HAS" ]; then
+    GREP_OPTION=
+elif [ "$2" == "DOES_NOT_HAVE" ]; then
+    GREP_OPTION=-v
+else
+    echo Unknown option "$2"
+    exit 1
+fi
+
+if [ ${OS} != "linux" ]; then
+    echo Skipping checkbin $2 $3 because os $OS is not linux
+    exit 0
+fi
+
+objdump -t "$1" | grep -q $GREP_OPTION "$3"

test/runnable/extra-files/printVersionUnittest.d

@@ -0,0 +1,6 @@
+module printVersionUnittest;
+
+version(unittest)
+{
+    pragma(msg, "version unittest");
+}

test/runnable/extra-files/printingUnittest.d

@@ -0,0 +1,7 @@
+module printingUnittest;
+
+unittest
+{
+    import core.stdc.stdio;
+    printf("unittesting printingUnittest\n");
+}

test/runnable/extra-files/selectiveUnittests.d

@@ -0,0 +1,6 @@
+import printingUnittest;
+void main()
+{
+    // have to print something so grep works
+    import core.stdc.stdio; printf("done\n");
+}

test/runnable/extra-files/selectiveUnittests2.d

@@ -0,0 +1,4 @@
+import printVersionUnittest;
+void main()
+{
+}

test/runnable/selectiveUnittests.sh

@@ -0,0 +1,29 @@
+#!/usr/bin/env bash
+
+obj_file=${OUTPUT_BASE}/tempobj${OBJ}
+exe_file=${OUTPUT_BASE}/tempexe${EXE}
+
+# Compile both modules, make sure unittest compiles/runs
+unittest_output="unittesting printingUnittest"
+$DMD -m${MODEL} -of=${exe_file} -I=${EXTRA_FILES} -unittest ${EXTRA_FILES}/printingUnittest.d ${EXTRA_FILES}/selectiveUnittests.d
+${exe_file} | grep -q "${unittest_output}"
+./runnable/extra-files/check_bin.sh ${exe_file} HAS __unittest
+
+# Precompile the module and make sure it's unittest doesn't get compiled/run
+$DMD -m${MODEL} -c -of=${obj_file} ${EXTRA_FILES}/printingUnittest.d
+./runnable/extra-files/check_bin.sh ${exe_file} DOES_NOT_HAVE __unittest
+
+rm -f ${exe_file}
+$DMD -m${MODEL} -of=${exe_file} -I=${EXTRA_FILES} -unittest ${obj_file} ${EXTRA_FILES}/selectiveUnittests.d
+${exe_file} | grep -q -v "${unittest_output}"
+
+# Make sure that version(unittest) still get analyzed
+unittest_output="version unittest"
+$DMD -m${MODEL} -I=${EXTRA_FILES} -o- -unittest ${EXTRA_FILES}/printVersionUnittest.d ${EXTRA_FILES}/selectiveUnittests2.d 2>&1 | grep -q "${unittest_output}"
+
+# Precompile the module and make sure version(unittest) is still enabled
+$DMD -m${MODEL} -c -of=${obj_file} ${EXTRA_FILES}/printVersionUnittest.d
+$DMD -m${MODEL} -I=${EXTRA_FILES} -o- -unittest ${obj_file} ${EXTRA_FILES}/selectiveUnittests2.d 2>&1 | grep -q "${unittest_output}"
+
+rm -f ${obj_file}
+rm -f ${exe_file}