[Clang][RFC] Bypass TAD during overload resolution if a perfect match exists (#133426)

This implements the same overload resolution behavior as GCC, 
as described in https://wg21.link/p3606 (sections 1-2, not 3)

If, during overload resolution, a non-template candidate is always
picked because each argument is a perfect match (i.e., the source and
target types are the same), we do not perform deduction for any template
candidate that might exist.

The goal is to be able to merge #122423 without being too disruptive.

This change means that the selection of the best viable candidate and
template argument deduction become interleaved.

To avoid rewriting half of Clang, we store in `OverloadCandidateSet`
enough information to deduce template candidates from
`OverloadCandidateSet::BestViableFunction`. This means the lifetime of
any object used by the template argument must outlive a call to
`Add*Template*Candidate`.

This two-phase resolution is not performed for some initialization as
there are cases where template candidates are a better match per the
standard. It's also bypassed for code completion.

The change has a nice impact on compile times

https://llvm-compile-time-tracker.com/compare.php?from=edc22c64e527171041876f26a491bb1d03d905d5&to=8170b860bd4b70917005796c05a9be013a95abb2&stat=instructions%3Au

Fixes #62096
Fixes #74581
Fixes #53454

Author: cor3ntin

clang/docs/ReleaseNotes.rst

@@ -96,6 +96,12 @@ C++ Language Changes
       asm((std::string_view("nop")) ::: (std::string_view("memory")));
     }
 
+- Clang now implements the changes to overload resolution proposed by section 1 and 2 of
+  `P3606 <https://wg21.link/P3606R0>`_. If a non-template candidate exists in an overload set that is
+  a perfect match (all conversion sequences are identity conversions) template candiates are not instantiated.
+  Diagnostics that would have resulted from the instantiation of these template candidates are no longer
+  produced. This aligns Clang closer to the behavior of GCC, and fixes (#GH62096), (#GH74581), and (#GH74581).
+
 C++2c Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 

clang/include/clang/Sema/Overload.h

@@ -407,6 +407,24 @@ class Sema;
              Third == ICK_Identity;
     }
 
+    /// A conversion sequence is perfect if it is an identity conversion and
+    /// the type of the source is the same as the type of the target.
+    bool isPerfect(const ASTContext &C) const {
+      if (!isIdentityConversion())
+        return false;
+      // If we are not performing a reference binding, we can skip comparing
+      // the types, which has a noticeable performance impact.
+      if (!ReferenceBinding) {
+        assert(First || C.hasSameUnqualifiedType(getFromType(), getToType(2)));
+        return true;
+      }
+      if (!C.hasSameType(getFromType(), getToType(2)))
+        return false;
+      if (BindsToRvalue && IsLvalueReference)
+        return false;
+      return true;
+    }
+
     ImplicitConversionRank getRank() const;
     NarrowingKind
     getNarrowingKind(ASTContext &Context, const Expr *Converted,
@@ -743,6 +761,12 @@ class Sema;
       Standard.setAllToTypes(T);
     }
 
+    /// A conversion sequence is perfect if it is an identity conversion and
+    /// the type of the source is the same as the type of the target.
+    bool isPerfect(const ASTContext &C) const {
+      return isStandard() && Standard.isPerfect(C);
+    }
+
     // True iff this is a conversion sequence from an initializer list to an
     // array or std::initializer.
     bool hasInitializerListContainerType() const {
@@ -979,6 +1003,20 @@ class Sema;
       return false;
     }
 
+    // An overload is a perfect match if the conversion
+    // sequences for each argument are perfect.
+    bool isPerfectMatch(const ASTContext &Ctx) const {
+      if (!Viable)
+        return false;
+      for (const auto &C : Conversions) {
+        if (!C.isInitialized() || !C.isPerfect(Ctx))
+          return false;
+      }
+      if (isa_and_nonnull<CXXConversionDecl>(Function))
+        return FinalConversion.isPerfect(Ctx);
+      return true;
+    }
+
     bool TryToFixBadConversion(unsigned Idx, Sema &S) {
       bool CanFix = Fix.tryToFixConversion(
                       Conversions[Idx].Bad.FromExpr,
@@ -1015,6 +1053,65 @@ class Sema;
           RewriteKind(CRK_None) {}
   };
 
+  struct DeferredTemplateOverloadCandidate {
+
+    // intrusive linked list support for allocateDeferredCandidate
+    DeferredTemplateOverloadCandidate *Next = nullptr;
+
+    enum Kind { Function, Method, Conversion };
+
+    LLVM_PREFERRED_TYPE(Kind)
+    unsigned Kind : 2;
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned AllowObjCConversionOnExplicit : 1;
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned AllowResultConversion : 1;
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned AllowExplicit : 1;
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned SuppressUserConversions : 1;
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned PartialOverloading : 1;
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned AggregateCandidateDeduction : 1;
+  };
+
+  struct DeferredFunctionTemplateOverloadCandidate
+      : public DeferredTemplateOverloadCandidate {
+    FunctionTemplateDecl *FunctionTemplate;
+    DeclAccessPair FoundDecl;
+    ArrayRef<Expr *> Args;
+    CallExpr::ADLCallKind IsADLCandidate;
+    OverloadCandidateParamOrder PO;
+  };
+  static_assert(std::is_trivially_destructible_v<
+                DeferredFunctionTemplateOverloadCandidate>);
+
+  struct DeferredMethodTemplateOverloadCandidate
+      : public DeferredTemplateOverloadCandidate {
+    FunctionTemplateDecl *FunctionTemplate;
+    DeclAccessPair FoundDecl;
+    ArrayRef<Expr *> Args;
+    CXXRecordDecl *ActingContext;
+    Expr::Classification ObjectClassification;
+    QualType ObjectType;
+    OverloadCandidateParamOrder PO;
+  };
+  static_assert(std::is_trivially_destructible_v<
+                DeferredMethodTemplateOverloadCandidate>);
+
+  struct DeferredConversionTemplateOverloadCandidate
+      : public DeferredTemplateOverloadCandidate {
+    FunctionTemplateDecl *FunctionTemplate;
+    DeclAccessPair FoundDecl;
+    CXXRecordDecl *ActingContext;
+    Expr *From;
+    QualType ToType;
+  };
+
+  static_assert(std::is_trivially_destructible_v<
+                DeferredConversionTemplateOverloadCandidate>);
+
   /// OverloadCandidateSet - A set of overload candidates, used in C++
   /// overload resolution (C++ 13.3).
   class OverloadCandidateSet {
@@ -1043,6 +1140,11 @@ class Sema;
       /// C++ [over.match.call.general]
       /// Resolve a call through the address of an overload set.
       CSK_AddressOfOverloadSet,
+
+      /// When doing overload resolution during code completion,
+      /// we want to show all viable candidates, including otherwise
+      /// deferred template candidates.
+      CSK_CodeCompletion,
     };
 
     /// Information about operator rewrites to consider when adding operator
@@ -1117,16 +1219,27 @@ class Sema;
     SmallVector<OverloadCandidate, 16> Candidates;
     llvm::SmallPtrSet<uintptr_t, 16> Functions;
 
-    // Allocator for ConversionSequenceLists. We store the first few of these
+    DeferredTemplateOverloadCandidate *FirstDeferredCandidate = nullptr;
+    unsigned DeferredCandidatesCount : 8 * sizeof(unsigned) - 2;
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned HasDeferredTemplateConstructors : 1;
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned ResolutionByPerfectCandidateIsDisabled : 1;
+
+    // Allocator for ConversionSequenceLists and deferred candidate args.
+    // We store the first few of these
     // inline to avoid allocation for small sets.
     llvm::BumpPtrAllocator SlabAllocator;
 
     SourceLocation Loc;
     CandidateSetKind Kind;
     OperatorRewriteInfo RewriteInfo;
 
+    /// Small storage size for ImplicitConversionSequences
+    /// and the persisted arguments of deferred candidates.
     constexpr static unsigned NumInlineBytes =
-        24 * sizeof(ImplicitConversionSequence);
+        32 * sizeof(ImplicitConversionSequence);
+
     unsigned NumInlineBytesUsed = 0;
     alignas(void *) char InlineSpace[NumInlineBytes];
 
@@ -1137,15 +1250,13 @@ class Sema;
     /// from the slab allocator.
     /// FIXME: It would probably be nice to have a SmallBumpPtrAllocator
     /// instead.
-    /// FIXME: Now that this only allocates ImplicitConversionSequences, do we
-    /// want to un-generalize this?
     template <typename T>
     T *slabAllocate(unsigned N) {
       // It's simpler if this doesn't need to consider alignment.
       static_assert(alignof(T) == alignof(void *),
                     "Only works for pointer-aligned types.");
-      static_assert(std::is_trivial<T>::value ||
-                        std::is_same<ImplicitConversionSequence, T>::value,
+      static_assert(std::is_trivially_destructible_v<T> ||
+                        (std::is_same_v<ImplicitConversionSequence, T>),
                     "Add destruction logic to OverloadCandidateSet::clear().");
 
       unsigned NBytes = sizeof(T) * N;
@@ -1159,12 +1270,34 @@ class Sema;
       return reinterpret_cast<T *>(FreeSpaceStart);
     }
 
+    // Because the size of OverloadCandidateSet has a noticeable impact on
+    // performance, we store each deferred template candidate in the slab
+    // allocator such that deferred candidates are ultimately a singly-linked
+    // intrusive linked list. This ends up being much more efficient than a
+    // SmallVector that is empty in the common case.
+    template <typename T> T *allocateDeferredCandidate() {
+      T *C = slabAllocate<T>(1);
+      if (!FirstDeferredCandidate)
+        FirstDeferredCandidate = C;
+      else {
+        auto *F = FirstDeferredCandidate;
+        while (F->Next)
+          F = F->Next;
+        F->Next = C;
+      }
+      DeferredCandidatesCount++;
+      return C;
+    }
+
     void destroyCandidates();
 
   public:
     OverloadCandidateSet(SourceLocation Loc, CandidateSetKind CSK,
                          OperatorRewriteInfo RewriteInfo = {})
-        : Loc(Loc), Kind(CSK), RewriteInfo(RewriteInfo) {}
+        : FirstDeferredCandidate(nullptr), DeferredCandidatesCount(0),
+          HasDeferredTemplateConstructors(false),
+          ResolutionByPerfectCandidateIsDisabled(false), Loc(Loc), Kind(CSK),
+          RewriteInfo(RewriteInfo) {}
     OverloadCandidateSet(const OverloadCandidateSet &) = delete;
     OverloadCandidateSet &operator=(const OverloadCandidateSet &) = delete;
     ~OverloadCandidateSet() { destroyCandidates(); }
@@ -1176,6 +1309,9 @@ class Sema;
     /// Whether diagnostics should be deferred.
     bool shouldDeferDiags(Sema &S, ArrayRef<Expr *> Args, SourceLocation OpLoc);
 
+    // Whether the resolution of template candidates should be deferred
+    bool shouldDeferTemplateArgumentDeduction(const LangOptions &Opts) const;
+
     /// Determine when this overload candidate will be new to the
     /// overload set.
     bool isNewCandidate(Decl *F, OverloadCandidateParamOrder PO =
@@ -1199,8 +1335,10 @@ class Sema;
     iterator begin() { return Candidates.begin(); }
     iterator end() { return Candidates.end(); }
 
-    size_t size() const { return Candidates.size(); }
-    bool empty() const { return Candidates.empty(); }
+    size_t size() const { return Candidates.size() + DeferredCandidatesCount; }
+    bool empty() const {
+      return Candidates.empty() && DeferredCandidatesCount == 0;
+    }
 
     /// Allocate storage for conversion sequences for NumConversions
     /// conversions.
@@ -1216,6 +1354,24 @@ class Sema;
       return ConversionSequenceList(Conversions, NumConversions);
     }
 
+    /// Provide storage for any Expr* arg that must be preserved
+    /// until deferred template candidates are deduced.
+    /// Typically this should be used for reversed operator arguments
+    /// and any time the argument array is transformed while adding
+    /// a template candidate.
+    llvm::MutableArrayRef<Expr *> getPersistentArgsArray(unsigned N) {
+      Expr **Exprs = slabAllocate<Expr *>(N);
+      return llvm::MutableArrayRef<Expr *>(Exprs, N);
+    }
+
+    template <typename... T>
+    llvm::MutableArrayRef<Expr *> getPersistentArgsArray(T *...Exprs) {
+      llvm::MutableArrayRef<Expr *> Arr =
+          getPersistentArgsArray(sizeof...(Exprs));
+      llvm::copy(std::initializer_list<Expr *>{Exprs...}, Arr.data());
+      return Arr;
+    }
+
     /// Add a new candidate with NumConversions conversion sequence slots
     /// to the overload set.
     OverloadCandidate &addCandidate(unsigned NumConversions = 0,
@@ -1231,6 +1387,32 @@ class Sema;
       return C;
     }
 
+    void AddDeferredTemplateCandidate(
+        FunctionTemplateDecl *FunctionTemplate, DeclAccessPair FoundDecl,
+        ArrayRef<Expr *> Args, bool SuppressUserConversions,
+        bool PartialOverloading, bool AllowExplicit,
+        CallExpr::ADLCallKind IsADLCandidate, OverloadCandidateParamOrder PO,
+        bool AggregateCandidateDeduction);
+
+    void AddDeferredMethodTemplateCandidate(
+        FunctionTemplateDecl *MethodTmpl, DeclAccessPair FoundDecl,
+        CXXRecordDecl *ActingContext, QualType ObjectType,
+        Expr::Classification ObjectClassification, ArrayRef<Expr *> Args,
+        bool SuppressUserConversions, bool PartialOverloading,
+        OverloadCandidateParamOrder PO);
+
+    void AddDeferredConversionTemplateCandidate(
+        FunctionTemplateDecl *FunctionTemplate, DeclAccessPair FoundDecl,
+        CXXRecordDecl *ActingContext, Expr *From, QualType ToType,
+        bool AllowObjCConversionOnExplicit, bool AllowExplicit,
+        bool AllowResultConversion);
+
+    void InjectNonDeducedTemplateCandidates(Sema &S);
+
+    void DisableResolutionByPerfectCandidate() {
+      ResolutionByPerfectCandidateIsDisabled = true;
+    }
+
     /// Find the best viable function on this overload set, if it exists.
     OverloadingResult BestViableFunction(Sema &S, SourceLocation Loc,
                                          OverloadCandidateSet::iterator& Best);
@@ -1263,6 +1445,15 @@ class Sema;
       DestAS = AS;
     }
 
+  private:
+    OverloadingResult ResultForBestCandidate(const iterator &Best);
+    void CudaExcludeWrongSideCandidates(
+        Sema &S, SmallVectorImpl<OverloadCandidate *> &Candidates);
+    OverloadingResult
+    BestViableFunctionImpl(Sema &S, SourceLocation Loc,
+                           OverloadCandidateSet::iterator &Best);
+    void PerfectViableFunction(Sema &S, SourceLocation Loc,
+                               OverloadCandidateSet::iterator &Best);
   };
 
   bool isBetterOverloadCandidate(Sema &S, const OverloadCandidate &Cand1,
@@ -1311,6 +1502,21 @@ class Sema;
   // parameter.
   bool shouldEnforceArgLimit(bool PartialOverloading, FunctionDecl *Function);
 
+  inline bool OverloadCandidateSet::shouldDeferTemplateArgumentDeduction(
+      const LangOptions &Opts) const {
+    return
+        // For user defined conversion we need to check against different
+        // combination of CV qualifiers and look at any explicit specifier, so
+        // always deduce template candidates.
+        Kind != CSK_InitByUserDefinedConversion
+        // When doing code completion, we want to see all the
+        // viable candidates.
+        && Kind != CSK_CodeCompletion
+        // CUDA may prefer template candidates even when a non-candidate
+        // is a perfect match
+        && !Opts.CUDA;
+  }
+
 } // namespace clang
 
 #endif // LLVM_CLANG_SEMA_OVERLOAD_H

clang/lib/Sema/SemaCodeComplete.cpp

@@ -6354,7 +6354,8 @@ SemaCodeCompletion::ProduceCallSignatureHelp(Expr *Fn, ArrayRef<Expr *> Args,
   Expr *NakedFn = Fn->IgnoreParenCasts();
   // Build an overload candidate set based on the functions we find.
   SourceLocation Loc = Fn->getExprLoc();
-  OverloadCandidateSet CandidateSet(Loc, OverloadCandidateSet::CSK_Normal);
+  OverloadCandidateSet CandidateSet(Loc,
+                                    OverloadCandidateSet::CSK_CodeCompletion);
 
   if (auto ULE = dyn_cast<UnresolvedLookupExpr>(NakedFn)) {
     SemaRef.AddOverloadedCallCandidates(ULE, ArgsWithoutDependentTypes,
@@ -6557,7 +6558,8 @@ QualType SemaCodeCompletion::ProduceConstructorSignatureHelp(
   // FIXME: Provide support for variadic template constructors.
 
   if (CRD) {
-    OverloadCandidateSet CandidateSet(Loc, OverloadCandidateSet::CSK_Normal);
+    OverloadCandidateSet CandidateSet(Loc,
+                                      OverloadCandidateSet::CSK_CodeCompletion);
     for (NamedDecl *C : SemaRef.LookupConstructors(CRD)) {
       if (auto *FD = dyn_cast<FunctionDecl>(C)) {
         // FIXME: we can't yet provide correct signature help for initializer

clang/lib/Sema/SemaInit.cpp

@@ -10029,12 +10029,19 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
     //   When [...] the constructor [...] is a candidate by
     //    - [over.match.copy] (in all cases)
     if (TD) {
-      SmallVector<Expr *, 8> TmpInits;
-      for (Expr *E : Inits)
+
+      // As template candidates are not deduced immediately,
+      // persist the array in the overload set.
+      MutableArrayRef<Expr *> TmpInits =
+          Candidates.getPersistentArgsArray(Inits.size());
+
+      for (auto [I, E] : llvm::enumerate(Inits)) {
         if (auto *DI = dyn_cast<DesignatedInitExpr>(E))
-          TmpInits.push_back(DI->getInit());
+          TmpInits[I] = DI->getInit();
         else
-          TmpInits.push_back(E);
+          TmpInits[I] = E;
+      }
+
       AddTemplateOverloadCandidate(
           TD, FoundDecl, /*ExplicitArgs=*/nullptr, TmpInits, Candidates,
           /*SuppressUserConversions=*/false,