From 69fb77bd6b7209302014b520b44c7e4e93dd505d Mon Sep 17 00:00:00 2001 From: Jonathan Marler Date: Thu, 12 Apr 2018 09:05:03 -0600 Subject: [PATCH] Only compile unittests in root modules --- src/dmd/astbase.d | 15 ++++ src/dmd/dmodule.d | 80 ++++++++----------- src/dmd/dtemplate.d | 5 +- src/dmd/glue.d | 20 ++--- src/dmd/module.h | 2 +- src/dmd/parse.d | 4 +- test/runnable/extra-files/check_bin.sh | 17 ++++ .../extra-files/printVersionUnittest.d | 6 ++ test/runnable/extra-files/printingUnittest.d | 7 ++ .../runnable/extra-files/selectiveUnittests.d | 6 ++ .../extra-files/selectiveUnittests2.d | 4 + test/runnable/selectiveUnittests.sh | 29 +++++++ 12 files changed, 133 insertions(+), 62 deletions(-) create mode 100755 test/runnable/extra-files/check_bin.sh create mode 100644 test/runnable/extra-files/printVersionUnittest.d create mode 100644 test/runnable/extra-files/printingUnittest.d create mode 100644 test/runnable/extra-files/selectiveUnittests.d create mode 100644 test/runnable/extra-files/selectiveUnittests2.d create mode 100644 test/runnable/selectiveUnittests.sh diff --git a/src/dmd/astbase.d b/src/dmd/astbase.d index c8001d010901..e7d629a76b51 100644 --- a/src/dmd/astbase.d +++ b/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); diff --git a/src/dmd/dmodule.d b/src/dmd/dmodule.d index 58d56129878c..9f76ee2e585d 100644 --- a/src/dmd/dmodule.d +++ b/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) diff --git a/src/dmd/dtemplate.d b/src/dmd/dtemplate.d index b662b115c269..483b56c41ba1 100644 --- a/src/dmd/dtemplate.d +++ b/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()) diff --git a/src/dmd/glue.d b/src/dmd/glue.d index ac36c1afceec..f26956b9b690 100644 --- a/src/dmd/glue.d +++ b/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; diff --git a/src/dmd/module.h b/src/dmd/module.h index 7482adee9cb7..3d703409c5ac 100644 --- a/src/dmd/module.h +++ b/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); diff --git a/src/dmd/parse.d b/src/dmd/parse.d index 4800145b69aa..b7840a0f348a 100644 --- a/src/dmd/parse.d +++ b/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) diff --git a/test/runnable/extra-files/check_bin.sh b/test/runnable/extra-files/check_bin.sh new file mode 100755 index 000000000000..2d1075b78dd6 --- /dev/null +++ b/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" diff --git a/test/runnable/extra-files/printVersionUnittest.d b/test/runnable/extra-files/printVersionUnittest.d new file mode 100644 index 000000000000..830acb54af63 --- /dev/null +++ b/test/runnable/extra-files/printVersionUnittest.d @@ -0,0 +1,6 @@ +module printVersionUnittest; + +version(unittest) +{ + pragma(msg, "version unittest"); +} diff --git a/test/runnable/extra-files/printingUnittest.d b/test/runnable/extra-files/printingUnittest.d new file mode 100644 index 000000000000..d1bd828a39cd --- /dev/null +++ b/test/runnable/extra-files/printingUnittest.d @@ -0,0 +1,7 @@ +module printingUnittest; + +unittest +{ + import core.stdc.stdio; + printf("unittesting printingUnittest\n"); +} diff --git a/test/runnable/extra-files/selectiveUnittests.d b/test/runnable/extra-files/selectiveUnittests.d new file mode 100644 index 000000000000..08c4a9fab938 --- /dev/null +++ b/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"); +} diff --git a/test/runnable/extra-files/selectiveUnittests2.d b/test/runnable/extra-files/selectiveUnittests2.d new file mode 100644 index 000000000000..6fe8290919ed --- /dev/null +++ b/test/runnable/extra-files/selectiveUnittests2.d @@ -0,0 +1,4 @@ +import printVersionUnittest; +void main() +{ +} diff --git a/test/runnable/selectiveUnittests.sh b/test/runnable/selectiveUnittests.sh new file mode 100644 index 000000000000..e66878e99d45 --- /dev/null +++ b/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}