[Clang][RFC] Bypass TAD during overload resolution if a perfect match exists

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,