diff --git a/compiler/include/dmd/dsymbol.h b/compiler/include/dmd/dsymbol.h index 22b468b141e5..54dd43382797 100644 --- a/compiler/include/dmd/dsymbol.h +++ b/compiler/include/dmd/dsymbol.h @@ -373,6 +373,7 @@ class OverloadSet final : public Dsymbol class ForwardingScopeDsymbol final : public ScopeDsymbol { public: + int32_t idCounter; Dsymbol *symtabInsert(Dsymbol *s) override; Dsymbol *symtabLookup(Dsymbol *s, Identifier *id) override; void importScope(Dsymbol *s, Visibility visibility) override; diff --git a/compiler/include/dmd/scope.h b/compiler/include/dmd/scope.h index 479c7a440601..a136381471b2 100644 --- a/compiler/include/dmd/scope.h +++ b/compiler/include/dmd/scope.h @@ -145,4 +145,6 @@ struct Scope final AliasDeclaration *aliasAsg; // if set, then aliasAsg is being assigned a new value, // do not set wasRead for it StructDeclaration *argStruct; // elimiate recursion when looking for rvalue construction + + int32_t idCounter; }; diff --git a/compiler/src/dmd/dscope.d b/compiler/src/dmd/dscope.d index 9a7f362b8c7a..a10ed7441f7f 100644 --- a/compiler/src/dmd/dscope.d +++ b/compiler/src/dmd/dscope.d @@ -211,6 +211,9 @@ extern (C++) struct Scope /// do not set wasRead for it StructDeclaration argStruct; /// elimiate recursion when looking for rvalue construction + int idCounter; /// disambiguator for `Identifier.generateIdWithLoc`, set per `static foreach` / + /// unrolled `foreach` iteration so generated identifiers are unique and stable + extern (D) __gshared Scope* freelist; extern (D) static Scope* alloc() diff --git a/compiler/src/dmd/dsymbol.d b/compiler/src/dmd/dsymbol.d index d4d0f0430003..5cecfe322720 100644 --- a/compiler/src/dmd/dsymbol.d +++ b/compiler/src/dmd/dsymbol.d @@ -1376,6 +1376,11 @@ extern (C++) final class OverloadSet : Dsymbol */ extern (C++) final class ForwardingScopeDsymbol : ScopeDsymbol { + /// Disambiguator for `Identifier.generateIdWithLoc`, carrying the + /// `static foreach` iteration index into the forwarded scope. + /// See `Scope.idCounter`. + int idCounter; + extern (D) this() nothrow @safe { super(); @@ -1577,6 +1582,33 @@ extern (C++) final class DsymbolTable : RootObject { return tab.length; } + + /************************** + * Pick a name derived from `ident` that is not yet taken in this table. + * + * Params: + * ident = base identifier + * Returns: + * `ident` if free, otherwise `ident_` for the lowest free `n` + */ + Identifier uniqueName(Identifier ident) + { + if (!lookup(ident)) + return ident; + const base = ident.toString(); + Identifier id; + int n = 1; + do + { + OutBuffer buf; + buf.writestring(base); + buf.writestring("_"); + buf.print(n++); + id = Identifier.idPool(buf[]); + } + while (lookup(id)); + return id; + } } /** diff --git a/compiler/src/dmd/dsymbolsem.d b/compiler/src/dmd/dsymbolsem.d index 9afd17e73777..352f713a6c8a 100644 --- a/compiler/src/dmd/dsymbolsem.d +++ b/compiler/src/dmd/dsymbolsem.d @@ -4089,7 +4089,7 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor tm.symtab = sc.parent.isScopeDsymbol().symtab; L1: assert(tm.symtab); - tm.ident = Identifier.generateIdWithLoc("__mixin", tm.loc, null, /*mustBeUnique:*/ false); + tm.ident = Identifier.generateIdWithLoc("__mixin", tm.loc, sc.idCounter); tm.symtab.insert(tm); } } @@ -6326,6 +6326,39 @@ void addMember(Dsymbol dsym, Scope* sc, ScopeDsymbol sds) dsym.accept(addMemberVisitor); } +/**************************************************** + * Disambiguate the location-based name of a compiler-generated symbol (static + * constructor, unittest) whose source location is shared by a copy: + * + * - When duplicated by a `static foreach`, the enclosing iteration index + * `Scope.idCounter` is appended + * - Otherwise the location can still clash, e.g. when two string mixins on the + * same source line each generate such a symbol (both get the same `-mixin-` + * pseudo location). The lowest free index is then appended. + */ +private void disambiguateLocBasedId(Dsymbol fd, Scope* sc, ScopeDsymbol sds) +{ + if (!fd.ident) + return; + if (sc && sc.idCounter != 0) + { + fd.ident = appendIdCounter(fd.ident, sc.idCounter); + return; + } + // Resolve clashes against the target symbol table (in source order). + if (sds && sds.symtab) + fd.ident = sds.symtab.uniqueName(fd.ident); +} + +private Identifier appendIdCounter(Identifier ident, int counter) +{ + OutBuffer buf; + buf.writestring(ident.toString()); + buf.writestring("_"); + buf.print(counter); + return Identifier.idPool(buf[]); +} + private void attribAddMember(AttribDeclaration atb, Scope* sc, ScopeDsymbol sds) { Dsymbols* d = atb.include(sc); @@ -6421,6 +6454,24 @@ private extern(C++) class AddMemberVisitor : Visitor // we didn't add anything } + override void visit(UnitTestDeclaration d) + { + disambiguateLocBasedId(d, sc, sds); + visit(cast(Dsymbol)d); + } + + override void visit(StaticCtorDeclaration d) + { + disambiguateLocBasedId(d, sc, sds); + visit(cast(Dsymbol)d); + } + + override void visit(StaticDtorDeclaration d) + { + disambiguateLocBasedId(d, sc, sds); + visit(cast(Dsymbol)d); + } + /***************************** * Add import to sd's symbol table. */ @@ -9186,6 +9237,7 @@ private extern(C++) class NewScopeVisitor : Visitor override void visit(ForwardingAttribDeclaration fad) { sc = sc.push(fad.sym); + sc.idCounter = fad.sym.idCounter; } override void visit(UserAttributeDeclaration uac) diff --git a/compiler/src/dmd/expressionsem.d b/compiler/src/dmd/expressionsem.d index 3bf47b420f4c..b398a857e802 100644 --- a/compiler/src/dmd/expressionsem.d +++ b/compiler/src/dmd/expressionsem.d @@ -7337,7 +7337,9 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor symtab = sds.symtab; } assert(symtab); - Identifier id = Identifier.generateIdWithLoc(s, exp.loc, cast(const void*) sc.parent); + // The same source location can be lowered to a function literal more + // than once - e.g. an inherited `in`/`out` contract + Identifier id = symtab.uniqueName(Identifier.generateIdWithLoc(s, exp.loc, sc.idCounter)); exp.fd.ident = id; if (exp.td) exp.td.ident = id; diff --git a/compiler/src/dmd/identifier.d b/compiler/src/dmd/identifier.d index 5ace70ed5b8b..471efb566419 100644 --- a/compiler/src/dmd/identifier.d +++ b/compiler/src/dmd/identifier.d @@ -213,24 +213,22 @@ nothrow: /*************************************** * Generate deterministic named identifier based on a source location, * such that the name is consistent across multiple compilations. - * A new unique name is generated. If the prefix+location is already in - * the stringtable, an extra suffix is added (starting the count at "_1"). + * + * The generated name is `_L_C`, optionally followed by + * `_` when `counter` is non-zero. The counter is used to make the + * name unique when the same source location is duplicated, which happens + * when a `static foreach` (or unrolled sequence `foreach`) copies its body. * * Params: - * prefix = first part of the identifier name. - * loc = source location to use in the identifier name. - * parent = (optional) extra part to be used in uniqueness check, - * if (prefix1, loc1) == (prefix2, loc2), but - * parent1 != parent2, no new name will be generated. - * mustBeUnique = require unique identifier. - * append counter to generated name if this prefix + - * location has been encountered before + * prefix = first part of the identifier name. + * loc = source location to use in the identifier name. + * counter = disambiguator for copies of the same source location, + * see `Scope.idCounter`. `0` means no suffix is appended. * Returns: * Identifier (inside Identifier.idPool) with deterministic name based * on the source location. */ - extern (D) static Identifier generateIdWithLoc(string prefix, Loc loc, - const void* parent = null, bool mustBeUnique = true) + extern (D) static Identifier generateIdWithLoc(string prefix, Loc loc, int counter = 0) { // generate `_L_C` auto sl = SourceLoc(loc); @@ -241,52 +239,10 @@ nothrow: idBuf.writestring("_C"); idBuf.print(sl.column); - if (!mustBeUnique) - return idPool(idBuf[]); - - /** - * Make sure the identifiers are unique per filename, i.e., per module/mixin - * (`path/to/foo.d` and `path/to/foo.d-mixin-`). See issues - * https://issues.dlang.org/show_bug.cgi?id=16995 - * https://issues.dlang.org/show_bug.cgi?id=18097 - * https://issues.dlang.org/show_bug.cgi?id=18111 - * https://issues.dlang.org/show_bug.cgi?id=18880 - * https://issues.dlang.org/show_bug.cgi?id=18868 - * https://issues.dlang.org/show_bug.cgi?id=19058 - * - * It is a bit trickier for lambdas/dgliterals: we want them to be unique per - * module/mixin + function/template instantiation context. So we use extra parent - * argument for that when dealing with lambdas. We could have added it to prefix - * directly, but that would unnecessary lengthen symbols names. See issue: - * https://issues.dlang.org/show_bug.cgi?id=23722 - */ - static struct Key { string locKey; string prefix; const(void)* parent; } - __gshared uint[Key] counters; - - const locKey = cast(string) (sl.filename ~ idBuf[]); - const key = Key(locKey, prefix, parent); - static if (__traits(compiles, counters.update(Key.init, () => 0u, (ref uint a) => 0u))) + if (counter) { - // 2.082+ - counters.update(key, - () => 1u, // insertion - (ref uint counter) // update - { - idBuf.writestring("_"); - idBuf.print(counter); - return counter + 1; - } - ); - } - else - { - if (auto pCounter = key in counters) - { - idBuf.writestring("_"); - idBuf.print((*pCounter)++); - } - else - counters[key] = 1; + idBuf.writestring("_"); + idBuf.print(counter); } return idPool(idBuf[]); diff --git a/compiler/src/dmd/statementsem.d b/compiler/src/dmd/statementsem.d index dbaf9023f8b1..a87afc4b03af 100644 --- a/compiler/src/dmd/statementsem.d +++ b/compiler/src/dmd/statementsem.d @@ -528,12 +528,18 @@ Statement statementSemanticVisit(Statement s, Scope* sc) scd.sbreak = uls; scd.scontinue = uls; + // Keep identifiers generated within each unrolled body (e.g. lambdas) + // unique and stable, the same way `static foreach` does. + const baseCounter = sc.idCounter; + const n = uls.statements.length; + Statement serror = null; foreach (i, ref s; *uls.statements) { if (s) { //printf("[%d]: %s\n", i, s.toChars()); + scd.idCounter = cast(int) (baseCounter * n + i); s = s.statementSemantic(scd); if (s && !serror) serror = s.isErrorStatement(); @@ -600,6 +606,7 @@ Statement statementSemanticVisit(Statement s, Scope* sc) ss.sym.parent = csc.scopesym; } sc = sc.push(ss.sym); + sc.idCounter = ss.sym.idCounter; sc.sbreak = ss; sc.scontinue = ss; ss.statement = ss.statement.statementSemantic(sc); @@ -4791,6 +4798,11 @@ public auto makeTupleForeach(Scope* sc, bool isStatic, bool isDecl, ForeachState s = new CompoundStatement(loc, stmts); } + // Disambiguator for identifiers generated within this iteration's body + // (e.g. unittests / lambdas). Combined with the enclosing scope's counter + // as a mixed-radix number so it stays unique under nested `static foreach`, + // and stable across compilations (it only depends on the unrolling indices). + const iterCounter = cast(int) (sc.idCounter * n + j); if (!isStatic) { s = new ScopeStatement(loc, s, fs.endloc); @@ -4798,11 +4810,15 @@ public auto makeTupleForeach(Scope* sc, bool isStatic, bool isDecl, ForeachState else if (isDecl) { import dmd.attrib: ForwardingAttribDeclaration; - d = new ForwardingAttribDeclaration(decls); + auto fad = new ForwardingAttribDeclaration(decls); + fad.sym.idCounter = iterCounter; + d = fad; } else { - s = new ForwardingStatement(loc, s); + auto fwd = new ForwardingStatement(loc, s); + fwd.sym.idCounter = iterCounter; + s = fwd; } if (isDecl) diff --git a/compiler/test/compilable/staticforeach_ids.d b/compiler/test/compilable/staticforeach_ids.d new file mode 100644 index 000000000000..b6586523b5f7 --- /dev/null +++ b/compiler/test/compilable/staticforeach_ids.d @@ -0,0 +1,23 @@ +/* +REQUIRED_ARGS: -unittest +TEST_OUTPUT: +--- +AliasSeq!(__unittest_L15_C5, __unittest_L19_C9, __unittest_L19_C9_1, __unittest_L15_C5_1, __unittest_L19_C9_2, __unittest_L19_C9_3) +--- +*/ + +// Test that generated identifiers for declarations duplicated by `static foreach` are disambiguated + +module staticforeach_ids; + +static foreach (i; 0 .. 2) +{ + unittest { } + + static foreach (j; 0 .. 2) + { + unittest { } + } +} + +pragma(msg, __traits(getUnitTests, staticforeach_ids)); diff --git a/compiler/test/compilable/test21330.d b/compiler/test/compilable/test21330.d index 66f9e29a63a9..d43f4c057b5c 100644 --- a/compiler/test/compilable/test21330.d +++ b/compiler/test/compilable/test21330.d @@ -2,8 +2,8 @@ REQUIRED_ARGS: -unittest TEST_OUTPUT: --- -AliasSeq!(__unittest_L14_C5_1, __unittest_L14_C5_2) -AliasSeq!(__unittest_L14_C5_2) +AliasSeq!(__unittest_L14_C5, __unittest_L14_C5) +AliasSeq!(__unittest_L14_C5) --- */ // https://issues.dlang.org/show_bug.cgi?id=21330