[clang] Diagnose default arguments defined in different scopes

clang/docs/ReleaseNotes.rst

@@ -73,6 +73,11 @@ C++17 Feature Support
 Resolutions to C++ Defect Reports
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Clang now diagnoses ambiguous default arguments declared in different scopes
+  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>`_)
+
 C Language Changes
 ------------------
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -5145,6 +5145,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

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 {

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;
@@ -488,6 +490,20 @@ 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.
+      //
+      // 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;
+
     // We found the right previous declaration.
     break;
   }

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,
@@ -5883,6 +5939,13 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
     // the call expression, before calling ConvertArgumentsForCall.
     assert((Call->getNumArgs() == NumParams) &&
            "We should have reserved space for the default arguments before!");
+
+    if (FDecl->isExternC() ||
+        std::any_of(
+            FDecl->redecls_begin(), FDecl->redecls_end(),
+            [](FunctionDecl *Redecl) { return Redecl->isLocalExternDecl(); }))
+      checkDefaultArgumentsAcrossScopes(*this, FDecl, Args.size(),
+                                        Call->getBeginLoc());
   }
 
   // If too many are passed and not variadic, error on the extras and drop

clang/lib/Sema/SemaOverload.cpp

@@ -10960,6 +10960,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;
 }
 

clang/test/CXX/drs/cwg0xx.cpp

@@ -11,48 +11,55 @@
 // 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: 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;
   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}}
     }
   }
 
   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);
+  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
 

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}}

clang/test/CXX/drs/cwg4xx.cpp

@@ -369,7 +369,7 @@ namespace cwg417 { // cwg417: no
   }
 } // namespace cwg417
 
-namespace cwg418 { // cwg418: no
+namespace cwg418 { // cwg418: 21
 namespace example1 {
 void f1(int, int = 0);
 void f1(int = 0, int);
@@ -398,18 +398,21 @@ 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;
 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

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();
-}

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();