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.vladis...@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@-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@-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@-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@-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@-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@-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.vladis...@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@-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.vladis...@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@-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@-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@-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@-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@-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@-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@-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@-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@-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@-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@-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@-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@-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@-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@-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@-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@-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.vladis...@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.vladis...@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.vladis...@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@-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@-1 {{friend declaration specifying a default argument must be the only declaration}} // expected-note@#cwg136-f {{previous declaration is here}} + // expected-error@-3 {{missing default argument on parameter}} friend void g(int, int, int = 0); // expected-error@-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@-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@-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@-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@-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@-1 {{redefinition of default argument}} +// expected-note@#l3 {{previous definition is here}} + +template <typename> +void L::l4(int = 4) {} +// expected-error@-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@-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.vladis...@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.vladis...@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.vladis...@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.vladis...@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@-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.vladis...@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.vladis...@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@-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.vladis...@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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
