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

Reverts #133426

This is failing on some bots
https://lab.llvm.org/buildbot/#/builders/163/builds/17265

Author: cor3ntin

clang/docs/ReleaseNotes.rst

@@ -96,12 +96,6 @@ 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,24 +407,6 @@ 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,
@@ -761,12 +743,6 @@ 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 {
@@ -1003,20 +979,6 @@ 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,
@@ -1053,65 +1015,6 @@ 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 {
@@ -1140,11 +1043,6 @@ 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
@@ -1219,27 +1117,16 @@ class Sema;
     SmallVector<OverloadCandidate, 16> Candidates;
     llvm::SmallPtrSet<uintptr_t, 16> Functions;
 
-    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
+    // Allocator for ConversionSequenceLists. 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 =
-        32 * sizeof(ImplicitConversionSequence);
-
+        24 * sizeof(ImplicitConversionSequence);
     unsigned NumInlineBytesUsed = 0;
     alignas(void *) char InlineSpace[NumInlineBytes];
 
@@ -1250,13 +1137,15 @@ 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_trivially_destructible_v<T> ||
-                        (std::is_same_v<ImplicitConversionSequence, T>),
+      static_assert(std::is_trivial<T>::value ||
+                        std::is_same<ImplicitConversionSequence, T>::value,
                     "Add destruction logic to OverloadCandidateSet::clear().");
 
       unsigned NBytes = sizeof(T) * N;
@@ -1270,34 +1159,12 @@ 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 = {})
-        : FirstDeferredCandidate(nullptr), DeferredCandidatesCount(0),
-          HasDeferredTemplateConstructors(false),
-          ResolutionByPerfectCandidateIsDisabled(false), Loc(Loc), Kind(CSK),
-          RewriteInfo(RewriteInfo) {}
+        : Loc(Loc), Kind(CSK), RewriteInfo(RewriteInfo) {}
     OverloadCandidateSet(const OverloadCandidateSet &) = delete;
     OverloadCandidateSet &operator=(const OverloadCandidateSet &) = delete;
     ~OverloadCandidateSet() { destroyCandidates(); }
@@ -1309,9 +1176,6 @@ 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 =
@@ -1335,10 +1199,8 @@ class Sema;
     iterator begin() { return Candidates.begin(); }
     iterator end() { return Candidates.end(); }
 
-    size_t size() const { return Candidates.size() + DeferredCandidatesCount; }
-    bool empty() const {
-      return Candidates.empty() && DeferredCandidatesCount == 0;
-    }
+    size_t size() const { return Candidates.size(); }
+    bool empty() const { return Candidates.empty(); }
 
     /// Allocate storage for conversion sequences for NumConversions
     /// conversions.
@@ -1354,24 +1216,6 @@ 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,
@@ -1387,32 +1231,6 @@ 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);
@@ -1445,15 +1263,6 @@ 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,
@@ -1502,21 +1311,6 @@ 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,8 +6354,7 @@ 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_CodeCompletion);
+  OverloadCandidateSet CandidateSet(Loc, OverloadCandidateSet::CSK_Normal);
 
   if (auto ULE = dyn_cast<UnresolvedLookupExpr>(NakedFn)) {
     SemaRef.AddOverloadedCallCandidates(ULE, ArgsWithoutDependentTypes,
@@ -6558,8 +6557,7 @@ QualType SemaCodeCompletion::ProduceConstructorSignatureHelp(
   // FIXME: Provide support for variadic template constructors.
 
   if (CRD) {
-    OverloadCandidateSet CandidateSet(Loc,
-                                      OverloadCandidateSet::CSK_CodeCompletion);
+    OverloadCandidateSet CandidateSet(Loc, OverloadCandidateSet::CSK_Normal);
     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,19 +10029,12 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
     //   When [...] the constructor [...] is a candidate by
     //    - [over.match.copy] (in all cases)
     if (TD) {
-
-      // 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)) {
+      SmallVector<Expr *, 8> TmpInits;
+      for (Expr *E : Inits)
         if (auto *DI = dyn_cast<DesignatedInitExpr>(E))
-          TmpInits[I] = DI->getInit();
+          TmpInits.push_back(DI->getInit());
         else
-          TmpInits[I] = E;
-      }
-
+          TmpInits.push_back(E);
       AddTemplateOverloadCandidate(
           TD, FoundDecl, /*ExplicitArgs=*/nullptr, TmpInits, Candidates,
           /*SuppressUserConversions=*/false,