From cab17a8744f920d083ff6afdfe5a3a06961ace56 Mon Sep 17 00:00:00 2001 From: Atila Neves Date: Tue, 9 Jan 2018 15:35:35 +0100 Subject: [PATCH 1/2] Fix issue 18097 - unittest symbol names can be used before semantic pass --- src/dmd/dmodule.d | 6 --- src/dmd/dsymbolsem.d | 6 --- src/dmd/func.d | 48 ++------------------- src/dmd/module.h | 6 --- test/compilable/imports/module_with_tests.d | 5 --- test/compilable/issue16995.d | 13 ------ test/compilable/issue18097.d | 12 ++++++ test/fail_compilation/fail7848.d | 8 ++-- test/fail_compilation/ice14424.d | 2 +- test/runnable/imports/module_with_tests.d | 1 + test/runnable/issue16995.d | 28 ++++++++++++ 11 files changed, 49 insertions(+), 86 deletions(-) delete mode 100644 test/compilable/imports/module_with_tests.d delete mode 100644 test/compilable/issue16995.d create mode 100644 test/compilable/issue18097.d create mode 100644 test/runnable/imports/module_with_tests.d create mode 100644 test/runnable/issue16995.d diff --git a/src/dmd/dmodule.d b/src/dmd/dmodule.d index 547d78f6a74c..ca21aba820e4 100644 --- a/src/dmd/dmodule.d +++ b/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 /************************************* diff --git a/src/dmd/dsymbolsem.d b/src/dmd/dsymbolsem.d index ac0a64b753b0..2e469a0ced0e 100644 --- a/src/dmd/dsymbolsem.d +++ b/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) diff --git a/src/dmd/func.d b/src/dmd/func.d index 13119e39bac6..a10d0f034b37 100644 --- a/src/dmd/func.d +++ b/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; diff --git a/src/dmd/module.h b/src/dmd/module.h index 7a40308b8b2d..ea03ffb01dba 100644 --- a/src/dmd/module.h +++ b/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 diff --git a/test/compilable/imports/module_with_tests.d b/test/compilable/imports/module_with_tests.d deleted file mode 100644 index 2a24c00d7089..000000000000 --- a/test/compilable/imports/module_with_tests.d +++ /dev/null @@ -1,5 +0,0 @@ -unittest {} -unittest -{ - assert(1); -} diff --git a/test/compilable/issue16995.d b/test/compilable/issue16995.d deleted file mode 100644 index fb1eebeed4e5..000000000000 --- a/test/compilable/issue16995.d +++ /dev/null @@ -1,13 +0,0 @@ -// COMPILED_IMPORTS: imports/module_with_tests.d -// REQUIRED_ARGS: -unittest -// COMPILE_SEPARATELY - - -import imports.module_with_tests; - -void main() { - import module_with_tests; - foreach(ut; __traits(getUnitTests, module_with_tests)) { - ut(); - } -} diff --git a/test/compilable/issue18097.d b/test/compilable/issue18097.d new file mode 100644 index 000000000000..0bc16135129c --- /dev/null +++ b/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, { }); +} diff --git a/test/fail_compilation/fail7848.d b/test/fail_compilation/fail7848.d index e4f9957cb537..a18720a89db3 100644 --- a/test/fail_compilation/fail7848.d +++ b/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` diff --git a/test/fail_compilation/ice14424.d b/test/fail_compilation/ice14424.d index 249607b5fb77..2599dc91cd8d 100644 --- a/test/fail_compilation/ice14424.d +++ b/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 --- */ diff --git a/test/runnable/imports/module_with_tests.d b/test/runnable/imports/module_with_tests.d new file mode 100644 index 000000000000..b74f653d300f --- /dev/null +++ b/test/runnable/imports/module_with_tests.d @@ -0,0 +1 @@ +unittest {} unittest { assert(false); } diff --git a/test/runnable/issue16995.d b/test/runnable/issue16995.d new file mode 100644 index 000000000000..c958a33689a2 --- /dev/null +++ b/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; + Runtime.moduleUnitTester = () => true; +} + +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"); + } + } +} From 7c2ce0c5677066b97d87cd9654a8872f84636648 Mon Sep 17 00:00:00 2001 From: Atila Neves Date: Thu, 25 Jan 2018 18:19:00 +0100 Subject: [PATCH 2/2] Use extendedModuleUnitTester in issue16995 test --- test/runnable/issue16995.d | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/runnable/issue16995.d b/test/runnable/issue16995.d index c958a33689a2..bd3ddefe3858 100644 --- a/test/runnable/issue16995.d +++ b/test/runnable/issue16995.d @@ -7,8 +7,8 @@ import core.exception: AssertError; shared static this() { - import core.runtime: Runtime; - Runtime.moduleUnitTester = () => true; + import core.runtime: Runtime, UnitTestResult; + Runtime.extendedModuleUnitTester = () => UnitTestResult.pass; } void main()