[clang] [clang] Diagnose default arguments defined in different scopes (PR #124844)
Vlad Serebrennikov via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 28 13:40:34 PST 2025
https://github.com/Endilll created https://github.com/llvm/llvm-project/pull/124844
This patch implements the rule described in N5001.[[over.match.best]/4](https://eel.is/c++draft/over.match.best#general-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[.](https://eel.is/c++draft/over.match.best#general-4.sentence-1)
Consequently, it implements [CWG1](https://cplusplus.github.io/CWG/issues/1.html) "What if two using-declarations refer to the same function but the declarations introduce different default-arguments?" and [CWG418](https://cplusplus.github.io/CWG/issues/418.html) "Imperfect wording on error on multiple default arguments on a called function" that were fixed by adding paragraph 4 to [over.match.best].
In the case of friend functions, we no longer diagnose redefinitions of default arguments across scopes. This doesn't change status quo w.r.t. what we accept and what we reject, because such cases also violate [[dcl.fct.default]/4](https://eel.is/c++draft/dcl.fct.default#4.sentence-7).
Background information on default arguments
---------------------------------------------------
Sets of default arguments are associated with lexical scope of function declarations they appear in, with a single exception of out-of-line definitions of member function (details below):
>For non-template functions, default arguments can be added in later declarations of a function that inhabit the same scope[.](https://eel.is/c++draft/dcl.fct.default#4.sentence-1)
Declarations that inhabit different scopes have completely distinct sets of default arguments[.](https://eel.is/c++draft/dcl.fct.default#4.sentence-2)
(Note that "inhabit" above is a word of power, which for our case uses the fallback definition in [[basic.scope.scope]](https://eel.is/c++draft/basic.scope.scope#2.3).)
Typically this is the same scope as (semantic) target scope of the declaration, so all redeclarations share the same set of default arguments. However, there are function declarations that (semantically) _belong_ to the different scope than the (lexical) scope they _inhabit_: `extern "C"` declarations, local function declarations, friend declarations, and out-of-line definitions of member functions.
Friend declarations and out-of-line definitions of member functions have additional provisions in the wording:
> If a friend declaration D specifies a default argument expression, that declaration shall be a definition and there shall be no other declaration of the function or function template which is reachable from D or from which D is reachable[.](https://eel.is/c++draft/dcl.fct.default#4.sentence-7)
> Except for member functions of templated classes, the default arguments in a member function definition that appears outside of the class definition are added to the set of default arguments provided by the member function declaration in the class definition; the program is ill-formed if a default constructor ([[class.default.ctor]](https://eel.is/c++draft/class.default.ctor)), copy or move constructor ([[class.copy.ctor]](https://eel.is/c++draft/class.copy.ctor)), or copy or move assignment operator ([[class.copy.assign]](https://eel.is/c++draft/class.copy.assign)) is so declared[.](https://eel.is/c++draft/dcl.fct.default#6.sentence-1)
Default arguments for a member function of a templated class shall be specified on the initial declaration of the member function within the templated class[.](https://eel.is/c++draft/dcl.fct.default#6.sentence-2)
Implementation approach
-----------------------------
`MergeCXXFunctionDecl` diagnoses redefinitions of default arguments across function redeclarations in the same scope, in other words, it handles default arguments that are a part of the same set of default arguments. This patch complements it by handling default arguments that are a part of different sets.
[over.match.best]/4 applies when functions are called, so checking it in `MergeCXXFunctionDecl` would be too early. `Sema::BestViableFunction` also doesn't fit, because we can skip it if lookup finds a single function. By the time `CheckFunctionCall` is called, number of arguments in `CallExpr` is adjusted to include default arguments, so it's too late, too. `ConvertArgumentsForCall` seems to be the best fit for this check.
>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/13] [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 f110b8cf765075..00b1e3b678dda8 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 774e5484cfa0e7..cfa47ec1c1f977 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 c7f2422b542dd1..8e0b9ef8b44b86 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 839b3a1cccdcc3..ba9c7c0237cd27 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 6ae9c51c06b315..fe8ae5a4190efe 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 15f469440c66f2..6b969925c8c191 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 bcaf7db04ad3b5..782ed70c5741b9 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 00000000000000..db20c48b1ab17c
--- /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 69ddd5e58b921a..d9eb8c26317f3b 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/13] 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 db20c48b1ab17c..4175b0d231ca51 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/13] 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 4d6e02fe2956e0..1c94f347f7613f 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 d5273d463d7c01..b2bed0c68490de 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 fe8ae5a4190efe..08cb5e54acd4be 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 6b969925c8c191..bc8e9c12a98dfa 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 2459ef1ad41fcd..002d9fa703ccfe 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 00000000000000..f61cc62c117e51
--- /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 4175b0d231ca51..00000000000000
--- 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 8345b2433a3fe1..675a3a61e4ca79 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/13] 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 28cd94fb855db2..8fb6e378ead7ba 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/13] 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 cf4ac6a0b9af44..26a4d5a822e3d8 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/13] 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 8fb6e378ead7ba..b13362426f192e 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 bc8e9c12a98dfa..4d45898a397766 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 15bcc20b7fa2a9..f6aa0b3ffb35c6 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 f61cc62c117e51..e78f120fea0485 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/13] 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 18d6944083cb27..ad2d7f02a59aad 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/13] 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 ad2d7f02a59aad..f2c722ce92841b 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/13] 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 473315ffd50f8f..528304409b8092 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 5dfcc04527b9f3..742f3452327687 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 26a4d5a822e3d8..0b616aa07528b7 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/13] 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 9fdfa36f891d50..f86431e21a7669 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 782ed70c5741b9..5f1aac1bdfe403 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 d9eb8c26317f3b..d8fb845b5bebae 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/13] 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 40989ef50eca78..b0e946abe7e9c4 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/13] 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 29f5dde294715c..b543f6a7ac8489 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 e78f120fea0485..cb7addedb7ec20 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/13] 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 b0e946abe7e9c4..75b66d8d6298d9 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 742f3452327687..b177811ef19d03 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
More information about the cfe-commits
mailing list