diff --git a/changelog/duplicate-implementations-deprecation.dd b/changelog/duplicate-implementations-deprecation.dd new file mode 100644 index 000000000000..c296839dc458 --- /dev/null +++ b/changelog/duplicate-implementations-deprecation.dd @@ -0,0 +1,33 @@ +Diagnostics for conflicting function definitions within a module + +Previously, multiple definitions of identical functions within a module were not +recognized, although they have the same mangling. This was problematic because a +binary cannot contain multiple definitions of a symbol, which caused undefined +behavior depending on the compiler backend. + +DMD will now raise an error message if there are conflicting implementations +within a single module: + +--- +void foo() {} +void foo() {} // error +--- + +Multiple declarations are still allowed as long as there is at most one definition: + +--- +void bar(int); +void bar(int) { } +void bar(int); +--- + +DMD will issue a deprecation for mangling schemes that don't support overloading +(`extern(C|Windows|System)`): + +--- +extern(C): +void foo(int) { } +void foo(double) { } // deprecation +--- + +This deprecation will become an error in 2.105. diff --git a/src/dmd/frontend.h b/src/dmd/frontend.h index 37f92bc9d32d..142af85b03cb 100644 --- a/src/dmd/frontend.h +++ b/src/dmd/frontend.h @@ -5388,6 +5388,7 @@ class TypeFunction : public TypeNext bool isInOutQual() const; void isInOutQual(bool v); bool iswild() const; + bool attributesEqual(const TypeFunction* const other) const; void accept(Visitor* v); }; diff --git a/src/dmd/mtype.d b/src/dmd/mtype.d index 4f2ea08bdd6d..8171b6d7f522 100644 --- a/src/dmd/mtype.d +++ b/src/dmd/mtype.d @@ -5198,6 +5198,18 @@ extern (C++) final class TypeFunction : TypeNext return (funcFlags & (FunctionFlag.inoutParam | FunctionFlag.inoutQual)) != 0; } + /// Returns: whether `this` function type has the same attributes (`@safe`,...) as `other` + bool attributesEqual(const scope TypeFunction other) const pure nothrow @safe @nogc + { + enum attributes = FunctionFlag.isnothrow + | FunctionFlag.isnogc + | FunctionFlag.islive; + + return this.trust == other.trust && + this.purity == other.purity && + (this.funcFlags & attributes) == (other.funcFlags & attributes); + } + override void accept(Visitor v) { v.visit(this); @@ -6674,6 +6686,33 @@ extern (C++) struct ParameterList { return ParameterList(Parameter.arraySyntaxCopy(parameters), varargs); } + + /// Compares this to another ParameterList (and expands tuples if necessary) + extern (D) bool opEquals(scope ref ParameterList other) const + { + if (stc != other.stc || varargs != other.varargs || (!parameters != !other.parameters)) + return false; + + if (this.parameters is other.parameters) + return true; + + size_t idx; + bool diff; + + // Pairwise compare each parameter + // Can this avoid the O(n) indexing for the second list? + foreach (_, p1; cast() this) + { + auto p2 = other[idx++]; + if (!p2 || p1 != p2) { + diff = true; + break; + } + } + + // Ensure no remaining parameters in `other` + return !diff && other[idx] is null; + } } diff --git a/src/dmd/semantic2.d b/src/dmd/semantic2.d index 266fe16c2d0c..ffb2f1515563 100644 --- a/src/dmd/semantic2.d +++ b/src/dmd/semantic2.d @@ -359,17 +359,19 @@ private extern(C++) final class Semantic2Visitor : Visitor //printf("FuncDeclaration::semantic2 [%s] fd0 = %s %s\n", loc.toChars(), toChars(), type.toChars()); - // https://issues.dlang.org/show_bug.cgi?id=18385 - // Disable for 2.079, s.t. a deprecation cycle can be started with 2.080 - if (0) - if (fd.overnext && !fd.errors) + // Only check valid functions which have a body to avoid errors + // for multiple declarations, e.g. + // void foo(); + // void foo(); + if (fd.fbody && fd.overnext && !fd.errors) { OutBuffer buf1; OutBuffer buf2; // Always starts the lookup from 'this', because the conflicts with // previous overloads are already reported. - auto f1 = fd; + alias f1 = fd; + auto tf1 = cast(TypeFunction) f1.type; mangleToFuncSignature(buf1, f1); overloadApply(f1, (Dsymbol s) @@ -379,7 +381,7 @@ private extern(C++) final class Semantic2Visitor : Visitor return 0; // Don't have to check conflict between declaration and definition. - if ((f1.fbody !is null) != (f2.fbody !is null)) + if (f2.fbody is null) return 0; /* Check for overload merging with base class member functions. @@ -393,35 +395,26 @@ private extern(C++) final class Semantic2Visitor : Visitor if (f1.overrides(f2)) return 0; + auto tf2 = cast(TypeFunction) f2.type; + // extern (C) functions always conflict each other. if (f1.ident == f2.ident && f1.toParent2() == f2.toParent2() && (f1.linkage != LINK.d && f1.linkage != LINK.cpp) && - (f2.linkage != LINK.d && f2.linkage != LINK.cpp)) + (f2.linkage != LINK.d && f2.linkage != LINK.cpp) && + + // But allow the hack to declare overloads with different parameters/STC's + (!tf1.attributesEqual(tf2) || tf1.parameterList != tf2.parameterList)) { - /* Allow the hack that is actually used in druntime, - * to ignore function attributes for extern (C) functions. - * TODO: Must be reconsidered in the future. - * BUG: https://issues.dlang.org/show_bug.cgi?id=18206 - * - * extern(C): - * alias sigfn_t = void function(int); - * alias sigfn_t2 = void function(int) nothrow @nogc; - * sigfn_t bsd_signal(int sig, sigfn_t func); - * sigfn_t2 bsd_signal(int sig, sigfn_t2 func) nothrow @nogc; // no error - */ - if (f1.fbody is null || f2.fbody is null) - return 0; - - auto tf2 = cast(TypeFunction)f2.type; - error(f2.loc, "%s `%s%s` cannot be overloaded with %s`extern(%s)` function at %s", - f2.kind(), - f2.toPrettyChars(), - parametersTypeToChars(tf2.parameterList), - (f1.linkage == f2.linkage ? "another " : "").ptr, - linkageToChars(f1.linkage), f1.loc.toChars()); - f2.type = Type.terror; - f2.errors = true; + // @@@DEPRECATED_2.094@@@ + // Deprecated in 2020-08, make this an error in 2.104 + f2.deprecation("cannot overload `extern(%s)` function at %s", + linkageToChars(f1.linkage), + f1.loc.toChars()); + + // Enable this when turning the deprecation into an error + // f2.type = Type.terror; + // f2.errors = true; return 0; } @@ -434,7 +427,6 @@ private extern(C++) final class Semantic2Visitor : Visitor //printf("+%s\n\ts1 = %s\n\ts2 = %s @ [%s]\n", toChars(), s1, s2, f2.loc.toChars()); if (strcmp(s1, s2) == 0) { - auto tf2 = cast(TypeFunction)f2.type; error(f2.loc, "%s `%s%s` conflicts with previous declaration at %s", f2.kind(), f2.toPrettyChars(), diff --git a/test/compilable/test18385.d b/test/compilable/test18385.d new file mode 100644 index 000000000000..5c789cb835cf --- /dev/null +++ b/test/compilable/test18385.d @@ -0,0 +1,42 @@ +/* +Reduced from the assertion failure in the glue layer when compiling DWT. +A `compilable` test because it needs codegen. + +Remove this test once the deprecation for conflicting deprecations ends, +see visit(FuncDeclaration) in semantic2.d for details. + +TEST_OUTPUT: +--- +compilable/test18385.d(23): Deprecation: function `test18385.is_paragraph_start` cannot overload `extern(C)` function at compilable/test18385.d(22) +compilable/test18385.d(26): Deprecation: function `test18385.foo` cannot overload `extern(C)` function at compilable/test18385.d(25) +compilable/test18385.d(29): Deprecation: function `test18385.trust` cannot overload `extern(C)` function at compilable/test18385.d(28) +compilable/test18385.d(32): Deprecation: function `test18385.purity` cannot overload `extern(C)` function at compilable/test18385.d(31) +compilable/test18385.d(35): Deprecation: function `test18385.nogc` cannot overload `extern(C)` function at compilable/test18385.d(34) +compilable/test18385.d(38): Deprecation: function `test18385.nothrow_` cannot overload `extern(C)` function at compilable/test18385.d(37) +compilable/test18385.d(41): Deprecation: function `test18385.live` cannot overload `extern(C)` function at compilable/test18385.d(40) +--- +*/ + +extern(C) +{ + uint is_paragraph_start(){ return 0; } + uint is_paragraph_start(int){ return 0; } + + void foo(char, bool) {} + void foo(byte, char) {} + + void trust() {} + void trust() @safe {} + + void purity() {} + void purity() pure {} + + void nogc() {} + void nogc() @safe {} + + void nothrow_() {} + void nothrow_() nothrow {} + + void live() {} + void live() @live {} +} diff --git a/test/fail_compilation/fail17492.d b/test/fail_compilation/fail17492.d index dca6f6fc0888..7236c22a8bcf 100644 --- a/test/fail_compilation/fail17492.d +++ b/test/fail_compilation/fail17492.d @@ -1,11 +1,9 @@ /* -https://issues.dlang.org/show_bug.cgi?id=18385 -Disabled for 2.079, s.t. a deprecation cycle can be started with 2.080 -DISABLED: win32 win64 osx linux freebsd dragonflybsd +REQUIRED_ARGS: -de TEST_OUTPUT: --- -fail_compilation/fail17492.d(20): Error: class `fail17492.C.testE.I` already exists at fail17492.d(13). Perhaps in another function with the same name? -fail_compilation/fail17492.d(37): Error: struct `fail17492.S.testE.I` already exists at fail17492.d(30). Perhaps in another function with the same name? +fail_compilation/fail17492.d(20): Error: function `fail17492.C.testE()` conflicts with previous declaration at fail_compilation/fail17492.d(13) +fail_compilation/fail17492.d(37): Error: function `fail17492.S.testE()` conflicts with previous declaration at fail_compilation/fail17492.d(30) --- https://issues.dlang.org/show_bug.cgi?id=17492 */ diff --git a/test/fail_compilation/fail2789.d b/test/fail_compilation/fail2789.d index ef9d858fcaab..4e9f2c416152 100644 --- a/test/fail_compilation/fail2789.d +++ b/test/fail_compilation/fail2789.d @@ -1,13 +1,14 @@ /* -DISABLED: win32 win64 osx linux freebsd dragonflybsd https://issues.dlang.org/show_bug.cgi?id=18385 -Disabled for 2.079, s.t. a deprecation cycle can be started with 2.080 + TEST_OUTPUT: --- fail_compilation/fail2789.d(15): Error: function `fail2789.A2789.m()` conflicts with previous declaration at fail_compilation/fail2789.d(10) fail_compilation/fail2789.d(25): Error: function `fail2789.A2789.m()` conflicts with previous declaration at fail_compilation/fail2789.d(10) --- */ +#line 7 + class A2789 { int m() @@ -33,12 +34,12 @@ class A2789 /* TEST_OUTPUT: --- -fail_compilation/fail2789.d(46): Error: function `fail2789.f3()` conflicts with previous declaration at fail_compilation/fail2789.d(45) fail_compilation/fail2789.d(49): Error: function `fail2789.f4()` conflicts with previous declaration at fail_compilation/fail2789.d(48) fail_compilation/fail2789.d(52): Error: function `fail2789.f5()` conflicts with previous declaration at fail_compilation/fail2789.d(51) fail_compilation/fail2789.d(55): Error: function `fail2789.f6()` conflicts with previous declaration at fail_compilation/fail2789.d(54) --- */ + void f1(); void f1() {} // ok @@ -60,9 +61,9 @@ auto f6() { return ""; } // string(), conflict /* TEST_OUTPUT: --- -fail_compilation/fail2789.d(67): Error: function `fail2789.f_ExternC1()` cannot be overloaded with another `extern(C)` function at fail_compilation/fail2789.d(66) -fail_compilation/fail2789.d(70): Error: function `fail2789.f_ExternC2(int)` cannot be overloaded with another `extern(C)` function at fail_compilation/fail2789.d(69) -fail_compilation/fail2789.d(73): Error: function `fail2789.f_ExternC3()` cannot be overloaded with another `extern(C)` function at fail_compilation/fail2789.d(72) +fail_compilation/fail2789.d(67): Error: function `fail2789.f_ExternC1()` conflicts with previous declaration at fail_compilation/fail2789.d(66) +fail_compilation/fail2789.d(70): Deprecation: function `fail2789.f_ExternC2` cannot overload `extern(C)` function at fail_compilation/fail2789.d(69) +fail_compilation/fail2789.d(73): Deprecation: function `fail2789.f_ExternC3` cannot overload `extern(C)` function at fail_compilation/fail2789.d(72) fail_compilation/fail2789.d(76): Error: function `fail2789.f_MixExtern1()` conflicts with previous declaration at fail_compilation/fail2789.d(75) --- */ diff --git a/test/fail_compilation/fail47.d b/test/fail_compilation/fail47.d index 4da1926dfec7..9da0c0927a28 100644 --- a/test/fail_compilation/fail47.d +++ b/test/fail_compilation/fail47.d @@ -1,12 +1,7 @@ /* TEST_OUTPUT: --- -fail_compilation/fail47.d(13): Error: variable `fail47._foo` is aliased to a function -fail_compilation/fail47.d(13): Error: variable `fail47._foo` is aliased to a function -fail_compilation/fail47.d(13): Error: variable `fail47._foo` is aliased to a function -fail_compilation/fail47.d(18): Error: none of the overloads of `foo` are callable using argument types `(int)`, candidates are: -fail_compilation/fail47.d(12): `fail47.foo()` -fail_compilation/fail47.d(13): Error: variable `fail47._foo` is aliased to a function +fail_compilation/fail47.d(8): Error: variable `fail47._foo` is aliased to a function --- */ void foo() {} diff --git a/test/fail_compilation/fail5634.d b/test/fail_compilation/fail5634.d index 87943f418756..e2442e65593f 100644 --- a/test/fail_compilation/fail5634.d +++ b/test/fail_compilation/fail5634.d @@ -1,8 +1,8 @@ /* TEST_OUTPUT: ---- -fail_compilation/fail5634.d(9): Error: only one `main`$?:windows=, `WinMain`, or `DllMain`$ allowed. Previously found `main` at fail_compilation/fail5634.d(8) ----- +fail_compilation/fail5634.d(9): Error: function `D main()` conflicts with previous declaration at fail_compilation/fail5634.d(8) +--- */ void main() { } diff --git a/test/fail_compilation/fail7443.d b/test/fail_compilation/fail7443.d index 3770879e7917..2c5c94007462 100644 --- a/test/fail_compilation/fail7443.d +++ b/test/fail_compilation/fail7443.d @@ -10,5 +10,5 @@ class Foo { public static { this() {}} public shared static { this() {}} - public {this() {}} + public {this(int) {}} } diff --git a/test/fail_compilation/test18282.d b/test/fail_compilation/test18282.d index 72190410f9d8..b85380163e50 100644 --- a/test/fail_compilation/test18282.d +++ b/test/fail_compilation/test18282.d @@ -46,7 +46,7 @@ S2000 bar2() } -void bar2() +void bar3() { int i; char c; diff --git a/test/fail_compilation/test18385.d b/test/fail_compilation/test18385.d new file mode 100644 index 000000000000..3ec8253bd627 --- /dev/null +++ b/test/fail_compilation/test18385.d @@ -0,0 +1,31 @@ +/* +REQUIRED_ARGS: -de +TEST_OUTPUT: +--- +fail_compilation/test18385.d(13): Deprecation: function `test18385.foo` cannot overload `extern(C)` function at fail_compilation/test18385.d(12) +fail_compilation/test18385.d(18): Deprecation: function `test18385.S.foo` cannot overload `extern(C)` function at fail_compilation/test18385.d(17) +--- +*/ + +extern (C): + +void foo(int) { } +void foo(double) { } + +struct S +{ + static void foo(int) {} + static void foo(double) {} +} + +void foo2(int) { } +extern(D) void foo2(double) { } // OK as it has a different mangling + +void foo3(int) { } +void foo3(double); // duplicate declarations are allowed + +void foo4(); +void foo4() { } + +extern(D) void foo5(); +extern(D) void foo5() { }