Version 2: implementation based on overload resolution

Author: Endilll

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -5143,8 +5143,8 @@ def err_addr_ovl_no_qualifier : Error<
   "cannot form member pointer of type %0 without '&' and class name">;
 
 def err_ovl_ambiguous_default_arg
-    : Error<"function call relies on ambiguous default argument %select{|for "
-            "parameter '%1'}0">;
+    : Error<"function call relies on default argument that has multiple "
+            "definitions">;
 
 } // let Deferrable
 

clang/include/clang/Sema/Overload.h

@@ -901,6 +901,13 @@ class Sema;
     LLVM_PREFERRED_TYPE(bool)
     unsigned Viable : 1;
 
+    /// Whether this candidate wouldn't be viable without default arguments.
+    /// This is needed to implement the special provision about ambiguous
+    /// default arguments of the best viable function described in
+    /// [over.match.best]/4.
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned ViableByDefaultArgument : 1;
+
     /// Whether this candidate is the best viable function, or tied for being
     /// the best viable function.
     ///
@@ -1177,11 +1184,38 @@ class Sema;
     /// Whether diagnostics should be deferred.
     bool shouldDeferDiags(Sema &S, ArrayRef<Expr *> Args, SourceLocation OpLoc);
 
-    /// Determine when this overload candidate will be new to the
-    /// overload set.
+    /// Determine when this overload candidate will be new to the overload set.
+    /// Typically the uniqueness is determined by canonical declaration
+    /// (i.e. semantic entity), but it's aware of a special case of default
+    /// arguments in redeclarations spanning across different lexical scopes.
+    /// See also eliminateDuplicatesWithUnusedDefaultArgs().
     bool isNewCandidate(Decl *F, OverloadCandidateParamOrder PO =
                                      OverloadCandidateParamOrder::Normal) {
       uintptr_t Key = reinterpret_cast<uintptr_t>(F->getCanonicalDecl());
+
+      // There is one special case when we can't rely on just semantic
+      // properties of the function (i.e. entity in the Standard sense),
+      // which is default arguments defined in redeclarations across
+      // different scopes, because sets of default arguments are associated
+      // with lexical scope of function declaration. Which means single
+      // function entity can have more than one set of default arguments,
+      // and which of those sets are considered depends solely on what
+      // name lookup has found for each individual call expression.
+      // See [over.match.best]/4 for more details.
+      // In such case, we use (lexical) function declaration to determine
+      // uniqueness of overload candidate, instead of (semantic) entity
+      // it represents. This would lead to duplicates, which will be
+      // eliminated by eliminateDuplicatesWithUnusedDefaultArgs()
+      // after every candidate will be considered.
+      if (const auto *FD = dyn_cast<FunctionDecl>(F);
+          FD && FD->getLexicalDeclContext() !=
+                    F->getCanonicalDecl()->getLexicalDeclContext()) {
+        auto HasDefaultArg = [](const ParmVarDecl *PDecl) {
+          return PDecl->hasDefaultArg();
+        };
+        if (llvm::any_of(FD->parameters(), HasDefaultArg))
+          Key = reinterpret_cast<uintptr_t>(FD);
+      }
       Key |= static_cast<uintptr_t>(PO);
       return Functions.insert(Key).second;
     }
@@ -1195,6 +1229,14 @@ class Sema;
     /// Clear out all of the candidates.
     void clear(CandidateSetKind CSK);
 
+    /// Clean up duplicates left by isNewCandidate()
+    /// (see comment in its implementation for details.)
+    /// When this function is called, we should know which of duplicated
+    /// candidates are viable because of their default arguments, and those
+    /// are the only duplicates we are interested in during subsequent
+    /// selection of the best viable function.
+    void eliminateDuplicatesWithUnusedDefaultArgs();
+
     using iterator = SmallVectorImpl<OverloadCandidate>::iterator;
 
     iterator begin() { return Candidates.begin(); }

clang/lib/AST/Decl.cpp

@@ -1883,6 +1883,33 @@ bool NamedDecl::declarationReplaces(const NamedDecl *OldD,
                         cast<UnresolvedUsingValueDecl>(OldD)->getQualifier());
   }
 
+  {
+    // When default arguments across redeclarations in different lexical scopes
+    // are involved, we can't let one UsingShadowDecl to replace another based
+    // on the fact that they refer to the same canonical function.
+    const auto *OldShadowD = dyn_cast<UsingShadowDecl>(OldD);
+    const auto *NewShadowD = dyn_cast<UsingShadowDecl>(this);
+    if (OldShadowD && NewShadowD &&
+        isa<FunctionDecl>(OldShadowD->getTargetDecl()) &&
+        isa<FunctionDecl>(NewShadowD->getTargetDecl())) {
+      const auto *OldFDecl = cast<FunctionDecl>(OldShadowD->getTargetDecl());
+      const auto *NewFDecl = cast<FunctionDecl>(NewShadowD->getTargetDecl());
+      const DeclContext *OldDeclCtx = OldFDecl->getLexicalDeclContext()
+                                          ->getNonTransparentContext()
+                                          ->getPrimaryContext();
+      const DeclContext *NewDeclCtx = NewFDecl->getLexicalDeclContext()
+                                          ->getNonTransparentContext()
+                                          ->getPrimaryContext();
+      auto hasDefaultArg = [](const ParmVarDecl *PDecl) {
+        return PDecl->hasDefaultArg();
+      };
+      if (OldDeclCtx != NewDeclCtx &&
+          llvm::any_of(OldFDecl->parameters(), hasDefaultArg) &&
+          llvm::any_of(NewFDecl->parameters(), hasDefaultArg))
+        return false;
+    }
+  }
+
   if (isRedeclarable(getKind())) {
     if (getCanonicalDecl() != OldD->getCanonicalDecl())
       return false;

clang/lib/Sema/SemaDeclCXX.cpp

@@ -490,18 +490,20 @@ bool Sema::MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old,
       continue;
     }
 
-    if (PrevForDefaultArgs->getLexicalDeclContext()->getPrimaryContext() !=
-            ScopeDC->getPrimaryContext() &&
+    if (PrevForDefaultArgs->getLexicalDeclContext()
+                ->getNonTransparentContext()
+                ->getPrimaryContext() !=
+            ScopeDC->getNonTransparentContext()->getPrimaryContext() &&
         !New->isCXXClassMember())
       // If previous declaration is lexically in a different scope,
       // we don't inherit its default arguments, except for out-of-line
       // 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.
+      // code (C++2c [over.match.best]/4.) Instead, this check is deferred to
+      // overload resolution. See handling of ambiguous overload resolution
+      // result in FinishOverloadedCallExpr().
       continue;
 
     // We found the right previous declaration.

clang/lib/Sema/SemaExpr.cpp

@@ -5826,62 +5826,6 @@ static bool isParenthetizedAndQualifiedAddressOfExpr(Expr *Fn) {
   return false;
 }
 
-/// @brief Checks that each default argument needed to make the call
-/// is defined only once, implementing [over.match.best]/4 rule.
-///
-/// @param FDecl Function declaration selected for the call
-/// @param NumArgs Number of argument explicitly specified in the call
-/// expression
-/// @param CallLoc Source location of the call expression
-static void checkDefaultArgumentsAcrossScopes(Sema &S, FunctionDecl *FDecl,
-                                              int NumArgs,
-                                              SourceLocation CallLoc) {
-  // [over.match.best]/4:
-  // If the best viable function resolves to a function
-  // for which multiple declarations were found,
-  // and if any two of these declarations inhabit different scopes
-  // and specify a default argument that made the function viable,
-  // the program is ill-formed.
-
-  // Calculate the range of parameters,
-  // default arguments of which made the candidate viable.
-  int FirstDefaultArgIndex = NumArgs;
-  int LastDefaultArgIndex = FDecl->getNumParams() - 1;
-
-  // For each such parameter, collect all redeclarations
-  // that have non-inherited default argument.
-  llvm::SmallDenseMap<int, llvm::TinyPtrVector<ParmVarDecl *>> ParamRedecls(
-      LastDefaultArgIndex - FirstDefaultArgIndex + 1);
-  for (FunctionDecl *Redecl : FDecl->redecls()) {
-    for (int i = FirstDefaultArgIndex; i <= LastDefaultArgIndex; ++i) {
-      ParmVarDecl *Param = Redecl->getParamDecl(i);
-      if (Param->hasDefaultArg() && !Param->hasInheritedDefaultArg())
-        ParamRedecls[i].push_back(Param);
-    }
-  }
-
-  // Emit the diagnostic if a given parameter has more than one declaration.
-  // MergeCXXFunctionDecl takes care of redeclarations of a default argument
-  // in the same scope, so if we found more than one,
-  // we assume they come from different scopes.
-  for (auto [ParamIndex, Redecls] : ParamRedecls) {
-    assert(!Redecls.empty());
-    if (Redecls.size() == 1)
-      continue;
-
-    ParmVarDecl *Param = FDecl->getParamDecl(ParamIndex);
-    if (!Param->getDeclName().isEmpty()) {
-      S.Diag(CallLoc, diag::err_ovl_ambiguous_default_arg)
-          << 1 << Param->getName();
-    } else
-      S.Diag(CallLoc, diag::err_ovl_ambiguous_default_arg) << 0;
-    for (ParmVarDecl *Param : Redecls) {
-      S.Diag(Param->getDefaultArg()->getExprLoc(),
-             diag::note_default_argument_declared_here);
-    }
-  }
-}
-
 bool
 Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
                               FunctionDecl *FDecl,
@@ -5955,13 +5899,6 @@ Sema::ConvertArgumentsForCall(CallExpr *Call, Expr *Fn,
     // the call expression, before calling ConvertArgumentsForCall.
     assert((Call->getNumArgs() == NumParams) &&
            "We should have reserved space for the default arguments before!");
-
-    if (FDecl->isExternC() ||
-        std::any_of(
-            FDecl->redecls_begin(), FDecl->redecls_end(),
-            [](FunctionDecl *Redecl) { return Redecl->isLocalExternDecl(); }))
-      checkDefaultArgumentsAcrossScopes(*this, FDecl, Args.size(),
-                                        Call->getBeginLoc());
   }
 
   // If too many are passed and not variadic, error on the extras and drop

clang/lib/Sema/SemaLookup.cpp

@@ -518,8 +518,28 @@ void LookupResult::resolveKind() {
   llvm::BitVector RemovedDecls(N);
 
   for (unsigned I = 0; I < N; I++) {
-    const NamedDecl *D = Decls[I]->getUnderlyingDecl();
-    D = cast<NamedDecl>(D->getCanonicalDecl());
+    const NamedDecl *NonCanonicalD = Decls[I]->getUnderlyingDecl();
+    const NamedDecl *D = cast<NamedDecl>(NonCanonicalD->getCanonicalDecl());
+
+    // When a function has redeclarations across different lexical scopes,
+    // and at least some of them have default arguments, we end up with
+    // multiple sets of default argument. Canonical declaration can't capture
+    // that, so we use non-canonical declarations instead. Overload resolution
+    // machinery is prepared to deal with that, see
+    // OverloadCandidateSet::isNewCandidate() and
+    // OverloadCandidateSet::eliminateDuplicatesWithUnusedDefaultArgs().
+    const DeclContext *DCtx = D->getLexicalDeclContext()
+                                  ->getNonTransparentContext()
+                                  ->getPrimaryContext();
+    if (const auto *FD = dyn_cast<FunctionDecl>(NonCanonicalD);
+        FD && DCtx != FD->getLexicalDeclContext()
+                          ->getNonTransparentContext()
+                          ->getPrimaryContext()) {
+      if (llvm::any_of(FD->parameters(), [](const ParmVarDecl *PDecl) {
+            return PDecl->hasDefaultArg();
+          }))
+        D = NonCanonicalD;
+    }
 
     // Ignore an invalid declaration unless it's the only one left.
     // Also ignore HLSLBufferDecl which not have name conflict with other Decls.
@@ -551,6 +571,8 @@ void LookupResult::resolveKind() {
         continue;
     }
 
+    // If this declaration is not the first declaration of an entity,
+    // this variable holds the index of the declaration we saw.
     std::optional<unsigned> ExistingI;
 
     // Redeclarations of types via typedef can occur both within a scope

clang/lib/Sema/SemaOverload.cpp

@@ -1125,6 +1125,15 @@ void OverloadCandidateSet::clear(CandidateSetKind CSK) {
   Kind = CSK;
 }
 
+void OverloadCandidateSet::eliminateDuplicatesWithUnusedDefaultArgs() {
+  llvm::SmallPtrSet<Decl *, 16> UniqueEntities;
+  llvm::erase_if(Candidates, [&](const OverloadCandidate &Candidate) {
+    auto [It, New] =
+        UniqueEntities.insert(Candidate.Function->getCanonicalDecl());
+    return !(New || Candidate.ViableByDefaultArgument);
+  });
+}
+
 namespace {
   class UnbridgedCastsSet {
     struct Entry {
@@ -6986,6 +6995,7 @@ void Sema::AddOverloadCandidate(
   Candidate.FoundDecl = FoundDecl;
   Candidate.Function = Function;
   Candidate.Viable = true;
+  Candidate.ViableByDefaultArgument = false;
   Candidate.RewriteKind =
       CandidateSet.getRewriteInfo().getRewriteKind(Function, PO);
   Candidate.IsADLCandidate = llvm::to_underlying(IsADLCandidate);
@@ -7113,6 +7123,10 @@ void Sema::AddOverloadCandidate(
     return;
   }
 
+  if (Args.size() < Function->getNumParams()) {
+    Candidate.ViableByDefaultArgument = true;
+  }
+
   // (CUDA B.1): Check for invalid calls between targets.
   if (getLangOpts().CUDA) {
     const FunctionDecl *Caller = getCurFunctionDecl(/*AllowLambda=*/true);
@@ -13726,6 +13740,8 @@ void Sema::AddOverloadedCallCandidates(UnresolvedLookupExpr *ULE,
                                CandidateSet, PartialOverloading,
                                /*KnownValid*/ true);
 
+  CandidateSet.eliminateDuplicatesWithUnusedDefaultArgs();
+
   if (ULE->requiresADL())
     AddArgumentDependentLookupCandidates(ULE->getName(), ULE->getExprLoc(),
                                          Args, ExplicitTemplateArgs,
@@ -14179,13 +14195,54 @@ static ExprResult FinishOverloadedCallExpr(Sema &SemaRef, Scope *S, Expr *Fn,
     break;
   }
 
-  case OR_Ambiguous:
+  case OR_Ambiguous: {
+    auto handleAmbiguousDefaultArgs = [&] {
+      unsigned FirstDefaultArgIndex = Args.size();
+      llvm::SmallVector<PartialDiagnosticAt, 4> Notes;
+      for (const OverloadCandidate &Cand : *CandidateSet) {
+        if (!Cand.Best)
+          continue;
+        if (isa<CXXMethodDecl>(Cand.Function) ||
+            Cand.Function->isFunctionTemplateSpecialization()) {
+          return false;
+        }
+        if (!Cand.ViableByDefaultArgument) {
+          // We found a candidate marked best which wasn't made viable by
+          // default argument, which means this is not an "ambiguous by
+          // default argument" situation.
+          return false;
+        }
+        const ParmVarDecl *Parameter =
+            Cand.Function->getParamDecl(FirstDefaultArgIndex);
+        Notes.emplace_back(
+            Parameter->getDefaultArg()->getExprLoc(),
+            SemaRef.PDiag(diag::note_default_argument_declared_here)
+                << Parameter->getDefaultArgRange());
+      }
+      if (Notes.empty())
+        return false;
+
+      assert(Notes.size() >= 2 &&
+             "Overloaded call is considered ambiguous by default arguments,"
+             " but we found less than two candidates");
+      SemaRef.Diag(Fn->getBeginLoc(), diag::err_ovl_ambiguous_default_arg)
+          << Fn->getSourceRange();
+      llvm::sort(Notes);
+      for (const PartialDiagnosticAt &PDiag : Notes) {
+        SemaRef.Diag(PDiag.first, PDiag.second);
+      }
+      return true;
+    };
+    if (handleAmbiguousDefaultArgs())
+      break;
+
     CandidateSet->NoteCandidates(
         PartialDiagnosticAt(Fn->getBeginLoc(),
                             SemaRef.PDiag(diag::err_ovl_ambiguous_call)
                                 << ULE->getName() << Fn->getSourceRange()),
         SemaRef, OCD_AmbiguousCandidates, Args);
     break;
+  }
 
   case OR_Deleted: {
     FunctionDecl *FDecl = (*Best)->Function;