[clang] [clang] Diagnose default arguments defined in different scopes (PR #124844)
Vlad Serebrennikov via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 10 10:07:49 PDT 2025
https://github.com/Endilll updated https://github.com/llvm/llvm-project/pull/124844
>From da30f708caee020677675277673e0b7c6f9c644f Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Fri, 24 Jan 2025 15:15:17 +0400
Subject: [PATCH 01/15] [clang] Diagnose default arguments defined in different
scopes
---
clang/docs/ReleaseNotes.rst | 5 ++
.../clang/Basic/DiagnosticSemaKinds.td | 4 ++
clang/include/clang/Sema/Overload.h | 3 +-
clang/lib/Sema/SemaDeclCXX.cpp | 5 ++
clang/lib/Sema/SemaOverload.cpp | 47 +++++++++++++++++++
clang/test/CXX/drs/cwg0xx.cpp | 14 ++++--
clang/test/CXX/drs/cwg4xx.cpp | 11 +++--
.../SemaCXX/default-argument-extern-c.cpp | 39 +++++++++++++++
clang/www/cxx_dr_status.html | 4 +-
9 files changed, 120 insertions(+), 12 deletions(-)
create mode 100644 clang/test/SemaCXX/default-argument-extern-c.cpp
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f110b8cf76507..00b1e3b678dda 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -375,6 +375,11 @@ Resolutions to C++ Defect Reports
by default.
(`CWG2521: User-defined literals and reserved identifiers <https://cplusplus.github.io/CWG/issues/2521.html>`_).
+- Clang now diagnoses ambiguous default arguments declared in different scopes
+ when calling an ``extern "C"`` function, implementing [over.best.match] p4.
+ (`CWG1: What if two using-declarations refer to the same function but the declarations introduce different default-arguments? <https://cplusplus.github.io/CWG/issues/1.html>`_,
+ `CWG418: Imperfect wording on error on multiple default arguments on a called function <https://cplusplus.github.io/CWG/issues/418.html>`_)
+
- Fix name lookup for a dependent base class that is the current instantiation.
(`CWG591: When a dependent base class is the current instantiation <https://cplusplus.github.io/CWG/issues/591.html>`_).
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 774e5484cfa0e..cfa47ec1c1f97 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5136,6 +5136,10 @@ def err_addr_ovl_not_func_ptrref : Error<
def err_addr_ovl_no_qualifier : Error<
"cannot form member pointer of type %0 without '&' and class name">;
+def err_ovl_ambiguous_default_arg
+ : Error<"function call relies on ambiguous default argument %select{|for "
+ "parameter '%1'}0">;
+
} // let Deferrable
// C++11 Literal Operators
diff --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.h
index c7f2422b542dd..8e0b9ef8b44b8 100644
--- a/clang/include/clang/Sema/Overload.h
+++ b/clang/include/clang/Sema/Overload.h
@@ -948,7 +948,8 @@ class Sema;
unsigned char FailureKind;
/// The number of call arguments that were explicitly provided,
- /// to be used while performing partial ordering of function templates.
+ /// to be used while performing partial ordering of function templates
+ /// and to diagnose ambiguous default arguments ([over.best.match]/4).
unsigned ExplicitCallArguments;
union {
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 839b3a1cccdcc..ba9c7c0237cd2 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -477,6 +477,11 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
// Ignore default arguments of old decl if they are not in
// the same scope and this is not an out-of-line definition of
// a member function.
+ //
+ // extern "C" functions can have default arguments across different
+ // scopes, but diagnosing that early would reject well-formed code
+ // (_N5001_.[over.best.match]/4.) Instead, they are checked
+ // in BestViableFunction after the best viable function has been selected.
continue;
}
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 6ae9c51c06b31..fe8ae5a4190ef 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -10928,6 +10928,53 @@ OverloadCandidateSet::BestViableFunction(Sema &S, SourceLocation Loc,
S.diagnoseEquivalentInternalLinkageDeclarations(Loc, Best->Function,
EquivalentCands);
+ // [over.match.best]/4:
+ // If the best viable function resolves to a function
+ // for which multiple declarations were found,
+ // and if any two of these declarations inhabit different scopes
+ // and specify a default argument that made the function viable,
+ // the program is ill-formed.
+ if (Best->Function && Best->Function->isExternC() &&
+ Best->ExplicitCallArguments < Best->Function->getNumParams()) {
+ // Calculate the range of parameters,
+ // default arguments of which made the candidate viable.
+ int FirstDefaultArgIndex = Best->ExplicitCallArguments;
+ int LastDefaultArgIndex = Best->Function->getNumParams() - 1;
+
+ // For each such parameter, collect all redeclarations
+ // that have non-inherited default argument.
+ llvm::SmallDenseMap<int, llvm::TinyPtrVector<ParmVarDecl *>> ParamRedecls(
+ LastDefaultArgIndex - FirstDefaultArgIndex + 1);
+ for (FunctionDecl *Redecl : Best->Function->redecls()) {
+ for (int i = FirstDefaultArgIndex; i <= LastDefaultArgIndex; ++i) {
+ ParmVarDecl *Param = Redecl->getParamDecl(i);
+ if (Param->hasDefaultArg() && !Param->hasInheritedDefaultArg())
+ ParamRedecls[i].push_back(Param);
+ }
+ }
+
+ // Emit the diagnostic if a given parameter has more than one declaration.
+ // MergeCXXFunctionDecl takes care of redeclarations of a default argument
+ // in the same scope, so if we found more than one,
+ // we assume they come from different scopes.
+ for (auto [ParamIndex, Redecls] : ParamRedecls) {
+ assert(!Redecls.empty());
+ if (Redecls.size() == 1)
+ continue;
+
+ ParmVarDecl *Param = Best->Function->getParamDecl(ParamIndex);
+ if (!Param->getDeclName().isEmpty()) {
+ S.Diag(Loc, diag::err_ovl_ambiguous_default_arg)
+ << 1 << Param->getName();
+ } else
+ S.Diag(Loc, diag::err_ovl_ambiguous_default_arg) << 0;
+ for (ParmVarDecl *Param : Redecls) {
+ S.Diag(Param->getDefaultArg()->getExprLoc(),
+ diag::note_default_argument_declared_here);
+ }
+ }
+ }
+
return OR_Success;
}
diff --git a/clang/test/CXX/drs/cwg0xx.cpp b/clang/test/CXX/drs/cwg0xx.cpp
index 15f469440c66f..6b969925c8c19 100644
--- a/clang/test/CXX/drs/cwg0xx.cpp
+++ b/clang/test/CXX/drs/cwg0xx.cpp
@@ -11,21 +11,25 @@
// cxx98-error at -1 {{variadic macros are a C99 feature}}
#endif
-namespace cwg1 { // cwg1: no
- namespace X { extern "C" void cwg1_f(int a = 1); }
- namespace Y { extern "C" void cwg1_f(int a = 1); }
+namespace cwg1 { // cwg1: 20
+ namespace X { extern "C" void cwg1_f(int a = 1); } // #cwg1-X
+ namespace Y { extern "C" void cwg1_f(int a = 1); } // #cwg1-Y
using X::cwg1_f; using Y::cwg1_f;
void g() {
cwg1_f(0);
- // FIXME: This should be rejected, due to the ambiguous default argument.
cwg1_f();
+ // expected-error at -1 {{function call relies on ambiguous default argument for parameter 'a'}}
+ // expected-note@#cwg1-Y {{default argument declared here}}
+ // expected-note@#cwg1-X {{default argument declared here}}
}
namespace X {
using Y::cwg1_f;
void h() {
cwg1_f(0);
- // FIXME: This should be rejected, due to the ambiguous default argument.
cwg1_f();
+ // expected-error at -1 {{function call relies on ambiguous default argument for parameter 'a'}}
+ // expected-note@#cwg1-Y {{default argument declared here}}
+ // expected-note@#cwg1-X {{default argument declared here}}
}
}
diff --git a/clang/test/CXX/drs/cwg4xx.cpp b/clang/test/CXX/drs/cwg4xx.cpp
index bcaf7db04ad3b..782ed70c5741b 100644
--- a/clang/test/CXX/drs/cwg4xx.cpp
+++ b/clang/test/CXX/drs/cwg4xx.cpp
@@ -369,7 +369,7 @@ namespace cwg417 { // cwg417: no
}
} // namespace cwg417
-namespace cwg418 { // cwg418: no
+namespace cwg418 { // cwg418: 20
namespace example1 {
void f1(int, int = 0);
void f1(int = 0, int);
@@ -398,10 +398,10 @@ void g2() {
// example from [over.match.best]/4
namespace example3 {
namespace A {
-extern "C" void f(int = 5);
+extern "C" void f(int = 5); // #cwg418-ex3-A
}
namespace B {
-extern "C" void f(int = 5);
+extern "C" void f(int = 5); // #cwg418-ex3-B
}
using A::f;
@@ -409,7 +409,10 @@ using B::f;
void use() {
f(3);
- f(); // FIXME: this should fail
+ f();
+ // expected-error at -1 {{function call relies on ambiguous default argument}}
+ // expected-note@#cwg418-ex3-B {{default argument declared here}}
+ // expected-note@#cwg418-ex3-A {{default argument declared here}}
}
} // namespace example3
} // namespace cwg418
diff --git a/clang/test/SemaCXX/default-argument-extern-c.cpp b/clang/test/SemaCXX/default-argument-extern-c.cpp
new file mode 100644
index 0000000000000..db20c48b1ab17
--- /dev/null
+++ b/clang/test/SemaCXX/default-argument-extern-c.cpp
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2c %s
+
+namespace A {
+ extern "C" void f1(...);
+ extern "C" void f2(int, ...);
+ extern "C" void f3(int = 0, ...); // #A-f3
+} // namespace A
+
+namespace B {
+ extern "C" void f1(...);
+ extern "C" void f2(int, ...); // #B-f2
+ extern "C" void f3(int = 0, ...); // #B-f3
+} // namespace B
+
+void f() {
+ using A::f1;
+ using A::f2;
+ using A::f3;
+ using B::f1;
+ using B::f2;
+ using B::f3;
+
+ f1();
+ f1(0);
+ f1(0, 0);
+ f2();
+ // expected-error at -1 {{no matching function for call to 'f2'}}
+ // expected-note@#B-f2 {{candidate function not viable: requires at least 1 argument, but 0 were provided}}
+ f2(0);
+ f2(0, 0);
+ f3();
+ // expected-error at -1 {{function call relies on ambiguous default argument}}
+ // expected-note@#B-f3 {{default argument declared here}}
+ // expected-note@#A-f3 {{default argument declared here}}
+ f3(0);
+ f3(0, 0);
+}
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 69ddd5e58b921..d9eb8c26317f3 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -51,7 +51,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/1.html">1</a></td>
<td>TC1</td>
<td>What if two using-declarations refer to the same function but the declarations introduce different default-arguments?</td>
- <td class="none" align="center">No</td>
+ <td class="unreleased" align="center">Clang 20</td>
</tr>
<tr class="open" id="2">
<td><a href="https://cplusplus.github.io/CWG/issues/2.html">2</a></td>
@@ -2555,7 +2555,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/418.html">418</a></td>
<td>CD6</td>
<td>Imperfect wording on error on multiple default arguments on a called function</td>
- <td class="none" align="center">No</td>
+ <td class="unreleased" align="center">Clang 20</td>
</tr>
<tr class="open" id="419">
<td><a href="https://cplusplus.github.io/CWG/issues/419.html">419</a></td>
>From a703624bcfe6f2e6b00c3091b63236caf3d5c09f Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Fri, 24 Jan 2025 15:32:44 +0400
Subject: [PATCH 02/15] Add a stress test
---
.../test/SemaCXX/default-argument-extern-c.cpp | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/clang/test/SemaCXX/default-argument-extern-c.cpp b/clang/test/SemaCXX/default-argument-extern-c.cpp
index db20c48b1ab17..4175b0d231ca5 100644
--- a/clang/test/SemaCXX/default-argument-extern-c.cpp
+++ b/clang/test/SemaCXX/default-argument-extern-c.cpp
@@ -37,3 +37,21 @@ void f() {
f3(0);
f3(0, 0);
}
+
+#define P_10(x) x, x, x, x, x, x, x, x, x, x,
+#define P_100(x) P_10(x) P_10(x) P_10(x) P_10(x) P_10(x) \
+ P_10(x) P_10(x) P_10(x) P_10(x) P_10(x)
+#define P_1000(x) P_100(x) P_100(x) P_100(x) P_100(x) P_100(x) \
+ P_100(x) P_100(x) P_100(x) P_100(x) P_100(x)
+#define P_10000(x) P_1000(x) P_1000(x) P_1000(x) P_1000(x) P_1000(x) \
+ P_1000(x) P_1000(x) P_1000(x) P_1000(x) P_1000(x)
+
+namespace C1 {
+extern "C" int g(
+ P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) int = 0
+ // expected-error at -1 {{too many function parameters; subsequent parameters will be ignored}}
+);
+} // namespace C1
+
+using C1::g;
+int h = g();
>From c911cba6c131bcb0873b8cccde1c68d72ff6042b Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Tue, 28 Jan 2025 16:58:50 +0400
Subject: [PATCH 03/15] Take extern and local function declarations into
account
---
clang/include/clang/Sema/Sema.h | 10 ++
clang/lib/Sema/SemaExpr.cpp | 8 ++
clang/lib/Sema/SemaOverload.cpp | 77 ++++++-------
clang/test/CXX/drs/cwg0xx.cpp | 15 ++-
clang/test/CodeGenCXX/default-arguments.cpp | 11 --
.../default-argument-different-scopes.cpp | 102 ++++++++++++++++++
.../SemaCXX/default-argument-extern-c.cpp | 57 ----------
clang/test/SemaCXX/default1.cpp | 7 +-
8 files changed, 174 insertions(+), 113 deletions(-)
create mode 100644 clang/test/SemaCXX/default-argument-different-scopes.cpp
delete mode 100644 clang/test/SemaCXX/default-argument-extern-c.cpp
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 4d6e02fe2956e..1c94f347f7613 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10388,6 +10388,16 @@ class Sema final : public SemaBase {
bool Complain = false,
SourceLocation Loc = SourceLocation());
+ /// @brief Checks that each default argument needed to make the call
+ /// is defined only once, implementing [over.match.best]/4 rule.
+ ///
+ /// @param FDecl Function declaration selected for the call
+ /// @param NumArgs Number of argument explicitly specified in the call
+ /// expression
+ /// @param CallLoc Source location of the call expression
+ void checkDefaultArgumentsAcrossScopes(FunctionDecl *FDecl, int NumArgs,
+ SourceLocation CallLoc);
+
// [PossiblyAFunctionType] --> [Return]
// NonFunctionType --> NonFunctionType
// R (A) --> R(A)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index d5273d463d7c0..b2bed0c68490d 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5883,6 +5883,14 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
// the call expression, before calling ConvertArgumentsForCall.
assert((Call->getNumArgs() == NumParams) &&
"We should have reserved space for the default arguments before!");
+
+ if (FDecl->isExternC() ||
+ std::any_of(
+ FDecl->redecls_begin(), FDecl->redecls_end(),
+ [](FunctionDecl *Redecl) { return Redecl->isLocalExternDecl(); })) {
+ checkDefaultArgumentsAcrossScopes(FDecl, Args.size(),
+ Call->getBeginLoc());
+ }
}
// If too many are passed and not variadic, error on the extras and drop
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index fe8ae5a4190ef..08cb5e54acd4b 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -10928,54 +10928,55 @@ OverloadCandidateSet::BestViableFunction(Sema &S, SourceLocation Loc,
S.diagnoseEquivalentInternalLinkageDeclarations(Loc, Best->Function,
EquivalentCands);
+ return OR_Success;
+}
+
+void Sema::checkDefaultArgumentsAcrossScopes(FunctionDecl *FDecl, int NumArgs,
+ SourceLocation CallLoc) {
// [over.match.best]/4:
// If the best viable function resolves to a function
// for which multiple declarations were found,
// and if any two of these declarations inhabit different scopes
// and specify a default argument that made the function viable,
// the program is ill-formed.
- if (Best->Function && Best->Function->isExternC() &&
- Best->ExplicitCallArguments < Best->Function->getNumParams()) {
- // Calculate the range of parameters,
- // default arguments of which made the candidate viable.
- int FirstDefaultArgIndex = Best->ExplicitCallArguments;
- int LastDefaultArgIndex = Best->Function->getNumParams() - 1;
-
- // For each such parameter, collect all redeclarations
- // that have non-inherited default argument.
- llvm::SmallDenseMap<int, llvm::TinyPtrVector<ParmVarDecl *>> ParamRedecls(
- LastDefaultArgIndex - FirstDefaultArgIndex + 1);
- for (FunctionDecl *Redecl : Best->Function->redecls()) {
- for (int i = FirstDefaultArgIndex; i <= LastDefaultArgIndex; ++i) {
- ParmVarDecl *Param = Redecl->getParamDecl(i);
- if (Param->hasDefaultArg() && !Param->hasInheritedDefaultArg())
- ParamRedecls[i].push_back(Param);
- }
- }
- // Emit the diagnostic if a given parameter has more than one declaration.
- // MergeCXXFunctionDecl takes care of redeclarations of a default argument
- // in the same scope, so if we found more than one,
- // we assume they come from different scopes.
- for (auto [ParamIndex, Redecls] : ParamRedecls) {
- assert(!Redecls.empty());
- if (Redecls.size() == 1)
- continue;
+ // Calculate the range of parameters,
+ // default arguments of which made the candidate viable.
+ int FirstDefaultArgIndex = NumArgs;
+ int LastDefaultArgIndex = FDecl->getNumParams() - 1;
+
+ // For each such parameter, collect all redeclarations
+ // that have non-inherited default argument.
+ llvm::SmallDenseMap<int, llvm::TinyPtrVector<ParmVarDecl *>> ParamRedecls(
+ LastDefaultArgIndex - FirstDefaultArgIndex + 1);
+ for (FunctionDecl *Redecl : FDecl->redecls()) {
+ for (int i = FirstDefaultArgIndex; i <= LastDefaultArgIndex; ++i) {
+ ParmVarDecl *Param = Redecl->getParamDecl(i);
+ if (Param->hasDefaultArg() && !Param->hasInheritedDefaultArg())
+ ParamRedecls[i].push_back(Param);
+ }
+ }
+
+ // Emit the diagnostic if a given parameter has more than one declaration.
+ // MergeCXXFunctionDecl takes care of redeclarations of a default argument
+ // in the same scope, so if we found more than one,
+ // we assume they come from different scopes.
+ for (auto [ParamIndex, Redecls] : ParamRedecls) {
+ assert(!Redecls.empty());
+ if (Redecls.size() == 1)
+ continue;
- ParmVarDecl *Param = Best->Function->getParamDecl(ParamIndex);
- if (!Param->getDeclName().isEmpty()) {
- S.Diag(Loc, diag::err_ovl_ambiguous_default_arg)
- << 1 << Param->getName();
- } else
- S.Diag(Loc, diag::err_ovl_ambiguous_default_arg) << 0;
- for (ParmVarDecl *Param : Redecls) {
- S.Diag(Param->getDefaultArg()->getExprLoc(),
- diag::note_default_argument_declared_here);
- }
+ ParmVarDecl *Param = FDecl->getParamDecl(ParamIndex);
+ if (!Param->getDeclName().isEmpty()) {
+ Diag(CallLoc, diag::err_ovl_ambiguous_default_arg)
+ << 1 << Param->getName();
+ } else
+ Diag(CallLoc, diag::err_ovl_ambiguous_default_arg) << 0;
+ for (ParmVarDecl *Param : Redecls) {
+ Diag(Param->getDefaultArg()->getExprLoc(),
+ diag::note_default_argument_declared_here);
}
}
-
- return OR_Success;
}
namespace {
diff --git a/clang/test/CXX/drs/cwg0xx.cpp b/clang/test/CXX/drs/cwg0xx.cpp
index 6b969925c8c19..bc8e9c12a98df 100644
--- a/clang/test/CXX/drs/cwg0xx.cpp
+++ b/clang/test/CXX/drs/cwg0xx.cpp
@@ -43,20 +43,25 @@ namespace cwg1 { // cwg1: 20
// expected-note@#cwg1-z {{previous definition is here}}
}
- void i(int = 1);
+ void i(int = 1); // #cwg1-i
void j() {
- void i(int = 1);
+ void i(int = 1); // #cwg1-i-redecl
using cwg1::i;
i(0);
- // FIXME: This should be rejected, due to the ambiguous default argument.
i();
+ // expected-error at -1 {{function call relies on ambiguous default argument}}
+ // expected-note@#cwg1-i-redecl {{default argument declared here}}
+ // expected-note@#cwg1-i {{default argument declared here}}
}
void k() {
using cwg1::i;
- void i(int = 1);
+ void i(int = 1); // #cwg1-i-redecl2
i(0);
- // FIXME: This should be rejected, due to the ambiguous default argument.
i();
+ // expected-error at -1 {{function call relies on ambiguous default argument}}
+ // expected-note@#cwg1-i-redecl2 {{default argument declared here}}
+ // expected-note@#cwg1-i-redecl {{default argument declared here}}
+ // expected-note@#cwg1-i {{default argument declared here}}
}
} // namespace cwg1
diff --git a/clang/test/CodeGenCXX/default-arguments.cpp b/clang/test/CodeGenCXX/default-arguments.cpp
index 2459ef1ad41fc..002d9fa703ccf 100644
--- a/clang/test/CodeGenCXX/default-arguments.cpp
+++ b/clang/test/CodeGenCXX/default-arguments.cpp
@@ -74,14 +74,3 @@ void f3() {
B *bs = new B[2];
delete bs;
}
-
-void f4() {
- void g4(int a, int b = 7);
- {
- void g4(int a, int b = 5);
- }
- void g4(int a = 5, int b);
-
- // CHECK: call void @_Z2g4ii(i32 noundef 5, i32 noundef 7)
- g4();
-}
diff --git a/clang/test/SemaCXX/default-argument-different-scopes.cpp b/clang/test/SemaCXX/default-argument-different-scopes.cpp
new file mode 100644
index 0000000000000..f61cc62c117e5
--- /dev/null
+++ b/clang/test/SemaCXX/default-argument-different-scopes.cpp
@@ -0,0 +1,102 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2c %s
+
+namespace A {
+ extern "C" void f1(...);
+ extern "C" void f2(int, ...);
+ extern "C" void f3(int = 0, ...); // #A-f3
+} // namespace A
+
+namespace B {
+ extern "C" void f1(...);
+ extern "C" void f2(int, ...); // #B-f2
+ extern "C" void f3(int = 0, ...); // #B-f3
+} // namespace B
+
+void f() {
+ using A::f1;
+ using A::f2;
+ using A::f3;
+ using B::f1;
+ using B::f2;
+ using B::f3;
+
+ f1();
+ f1(0);
+ f1(0, 0);
+ f2();
+ // expected-error at -1 {{no matching function for call to 'f2'}}
+ // expected-note@#B-f2 {{candidate function not viable: requires at least 1 argument, but 0 were provided}}
+ f2(0);
+ f2(0, 0);
+ f3();
+ // expected-error at -1 {{function call relies on ambiguous default argument}}
+ // expected-note@#B-f3 {{default argument declared here}}
+ // expected-note@#A-f3 {{default argument declared here}}
+ f3(0);
+ f3(0, 0);
+}
+
+#define P_10(x) x, x, x, x, x, x, x, x, x, x,
+#define P_100(x) P_10(x) P_10(x) P_10(x) P_10(x) P_10(x) \
+ P_10(x) P_10(x) P_10(x) P_10(x) P_10(x)
+#define P_1000(x) P_100(x) P_100(x) P_100(x) P_100(x) P_100(x) \
+ P_100(x) P_100(x) P_100(x) P_100(x) P_100(x)
+#define P_10000(x) P_1000(x) P_1000(x) P_1000(x) P_1000(x) P_1000(x) \
+ P_1000(x) P_1000(x) P_1000(x) P_1000(x) P_1000(x)
+
+namespace C1 {
+extern "C" int g(
+ P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) int = 0
+ // expected-error at -1 {{too many function parameters; subsequent parameters will be ignored}}
+);
+} // namespace C1
+
+using C1::g;
+int h = g();
+
+void i1(int = 2); // #i1
+void i2(int = 2); // #i2
+extern "C" void j1(int = 2); // #j1
+extern "C" void j2(int = 2); // #j2
+
+void f2() {
+ void i1(int = 3); // #i1-redecl
+ extern void i2(int = 3); // #i2-redecl
+ void j1(int = 3); // #j1-redecl
+ extern void j2(int = 3); // #j2-redecl
+
+ i1();
+ // expected-error at -1 {{function call relies on ambiguous default argument}}
+ // expected-note@#i1-redecl {{default argument declared here}}
+ // expected-note@#i1 {{default argument declared here}}
+ ::i1();
+ // expected-error at -1 {{function call relies on ambiguous default argument}}
+ // expected-note@#i1 {{default argument declared here}}
+ // expected-note@#i1-redecl {{default argument declared here}}
+ i2();
+ // expected-error at -1 {{function call relies on ambiguous default argument}}
+ // expected-note@#i2-redecl {{default argument declared here}}
+ // expected-note@#i2 {{default argument declared here}}
+ ::i2();
+ // expected-error at -1 {{function call relies on ambiguous default argument}}
+ // expected-note@#i2 {{default argument declared here}}
+ // expected-note@#i2-redecl {{default argument declared here}}
+ j1();
+ // expected-error at -1 {{function call relies on ambiguous default argument}}
+ // expected-note@#j1-redecl {{default argument declared here}}
+ // expected-note@#j1 {{default argument declared here}}
+ ::j1();
+ // expected-error at -1 {{function call relies on ambiguous default argument}}
+ // expected-note@#j1 {{default argument declared here}}
+ // expected-note@#j1-redecl {{default argument declared here}}
+ j2();
+ // expected-error at -1 {{function call relies on ambiguous default argument}}
+ // expected-note@#j2-redecl {{default argument declared here}}
+ // expected-note@#j2 {{default argument declared here}}
+ ::j2();
+ // expected-error at -1 {{function call relies on ambiguous default argument}}
+ // expected-note@#j2 {{default argument declared here}}
+ // expected-note@#j2-redecl {{default argument declared here}}
+}
diff --git a/clang/test/SemaCXX/default-argument-extern-c.cpp b/clang/test/SemaCXX/default-argument-extern-c.cpp
deleted file mode 100644
index 4175b0d231ca5..0000000000000
--- a/clang/test/SemaCXX/default-argument-extern-c.cpp
+++ /dev/null
@@ -1,57 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2c %s
-
-namespace A {
- extern "C" void f1(...);
- extern "C" void f2(int, ...);
- extern "C" void f3(int = 0, ...); // #A-f3
-} // namespace A
-
-namespace B {
- extern "C" void f1(...);
- extern "C" void f2(int, ...); // #B-f2
- extern "C" void f3(int = 0, ...); // #B-f3
-} // namespace B
-
-void f() {
- using A::f1;
- using A::f2;
- using A::f3;
- using B::f1;
- using B::f2;
- using B::f3;
-
- f1();
- f1(0);
- f1(0, 0);
- f2();
- // expected-error at -1 {{no matching function for call to 'f2'}}
- // expected-note@#B-f2 {{candidate function not viable: requires at least 1 argument, but 0 were provided}}
- f2(0);
- f2(0, 0);
- f3();
- // expected-error at -1 {{function call relies on ambiguous default argument}}
- // expected-note@#B-f3 {{default argument declared here}}
- // expected-note@#A-f3 {{default argument declared here}}
- f3(0);
- f3(0, 0);
-}
-
-#define P_10(x) x, x, x, x, x, x, x, x, x, x,
-#define P_100(x) P_10(x) P_10(x) P_10(x) P_10(x) P_10(x) \
- P_10(x) P_10(x) P_10(x) P_10(x) P_10(x)
-#define P_1000(x) P_100(x) P_100(x) P_100(x) P_100(x) P_100(x) \
- P_100(x) P_100(x) P_100(x) P_100(x) P_100(x)
-#define P_10000(x) P_1000(x) P_1000(x) P_1000(x) P_1000(x) P_1000(x) \
- P_1000(x) P_1000(x) P_1000(x) P_1000(x) P_1000(x)
-
-namespace C1 {
-extern "C" int g(
- P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) int = 0
- // expected-error at -1 {{too many function parameters; subsequent parameters will be ignored}}
-);
-} // namespace C1
-
-using C1::g;
-int h = g();
diff --git a/clang/test/SemaCXX/default1.cpp b/clang/test/SemaCXX/default1.cpp
index 8345b2433a3fe..675a3a61e4ca7 100644
--- a/clang/test/SemaCXX/default1.cpp
+++ b/clang/test/SemaCXX/default1.cpp
@@ -41,11 +41,14 @@ void kk(Y = 17); // expected-error{{no viable conversion}} \
// expected-note{{passing argument to parameter here}}
int l () {
- int m(int i, int j, int k = 3);
+ int m(int i, int j, int k = 3); // #m
if (1)
{
- int m(int i, int j = 2, int k = 4);
+ int m(int i, int j = 2, int k = 4); // #m-redecl
m(8);
+ // expected-error at -1 {{function call relies on ambiguous default argument for parameter 'k'}}
+ // expected-note@#m-redecl {{default argument declared here}}
+ // expected-note@#m {{default argument declared here}}
}
return 0;
}
>From 5a76dc9eba67f45f87595eb86f85872ccf066923 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Tue, 28 Jan 2025 17:01:22 +0400
Subject: [PATCH 04/15] Fix type in the reference to the Standard
---
clang/lib/Sema/SemaDeclCXX.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 28cd94fb855db..8fb6e378ead7b 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -480,7 +480,7 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
//
// extern "C" functions can have default arguments across different
// scopes, but diagnosing that early would reject well-formed code
- // (_N5001_.[over.best.match]/4.) Instead, they are checked
+ // (_N5001_.[over.match.best]/4.) Instead, they are checked
// in BestViableFunction after the best viable function has been selected.
continue;
}
>From ce204e4905bec9c042d4b355eb431200e8788a52 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Tue, 28 Jan 2025 17:08:49 +0400
Subject: [PATCH 05/15] Leave a comment in BestViableFunction
---
clang/lib/Sema/SemaOverload.cpp | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index cf4ac6a0b9af4..26a4d5a822e3d 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -10951,6 +10951,10 @@ OverloadCandidateSet::BestViableFunction(Sema &S, SourceLocation Loc,
S.diagnoseEquivalentInternalLinkageDeclarations(Loc, Best->Function,
EquivalentCands);
+ // [over.match.best]/4 is checked for in Sema::ConvertArgumentsForCall,
+ // because not every function call goes through our overload resolution
+ // machinery, even if the Standard says it supposed to.
+
return OR_Success;
}
>From f47e8cc9a659743abb34e91ee53fa5d0af4cb416 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Wed, 29 Jan 2025 00:17:30 +0400
Subject: [PATCH 06/15] Handle friend declarations
---
clang/lib/Sema/SemaDeclCXX.cpp | 21 +++++--
clang/test/CXX/drs/cwg0xx.cpp | 6 +-
clang/test/CXX/drs/cwg1xx.cpp | 1 +
.../default-argument-different-scopes.cpp | 62 +++++++++++++++++++
4 files changed, 81 insertions(+), 9 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 8fb6e378ead7b..b13362426f192 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -454,11 +454,13 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
bool Invalid = false;
// The declaration context corresponding to the scope is the semantic
- // parent, unless this is a local function declaration, in which case
- // it is that surrounding function.
- DeclContext *ScopeDC = New->isLocalExternDecl()
- ? New->getLexicalDeclContext()
- : New->getDeclContext();
+ // parent, unless this is a local function declaration
+ // or a friend declaration, in which case it is that surrounding function.
+ DeclContext *ScopeDC =
+ New->isLocalExternDecl() ||
+ New->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend)
+ ? New->getLexicalDeclContext()
+ : New->getDeclContext();
// Find the previous declaration for the purpose of default arguments.
FunctionDecl *PrevForDefaultArgs = Old;
@@ -493,6 +495,15 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
continue;
}
+ if (PrevForDefaultArgs->getLexicalDeclContext()->getPrimaryContext() !=
+ ScopeDC->getPrimaryContext() &&
+ !New->isCXXClassMember()) {
+ // If previous declaration is lexically in a different scope,
+ // we don't inherit its default arguments, except for out-of-line
+ // declarations of member functions.
+ continue;
+ }
+
// We found the right previous declaration.
break;
}
diff --git a/clang/test/CXX/drs/cwg0xx.cpp b/clang/test/CXX/drs/cwg0xx.cpp
index bc8e9c12a98df..4d45898a39776 100644
--- a/clang/test/CXX/drs/cwg0xx.cpp
+++ b/clang/test/CXX/drs/cwg0xx.cpp
@@ -36,11 +36,9 @@ namespace cwg1 { // cwg1: 20
namespace X {
void z(int);
}
- void X::z(int = 1) {} // #cwg1-z
+ void X::z(int = 1) {}
namespace X {
- void z(int = 1);
- // expected-error at -1 {{redefinition of default argument}}
- // expected-note@#cwg1-z {{previous definition is here}}
+ void z(int = 1); // OK, namespace X has a distinct set of default arguments
}
void i(int = 1); // #cwg1-i
diff --git a/clang/test/CXX/drs/cwg1xx.cpp b/clang/test/CXX/drs/cwg1xx.cpp
index 15bcc20b7fa2a..f6aa0b3ffb35c 100644
--- a/clang/test/CXX/drs/cwg1xx.cpp
+++ b/clang/test/CXX/drs/cwg1xx.cpp
@@ -518,6 +518,7 @@ namespace cwg136 { // cwg136: 3.4
friend void f(int, int = 0, int);
// expected-error at -1 {{friend declaration specifying a default argument must be the only declaration}}
// expected-note@#cwg136-f {{previous declaration is here}}
+ // expected-error at -3 {{missing default argument on parameter}}
friend void g(int, int, int = 0);
// expected-error at -1 {{friend declaration specifying a default argument must be the only declaration}}
// expected-note@#cwg136-g {{previous declaration is here}}
diff --git a/clang/test/SemaCXX/default-argument-different-scopes.cpp b/clang/test/SemaCXX/default-argument-different-scopes.cpp
index f61cc62c117e5..e78f120fea048 100644
--- a/clang/test/SemaCXX/default-argument-different-scopes.cpp
+++ b/clang/test/SemaCXX/default-argument-different-scopes.cpp
@@ -100,3 +100,65 @@ void f2() {
// expected-note@#j2 {{default argument declared here}}
// expected-note@#j2-redecl {{default argument declared here}}
}
+
+// In 'k' group of tests, no redefinition of default arguments occur,
+// because sets of default arguments are associated with lexical scopes
+// of function declarations.
+
+void k1(int); // #k1
+void k2(int = 2);
+void k3(int = 3); // #k3
+
+struct K {
+ friend void k1(int = 1) {}
+ // expected-error at -1 {{friend declaration specifying a default argument must be the only declaration}}
+ // expected-note@#k1 {{previous declaration is here}}
+ friend void k2(int) {}
+ friend void k3(int = 3) {}
+ // expected-error at -1 {{friend declaration specifying a default argument must be the only declaration}}
+ // expected-note@#k3 {{previous declaration is here}}
+
+ friend void k4(int = 4) {} // #k4
+ friend void k5(int) {}
+ friend void k6(int = 6) {} // #k6
+};
+
+void k4(int);
+// expected-error at -1 {{friend declaration specifying a default argument must be the only declaration}}
+// expected-note@#k4 {{previous declaration is here}}
+void k5(int = 5);
+void k6(int = 6);
+// expected-error at -1 {{friend declaration specifying a default argument must be the only declaration}}
+// expected-note@#k6 {{previous declaration is here}}
+
+struct L {
+ void l1(int);
+ void l2(int = 2);
+ void l3(int = 3); // #l3
+
+ template <typename>
+ void l4(int); // #l4
+ template <typename>
+ void l5(int = 5);
+ template <typename>
+ void l6(int = 6); // #l6
+};
+
+void L::l1(int = 1) {}
+void L::l2(int) {}
+void L::l3(int = 3) {}
+// expected-error at -1 {{redefinition of default argument}}
+// expected-note@#l3 {{previous definition is here}}
+
+template <typename>
+void L::l4(int = 4) {}
+// expected-error at -1 {{default arguments cannot be added to a function template that has already been declared}}
+// expected-note@#l4 {{previous template declaration is here}}
+
+template <typename>
+void L::l5(int) {}
+
+template <typename>
+void L::l6(int = 6) {}
+// expected-error at -1 {{redefinition of default argument}}
+// expected-note@#l6 {{previous definition is here}}
>From bfa5851a318826018f8c592c88e853726383dcbe Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Wed, 29 Jan 2025 00:22:51 +0400
Subject: [PATCH 07/15] Update release notes
---
clang/docs/ReleaseNotes.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 18d6944083cb2..ad2d7f02a59aa 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -384,7 +384,7 @@ Resolutions to C++ Defect Reports
(`CWG2521: User-defined literals and reserved identifiers <https://cplusplus.github.io/CWG/issues/2521.html>`_).
- Clang now diagnoses ambiguous default arguments declared in different scopes
- when calling an ``extern "C"`` function, implementing [over.best.match] p4.
+ when calling functions, implementing [over.best.match] p4.
(`CWG1: What if two using-declarations refer to the same function but the declarations introduce different default-arguments? <https://cplusplus.github.io/CWG/issues/1.html>`_,
`CWG418: Imperfect wording on error on multiple default arguments on a called function <https://cplusplus.github.io/CWG/issues/418.html>`_)
>From 084fc667bb869d666b623b933b8aa613ab941ccb Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Wed, 29 Jan 2025 00:23:41 +0400
Subject: [PATCH 08/15] Fix stable name in release notes
---
clang/docs/ReleaseNotes.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ad2d7f02a59aa..f2c722ce92841 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -384,7 +384,7 @@ Resolutions to C++ Defect Reports
(`CWG2521: User-defined literals and reserved identifiers <https://cplusplus.github.io/CWG/issues/2521.html>`_).
- Clang now diagnoses ambiguous default arguments declared in different scopes
- when calling functions, implementing [over.best.match] p4.
+ when calling functions, implementing [over.match.best] p4.
(`CWG1: What if two using-declarations refer to the same function but the declarations introduce different default-arguments? <https://cplusplus.github.io/CWG/issues/1.html>`_,
`CWG418: Imperfect wording on error on multiple default arguments on a called function <https://cplusplus.github.io/CWG/issues/418.html>`_)
>From e7411f8b5bc7fc2e69c39ca3c375b9a1ae6523d8 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Wed, 29 Jan 2025 01:02:44 +0400
Subject: [PATCH 09/15] Move the new check function to `SemaExpr.cpp` where
it's now used
---
clang/include/clang/Sema/Sema.h | 10 ------
clang/lib/Sema/SemaExpr.cpp | 58 ++++++++++++++++++++++++++++++++-
clang/lib/Sema/SemaOverload.cpp | 48 ---------------------------
3 files changed, 57 insertions(+), 59 deletions(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 473315ffd50f8..528304409b809 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10389,16 +10389,6 @@ class Sema final : public SemaBase {
bool Complain = false,
SourceLocation Loc = SourceLocation());
- /// @brief Checks that each default argument needed to make the call
- /// is defined only once, implementing [over.match.best]/4 rule.
- ///
- /// @param FDecl Function declaration selected for the call
- /// @param NumArgs Number of argument explicitly specified in the call
- /// expression
- /// @param CallLoc Source location of the call expression
- void checkDefaultArgumentsAcrossScopes(FunctionDecl *FDecl, int NumArgs,
- SourceLocation CallLoc);
-
// [PossiblyAFunctionType] --> [Return]
// NonFunctionType --> NonFunctionType
// R (A) --> R(A)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 5dfcc04527b9f..742f345232768 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5810,6 +5810,62 @@ static bool isParenthetizedAndQualifiedAddressOfExpr(Expr *Fn) {
return false;
}
+/// @brief Checks that each default argument needed to make the call
+/// is defined only once, implementing [over.match.best]/4 rule.
+///
+/// @param FDecl Function declaration selected for the call
+/// @param NumArgs Number of argument explicitly specified in the call
+/// expression
+/// @param CallLoc Source location of the call expression
+static void checkDefaultArgumentsAcrossScopes(Sema &S, FunctionDecl *FDecl,
+ int NumArgs,
+ SourceLocation CallLoc) {
+ // [over.match.best]/4:
+ // If the best viable function resolves to a function
+ // for which multiple declarations were found,
+ // and if any two of these declarations inhabit different scopes
+ // and specify a default argument that made the function viable,
+ // the program is ill-formed.
+
+ // Calculate the range of parameters,
+ // default arguments of which made the candidate viable.
+ int FirstDefaultArgIndex = NumArgs;
+ int LastDefaultArgIndex = FDecl->getNumParams() - 1;
+
+ // For each such parameter, collect all redeclarations
+ // that have non-inherited default argument.
+ llvm::SmallDenseMap<int, llvm::TinyPtrVector<ParmVarDecl *>> ParamRedecls(
+ LastDefaultArgIndex - FirstDefaultArgIndex + 1);
+ for (FunctionDecl *Redecl : FDecl->redecls()) {
+ for (int i = FirstDefaultArgIndex; i <= LastDefaultArgIndex; ++i) {
+ ParmVarDecl *Param = Redecl->getParamDecl(i);
+ if (Param->hasDefaultArg() && !Param->hasInheritedDefaultArg())
+ ParamRedecls[i].push_back(Param);
+ }
+ }
+
+ // Emit the diagnostic if a given parameter has more than one declaration.
+ // MergeCXXFunctionDecl takes care of redeclarations of a default argument
+ // in the same scope, so if we found more than one,
+ // we assume they come from different scopes.
+ for (auto [ParamIndex, Redecls] : ParamRedecls) {
+ assert(!Redecls.empty());
+ if (Redecls.size() == 1)
+ continue;
+
+ ParmVarDecl *Param = FDecl->getParamDecl(ParamIndex);
+ if (!Param->getDeclName().isEmpty()) {
+ S.Diag(CallLoc, diag::err_ovl_ambiguous_default_arg)
+ << 1 << Param->getName();
+ } else
+ S.Diag(CallLoc, diag::err_ovl_ambiguous_default_arg) << 0;
+ for (ParmVarDecl *Param : Redecls) {
+ S.Diag(Param->getDefaultArg()->getExprLoc(),
+ diag::note_default_argument_declared_here);
+ }
+ }
+}
+
bool
Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
FunctionDecl *FDecl,
@@ -5888,7 +5944,7 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
std::any_of(
FDecl->redecls_begin(), FDecl->redecls_end(),
[](FunctionDecl *Redecl) { return Redecl->isLocalExternDecl(); })) {
- checkDefaultArgumentsAcrossScopes(FDecl, Args.size(),
+ checkDefaultArgumentsAcrossScopes(*this, FDecl, Args.size(),
Call->getBeginLoc());
}
}
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 26a4d5a822e3d..0b616aa07528b 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -10958,54 +10958,6 @@ OverloadCandidateSet::BestViableFunction(Sema &S, SourceLocation Loc,
return OR_Success;
}
-void Sema::checkDefaultArgumentsAcrossScopes(FunctionDecl *FDecl, int NumArgs,
- SourceLocation CallLoc) {
- // [over.match.best]/4:
- // If the best viable function resolves to a function
- // for which multiple declarations were found,
- // and if any two of these declarations inhabit different scopes
- // and specify a default argument that made the function viable,
- // the program is ill-formed.
-
- // Calculate the range of parameters,
- // default arguments of which made the candidate viable.
- int FirstDefaultArgIndex = NumArgs;
- int LastDefaultArgIndex = FDecl->getNumParams() - 1;
-
- // For each such parameter, collect all redeclarations
- // that have non-inherited default argument.
- llvm::SmallDenseMap<int, llvm::TinyPtrVector<ParmVarDecl *>> ParamRedecls(
- LastDefaultArgIndex - FirstDefaultArgIndex + 1);
- for (FunctionDecl *Redecl : FDecl->redecls()) {
- for (int i = FirstDefaultArgIndex; i <= LastDefaultArgIndex; ++i) {
- ParmVarDecl *Param = Redecl->getParamDecl(i);
- if (Param->hasDefaultArg() && !Param->hasInheritedDefaultArg())
- ParamRedecls[i].push_back(Param);
- }
- }
-
- // Emit the diagnostic if a given parameter has more than one declaration.
- // MergeCXXFunctionDecl takes care of redeclarations of a default argument
- // in the same scope, so if we found more than one,
- // we assume they come from different scopes.
- for (auto [ParamIndex, Redecls] : ParamRedecls) {
- assert(!Redecls.empty());
- if (Redecls.size() == 1)
- continue;
-
- ParmVarDecl *Param = FDecl->getParamDecl(ParamIndex);
- if (!Param->getDeclName().isEmpty()) {
- Diag(CallLoc, diag::err_ovl_ambiguous_default_arg)
- << 1 << Param->getName();
- } else
- Diag(CallLoc, diag::err_ovl_ambiguous_default_arg) << 0;
- for (ParmVarDecl *Param : Redecls) {
- Diag(Param->getDefaultArg()->getExprLoc(),
- diag::note_default_argument_declared_here);
- }
- }
-}
-
namespace {
enum OverloadCandidateKind {
>From b250d9020af1b740cbf47a83eee30782187f50a4 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Wed, 29 Jan 2025 01:09:10 +0400
Subject: [PATCH 10/15] Mark DRs are available in Clang 21
---
clang/test/CXX/drs/cwg0xx.cpp | 2 +-
clang/test/CXX/drs/cwg4xx.cpp | 2 +-
clang/www/cxx_dr_status.html | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang/test/CXX/drs/cwg0xx.cpp b/clang/test/CXX/drs/cwg0xx.cpp
index 9fdfa36f891d5..f86431e21a766 100644
--- a/clang/test/CXX/drs/cwg0xx.cpp
+++ b/clang/test/CXX/drs/cwg0xx.cpp
@@ -11,7 +11,7 @@
// cxx98-error at -1 {{variadic macros are a C99 feature}}
#endif
-namespace cwg1 { // cwg1: 20
+namespace cwg1 { // cwg1: 21
namespace X { extern "C" void cwg1_f(int a = 1); } // #cwg1-X
namespace Y { extern "C" void cwg1_f(int a = 1); } // #cwg1-Y
using X::cwg1_f; using Y::cwg1_f;
diff --git a/clang/test/CXX/drs/cwg4xx.cpp b/clang/test/CXX/drs/cwg4xx.cpp
index 782ed70c5741b..5f1aac1bdfe40 100644
--- a/clang/test/CXX/drs/cwg4xx.cpp
+++ b/clang/test/CXX/drs/cwg4xx.cpp
@@ -369,7 +369,7 @@ namespace cwg417 { // cwg417: no
}
} // namespace cwg417
-namespace cwg418 { // cwg418: 20
+namespace cwg418 { // cwg418: 21
namespace example1 {
void f1(int, int = 0);
void f1(int = 0, int);
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index d9eb8c26317f3..d8fb845b5beba 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -51,7 +51,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/1.html">1</a></td>
<td>TC1</td>
<td>What if two using-declarations refer to the same function but the declarations introduce different default-arguments?</td>
- <td class="unreleased" align="center">Clang 20</td>
+ <td class="unreleased" align="center">Clang 21</td>
</tr>
<tr class="open" id="2">
<td><a href="https://cplusplus.github.io/CWG/issues/2.html">2</a></td>
@@ -2555,7 +2555,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/418.html">418</a></td>
<td>CD6</td>
<td>Imperfect wording on error on multiple default arguments on a called function</td>
- <td class="unreleased" align="center">Clang 20</td>
+ <td class="unreleased" align="center">Clang 21</td>
</tr>
<tr class="open" id="419">
<td><a href="https://cplusplus.github.io/CWG/issues/419.html">419</a></td>
>From afdf43bfaf1c78a309741e83b1c40cb64ffbaee6 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Wed, 29 Jan 2025 01:14:55 +0400
Subject: [PATCH 11/15] Update comment in MergeCXXFunctionDecl
---
clang/lib/Sema/SemaDeclCXX.cpp | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 40989ef50eca7..b0e946abe7e9c 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -479,11 +479,6 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
// Ignore default arguments of old decl if they are not in
// the same scope and this is not an out-of-line definition of
// a member function.
- //
- // extern "C" functions can have default arguments across different
- // scopes, but diagnosing that early would reject well-formed code
- // (_N5001_.[over.match.best]/4.) Instead, they are checked
- // in BestViableFunction after the best viable function has been selected.
continue;
}
@@ -501,6 +496,12 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
// If previous declaration is lexically in a different scope,
// we don't inherit its default arguments, except for out-of-line
// declarations of member functions.
+ //
+ // extern "C" and local functions can have default arguments across
+ // different scopes, but diagnosing that early would reject well-formed
+ // code (_N5001_.[over.match.best]/4.) Instead, they are checked
+ // in ConvertArgumentsForCall, after the best viable function has been
+ // selected.
continue;
}
>From 344cd4c7248c933850cc509a37a5d516efc16c96 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Wed, 29 Jan 2025 01:32:53 +0400
Subject: [PATCH 12/15] Move the stress test around
---
.../test/Parser/function-parameter-limit.cpp | 20 +++++++++++++++++++
.../default-argument-different-scopes.cpp | 18 -----------------
2 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/clang/test/Parser/function-parameter-limit.cpp b/clang/test/Parser/function-parameter-limit.cpp
index 29f5dde294715..b543f6a7ac848 100644
--- a/clang/test/Parser/function-parameter-limit.cpp
+++ b/clang/test/Parser/function-parameter-limit.cpp
@@ -27,3 +27,23 @@ extern double(*func2)(
P_10000(int u)
P_10000(int v) // expected-error {{too many function parameters; subsequent parameters will be ignored}}
int w);
+
+#define PD_10(x) x, x, x, x, x, x, x, x, x, x,
+#define PD_100(x) PD_10(x) PD_10(x) PD_10(x) PD_10(x) PD_10(x) \
+ PD_10(x) PD_10(x) PD_10(x) PD_10(x) PD_10(x)
+#define PD_1000(x) PD_100(x) PD_100(x) PD_100(x) PD_100(x) PD_100(x) \
+ PD_100(x) PD_100(x) PD_100(x) PD_100(x) PD_100(x)
+#define PD_10000(x) PD_1000(x) PD_1000(x) PD_1000(x) PD_1000(x) PD_1000(x) \
+ PD_1000(x) PD_1000(x) PD_1000(x) PD_1000(x) PD_1000(x)
+
+extern "C" int func3(
+ PD_10000(int = 0)
+ PD_10000(int = 0)
+ PD_10000(int = 0)
+ PD_10000(int = 0)
+ PD_10000(int = 0)
+ PD_10000(int = 0)
+ PD_10000(int = 0) // expected-error {{too many function parameters; subsequent parameters will be ignored}}
+ int = 0);
+
+int h = func3();
diff --git a/clang/test/SemaCXX/default-argument-different-scopes.cpp b/clang/test/SemaCXX/default-argument-different-scopes.cpp
index e78f120fea048..cb7addedb7ec2 100644
--- a/clang/test/SemaCXX/default-argument-different-scopes.cpp
+++ b/clang/test/SemaCXX/default-argument-different-scopes.cpp
@@ -38,24 +38,6 @@ void f() {
f3(0, 0);
}
-#define P_10(x) x, x, x, x, x, x, x, x, x, x,
-#define P_100(x) P_10(x) P_10(x) P_10(x) P_10(x) P_10(x) \
- P_10(x) P_10(x) P_10(x) P_10(x) P_10(x)
-#define P_1000(x) P_100(x) P_100(x) P_100(x) P_100(x) P_100(x) \
- P_100(x) P_100(x) P_100(x) P_100(x) P_100(x)
-#define P_10000(x) P_1000(x) P_1000(x) P_1000(x) P_1000(x) P_1000(x) \
- P_1000(x) P_1000(x) P_1000(x) P_1000(x) P_1000(x)
-
-namespace C1 {
-extern "C" int g(
- P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) P_10000(int = 0) int = 0
- // expected-error at -1 {{too many function parameters; subsequent parameters will be ignored}}
-);
-} // namespace C1
-
-using C1::g;
-int h = g();
-
void i1(int = 2); // #i1
void i2(int = 2); // #i2
extern "C" void j1(int = 2); // #j1
>From 0e24e378a7ca98064137eb49e85d5850d0638705 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Wed, 29 Jan 2025 01:37:35 +0400
Subject: [PATCH 13/15] I love our coding standard w.r.t. single-line if
statements
---
clang/lib/Sema/SemaDeclCXX.cpp | 3 +--
clang/lib/Sema/SemaExpr.cpp | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index b0e946abe7e9c..75b66d8d6298d 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -492,7 +492,7 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
if (PrevForDefaultArgs->getLexicalDeclContext()->getPrimaryContext() !=
ScopeDC->getPrimaryContext() &&
- !New->isCXXClassMember()) {
+ !New->isCXXClassMember())
// If previous declaration is lexically in a different scope,
// we don't inherit its default arguments, except for out-of-line
// declarations of member functions.
@@ -503,7 +503,6 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
// in ConvertArgumentsForCall, after the best viable function has been
// selected.
continue;
- }
// We found the right previous declaration.
break;
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 742f345232768..b177811ef19d0 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5943,10 +5943,9 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
if (FDecl->isExternC() ||
std::any_of(
FDecl->redecls_begin(), FDecl->redecls_end(),
- [](FunctionDecl *Redecl) { return Redecl->isLocalExternDecl(); })) {
+ [](FunctionDecl *Redecl) { return Redecl->isLocalExternDecl(); }))
checkDefaultArgumentsAcrossScopes(*this, FDecl, Args.size(),
Call->getBeginLoc());
- }
}
// If too many are passed and not variadic, error on the extras and drop
>From 59c9a932ba21833d0b01683853022dede536ad15 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Wed, 29 Jan 2025 08:58:21 +0400
Subject: [PATCH 14/15] Add a test with class template
---
.../default-argument-different-scopes.cpp | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/clang/test/SemaCXX/default-argument-different-scopes.cpp b/clang/test/SemaCXX/default-argument-different-scopes.cpp
index cb7addedb7ec2..ad20e18a21604 100644
--- a/clang/test/SemaCXX/default-argument-different-scopes.cpp
+++ b/clang/test/SemaCXX/default-argument-different-scopes.cpp
@@ -144,3 +144,22 @@ template <typename>
void L::l6(int = 6) {}
// expected-error at -1 {{redefinition of default argument}}
// expected-note@#l6 {{previous definition is here}}
+
+template <typename>
+struct M {
+ void m1(int);
+ void m2(int = 2);
+ void m3(int = 3); // #m3
+};
+
+template <typename T>
+void M<T>::m1(int = 1) {}
+// expected-error at -1 {{default arguments cannot be added to an out-of-line definition of a member of a class template}}
+
+template <typename T>
+void M<T>::m2(int) {}
+
+template <typename T>
+void M<T>::m3(int = 3) {}
+// expected-error at -1 {{redefinition of default argument}}
+// expected-note@#m3 {{previous definition is here}}
>From 91546caf6ed464f779af641faab283431a5e9469 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
Date: Fri, 7 Mar 2025 00:18:01 +0400
Subject: [PATCH 15/15] Version 2: implementation based on overload resolution
---
.../clang/Basic/DiagnosticSemaKinds.td | 4 +-
clang/include/clang/Sema/Overload.h | 46 +++++++-
clang/lib/AST/Decl.cpp | 27 +++++
clang/lib/Sema/SemaDeclCXX.cpp | 12 +-
clang/lib/Sema/SemaExpr.cpp | 63 ----------
clang/lib/Sema/SemaLookup.cpp | 26 ++++-
clang/lib/Sema/SemaOverload.cpp | 59 +++++++++-
clang/test/CXX/drs/cwg0xx.cpp | 17 ++-
clang/test/CXX/drs/cwg4xx.cpp | 16 ++-
.../default-argument-different-scopes.cpp | 109 +++++++++++-------
clang/test/SemaCXX/default1.cpp | 7 +-
11 files changed, 251 insertions(+), 135 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8b4e749c70d8f..39f05fb083ed1 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5143,8 +5143,8 @@ def err_addr_ovl_no_qualifier : Error<
"cannot form member pointer of type %0 without '&' and class name">;
def err_ovl_ambiguous_default_arg
- : Error<"function call relies on ambiguous default argument %select{|for "
- "parameter '%1'}0">;
+ : Error<"function call relies on default argument that has multiple "
+ "definitions">;
} // let Deferrable
diff --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.h
index bd04d4b073605..10a02864f76e0 100644
--- a/clang/include/clang/Sema/Overload.h
+++ b/clang/include/clang/Sema/Overload.h
@@ -901,6 +901,13 @@ class Sema;
LLVM_PREFERRED_TYPE(bool)
unsigned Viable : 1;
+ /// Whether this candidate wouldn't be viable without default arguments.
+ /// This is needed to implement the special provision about ambiguous
+ /// default arguments of the best viable function described in
+ /// [over.match.best]/4.
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned ViableByDefaultArgument : 1;
+
/// Whether this candidate is the best viable function, or tied for being
/// the best viable function.
///
@@ -1177,11 +1184,38 @@ class Sema;
/// Whether diagnostics should be deferred.
bool shouldDeferDiags(Sema &S, ArrayRef<Expr *> Args, SourceLocation OpLoc);
- /// Determine when this overload candidate will be new to the
- /// overload set.
+ /// Determine when this overload candidate will be new to the overload set.
+ /// Typically the uniqueness is determined by canonical declaration
+ /// (i.e. semantic entity), but it's aware of a special case of default
+ /// arguments in redeclarations spanning across different lexical scopes.
+ /// See also eliminateDuplicatesWithUnusedDefaultArgs().
bool isNewCandidate(Decl *F, OverloadCandidateParamOrder PO =
OverloadCandidateParamOrder::Normal) {
uintptr_t Key = reinterpret_cast<uintptr_t>(F->getCanonicalDecl());
+
+ // There is one special case when we can't rely on just semantic
+ // properties of the function (i.e. entity in the Standard sense),
+ // which is default arguments defined in redeclarations across
+ // different scopes, because sets of default arguments are associated
+ // with lexical scope of function declaration. Which means single
+ // function entity can have more than one set of default arguments,
+ // and which of those sets are considered depends solely on what
+ // name lookup has found for each individual call expression.
+ // See [over.match.best]/4 for more details.
+ // In such case, we use (lexical) function declaration to determine
+ // uniqueness of overload candidate, instead of (semantic) entity
+ // it represents. This would lead to duplicates, which will be
+ // eliminated by eliminateDuplicatesWithUnusedDefaultArgs()
+ // after every candidate will be considered.
+ if (const auto *FD = dyn_cast<FunctionDecl>(F);
+ FD && FD->getLexicalDeclContext() !=
+ F->getCanonicalDecl()->getLexicalDeclContext()) {
+ auto HasDefaultArg = [](const ParmVarDecl *PDecl) {
+ return PDecl->hasDefaultArg();
+ };
+ if (llvm::any_of(FD->parameters(), HasDefaultArg))
+ Key = reinterpret_cast<uintptr_t>(FD);
+ }
Key |= static_cast<uintptr_t>(PO);
return Functions.insert(Key).second;
}
@@ -1195,6 +1229,14 @@ class Sema;
/// Clear out all of the candidates.
void clear(CandidateSetKind CSK);
+ /// Clean up duplicates left by isNewCandidate()
+ /// (see comment in its implementation for details.)
+ /// When this function is called, we should know which of duplicated
+ /// candidates are viable because of their default arguments, and those
+ /// are the only duplicates we are interested in during subsequent
+ /// selection of the best viable function.
+ void eliminateDuplicatesWithUnusedDefaultArgs();
+
using iterator = SmallVectorImpl<OverloadCandidate>::iterator;
iterator begin() { return Candidates.begin(); }
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 610207cf8b9a4..3b7a6f6a40e50 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -1883,6 +1883,33 @@ bool NamedDecl::declarationReplaces(const NamedDecl *OldD,
cast<UnresolvedUsingValueDecl>(OldD)->getQualifier());
}
+ {
+ // When default arguments across redeclarations in different lexical scopes
+ // are involved, we can't let one UsingShadowDecl to replace another based
+ // on the fact that they refer to the same canonical function.
+ const auto *OldShadowD = dyn_cast<UsingShadowDecl>(OldD);
+ const auto *NewShadowD = dyn_cast<UsingShadowDecl>(this);
+ if (OldShadowD && NewShadowD &&
+ isa<FunctionDecl>(OldShadowD->getTargetDecl()) &&
+ isa<FunctionDecl>(NewShadowD->getTargetDecl())) {
+ const auto *OldFDecl = cast<FunctionDecl>(OldShadowD->getTargetDecl());
+ const auto *NewFDecl = cast<FunctionDecl>(NewShadowD->getTargetDecl());
+ const DeclContext *OldDeclCtx = OldFDecl->getLexicalDeclContext()
+ ->getNonTransparentContext()
+ ->getPrimaryContext();
+ const DeclContext *NewDeclCtx = NewFDecl->getLexicalDeclContext()
+ ->getNonTransparentContext()
+ ->getPrimaryContext();
+ auto hasDefaultArg = [](const ParmVarDecl *PDecl) {
+ return PDecl->hasDefaultArg();
+ };
+ if (OldDeclCtx != NewDeclCtx &&
+ llvm::any_of(OldFDecl->parameters(), hasDefaultArg) &&
+ llvm::any_of(NewFDecl->parameters(), hasDefaultArg))
+ return false;
+ }
+ }
+
if (isRedeclarable(getKind())) {
if (getCanonicalDecl() != OldD->getCanonicalDecl())
return false;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index af71acd69d45c..8eefcb956d6b2 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -490,8 +490,10 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
continue;
}
- if (PrevForDefaultArgs->getLexicalDeclContext()->getPrimaryContext() !=
- ScopeDC->getPrimaryContext() &&
+ if (PrevForDefaultArgs->getLexicalDeclContext()
+ ->getNonTransparentContext()
+ ->getPrimaryContext() !=
+ ScopeDC->getNonTransparentContext()->getPrimaryContext() &&
!New->isCXXClassMember())
// If previous declaration is lexically in a different scope,
// we don't inherit its default arguments, except for out-of-line
@@ -499,9 +501,9 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
//
// extern "C" and local functions can have default arguments across
// different scopes, but diagnosing that early would reject well-formed
- // code (_N5001_.[over.match.best]/4.) Instead, they are checked
- // in ConvertArgumentsForCall, after the best viable function has been
- // selected.
+ // code (C++2c [over.match.best]/4.) Instead, this check is deferred to
+ // overload resolution. See handling of ambiguous overload resolution
+ // result in FinishOverloadedCallExpr().
continue;
// We found the right previous declaration.
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index fb5b864292049..fad15bf95c415 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5826,62 +5826,6 @@ static bool isParenthetizedAndQualifiedAddressOfExpr(Expr *Fn) {
return false;
}
-/// @brief Checks that each default argument needed to make the call
-/// is defined only once, implementing [over.match.best]/4 rule.
-///
-/// @param FDecl Function declaration selected for the call
-/// @param NumArgs Number of argument explicitly specified in the call
-/// expression
-/// @param CallLoc Source location of the call expression
-static void checkDefaultArgumentsAcrossScopes(Sema &S, FunctionDecl *FDecl,
- int NumArgs,
- SourceLocation CallLoc) {
- // [over.match.best]/4:
- // If the best viable function resolves to a function
- // for which multiple declarations were found,
- // and if any two of these declarations inhabit different scopes
- // and specify a default argument that made the function viable,
- // the program is ill-formed.
-
- // Calculate the range of parameters,
- // default arguments of which made the candidate viable.
- int FirstDefaultArgIndex = NumArgs;
- int LastDefaultArgIndex = FDecl->getNumParams() - 1;
-
- // For each such parameter, collect all redeclarations
- // that have non-inherited default argument.
- llvm::SmallDenseMap<int, llvm::TinyPtrVector<ParmVarDecl *>> ParamRedecls(
- LastDefaultArgIndex - FirstDefaultArgIndex + 1);
- for (FunctionDecl *Redecl : FDecl->redecls()) {
- for (int i = FirstDefaultArgIndex; i <= LastDefaultArgIndex; ++i) {
- ParmVarDecl *Param = Redecl->getParamDecl(i);
- if (Param->hasDefaultArg() && !Param->hasInheritedDefaultArg())
- ParamRedecls[i].push_back(Param);
- }
- }
-
- // Emit the diagnostic if a given parameter has more than one declaration.
- // MergeCXXFunctionDecl takes care of redeclarations of a default argument
- // in the same scope, so if we found more than one,
- // we assume they come from different scopes.
- for (auto [ParamIndex, Redecls] : ParamRedecls) {
- assert(!Redecls.empty());
- if (Redecls.size() == 1)
- continue;
-
- ParmVarDecl *Param = FDecl->getParamDecl(ParamIndex);
- if (!Param->getDeclName().isEmpty()) {
- S.Diag(CallLoc, diag::err_ovl_ambiguous_default_arg)
- << 1 << Param->getName();
- } else
- S.Diag(CallLoc, diag::err_ovl_ambiguous_default_arg) << 0;
- for (ParmVarDecl *Param : Redecls) {
- S.Diag(Param->getDefaultArg()->getExprLoc(),
- diag::note_default_argument_declared_here);
- }
- }
-}
-
bool
Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
FunctionDecl *FDecl,
@@ -5955,13 +5899,6 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
// the call expression, before calling ConvertArgumentsForCall.
assert((Call->getNumArgs() == NumParams) &&
"We should have reserved space for the default arguments before!");
-
- if (FDecl->isExternC() ||
- std::any_of(
- FDecl->redecls_begin(), FDecl->redecls_end(),
- [](FunctionDecl *Redecl) { return Redecl->isLocalExternDecl(); }))
- checkDefaultArgumentsAcrossScopes(*this, FDecl, Args.size(),
- Call->getBeginLoc());
}
// If too many are passed and not variadic, error on the extras and drop
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 0f5b7426e743e..25d9b27375935 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -518,8 +518,28 @@ void LookupResult::resolveKind() {
llvm::BitVector RemovedDecls(N);
for (unsigned I = 0; I < N; I++) {
- const NamedDecl *D = Decls[I]->getUnderlyingDecl();
- D = cast<NamedDecl>(D->getCanonicalDecl());
+ const NamedDecl *NonCanonicalD = Decls[I]->getUnderlyingDecl();
+ const NamedDecl *D = cast<NamedDecl>(NonCanonicalD->getCanonicalDecl());
+
+ // When a function has redeclarations across different lexical scopes,
+ // and at least some of them have default arguments, we end up with
+ // multiple sets of default argument. Canonical declaration can't capture
+ // that, so we use non-canonical declarations instead. Overload resolution
+ // machinery is prepared to deal with that, see
+ // OverloadCandidateSet::isNewCandidate() and
+ // OverloadCandidateSet::eliminateDuplicatesWithUnusedDefaultArgs().
+ const DeclContext *DCtx = D->getLexicalDeclContext()
+ ->getNonTransparentContext()
+ ->getPrimaryContext();
+ if (const auto *FD = dyn_cast<FunctionDecl>(NonCanonicalD);
+ FD && DCtx != FD->getLexicalDeclContext()
+ ->getNonTransparentContext()
+ ->getPrimaryContext()) {
+ if (llvm::any_of(FD->parameters(), [](const ParmVarDecl *PDecl) {
+ return PDecl->hasDefaultArg();
+ }))
+ D = NonCanonicalD;
+ }
// Ignore an invalid declaration unless it's the only one left.
// Also ignore HLSLBufferDecl which not have name conflict with other Decls.
@@ -551,6 +571,8 @@ void LookupResult::resolveKind() {
continue;
}
+ // If this declaration is not the first declaration of an entity,
+ // this variable holds the index of the declaration we saw.
std::optional<unsigned> ExistingI;
// Redeclarations of types via typedef can occur both within a scope
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 13e1fe259f419..cb640cec4a749 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -1125,6 +1125,15 @@ void OverloadCandidateSet::clear(CandidateSetKind CSK) {
Kind = CSK;
}
+void OverloadCandidateSet::eliminateDuplicatesWithUnusedDefaultArgs() {
+ llvm::SmallPtrSet<Decl *, 16> UniqueEntities;
+ llvm::erase_if(Candidates, [&](const OverloadCandidate &Candidate) {
+ auto [It, New] =
+ UniqueEntities.insert(Candidate.Function->getCanonicalDecl());
+ return !(New || Candidate.ViableByDefaultArgument);
+ });
+}
+
namespace {
class UnbridgedCastsSet {
struct Entry {
@@ -6986,6 +6995,7 @@ void Sema::AddOverloadCandidate(
Candidate.FoundDecl = FoundDecl;
Candidate.Function = Function;
Candidate.Viable = true;
+ Candidate.ViableByDefaultArgument = false;
Candidate.RewriteKind =
CandidateSet.getRewriteInfo().getRewriteKind(Function, PO);
Candidate.IsADLCandidate = llvm::to_underlying(IsADLCandidate);
@@ -7113,6 +7123,10 @@ void Sema::AddOverloadCandidate(
return;
}
+ if (Args.size() < Function->getNumParams()) {
+ Candidate.ViableByDefaultArgument = true;
+ }
+
// (CUDA B.1): Check for invalid calls between targets.
if (getLangOpts().CUDA) {
const FunctionDecl *Caller = getCurFunctionDecl(/*AllowLambda=*/true);
@@ -13726,6 +13740,8 @@ void Sema::AddOverloadedCallCandidates(UnresolvedLookupExpr *ULE,
CandidateSet, PartialOverloading,
/*KnownValid*/ true);
+ CandidateSet.eliminateDuplicatesWithUnusedDefaultArgs();
+
if (ULE->requiresADL())
AddArgumentDependentLookupCandidates(ULE->getName(), ULE->getExprLoc(),
Args, ExplicitTemplateArgs,
@@ -14179,13 +14195,54 @@ static ExprResult FinishOverloadedCallExpr(Sema &SemaRef, Scope *S, Expr *Fn,
break;
}
- case OR_Ambiguous:
+ case OR_Ambiguous: {
+ auto handleAmbiguousDefaultArgs = [&] {
+ unsigned FirstDefaultArgIndex = Args.size();
+ llvm::SmallVector<PartialDiagnosticAt, 4> Notes;
+ for (const OverloadCandidate &Cand : *CandidateSet) {
+ if (!Cand.Best)
+ continue;
+ if (isa<CXXMethodDecl>(Cand.Function) ||
+ Cand.Function->isFunctionTemplateSpecialization()) {
+ return false;
+ }
+ if (!Cand.ViableByDefaultArgument) {
+ // We found a candidate marked best which wasn't made viable by
+ // default argument, which means this is not an "ambiguous by
+ // default argument" situation.
+ return false;
+ }
+ const ParmVarDecl *Parameter =
+ Cand.Function->getParamDecl(FirstDefaultArgIndex);
+ Notes.emplace_back(
+ Parameter->getDefaultArg()->getExprLoc(),
+ SemaRef.PDiag(diag::note_default_argument_declared_here)
+ << Parameter->getDefaultArgRange());
+ }
+ if (Notes.empty())
+ return false;
+
+ assert(Notes.size() >= 2 &&
+ "Overloaded call is considered ambiguous by default arguments,"
+ " but we found less than two candidates");
+ SemaRef.Diag(Fn->getBeginLoc(), diag::err_ovl_ambiguous_default_arg)
+ << Fn->getSourceRange();
+ llvm::sort(Notes);
+ for (const PartialDiagnosticAt &PDiag : Notes) {
+ SemaRef.Diag(PDiag.first, PDiag.second);
+ }
+ return true;
+ };
+ if (handleAmbiguousDefaultArgs())
+ break;
+
CandidateSet->NoteCandidates(
PartialDiagnosticAt(Fn->getBeginLoc(),
SemaRef.PDiag(diag::err_ovl_ambiguous_call)
<< ULE->getName() << Fn->getSourceRange()),
SemaRef, OCD_AmbiguousCandidates, Args);
break;
+ }
case OR_Deleted: {
FunctionDecl *FDecl = (*Best)->Function;
diff --git a/clang/test/CXX/drs/cwg0xx.cpp b/clang/test/CXX/drs/cwg0xx.cpp
index f86431e21a766..d115a8f9da691 100644
--- a/clang/test/CXX/drs/cwg0xx.cpp
+++ b/clang/test/CXX/drs/cwg0xx.cpp
@@ -18,18 +18,18 @@ namespace cwg1 { // cwg1: 21
void g() {
cwg1_f(0);
cwg1_f();
- // expected-error at -1 {{function call relies on ambiguous default argument for parameter 'a'}}
- // expected-note@#cwg1-Y {{default argument declared here}}
+ // expected-error at -1 {{function call relies on default argument that has multiple definitions}}
// expected-note@#cwg1-X {{default argument declared here}}
+ // expected-note@#cwg1-Y {{default argument declared here}}
}
namespace X {
using Y::cwg1_f;
void h() {
cwg1_f(0);
cwg1_f();
- // expected-error at -1 {{function call relies on ambiguous default argument for parameter 'a'}}
- // expected-note@#cwg1-Y {{default argument declared here}}
+ // expected-error at -1 {{function call relies on default argument that has multiple definitions}}
// expected-note@#cwg1-X {{default argument declared here}}
+ // expected-note@#cwg1-Y {{default argument declared here}}
}
}
@@ -47,19 +47,18 @@ namespace cwg1 { // cwg1: 21
using cwg1::i;
i(0);
i();
- // expected-error at -1 {{function call relies on ambiguous default argument}}
- // expected-note@#cwg1-i-redecl {{default argument declared here}}
+ // expected-error at -1 {{function call relies on default argument that has multiple definitions}}
// expected-note@#cwg1-i {{default argument declared here}}
+ // expected-note@#cwg1-i-redecl {{default argument declared here}}
}
void k() {
using cwg1::i;
void i(int = 1); // #cwg1-i-redecl2
i(0);
i();
- // expected-error at -1 {{function call relies on ambiguous default argument}}
- // expected-note@#cwg1-i-redecl2 {{default argument declared here}}
- // expected-note@#cwg1-i-redecl {{default argument declared here}}
+ // expected-error at -1 {{function call relies on default argument that has multiple definitions}}
// expected-note@#cwg1-i {{default argument declared here}}
+ // expected-note@#cwg1-i-redecl2 {{default argument declared here}}
}
} // namespace cwg1
diff --git a/clang/test/CXX/drs/cwg4xx.cpp b/clang/test/CXX/drs/cwg4xx.cpp
index 5f1aac1bdfe40..7df9bb9361798 100644
--- a/clang/test/CXX/drs/cwg4xx.cpp
+++ b/clang/test/CXX/drs/cwg4xx.cpp
@@ -371,8 +371,10 @@ namespace cwg417 { // cwg417: no
namespace cwg418 { // cwg418: 21
namespace example1 {
+void f1(int, int);
void f1(int, int = 0);
void f1(int = 0, int);
+void f1(int, int);
void g() { f1(); }
} // namespace example1
@@ -385,7 +387,9 @@ namespace B {
using A::f2;
}
namespace A {
+void f2(int);
void f2(int = 3);
+void f2(int);
}
void g2() {
using B::f2;
@@ -395,13 +399,17 @@ void g2() {
}
} // namespace example2
-// example from [over.match.best]/4
+// based on example from [over.match.best]/4
namespace example3 {
namespace A {
+extern "C" void f(int);
extern "C" void f(int = 5); // #cwg418-ex3-A
+extern "C" void f(int);
}
namespace B {
+extern "C" void f(int);
extern "C" void f(int = 5); // #cwg418-ex3-B
+extern "C" void f(int);
}
using A::f;
@@ -410,9 +418,13 @@ using B::f;
void use() {
f(3);
f();
- // expected-error at -1 {{function call relies on ambiguous default argument}}
+ // expected-error at -1 {{function call relies on default argument that has multiple definitions}}
+ // expected-note@#cwg418-ex3-A {{default argument declared here}}
// expected-note@#cwg418-ex3-B {{default argument declared here}}
+ example3::f();
+ // expected-error at -1 {{function call relies on default argument that has multiple definitions}}
// expected-note@#cwg418-ex3-A {{default argument declared here}}
+ // expected-note@#cwg418-ex3-B {{default argument declared here}}
}
} // namespace example3
} // namespace cwg418
diff --git a/clang/test/SemaCXX/default-argument-different-scopes.cpp b/clang/test/SemaCXX/default-argument-different-scopes.cpp
index ad20e18a21604..b87d7cb7b8d90 100644
--- a/clang/test/SemaCXX/default-argument-different-scopes.cpp
+++ b/clang/test/SemaCXX/default-argument-different-scopes.cpp
@@ -14,78 +14,88 @@ namespace B {
extern "C" void f3(int = 0, ...); // #B-f3
} // namespace B
+void f3(char);
+void f4(int = 0); // #f4-global
+void f5(int = 0); // #f5-global
+
void f() {
using A::f1;
- using A::f2;
- using A::f3;
using B::f1;
- using B::f2;
- using B::f3;
f1();
f1(0);
f1(0, 0);
+
+ using A::f2;
+ using B::f2;
+
f2();
// expected-error at -1 {{no matching function for call to 'f2'}}
// expected-note@#B-f2 {{candidate function not viable: requires at least 1 argument, but 0 were provided}}
f2(0);
f2(0, 0);
+
+ using A::f3;
+ using B::f3;
+
f3();
- // expected-error at -1 {{function call relies on ambiguous default argument}}
- // expected-note@#B-f3 {{default argument declared here}}
+ // expected-error at -1 {{function call relies on default argument that has multiple definitions}}
// expected-note@#A-f3 {{default argument declared here}}
+ // expected-note@#B-f3 {{default argument declared here}}
f3(0);
f3(0, 0);
+ f3('a');
+
+ using ::f4;
+ void f4(int = 1); // #f4-local
+ f4();
+ // expected-error at -1 {{function call relies on default argument that has multiple definitions}}
+ // expected-note@#f4-global {{default argument declared here}}
+ // expected-note@#f4-local {{default argument declared here}}
+
+ using ::f5;
+ extern void f5(int = 1); // #f5-local
+ extern void f5(int);
+ f5();
+ // expected-error at -1 {{function call relies on default argument that has multiple definitions}}
+ // expected-note@#f5-global {{default argument declared here}}
+ // expected-note@#f5-local {{default argument declared here}}
}
-void i1(int = 2); // #i1
-void i2(int = 2); // #i2
-extern "C" void j1(int = 2); // #j1
-extern "C" void j2(int = 2); // #j2
+// Two declarations in different scopes have to be found and be viable
+// candidates to run into ambiguous default arguments situation.
+// In the tests below, calls with qualified names finds declarations only
+// at namespace scope, whereas calls with unqualified names find only
+// declarations at block scope. As a result, all calls are well-formed
+// in 'i' group of tests.
-void f2() {
- void i1(int = 3); // #i1-redecl
- extern void i2(int = 3); // #i2-redecl
- void j1(int = 3); // #j1-redecl
- extern void j2(int = 3); // #j2-redecl
+void i1(int = 2);
+void i2(int = 2);
+extern "C" void j1(int = 2);
+extern "C" void j2(int = 2);
- i1();
- // expected-error at -1 {{function call relies on ambiguous default argument}}
- // expected-note@#i1-redecl {{default argument declared here}}
- // expected-note@#i1 {{default argument declared here}}
+void i() {
+ void i1(int = 3);
::i1();
- // expected-error at -1 {{function call relies on ambiguous default argument}}
- // expected-note@#i1 {{default argument declared here}}
- // expected-note@#i1-redecl {{default argument declared here}}
- i2();
- // expected-error at -1 {{function call relies on ambiguous default argument}}
- // expected-note@#i2-redecl {{default argument declared here}}
- // expected-note@#i2 {{default argument declared here}}
+ i1();
+
+ extern void i2(int = 3);
::i2();
- // expected-error at -1 {{function call relies on ambiguous default argument}}
- // expected-note@#i2 {{default argument declared here}}
- // expected-note@#i2-redecl {{default argument declared here}}
- j1();
- // expected-error at -1 {{function call relies on ambiguous default argument}}
- // expected-note@#j1-redecl {{default argument declared here}}
- // expected-note@#j1 {{default argument declared here}}
+ i2();
+
+ void j1(int = 3);
::j1();
- // expected-error at -1 {{function call relies on ambiguous default argument}}
- // expected-note@#j1 {{default argument declared here}}
- // expected-note@#j1-redecl {{default argument declared here}}
- j2();
- // expected-error at -1 {{function call relies on ambiguous default argument}}
- // expected-note@#j2-redecl {{default argument declared here}}
- // expected-note@#j2 {{default argument declared here}}
+ j1();
+
+ extern void j2(int = 3);
::j2();
- // expected-error at -1 {{function call relies on ambiguous default argument}}
- // expected-note@#j2 {{default argument declared here}}
- // expected-note@#j2-redecl {{default argument declared here}}
+ j2();
}
// In 'k' group of tests, no redefinition of default arguments occur,
// because sets of default arguments are associated with lexical scopes
-// of function declarations.
+// of function declarations. There are exceptions from this rule,
+// described below.
void k1(int); // #k1
void k2(int = 2);
@@ -113,6 +123,11 @@ void k6(int = 6);
// expected-error at -1 {{friend declaration specifying a default argument must be the only declaration}}
// expected-note@#k6 {{previous declaration is here}}
+// The only exception from the rule that default arguments are associated with
+// their lexical scope is out-of-line definitions of member functions of
+// non-templated classes. Such default arguments contribute to the set of
+// default arguments associated with the class scope.
+
struct L {
void l1(int);
void l2(int = 2);
@@ -145,6 +160,10 @@ void L::l6(int = 6) {}
// expected-error at -1 {{redefinition of default argument}}
// expected-note@#l6 {{previous definition is here}}
+// Default arguments are not allowed in out-of-line declarations
+// of member functions of class templates. They have to be specified within
+// member-specification.
+
template <typename>
struct M {
void m1(int);
@@ -159,6 +178,8 @@ void M<T>::m1(int = 1) {}
template <typename T>
void M<T>::m2(int) {}
+// FIXME: the real problem is that default argument is not allowed here,
+// and not that it's redefined.
template <typename T>
void M<T>::m3(int = 3) {}
// expected-error at -1 {{redefinition of default argument}}
diff --git a/clang/test/SemaCXX/default1.cpp b/clang/test/SemaCXX/default1.cpp
index 675a3a61e4ca7..8345b2433a3fe 100644
--- a/clang/test/SemaCXX/default1.cpp
+++ b/clang/test/SemaCXX/default1.cpp
@@ -41,14 +41,11 @@ void kk(Y = 17); // expected-error{{no viable conversion}} \
// expected-note{{passing argument to parameter here}}
int l () {
- int m(int i, int j, int k = 3); // #m
+ int m(int i, int j, int k = 3);
if (1)
{
- int m(int i, int j = 2, int k = 4); // #m-redecl
+ int m(int i, int j = 2, int k = 4);
m(8);
- // expected-error at -1 {{function call relies on ambiguous default argument for parameter 'k'}}
- // expected-note@#m-redecl {{default argument declared here}}
- // expected-note@#m {{default argument declared here}}
}
return 0;
}
More information about the cfe-commits
mailing list