From f527810984133fb379f99c4b12a357257a53eefb Mon Sep 17 00:00:00 2001 From: Atila Neves Date: Tue, 25 Apr 2017 00:56:53 +0200 Subject: [PATCH 1/2] Fix Issue 16995 - __traits(getUnittests) doesn't work with separate compilation --- src/ddmd/dmodule.d | 2 +- src/ddmd/dsymbolsem.d | 5 +++ src/ddmd/func.d | 39 +++++++++++++++------ src/ddmd/module.h | 2 +- test/compilable/imports/module_with_tests.d | 1 + test/compilable/issue16995.d | 13 +++++++ test/fail_compilation/fail7848.d | 16 ++++----- test/fail_compilation/ice14424.d | 2 +- 8 files changed, 58 insertions(+), 22 deletions(-) create mode 100644 test/compilable/imports/module_with_tests.d create mode 100644 test/compilable/issue16995.d diff --git a/src/ddmd/dmodule.d b/src/ddmd/dmodule.d index c63536477a8b..466a348d690b 100644 --- a/src/ddmd/dmodule.d +++ b/src/ddmd/dmodule.d @@ -319,7 +319,7 @@ 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; - + 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/ddmd/dsymbolsem.d b/src/ddmd/dsymbolsem.d index 96cc57340c95..3916e6d73e73 100644 --- a/src/ddmd/dsymbolsem.d +++ b/src/ddmd/dsymbolsem.d @@ -5144,6 +5144,11 @@ extern(C++) final class DsymbolSemanticVisitor : Visitor override void visit(UnitTestDeclaration utd) { + // the identifier has to be generated here in order to be able to link + // or not the files are compiled separately or all at once. + // See bugzilla #16995 + utd.setIdentifier; + if (utd.semanticRun >= PASSsemanticdone) return; if (utd._scope) diff --git a/src/ddmd/func.d b/src/ddmd/func.d index 130816110053..c7ca4b6332bc 100644 --- a/src/ddmd/func.d +++ b/src/ddmd/func.d @@ -3156,16 +3156,6 @@ extern (C++) final class InvariantDeclaration : FuncDeclaration } } -/*********************************************************** - * Generate unique unittest function Id so we can have multiple - * instances per module. - */ -private Identifier unitTestId(Loc loc) -{ - OutBuffer buf; - buf.printf("__unittestL%u_", loc.linnum); - return Identifier.generateId(buf.peekString()); -} /*********************************************************** */ @@ -3178,7 +3168,10 @@ extern (C++) final class UnitTestDeclaration : FuncDeclaration extern (D) this(Loc loc, Loc endloc, StorageClass stc, char* codedoc) { - super(loc, endloc, unitTestId(loc), stc, null); + // 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); this.codedoc = codedoc; } @@ -3189,6 +3182,30 @@ extern (C++) final class UnitTestDeclaration : FuncDeclaration return FuncDeclaration.syntaxCopy(utd); } + final void setIdentifier() + { + ident = createIdentifier(loc, _scope); + } + + /*********************************************************** + * Generate unique unittest function Id so we can have multiple + * instances per module. + */ + private static Identifier createIdentifier(Loc loc, Scope* sc) + { + + OutBuffer buf; + auto index = sc ? sc._module.unitTestCounter++ : 0; + buf.printf("__unittest_%s_%u_%d", loc.filename, 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] = '_'; + + return Identifier.idPool(buf.peekSlice); + } + override AggregateDeclaration isThis() { return null; diff --git a/src/ddmd/module.h b/src/ddmd/module.h index 787987af8f5c..37de7b6d6362 100644 --- a/src/ddmd/module.h +++ b/src/ddmd/module.h @@ -81,7 +81,7 @@ 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; - + unsigned unitTestCounter; // how many unittests have been seen so far 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 new file mode 100644 index 000000000000..27cd555429bb --- /dev/null +++ b/test/compilable/imports/module_with_tests.d @@ -0,0 +1 @@ +unittest {} unittest {} diff --git a/test/compilable/issue16995.d b/test/compilable/issue16995.d new file mode 100644 index 000000000000..07dfa5b462f7 --- /dev/null +++ b/test/compilable/issue16995.d @@ -0,0 +1,13 @@ +// EXTRA_SOURCES: 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/fail_compilation/fail7848.d b/test/fail_compilation/fail7848.d index 77fcdfa35018..18bbb69e7668 100644 --- a/test/fail_compilation/fail7848.d +++ b/test/fail_compilation/fail7848.d @@ -3,16 +3,16 @@ /* TEST_OUTPUT: --- -fail_compilation/fail7848.d(35): Error: pure function 'fail7848.C.__unittestL33_$n$' cannot call impure function 'fail7848.func' -fail_compilation/fail7848.d(35): Error: @safe function 'fail7848.C.__unittestL33_$n$' cannot call @system function 'fail7848.func' -fail_compilation/fail7848.d(35): Error: @nogc function 'fail7848.C.__unittestL33_$n$' cannot call non-@nogc function 'fail7848.func' +fail_compilation/fail7848.d(35): Error: pure function 'fail7848.C.__unittest_fail_compilation_fail7848_d_33_0' cannot call impure function 'fail7848.func' +fail_compilation/fail7848.d(35): Error: @safe function 'fail7848.C.__unittest_fail_compilation_fail7848_d_33_0' cannot call @system function 'fail7848.func' +fail_compilation/fail7848.d(35): Error: @nogc function 'fail7848.C.__unittest_fail_compilation_fail7848_d_33_0' 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.__unittestL33_$n$` may throw -fail_compilation/fail7848.d(40): Error: pure function 'fail7848.C.__invariant2' cannot call impure function 'fail7848.func' -fail_compilation/fail7848.d(40): Error: @safe function 'fail7848.C.__invariant2' cannot call @system function 'fail7848.func' -fail_compilation/fail7848.d(40): Error: @nogc function 'fail7848.C.__invariant2' cannot call non-@nogc function 'fail7848.func' +fail_compilation/fail7848.d(33): Error: nothrow function `fail7848.C.__unittest_fail_compilation_fail7848_d_33_0` 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' fail_compilation/fail7848.d(40): Error: function `fail7848.func` is not nothrow -fail_compilation/fail7848.d(38): Error: nothrow function `fail7848.C.__invariant2` may throw +fail_compilation/fail7848.d(38): Error: nothrow function `fail7848.C.__invariant1` may throw fail_compilation/fail7848.d(45): Error: pure allocator 'fail7848.C.new' cannot call impure function 'fail7848.func' fail_compilation/fail7848.d(45): Error: @safe allocator 'fail7848.C.new' cannot call @system function 'fail7848.func' fail_compilation/fail7848.d(45): Error: @nogc allocator 'fail7848.C.new' cannot call non-@nogc function 'fail7848.func' diff --git a/test/fail_compilation/ice14424.d b/test/fail_compilation/ice14424.d index 29fe66612019..41d136c1e0ce 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` has no effect in expression `tuple(__unittestL3_$n$)` +fail_compilation/ice14424.d(12): Error: `tuple` has no effect in expression `tuple(__unittest_fail_compilation_imports_a14424_d_3_0)` --- */ From 0f26862ce9d300af868113296a5d7787b5c941d2 Mon Sep 17 00:00:00 2001 From: Atila Neves Date: Sat, 30 Sep 2017 11:32:54 +0200 Subject: [PATCH 2/2] Address review comments --- src/ddmd/dmodule.d | 5 +++++ src/ddmd/dsymbolsem.d | 9 +++++---- src/ddmd/func.d | 9 +++++++-- src/ddmd/module.h | 7 ++++++- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/ddmd/dmodule.d b/src/ddmd/dmodule.d index 466a348d690b..e5a956009958 100644 --- a/src/ddmd/dmodule.d +++ b/src/ddmd/dmodule.d @@ -319,6 +319,11 @@ 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/ddmd/dsymbolsem.d b/src/ddmd/dsymbolsem.d index 3916e6d73e73..a89d85e953e2 100644 --- a/src/ddmd/dsymbolsem.d +++ b/src/ddmd/dsymbolsem.d @@ -5144,10 +5144,11 @@ extern(C++) final class DsymbolSemanticVisitor : Visitor override void visit(UnitTestDeclaration utd) { - // the identifier has to be generated here in order to be able to link - // or not the files are compiled separately or all at once. - // See bugzilla #16995 - utd.setIdentifier; + // 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 >= PASSsemanticdone) return; diff --git a/src/ddmd/func.d b/src/ddmd/func.d index c7ca4b6332bc..0c30b56a8fab 100644 --- a/src/ddmd/func.d +++ b/src/ddmd/func.d @@ -3182,6 +3182,12 @@ 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); @@ -3193,7 +3199,6 @@ extern (C++) final class UnitTestDeclaration : FuncDeclaration */ private static Identifier createIdentifier(Loc loc, Scope* sc) { - OutBuffer buf; auto index = sc ? sc._module.unitTestCounter++ : 0; buf.printf("__unittest_%s_%u_%d", loc.filename, loc.linnum, index); @@ -3203,7 +3208,7 @@ extern (C++) final class UnitTestDeclaration : FuncDeclaration for(int i = 0; str[i] != 0; ++i) if(str[i] == '/' || str[i] == '\\' || str[i] == '.') str[i] = '_'; - return Identifier.idPool(buf.peekSlice); + return Identifier.idPool(buf.peekSlice()); } override AggregateDeclaration isThis() diff --git a/src/ddmd/module.h b/src/ddmd/module.h index 37de7b6d6362..85e465b19057 100644 --- a/src/ddmd/module.h +++ b/src/ddmd/module.h @@ -81,7 +81,12 @@ 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; - unsigned unitTestCounter; // how many unittests have been seen so far + /** + 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