[Clang] Explain why a type trait evaluated to false.

[cor3ntin]

static_assert(std::is_xx_v<MyType>); is a common pattern to check that a type meets a requirement.

This patch produces diagnostics notes when such assertion fails. The first type trait for which we provide detailed explanation is std::is_trivially_relocatable.

We employ the same mechanisn when a type trait appears an an unsatisfied atomic constraint.

I plan to also support std::is_trivially_replaceable in a follow up PR, and hopefully, over time we can support more type traits.

[llvmbot]

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes

static_assert(std::is_xx_v<MyType>); is a common pattern to check that a type meets a requirement.

This patch produces diagnostics notes when such assertion fails. The first type trait for which we provide detailed explanation is std::is_trivially_relocatable.

We employ the same mechanisn when a type trait appears an an unsatisfied atomic constraint.

I plan to also support std::is_trivially_replaceable in a follow up PR, and hopefully, over time we can support more type traits.

---

Patch is 47.43 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/141238.diff

10 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+23)
• (modified) clang/include/clang/Sema/Sema.h (+5)
• (modified) clang/lib/Sema/CMakeLists.txt (+1)
• (modified) clang/lib/Sema/SemaConcept.cpp (+1)
• (modified) clang/lib/Sema/SemaDeclCXX.cpp (+2-273)
• (modified) clang/lib/Sema/SemaExprCXX.cpp (+2-63)
• (added) clang/lib/Sema/SemaTypeTraits.cpp (+539)
• (added) clang/test/SemaCXX/type-traits-unsatisfied-diags-std.cpp (+101)
• (added) clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp (+146)
• (modified) clang/test/SemaObjCXX/objc-weak-type-traits.mm (+6-1)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 2835e3a9d9960..f2fcfe5d351f4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1762,6 +1762,29 @@ def err_user_defined_msg_constexpr : Error<
   "%sub{subst_user_defined_msg}0 must be produced by a "
   "constant expression">;
 
+// Type traits explanations
+def note_unsatisfied_trait : Note<"%0 is not %enum_select<TraitName>{"
+                                  "%TriviallyRelocatable{trivially relocatable}"
+                                  "}1">;
+
+def note_unsatisfied_trait_reason
+    : Note<"because it "
+           "%enum_select<TraitNotSatisfiedReason>{"
+           "%Ref{is a reference type}|"
+           "%HasArcLifetime{has an ARC lifetime qualifier}|"
+           "%VLA{is a variably-modified type}|"
+           "%VBase{has a virtual base %1}|"
+           "%NRBase{has a non-trivially-relocatable base %1}|"
+           "%NRField{has a non-trivially-relocatable member %1 of type %2}|"
+           "%DeletedDtr{has a %select{deleted|user-provided}1 destructor}|"
+           "%UserProvidedCtr{has a user provided %select{copy|move}1 "
+           "constructor}|"
+           "%UserProvidedAssign{has a user provided %select{copy|move}1 "
+           "assignment operator}|"
+           "%UnionWithUserDeclaredSMF{is a union with a user-declared "
+           "%sub{select_special_member_kind}1}"
+           "}0">;
+
 def warn_consteval_if_always_true : Warning<
   "consteval if is always true in an %select{unevaluated|immediate}0 context">,
   InGroup<DiagGroup<"redundant-consteval-if">>;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1091a7f504b57..bbc5c181c6a10 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5910,6 +5910,11 @@ class Sema final : public SemaBase {
   /// with expression \E
   void DiagnoseStaticAssertDetails(const Expr *E);
 
+  /// If E represents a built-in type trait, or a known standard type trait,
+  /// try to print more information about why the type type-trait failed.
+  /// This assumes we already evaluated the expression to a false boolean value.
+  void DiagnoseTypeTraitDetails(const Expr *E);
+
   /// Handle a friend type declaration.  This works in tandem with
   /// ActOnTag.
   ///
diff --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt
index 4b87004e4b8ea..51e0ee10b080b 100644
--- a/clang/lib/Sema/CMakeLists.txt
+++ b/clang/lib/Sema/CMakeLists.txt
@@ -96,6 +96,7 @@ add_clang_library(clangSema
   SemaTemplateInstantiateDecl.cpp
   SemaTemplateVariadic.cpp
   SemaType.cpp
+  SemaTypeTraits.cpp
   SemaWasm.cpp
   SemaX86.cpp
   TypeLocBuilder.cpp
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 7da8e696c90bd..c6a54dc141ded 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -1320,6 +1320,7 @@ static void diagnoseWellFormedUnsatisfiedConstraintExpr(Sema &S,
   S.Diag(SubstExpr->getSourceRange().getBegin(),
          diag::note_atomic_constraint_evaluated_to_false)
       << (int)First << SubstExpr;
+  S.DiagnoseTypeTraitDetails(SubstExpr);
 }
 
 template <typename SubstitutionDiagnostic>
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index fe92191b6a687..770ac9839eb98 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7368,279 +7368,6 @@ void Sema::CheckCompletedCXXClass(Scope *S, CXXRecordDecl *Record) {
   CheckMismatchedTypeAwareAllocators(OO_Array_New, OO_Array_Delete);
 }
 
-static CXXMethodDecl *LookupSpecialMemberFromXValue(Sema &SemaRef,
-                                                    const CXXRecordDecl *RD,
-                                                    bool Assign) {
-  RD = RD->getDefinition();
-  SourceLocation LookupLoc = RD->getLocation();
-
-  CanQualType CanTy = SemaRef.getASTContext().getCanonicalType(
-      SemaRef.getASTContext().getTagDeclType(RD));
-  DeclarationName Name;
-  Expr *Arg = nullptr;
-  unsigned NumArgs;
-
-  QualType ArgType = CanTy;
-  ExprValueKind VK = clang::VK_XValue;
-
-  if (Assign)
-    Name =
-        SemaRef.getASTContext().DeclarationNames.getCXXOperatorName(OO_Equal);
-  else
-    Name =
-        SemaRef.getASTContext().DeclarationNames.getCXXConstructorName(CanTy);
-
-  OpaqueValueExpr FakeArg(LookupLoc, ArgType, VK);
-  NumArgs = 1;
-  Arg = &FakeArg;
-
-  // Create the object argument
-  QualType ThisTy = CanTy;
-  Expr::Classification Classification =
-      OpaqueValueExpr(LookupLoc, ThisTy, VK_LValue)
-          .Classify(SemaRef.getASTContext());
-
-  // Now we perform lookup on the name we computed earlier and do overload
-  // resolution. Lookup is only performed directly into the class since there
-  // will always be a (possibly implicit) declaration to shadow any others.
-  OverloadCandidateSet OCS(LookupLoc, OverloadCandidateSet::CSK_Normal);
-  DeclContext::lookup_result R = RD->lookup(Name);
-
-  if (R.empty())
-    return nullptr;
-
-  // Copy the candidates as our processing of them may load new declarations
-  // from an external source and invalidate lookup_result.
-  SmallVector<NamedDecl *, 8> Candidates(R.begin(), R.end());
-
-  for (NamedDecl *CandDecl : Candidates) {
-    if (CandDecl->isInvalidDecl())
-      continue;
-
-    DeclAccessPair Cand = DeclAccessPair::make(CandDecl, clang::AS_none);
-    auto CtorInfo = getConstructorInfo(Cand);
-    if (CXXMethodDecl *M = dyn_cast<CXXMethodDecl>(Cand->getUnderlyingDecl())) {
-      if (Assign)
-        SemaRef.AddMethodCandidate(M, Cand, const_cast<CXXRecordDecl *>(RD),
-                                   ThisTy, Classification,
-                                   llvm::ArrayRef(&Arg, NumArgs), OCS, true);
-      else {
-        assert(CtorInfo);
-        SemaRef.AddOverloadCandidate(CtorInfo.Constructor, CtorInfo.FoundDecl,
-                                     llvm::ArrayRef(&Arg, NumArgs), OCS,
-                                     /*SuppressUserConversions*/ true);
-      }
-    } else if (FunctionTemplateDecl *Tmpl =
-                   dyn_cast<FunctionTemplateDecl>(Cand->getUnderlyingDecl())) {
-      if (Assign)
-        SemaRef.AddMethodTemplateCandidate(
-            Tmpl, Cand, const_cast<CXXRecordDecl *>(RD), nullptr, ThisTy,
-            Classification, llvm::ArrayRef(&Arg, NumArgs), OCS, true);
-      else {
-        assert(CtorInfo);
-        SemaRef.AddTemplateOverloadCandidate(
-            CtorInfo.ConstructorTmpl, CtorInfo.FoundDecl, nullptr,
-            llvm::ArrayRef(&Arg, NumArgs), OCS, true);
-      }
-    }
-  }
-
-  OverloadCandidateSet::iterator Best;
-  switch (OCS.BestViableFunction(SemaRef, LookupLoc, Best)) {
-  case OR_Success:
-    return cast<CXXMethodDecl>(Best->Function);
-  default:
-    return nullptr;
-  }
-}
-
-static bool hasSuitableConstructorForRelocation(Sema &SemaRef,
-                                                const CXXRecordDecl *D,
-                                                bool AllowUserDefined) {
-  assert(D->hasDefinition() && !D->isInvalidDecl());
-
-  if (D->hasSimpleMoveConstructor() || D->hasSimpleCopyConstructor())
-    return true;
-
-  CXXMethodDecl *Decl =
-      LookupSpecialMemberFromXValue(SemaRef, D, /*Assign=*/false);
-  return Decl && Decl->isUserProvided() == AllowUserDefined;
-}
-
-static bool hasSuitableMoveAssignmentOperatorForRelocation(
-    Sema &SemaRef, const CXXRecordDecl *D, bool AllowUserDefined) {
-  assert(D->hasDefinition() && !D->isInvalidDecl());
-
-  if (D->hasSimpleMoveAssignment() || D->hasSimpleCopyAssignment())
-    return true;
-
-  CXXMethodDecl *Decl =
-      LookupSpecialMemberFromXValue(SemaRef, D, /*Assign=*/true);
-  if (!Decl)
-    return false;
-
-  return Decl && Decl->isUserProvided() == AllowUserDefined;
-}
-
-// [C++26][class.prop]
-// A class C is default-movable if
-// - overload resolution for direct-initializing an object of type C
-// from an xvalue of type C selects a constructor that is a direct member of C
-// and is neither user-provided nor deleted,
-// - overload resolution for assigning to an lvalue of type C from an xvalue of
-// type C selects an assignment operator function that is a direct member of C
-// and is neither user-provided nor deleted, and C has a destructor that is
-// neither user-provided nor deleted.
-static bool IsDefaultMovable(Sema &SemaRef, const CXXRecordDecl *D) {
-  if (!hasSuitableConstructorForRelocation(SemaRef, D,
-                                           /*AllowUserDefined=*/false))
-    return false;
-
-  if (!hasSuitableMoveAssignmentOperatorForRelocation(
-          SemaRef, D, /*AllowUserDefined=*/false))
-    return false;
-
-  CXXDestructorDecl *Dtr = D->getDestructor();
-
-  if (!Dtr)
-    return true;
-
-  if (Dtr->isUserProvided() && (!Dtr->isDefaulted() || Dtr->isDeleted()))
-    return false;
-
-  return !Dtr->isDeleted();
-}
-
-// [C++26][class.prop]
-// A class is eligible for trivial relocation unless it...
-static bool IsEligibleForTrivialRelocation(Sema &SemaRef,
-                                           const CXXRecordDecl *D) {
-
-  for (const CXXBaseSpecifier &B : D->bases()) {
-    const auto *BaseDecl = B.getType()->getAsCXXRecordDecl();
-    if (!BaseDecl)
-      continue;
-    // ... has any virtual base classes
-    // ... has a base class that is not a trivially relocatable class
-    if (B.isVirtual() || (!BaseDecl->isDependentType() &&
-                          !SemaRef.IsCXXTriviallyRelocatableType(B.getType())))
-      return false;
-  }
-
-  for (const FieldDecl *Field : D->fields()) {
-    if (Field->getType()->isDependentType())
-      continue;
-    if (Field->getType()->isReferenceType())
-      continue;
-    // ... has a non-static data member of an object type that is not
-    // of a trivially relocatable type
-    if (!SemaRef.IsCXXTriviallyRelocatableType(Field->getType()))
-      return false;
-  }
-  return !D->hasDeletedDestructor();
-}
-
-// [C++26][class.prop]
-// A class C is eligible for replacement unless
-static bool IsEligibleForReplacement(Sema &SemaRef, const CXXRecordDecl *D) {
-
-  for (const CXXBaseSpecifier &B : D->bases()) {
-    const auto *BaseDecl = B.getType()->getAsCXXRecordDecl();
-    if (!BaseDecl)
-      continue;
-    // it has a base class that is not a replaceable class
-    if (!BaseDecl->isDependentType() &&
-        !SemaRef.IsCXXReplaceableType(B.getType()))
-      return false;
-  }
-
-  for (const FieldDecl *Field : D->fields()) {
-    if (Field->getType()->isDependentType())
-      continue;
-
-    // it has a non-static data member that is not of a replaceable type,
-    if (!SemaRef.IsCXXReplaceableType(Field->getType()))
-      return false;
-  }
-  return !D->hasDeletedDestructor();
-}
-
-ASTContext::CXXRecordDeclRelocationInfo
-Sema::CheckCXX2CRelocatableAndReplaceable(const CXXRecordDecl *D) {
-  ASTContext::CXXRecordDeclRelocationInfo Info{false, false};
-
-  if (!getLangOpts().CPlusPlus || D->isInvalidDecl())
-    return Info;
-
-  assert(D->hasDefinition());
-
-  // This is part of "eligible for replacement", however we defer it
-  // to avoid extraneous computations.
-  auto HasSuitableSMP = [&] {
-    return hasSuitableConstructorForRelocation(*this, D,
-                                               /*AllowUserDefined=*/true) &&
-           hasSuitableMoveAssignmentOperatorForRelocation(
-               *this, D, /*AllowUserDefined=*/true);
-  };
-
-  auto IsUnion = [&, Is = std::optional<bool>{}]() mutable {
-    if (!Is.has_value())
-      Is = D->isUnion() && !D->hasUserDeclaredCopyConstructor() &&
-           !D->hasUserDeclaredCopyAssignment() &&
-           !D->hasUserDeclaredMoveOperation() &&
-           !D->hasUserDeclaredDestructor();
-    return *Is;
-  };
-
-  auto IsDefaultMovable = [&, Is = std::optional<bool>{}]() mutable {
-    if (!Is.has_value())
-      Is = ::IsDefaultMovable(*this, D);
-    return *Is;
-  };
-
-  Info.IsRelocatable = [&] {
-    if (D->isDependentType())
-      return false;
-
-    // if it is eligible for trivial relocation
-    if (!IsEligibleForTrivialRelocation(*this, D))
-      return false;
-
-    // has the trivially_relocatable_if_eligible class-property-specifier,
-    if (D->hasAttr<TriviallyRelocatableAttr>())
-      return true;
-
-    // is a union with no user-declared special member functions, or
-    if (IsUnion())
-      return true;
-
-    // is default-movable.
-    return IsDefaultMovable();
-  }();
-
-  Info.IsReplaceable = [&] {
-    if (D->isDependentType())
-      return false;
-
-    // A class C is a replaceable class if it is eligible for replacement
-    if (!IsEligibleForReplacement(*this, D))
-      return false;
-
-    // has the replaceable_if_eligible class-property-specifier
-    if (D->hasAttr<ReplaceableAttr>())
-      return HasSuitableSMP();
-
-    // is a union with no user-declared special member functions, or
-    if (IsUnion())
-      return HasSuitableSMP();
-
-    // is default-movable.
-    return IsDefaultMovable();
-  }();
-
-  return Info;
-}
-
 /// Look up the special member function that would be called by a special
 /// member function for a subobject of class type.
 ///
@@ -17967,6 +17694,8 @@ void Sema::DiagnoseStaticAssertDetails(const Expr *E) {
           << DiagSide[0].ValueString << Op->getOpcodeStr()
           << DiagSide[1].ValueString << Op->getSourceRange();
     }
+  } else {
+    DiagnoseTypeTraitDetails(E);
   }
 }
 
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index b53877c40668d..3640483fd3706 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -5590,69 +5590,8 @@ static bool isTriviallyEqualityComparableType(Sema &S, QualType Type, SourceLoca
       CanonicalType, /*CheckIfTriviallyCopyable=*/false);
 }
 
-static bool IsCXXTriviallyRelocatableType(Sema &S, const CXXRecordDecl *RD) {
-  if (std::optional<ASTContext::CXXRecordDeclRelocationInfo> Info =
-          S.getASTContext().getRelocationInfoForCXXRecord(RD))
-    return Info->IsRelocatable;
-  ASTContext::CXXRecordDeclRelocationInfo Info =
-      S.CheckCXX2CRelocatableAndReplaceable(RD);
-  S.getASTContext().setRelocationInfoForCXXRecord(RD, Info);
-  return Info.IsRelocatable;
-}
-
-bool Sema::IsCXXTriviallyRelocatableType(QualType Type) {
-
-  QualType BaseElementType = getASTContext().getBaseElementType(Type);
-
-  if (Type->isVariableArrayType())
-    return false;
-
-  if (BaseElementType.hasNonTrivialObjCLifetime())
-    return false;
-
-  if (BaseElementType.hasAddressDiscriminatedPointerAuth())
-    return false;
-
-  if (BaseElementType->isIncompleteType())
-    return false;
-
-  if (BaseElementType->isScalarType() || BaseElementType->isVectorType())
-    return true;
-
-  if (const auto *RD = BaseElementType->getAsCXXRecordDecl())
-    return ::IsCXXTriviallyRelocatableType(*this, RD);
-
-  return false;
-}
-
-static bool IsCXXReplaceableType(Sema &S, const CXXRecordDecl *RD) {
-  if (std::optional<ASTContext::CXXRecordDeclRelocationInfo> Info =
-          S.getASTContext().getRelocationInfoForCXXRecord(RD))
-    return Info->IsReplaceable;
-  ASTContext::CXXRecordDeclRelocationInfo Info =
-      S.CheckCXX2CRelocatableAndReplaceable(RD);
-  S.getASTContext().setRelocationInfoForCXXRecord(RD, Info);
-  return Info.IsReplaceable;
-}
-
-bool Sema::IsCXXReplaceableType(QualType Type) {
-  if (Type.isConstQualified() || Type.isVolatileQualified())
-    return false;
-
-  if (Type->isVariableArrayType())
-    return false;
-
-  QualType BaseElementType =
-      getASTContext().getBaseElementType(Type.getUnqualifiedType());
-  if (BaseElementType->isIncompleteType())
-    return false;
-  if (BaseElementType->isScalarType())
-    return true;
-  if (const auto *RD = BaseElementType->getAsCXXRecordDecl())
-    return ::IsCXXReplaceableType(*this, RD);
-  return false;
-}
-
+// FIXME : Move the type traits logic to SemaTypeTraits.h
+extern bool IsCXXTriviallyRelocatableType(Sema &S, const CXXRecordDecl *RD);
 static bool IsTriviallyRelocatableType(Sema &SemaRef, QualType T) {
   QualType BaseElementType = SemaRef.getASTContext().getBaseElementType(T);
 
diff --git a/clang/lib/Sema/SemaTypeTraits.cpp b/clang/lib/Sema/SemaTypeTraits.cpp
new file mode 100644
index 0000000000000..d16989e8dfdbe
--- /dev/null
+++ b/clang/lib/Sema/SemaTypeTraits.cpp
@@ -0,0 +1,539 @@
+//===----- SemaTypeTraits.cpp - Semantic Analysis for C++ Type Traits -----===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file implements semantic analysis for C++ type traits.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/AST/DeclCXX.h"
+#include "clang/Basic/DiagnosticSema.h"
+#include "clang/Sema/Overload.h"
+#include "clang/Sema/Sema.h"
+
+using namespace clang;
+
+static CXXMethodDecl *LookupSpecialMemberFromXValue(Sema &SemaRef,
+                                                    const CXXRecordDecl *RD,
+                                                    bool Assign) {
+  RD = RD->getDefinition();
+  SourceLocation LookupLoc = RD->getLocation();
+
+  CanQualType CanTy = SemaRef.getASTContext().getCanonicalType(
+      SemaRef.getASTContext().getTagDeclType(RD));
+  DeclarationName Name;
+  Expr *Arg = nullptr;
+  unsigned NumArgs;
+
+  QualType ArgType = CanTy;
+  ExprValueKind VK = clang::VK_XValue;
+
+  if (Assign)
+    Name =
+        SemaRef.getASTContext().DeclarationNames.getCXXOperatorName(OO_Equal);
+  else
+    Name =
+        SemaRef.getASTContext().DeclarationNames.getCXXConstructorName(CanTy);
+
+  OpaqueValueExpr FakeArg(LookupLoc, ArgType, VK);
+  NumArgs = 1;
+  Arg = &FakeArg;
+
+  // Create the object argument
+  QualType ThisTy = CanTy;
+  Expr::Classification Classification =
+      OpaqueValueExpr(LookupLoc, ThisTy, VK_LValue)
+          .Classify(SemaRef.getASTContext());
+
+  // Now we perform lookup on the name we computed earlier and do overload
+  // resolution. Lookup is only performed directly into the class since there
+  // will always be a (possibly implicit) declaration to shadow any others.
+  OverloadCandidateSet OCS(LookupLoc, OverloadCandidateSet::CSK_Normal);
+  DeclContext::lookup_result R = RD->lookup(Name);
+
+  if (R.empty())
+    return nullptr;
+
+  // Copy the candidates as our processing of them may load new declarations
+  // from an external source and invalidate lookup_result.
+  SmallVector<NamedDecl *, 8> Candidates(R.begin(), R.end());
+
+  for (NamedDecl *CandDecl : Candidates) {
+    if (CandDecl->isInvalidDecl())
+      continue;
+
+    DeclAccessPair Cand = DeclAccessPair::make(CandDecl, clang::AS_none);
+    auto CtorInfo = getConstructorInfo(Cand);
+    if (CXXMethodDecl *M = dyn_cast<CXXMethodDecl>(Cand->getUnderlyingDecl())) {
+      if (Assign)
+        SemaRef.AddMethodCandidate(M, Cand, const_cast<CXXRecordDecl *>(RD),
+                                   ThisTy, Classification,
+                                   llvm::ArrayRef(&Arg, NumArgs), OCS, true);
+      else {
+        assert(CtorInfo);
+        SemaRef.AddOverloadCandidate(CtorInfo.Constructor, CtorInfo.FoundDecl,
+                                     llvm::ArrayRef(&Arg, NumArgs), OCS,
+                                     /*SuppressUserConversions*/ true);
+      }
+    } else if (FunctionTemplateDecl *Tmpl =
+                   dyn_cast<FunctionTemplateDecl>(Cand->getUnderlyingDecl())) {
+      if (Assign)
+        SemaRef.AddMethodTemplateCandidate(
+            Tmpl, Cand, const_cast<CXXRecordDecl *>(RD), nullptr, ThisTy,
+            Classification, llvm::ArrayRef(&Arg, NumArgs), OCS, true);
+      else {
+        assert(CtorInfo);
+        SemaRef.AddTemplateOverloadCandidate(
+            CtorInfo.ConstructorTmpl, CtorInfo.FoundDecl, nullptr,
+            llvm::ArrayRef(&Arg, NumArgs), OCS, true);
+      }
+    }
+  }
+
+  OverloadCandidateSet::iterator Best;
+  switch (OCS.BestViableFunction(SemaRef, LookupLoc, Best)) {
+  case OR_Success:
+    return cast<CXXMethodDecl>(Best->Function);
+  default:
+    return nullptr;
+  }
+}
+
+stat...
[truncated]

[cor3ntin]

I started to move some logic to SemaTypeTraits.cpp to keep the size of SemaExprCXX.cpp reasonable

[erichkeane]

My word... I'm VERY glad we have enum_select :D

[cor3ntin]

Yeah, in these cases it's super useful, thanks :D

[erichkeane]

There is no such file though? its a little sad to not have this done here.

[erichkeane]

This is one of those auto shouldn't be used here situations (I was quite confused by what we were looking at :D), though the optional around a pair is sorta self inflicted... I wonder if we should have a type-alias for that.

[erichkeane]

Could we split the 'SemaTypeTraits.cpp file creation' part of this patch (including MOVING stuff) to a different review/commit? It is making figuring out what to review pretty difficult.

[erichkeane]

.... CAN this happen?

[erichkeane]

Do a if (D->isInvalidDecl()) return; here. Else this can result in some weirdness at times I think?

[erichkeane]

I might suggest combining the two 'if' statements.