Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][NFC] Refactor Sema::TemplateDeductionResult #81398

Merged
merged 4 commits into from
Feb 12, 2024

Conversation

Endilll
Copy link
Contributor

@Endilll Endilll commented Feb 11, 2024

This patch converts Sema::TemplateDeductionResult into a scoped enum in namespace scope, making it eligible for forward declaring. This is useful in certain contexts, such as preferred_type annotations on bit-fields.

This patch converts `Sema::TemplateDeductionResult` into a scoped enum in namespace scope, making it eligible for forward declaring. This is useful in certain contexts, such as `preferred_type` annotations on bit-fields.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 11, 2024

@llvm/pr-subscribers-clang

Author: Vlad Serebrennikov (Endilll)

Changes

This patch converts Sema::TemplateDeductionResult into a scoped enum in namespace scope, making it eligible for forward declaring. This is useful in certain contexts, such as preferred_type annotations on bit-fields.


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

10 Files Affected:

  • (modified) clang/include/clang/Sema/Sema.h (+67-67)
  • (modified) clang/include/clang/Sema/TemplateDeduction.h (+7-1)
  • (modified) clang/lib/Sema/SemaDecl.cpp (+2-1)
  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+7-5)
  • (modified) clang/lib/Sema/SemaLookup.cpp (+2-2)
  • (modified) clang/lib/Sema/SemaOverload.cpp (+195-182)
  • (modified) clang/lib/Sema/SemaStmt.cpp (+9-7)
  • (modified) clang/lib/Sema/SemaTemplate.cpp (+14-12)
  • (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+369-325)
  • (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+6-5)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 3c26003b5bda7f..a3cda590503fef 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -353,6 +353,72 @@ class PreferredTypeBuilder {
   llvm::function_ref<QualType()> ComputeType;
 };
 
+/// Describes the result of template argument deduction.
+///
+/// The TemplateDeductionResult enumeration describes the result of
+/// template argument deduction, as returned from
+/// DeduceTemplateArguments(). The separate TemplateDeductionInfo
+/// structure provides additional information about the results of
+/// template argument deduction, e.g., the deduced template argument
+/// list (if successful) or the specific template parameters or
+/// deduced arguments that were involved in the failure.
+enum class TemplateDeductionResult {
+  /// Template argument deduction was successful.
+  Success = 0,
+  /// The declaration was invalid; do nothing.
+  Invalid,
+  /// Template argument deduction exceeded the maximum template
+  /// instantiation depth (which has already been diagnosed).
+  InstantiationDepth,
+  /// Template argument deduction did not deduce a value
+  /// for every template parameter.
+  Incomplete,
+  /// Template argument deduction did not deduce a value for every
+  /// expansion of an expanded template parameter pack.
+  IncompletePack,
+  /// Template argument deduction produced inconsistent
+  /// deduced values for the given template parameter.
+  Inconsistent,
+  /// Template argument deduction failed due to inconsistent
+  /// cv-qualifiers on a template parameter type that would
+  /// otherwise be deduced, e.g., we tried to deduce T in "const T"
+  /// but were given a non-const "X".
+  Underqualified,
+  /// Substitution of the deduced template argument values
+  /// resulted in an error.
+  SubstitutionFailure,
+  /// After substituting deduced template arguments, a dependent
+  /// parameter type did not match the corresponding argument.
+  DeducedMismatch,
+  /// After substituting deduced template arguments, an element of
+  /// a dependent parameter type did not match the corresponding element
+  /// of the corresponding argument (when deducing from an initializer list).
+  DeducedMismatchNested,
+  /// A non-depnedent component of the parameter did not match the
+  /// corresponding component of the argument.
+  NonDeducedMismatch,
+  /// When performing template argument deduction for a function
+  /// template, there were too many call arguments.
+  TooManyArguments,
+  /// When performing template argument deduction for a function
+  /// template, there were too few call arguments.
+  TooFewArguments,
+  /// The explicitly-specified template arguments were not valid
+  /// template arguments for the given template.
+  InvalidExplicitArguments,
+  /// Checking non-dependent argument conversions failed.
+  NonDependentConversionFailure,
+  /// The deduced arguments did not satisfy the constraints associated
+  /// with the template.
+  ConstraintsNotSatisfied,
+  /// Deduction failed; that's all we know.
+  MiscellaneousDeductionFailure,
+  /// CUDA Target attributes do not match.
+  CUDATargetMismatch,
+  /// Some error which was already diagnosed.
+  AlreadyDiagnosed
+};
+
 /// Sema - This implements semantic analysis and AST building for C.
 class Sema final {
   Sema(const Sema &) = delete;
@@ -9262,72 +9328,6 @@ class Sema final {
   QualType adjustCCAndNoReturn(QualType ArgFunctionType, QualType FunctionType,
                                bool AdjustExceptionSpec = false);
 
-  /// Describes the result of template argument deduction.
-  ///
-  /// The TemplateDeductionResult enumeration describes the result of
-  /// template argument deduction, as returned from
-  /// DeduceTemplateArguments(). The separate TemplateDeductionInfo
-  /// structure provides additional information about the results of
-  /// template argument deduction, e.g., the deduced template argument
-  /// list (if successful) or the specific template parameters or
-  /// deduced arguments that were involved in the failure.
-  enum TemplateDeductionResult {
-    /// Template argument deduction was successful.
-    TDK_Success = 0,
-    /// The declaration was invalid; do nothing.
-    TDK_Invalid,
-    /// Template argument deduction exceeded the maximum template
-    /// instantiation depth (which has already been diagnosed).
-    TDK_InstantiationDepth,
-    /// Template argument deduction did not deduce a value
-    /// for every template parameter.
-    TDK_Incomplete,
-    /// Template argument deduction did not deduce a value for every
-    /// expansion of an expanded template parameter pack.
-    TDK_IncompletePack,
-    /// Template argument deduction produced inconsistent
-    /// deduced values for the given template parameter.
-    TDK_Inconsistent,
-    /// Template argument deduction failed due to inconsistent
-    /// cv-qualifiers on a template parameter type that would
-    /// otherwise be deduced, e.g., we tried to deduce T in "const T"
-    /// but were given a non-const "X".
-    TDK_Underqualified,
-    /// Substitution of the deduced template argument values
-    /// resulted in an error.
-    TDK_SubstitutionFailure,
-    /// After substituting deduced template arguments, a dependent
-    /// parameter type did not match the corresponding argument.
-    TDK_DeducedMismatch,
-    /// After substituting deduced template arguments, an element of
-    /// a dependent parameter type did not match the corresponding element
-    /// of the corresponding argument (when deducing from an initializer list).
-    TDK_DeducedMismatchNested,
-    /// A non-depnedent component of the parameter did not match the
-    /// corresponding component of the argument.
-    TDK_NonDeducedMismatch,
-    /// When performing template argument deduction for a function
-    /// template, there were too many call arguments.
-    TDK_TooManyArguments,
-    /// When performing template argument deduction for a function
-    /// template, there were too few call arguments.
-    TDK_TooFewArguments,
-    /// The explicitly-specified template arguments were not valid
-    /// template arguments for the given template.
-    TDK_InvalidExplicitArguments,
-    /// Checking non-dependent argument conversions failed.
-    TDK_NonDependentConversionFailure,
-    /// The deduced arguments did not satisfy the constraints associated
-    /// with the template.
-    TDK_ConstraintsNotSatisfied,
-    /// Deduction failed; that's all we know.
-    TDK_MiscellaneousDeductionFailure,
-    /// CUDA Target attributes do not match.
-    TDK_CUDATargetMismatch,
-    /// Some error which was already diagnosed.
-    TDK_AlreadyDiagnosed
-  };
-
   TemplateDeductionResult
   DeduceTemplateArguments(ClassTemplatePartialSpecializationDecl *Partial,
                           ArrayRef<TemplateArgument> TemplateArgs,
@@ -14445,7 +14445,7 @@ class Sema final {
 };
 
 DeductionFailureInfo
-MakeDeductionFailureInfo(ASTContext &Context, Sema::TemplateDeductionResult TDK,
+MakeDeductionFailureInfo(ASTContext &Context, TemplateDeductionResult TDK,
                          sema::TemplateDeductionInfo &Info);
 
 /// Contains a late templated function.
diff --git a/clang/include/clang/Sema/TemplateDeduction.h b/clang/include/clang/Sema/TemplateDeduction.h
index 85691c66a04433..4ca03056e679e8 100644
--- a/clang/include/clang/Sema/TemplateDeduction.h
+++ b/clang/include/clang/Sema/TemplateDeduction.h
@@ -33,6 +33,7 @@ namespace clang {
 class Decl;
 struct DeducedPack;
 class Sema;
+enum class TemplateDeductionResult;
 
 namespace sema {
 
@@ -256,10 +257,11 @@ class TemplateDeductionInfo {
 /// A structure used to record information about a failed
 /// template argument deduction, for diagnosis.
 struct DeductionFailureInfo {
-  /// A Sema::TemplateDeductionResult.
+  LLVM_PREFERRED_TYPE(TemplateDeductionResult)
   unsigned Result : 8;
 
   /// Indicates whether a diagnostic is stored in Diagnostic.
+  LLVM_PREFERRED_TYPE(bool)
   unsigned HasDiagnostic : 1;
 
   /// Opaque pointer containing additional data about
@@ -295,6 +297,10 @@ struct DeductionFailureInfo {
 
   /// Free any memory associated with this deduction failure.
   void Destroy();
+
+  TemplateDeductionResult getResult() const {
+    return static_cast<TemplateDeductionResult>(Result);
+  }
 };
 
 /// TemplateSpecCandidate - This is a generalization of OverloadCandidate
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 2c526cd0d0e675..5ddaac3ec6c92d 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -13049,7 +13049,8 @@ QualType Sema::deduceVarTypeFromInitializer(VarDecl *VDecl,
   TemplateDeductionInfo Info(DeduceInit->getExprLoc());
   TemplateDeductionResult Result =
       DeduceAutoType(TSI->getTypeLoc(), DeduceInit, DeducedType, Info);
-  if (Result != TDK_Success && Result != TDK_AlreadyDiagnosed) {
+  if (Result != TemplateDeductionResult::Success &&
+      Result != TemplateDeductionResult::AlreadyDiagnosed) {
     if (!IsInitCapture)
       DiagnoseAutoDeductionFailure(VDecl, DeduceInit);
     else if (isa<InitListExpr>(Init))
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 246d2313e089f3..f2b89135af21cf 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1554,12 +1554,13 @@ Sema::BuildCXXTypeConstructExpr(TypeSourceInfo *TInfo,
     TemplateDeductionInfo Info(Deduce->getExprLoc());
     TemplateDeductionResult Result =
         DeduceAutoType(TInfo->getTypeLoc(), Deduce, DeducedType, Info);
-    if (Result != TDK_Success && Result != TDK_AlreadyDiagnosed)
+    if (Result != TemplateDeductionResult::Success &&
+        Result != TemplateDeductionResult::AlreadyDiagnosed)
       return ExprError(Diag(TyBeginLoc, diag::err_auto_expr_deduction_failure)
                        << Ty << Deduce->getType() << FullRange
                        << Deduce->getSourceRange());
     if (DeducedType.isNull()) {
-      assert(Result == TDK_AlreadyDiagnosed);
+      assert(Result == TemplateDeductionResult::AlreadyDiagnosed);
       return ExprError();
     }
 
@@ -2098,12 +2099,13 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
     TemplateDeductionInfo Info(Deduce->getExprLoc());
     TemplateDeductionResult Result =
         DeduceAutoType(AllocTypeInfo->getTypeLoc(), Deduce, DeducedType, Info);
-    if (Result != TDK_Success && Result != TDK_AlreadyDiagnosed)
+    if (Result != TemplateDeductionResult::Success &&
+        Result != TemplateDeductionResult::AlreadyDiagnosed)
       return ExprError(Diag(StartLoc, diag::err_auto_new_deduction_failure)
                        << AllocType << Deduce->getType() << TypeRange
                        << Deduce->getSourceRange());
     if (DeducedType.isNull()) {
-      assert(Result == TDK_AlreadyDiagnosed);
+      assert(Result == TemplateDeductionResult::AlreadyDiagnosed);
       return ExprError();
     }
     AllocType = DeducedType;
@@ -2883,7 +2885,7 @@ bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range,
         // expected function type.
         TemplateDeductionInfo Info(StartLoc);
         if (DeduceTemplateArguments(FnTmpl, nullptr, ExpectedFunctionType, Fn,
-                                    Info))
+                                    Info) != TemplateDeductionResult::Success)
           continue;
       } else
         Fn = cast<FunctionDecl>((*D)->getUnderlyingDecl());
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 02b1a045df44c2..d3a9c7abd0e944 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -1200,8 +1200,8 @@ static bool LookupDirect(Sema &S, LookupResult &R, const DeclContext *DC) {
     // Perform template argument deduction against the type that we would
     // expect the function to have.
     if (R.getSema().DeduceTemplateArguments(ConvTemplate, nullptr, ExpectedType,
-                                            Specialization, Info)
-          == Sema::TDK_Success) {
+                                            Specialization, Info) ==
+        TemplateDeductionResult::Success) {
       R.addDecl(Specialization);
       Found = true;
     }
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 42960c229077c3..7c15926c9afbd7 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -628,28 +628,28 @@ namespace {
 /// to the form used in overload-candidate information.
 DeductionFailureInfo
 clang::MakeDeductionFailureInfo(ASTContext &Context,
-                                Sema::TemplateDeductionResult TDK,
+                                TemplateDeductionResult TDK,
                                 TemplateDeductionInfo &Info) {
   DeductionFailureInfo Result;
   Result.Result = static_cast<unsigned>(TDK);
   Result.HasDiagnostic = false;
   switch (TDK) {
-  case Sema::TDK_Invalid:
-  case Sema::TDK_InstantiationDepth:
-  case Sema::TDK_TooManyArguments:
-  case Sema::TDK_TooFewArguments:
-  case Sema::TDK_MiscellaneousDeductionFailure:
-  case Sema::TDK_CUDATargetMismatch:
+  case TemplateDeductionResult::Invalid:
+  case TemplateDeductionResult::InstantiationDepth:
+  case TemplateDeductionResult::TooManyArguments:
+  case TemplateDeductionResult::TooFewArguments:
+  case TemplateDeductionResult::MiscellaneousDeductionFailure:
+  case TemplateDeductionResult::CUDATargetMismatch:
     Result.Data = nullptr;
     break;
 
-  case Sema::TDK_Incomplete:
-  case Sema::TDK_InvalidExplicitArguments:
+  case TemplateDeductionResult::Incomplete:
+  case TemplateDeductionResult::InvalidExplicitArguments:
     Result.Data = Info.Param.getOpaqueValue();
     break;
 
-  case Sema::TDK_DeducedMismatch:
-  case Sema::TDK_DeducedMismatchNested: {
+  case TemplateDeductionResult::DeducedMismatch:
+  case TemplateDeductionResult::DeducedMismatchNested: {
     // FIXME: Should allocate from normal heap so that we can free this later.
     auto *Saved = new (Context) DFIDeducedMismatchArgs;
     Saved->FirstArg = Info.FirstArg;
@@ -660,7 +660,7 @@ clang::MakeDeductionFailureInfo(ASTContext &Context,
     break;
   }
 
-  case Sema::TDK_NonDeducedMismatch: {
+  case TemplateDeductionResult::NonDeducedMismatch: {
     // FIXME: Should allocate from normal heap so that we can free this later.
     DFIArguments *Saved = new (Context) DFIArguments;
     Saved->FirstArg = Info.FirstArg;
@@ -669,10 +669,10 @@ clang::MakeDeductionFailureInfo(ASTContext &Context,
     break;
   }
 
-  case Sema::TDK_IncompletePack:
+  case TemplateDeductionResult::IncompletePack:
     // FIXME: It's slightly wasteful to allocate two TemplateArguments for this.
-  case Sema::TDK_Inconsistent:
-  case Sema::TDK_Underqualified: {
+  case TemplateDeductionResult::Inconsistent:
+  case TemplateDeductionResult::Underqualified: {
     // FIXME: Should allocate from normal heap so that we can free this later.
     DFIParamWithArguments *Saved = new (Context) DFIParamWithArguments;
     Saved->Param = Info.Param;
@@ -682,7 +682,7 @@ clang::MakeDeductionFailureInfo(ASTContext &Context,
     break;
   }
 
-  case Sema::TDK_SubstitutionFailure:
+  case TemplateDeductionResult::SubstitutionFailure:
     Result.Data = Info.takeSugared();
     if (Info.hasSFINAEDiagnostic()) {
       PartialDiagnosticAt *Diag = new (Result.Diagnostic) PartialDiagnosticAt(
@@ -692,7 +692,7 @@ clang::MakeDeductionFailureInfo(ASTContext &Context,
     }
     break;
 
-  case Sema::TDK_ConstraintsNotSatisfied: {
+  case TemplateDeductionResult::ConstraintsNotSatisfied: {
     CNSInfo *Saved = new (Context) CNSInfo;
     Saved->TemplateArgs = Info.takeSugared();
     Saved->Satisfaction = Info.AssociatedConstraintsSatisfaction;
@@ -700,9 +700,9 @@ clang::MakeDeductionFailureInfo(ASTContext &Context,
     break;
   }
 
-  case Sema::TDK_Success:
-  case Sema::TDK_NonDependentConversionFailure:
-  case Sema::TDK_AlreadyDiagnosed:
+  case TemplateDeductionResult::Success:
+  case TemplateDeductionResult::NonDependentConversionFailure:
+  case TemplateDeductionResult::AlreadyDiagnosed:
     llvm_unreachable("not a deduction failure");
   }
 
@@ -710,29 +710,29 @@ clang::MakeDeductionFailureInfo(ASTContext &Context,
 }
 
 void DeductionFailureInfo::Destroy() {
-  switch (static_cast<Sema::TemplateDeductionResult>(Result)) {
-  case Sema::TDK_Success:
-  case Sema::TDK_Invalid:
-  case Sema::TDK_InstantiationDepth:
-  case Sema::TDK_Incomplete:
-  case Sema::TDK_TooManyArguments:
-  case Sema::TDK_TooFewArguments:
-  case Sema::TDK_InvalidExplicitArguments:
-  case Sema::TDK_CUDATargetMismatch:
-  case Sema::TDK_NonDependentConversionFailure:
+  switch (static_cast<TemplateDeductionResult>(Result)) {
+  case TemplateDeductionResult::Success:
+  case TemplateDeductionResult::Invalid:
+  case TemplateDeductionResult::InstantiationDepth:
+  case TemplateDeductionResult::Incomplete:
+  case TemplateDeductionResult::TooManyArguments:
+  case TemplateDeductionResult::TooFewArguments:
+  case TemplateDeductionResult::InvalidExplicitArguments:
+  case TemplateDeductionResult::CUDATargetMismatch:
+  case TemplateDeductionResult::NonDependentConversionFailure:
     break;
 
-  case Sema::TDK_IncompletePack:
-  case Sema::TDK_Inconsistent:
-  case Sema::TDK_Underqualified:
-  case Sema::TDK_DeducedMismatch:
-  case Sema::TDK_DeducedMismatchNested:
-  case Sema::TDK_NonDeducedMismatch:
+  case TemplateDeductionResult::IncompletePack:
+  case TemplateDeductionResult::Inconsistent:
+  case TemplateDeductionResult::Underqualified:
+  case TemplateDeductionResult::DeducedMismatch:
+  case TemplateDeductionResult::DeducedMismatchNested:
+  case TemplateDeductionResult::NonDeducedMismatch:
     // FIXME: Destroy the data?
     Data = nullptr;
     break;
 
-  case Sema::TDK_SubstitutionFailure:
+  case TemplateDeductionResult::SubstitutionFailure:
     // FIXME: Destroy the template argument list?
     Data = nullptr;
     if (PartialDiagnosticAt *Diag = getSFINAEDiagnostic()) {
@@ -741,7 +741,7 @@ void DeductionFailureInfo::Destroy() {
     }
     break;
 
-  case Sema::TDK_ConstraintsNotSatisfied:
+  case TemplateDeductionResult::ConstraintsNotSatisfied:
     // FIXME: Destroy the template argument list?
     Data = nullptr;
     if (PartialDiagnosticAt *Diag = getSFINAEDiagnostic()) {
@@ -751,8 +751,8 @@ void DeductionFailureInfo::Destroy() {
     break;
 
   // Unhandled
-  case Sema::TDK_MiscellaneousDeductionFailure:
-  case Sema::TDK_AlreadyDiagnosed:
+  case TemplateDeductionResult::MiscellaneousDeductionFailure:
+  case TemplateDeductionResult::AlreadyDiagnosed:
     break;
   }
 }
@@ -764,33 +764,33 @@ PartialDiagnosticAt *DeductionFailureInfo::getSFINAEDiagnostic() {
 }
 
 TemplateParameter DeductionFailureInfo::getTemplateParameter() {
-  switch (static_cast<Sema::TemplateDeductionResult>(Result)) {
-  case Sema::TDK_Success:
-  case Sema::TDK_Invalid:
-  case Sema::TDK_InstantiationDepth:
-  case Sema::TDK_TooManyArguments:
-  case Sema::TDK_TooFewArguments:
-  case Sema::TDK_SubstitutionFailure:
-  case Sema::TDK_DeducedMismatch:
-  case Sema::TDK_DeducedMismatchNested:
-  case Sema::TDK_NonDeducedMismatch:
-  case Sema::TDK_CUDATargetMismatch:
-  case Sema::TDK_NonDependentConversionFailure:
-  case Sema::TDK_ConstraintsNotSatisfied:
+  switch (static_cast<TemplateDeductionResult>(Result)) {
+  case TemplateDeductionResult::Success:
+  case TemplateDeductionResult::Invalid:
+  case TemplateDeductionResult::InstantiationDepth:
+  case TemplateDeductionResult::TooManyArguments:
+  case TemplateDeductionResult::TooFewArguments:
+  case TemplateDeductionResult::SubstitutionFailure:
+  case TemplateDeductionResult::DeducedMismatch:
+  case TemplateDeductionResult::DeducedMismatchNested:
+  case TemplateDeductionResult::NonDeducedMismatch:
+  case TemplateDeductionResult::CUDATargetMismatch:
+  case TemplateDeductionResult::NonDependentConversionFailure:
+  case TemplateDeductionResult::ConstraintsNotSatisfied:
     return TemplateParameter();
 
-  case Sema::TDK_Incomplete:
-  case Sema::TDK_InvalidExplicitArguments:
+  case TemplateDeductionResult::Incomplete:
+  case TemplateDeductionResult::InvalidExplicitArguments:
     return TemplateParameter::getFromOpaqueValue(Data);
 
-  case Sema::TDK_IncompletePack:
-  case Sema::TDK_Inconsistent:
-  case Sema::TDK_Underqualified:
+  ...
[truncated]

Those annotations are out of scope of the PR
Copy link
Contributor Author

@Endilll Endilll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this patch is rather large, I left two comments highlighting potentially interesting parts of it.

case TemplateDeductionResult::Invalid:
case TemplateDeductionResult::NonDependentConversionFailure:
case TemplateDeductionResult::AlreadyDiagnosed:
return;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erichkeane This is worth checking

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, shoot... Invalid and AlreadyDiagnosed make sense to me to be here. I have no idea on the NonDependentConversionFailure, that one MIGHT be worth diagnosing, but I have no idea what such a diagnostic could look like.

Could you replace the 'return;' for that one with an assert and run the lit tests to see if it is something that might be valuable doing something with?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you replace the 'return;' for that one with an assert and run the lit tests to see if it is something that might be valuable doing something with?

No crashes in Clang test suite, so I pushed the change.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. It'll be interesting to see if we ever end up here, and the assert message is good enough we can figure out what to do pretty easily based on bug reports. Thanks!

@@ -295,6 +297,10 @@ struct DeductionFailureInfo {

/// Free any memory associated with this deduction failure.
void Destroy();

TemplateDeductionResult getResult() const {
return static_cast<TemplateDeductionResult>(Result);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erichkeane This is also worth checking. Code itself is boring, but the approach might be not.

@@ -33,6 +33,7 @@ namespace clang {
class Decl;
struct DeducedPack;
class Sema;
enum class TemplateDeductionResult;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we specify the underlying type in all of these? I wonder if we can just specify it as uint8_t? Would mean not having to store it as a bitfield if we didn't want.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is far from the first time we do unscoped enum → scoped enum refactoring so that we can forward declare enums. Me and @AaronBallman decided to stick to default int for scoped enums that are stored in bit-fieds. (This one is stored in bit-fields of width 8).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok then, I'm ok sticking to our existing practice if this is a policy that was already made.

case TemplateDeductionResult::Invalid:
case TemplateDeductionResult::NonDependentConversionFailure:
case TemplateDeductionResult::AlreadyDiagnosed:
return;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, shoot... Invalid and AlreadyDiagnosed make sense to me to be here. I have no idea on the NonDependentConversionFailure, that one MIGHT be worth diagnosing, but I have no idea what such a diagnostic could look like.

Could you replace the 'return;' for that one with an assert and run the lit tests to see if it is something that might be valuable doing something with?

Copy link

github-actions bot commented Feb 12, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@Endilll Endilll merged commit dfbe2bc into llvm:main Feb 12, 2024
4 checks passed
@Endilll Endilll deleted the refactor-template-deduction-result branch February 12, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants