[clang] NFC: cleanup check template argument #124668
Merged
+307
−301
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesPatch is 77.45 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/124668.diff 5 Files Affected:
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1ea7c62cb36f05..ef097f24fb3f52 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11650,6 +11650,33 @@ class Sema final : public SemaBase {
CTAK_DeducedFromArrayBound
};
+ struct CheckTemplateArgumentInfo {
+ explicit CheckTemplateArgumentInfo(bool PartialOrdering = false,
+ bool MatchingTTP = false)
+ : PartialOrdering(PartialOrdering), MatchingTTP(MatchingTTP) {}
+ CheckTemplateArgumentInfo(const CheckTemplateArgumentInfo &) = delete;
+ CheckTemplateArgumentInfo &
+ operator=(const CheckTemplateArgumentInfo &) = delete;
+
+ /// The checked, converted argument will be added to the
+ /// end of these vectors.
+ SmallVector<TemplateArgument, 4> SugaredConverted, CanonicalConverted;
+
+ /// The check is being performed in the context of partial ordering.
+ bool PartialOrdering;
+
+ /// If true, assume these template arguments are
+ /// the injected template arguments for a template template parameter.
+ /// This will relax the requirement that all its possible uses are valid:
+ /// TTP checking is loose, and assumes that invalid uses will be diagnosed
+ /// during instantiation.
+ bool MatchingTTP;
+
+ /// Is set to true when, in the context of TTP matching, a pack parameter
+ /// matches non-pack arguments.
+ bool MatchedPackOnParmToNonPackOnArg;
+ };
+
/// Check that the given template argument corresponds to the given
/// template parameter.
///
@@ -11669,22 +11696,16 @@ class Sema final : public SemaBase {
/// \param ArgumentPackIndex The index into the argument pack where this
/// argument will be placed. Only valid if the parameter is a parameter pack.
///
- /// \param Converted The checked, converted argument will be added to the
- /// end of this small vector.
- ///
/// \param CTAK Describes how we arrived at this particular template argument:
/// explicitly written, deduced, etc.
///
/// \returns true on error, false otherwise.
- bool
- CheckTemplateArgument(NamedDecl *Param, TemplateArgumentLoc &Arg,
- NamedDecl *Template, SourceLocation TemplateLoc,
- SourceLocation RAngleLoc, unsigned ArgumentPackIndex,
- SmallVectorImpl<TemplateArgument> &SugaredConverted,
- SmallVectorImpl<TemplateArgument> &CanonicalConverted,
- CheckTemplateArgumentKind CTAK, bool PartialOrdering,
- bool PartialOrderingTTP,
- bool *MatchedPackOnParmToNonPackOnArg);
+ bool CheckTemplateArgument(NamedDecl *Param, TemplateArgumentLoc &Arg,
+ NamedDecl *Template, SourceLocation TemplateLoc,
+ SourceLocation RAngleLoc,
+ unsigned ArgumentPackIndex,
+ CheckTemplateArgumentInfo &CTAI,
+ CheckTemplateArgumentKind CTAK);
/// Check that the given template arguments can be provided to
/// the given template, converting the arguments along the way.
@@ -11717,22 +11738,15 @@ class Sema final : public SemaBase {
/// \param DefaultArgs any default arguments from template specialization
/// deduction.
///
- /// \param PartialOrderingTTP If true, assume these template arguments are
- /// the injected template arguments for a template template parameter.
- /// This will relax the requirement that all its possible uses are valid:
- /// TTP checking is loose, and assumes that invalid uses will be diagnosed
- /// during instantiation.
- ///
/// \returns true if an error occurred, false otherwise.
- bool CheckTemplateArgumentList(
- TemplateDecl *Template, SourceLocation TemplateLoc,
- TemplateArgumentListInfo &TemplateArgs,
- const DefaultArguments &DefaultArgs, bool PartialTemplateArgs,
- SmallVectorImpl<TemplateArgument> &SugaredConverted,
- SmallVectorImpl<TemplateArgument> &CanonicalConverted,
- bool UpdateArgsWithConversions = true,
- bool *ConstraintsNotSatisfied = nullptr, bool PartialOrderingTTP = false,
- bool *MatchedPackOnParmToNonPackOnArg = nullptr);
+ bool CheckTemplateArgumentList(TemplateDecl *Template,
+ SourceLocation TemplateLoc,
+ TemplateArgumentListInfo &TemplateArgs,
+ const DefaultArguments &DefaultArgs,
+ bool PartialTemplateArgs,
+ CheckTemplateArgumentInfo &CTAI,
+ bool UpdateArgsWithConversions = true,
+ bool *ConstraintsNotSatisfied = nullptr);
bool CheckTemplateTypeArgument(
TemplateTypeParmDecl *Param, TemplateArgumentLoc &Arg,
@@ -11757,7 +11771,7 @@ class Sema final : public SemaBase {
QualType InstantiatedParamType, Expr *Arg,
TemplateArgument &SugaredConverted,
TemplateArgument &CanonicalConverted,
- bool PartialOrderingTTP,
+ bool MatchingTTP,
CheckTemplateArgumentKind CTAK);
/// Check a template argument against its corresponding
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 26f53532a1491f..64c6ebba66e144 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -3671,13 +3671,11 @@ Sema::LookupLiteralOperator(Scope *S, LookupResult &R,
// is a well-formed template argument for the template parameter.
if (StringLit) {
SFINAETrap Trap(*this);
- SmallVector<TemplateArgument, 1> SugaredChecked, CanonicalChecked;
+ CheckTemplateArgumentInfo CTAI;
TemplateArgumentLoc Arg(TemplateArgument(StringLit), StringLit);
if (CheckTemplateArgument(
Params->getParam(0), Arg, FD, R.getNameLoc(), R.getNameLoc(),
- 0, SugaredChecked, CanonicalChecked, CTAK_Specified,
- /*PartialOrdering=*/false, /*PartialOrderingTTP=*/false,
- /*MatchedPackOnParmToNonPackOnArg=*/nullptr) ||
+ /*ArgumentPackIndex=*/0, CTAI, CTAK_Specified) ||
Trap.hasErrorOccurred())
IsTemplate = false;
}
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index cb9d78734e6bbf..3944c4f67bab9a 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -38,6 +38,7 @@
#include "clang/Sema/TemplateDeduction.h"
#include "llvm/ADT/SmallBitVector.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/SaveAndRestore.h"
#include <optional>
using namespace clang;
@@ -3497,10 +3498,10 @@ QualType Sema::CheckTemplateIdType(TemplateName Name,
// Check that the template argument list is well-formed for this
// template.
- SmallVector<TemplateArgument, 4> SugaredConverted, CanonicalConverted;
+ CheckTemplateArgumentInfo CTAI;
if (CheckTemplateArgumentList(Template, TemplateLoc, TemplateArgs,
- DefaultArgs, false, SugaredConverted,
- CanonicalConverted,
+ DefaultArgs, /*PartialTemplateArgs=*/false,
+ CTAI,
/*UpdateArgsWithConversions=*/true))
return QualType();
@@ -3522,7 +3523,7 @@ QualType Sema::CheckTemplateIdType(TemplateName Name,
// template type alias specializations apart.
MultiLevelTemplateArgumentList TemplateArgLists;
TemplateArgLists.addOuterTemplateArguments(
- Template, SugaredConverted,
+ Template, CTAI.SugaredConverted,
/*Final=*/!getLangOpts().RetainSubstTemplateTypeParmTypeAstNodes);
TemplateArgLists.addOuterRetainedLevels(
AliasTemplate->getTemplateParameters()->getDepth());
@@ -3582,11 +3583,11 @@ QualType Sema::CheckTemplateIdType(TemplateName Name,
return QualType();
}
} else if (auto *BTD = dyn_cast<BuiltinTemplateDecl>(Template)) {
- CanonType = checkBuiltinTemplateIdType(*this, BTD, SugaredConverted,
+ CanonType = checkBuiltinTemplateIdType(*this, BTD, CTAI.SugaredConverted,
TemplateLoc, TemplateArgs);
} else if (Name.isDependent() ||
TemplateSpecializationType::anyDependentTemplateArguments(
- TemplateArgs, CanonicalConverted)) {
+ TemplateArgs, CTAI.CanonicalConverted)) {
// This class template specialization is a dependent
// type. Therefore, its canonical type is another class template
// specialization type that contains all of the converted
@@ -3595,7 +3596,7 @@ QualType Sema::CheckTemplateIdType(TemplateName Name,
//
// template<typename T, typename U = T> struct A;
CanonType = Context.getCanonicalTemplateSpecializationType(
- Name, CanonicalConverted);
+ Name, CTAI.CanonicalConverted);
// This might work out to be a current instantiation, in which
// case the canonical type needs to be the InjectedClassNameType.
@@ -3640,7 +3641,7 @@ QualType Sema::CheckTemplateIdType(TemplateName Name,
// corresponds to these arguments.
void *InsertPos = nullptr;
ClassTemplateSpecializationDecl *Decl =
- ClassTemplate->findSpecialization(CanonicalConverted, InsertPos);
+ ClassTemplate->findSpecialization(CTAI.CanonicalConverted, InsertPos);
if (!Decl) {
// This is the first time we have referenced this class template
// specialization. Create the canonical declaration and add it to
@@ -3649,7 +3650,7 @@ QualType Sema::CheckTemplateIdType(TemplateName Name,
Context, ClassTemplate->getTemplatedDecl()->getTagKind(),
ClassTemplate->getDeclContext(),
ClassTemplate->getTemplatedDecl()->getBeginLoc(),
- ClassTemplate->getLocation(), ClassTemplate, CanonicalConverted,
+ ClassTemplate->getLocation(), ClassTemplate, CTAI.CanonicalConverted,
nullptr);
ClassTemplate->AddSpecialization(Decl, InsertPos);
if (ClassTemplate->isOutOfLine())
@@ -3661,7 +3662,7 @@ QualType Sema::CheckTemplateIdType(TemplateName Name,
InstantiatingTemplate Inst(*this, TemplateLoc, Decl);
if (!Inst.isInvalid()) {
MultiLevelTemplateArgumentList TemplateArgLists(Template,
- CanonicalConverted,
+ CTAI.CanonicalConverted,
/*Final=*/false);
InstantiateAttrsForDecl(TemplateArgLists,
ClassTemplate->getTemplatedDecl(), Decl);
@@ -4183,10 +4184,10 @@ DeclResult Sema::ActOnVarTemplateSpecialization(
// Check that the template argument list is well-formed for this
// template.
- SmallVector<TemplateArgument, 4> SugaredConverted, CanonicalConverted;
+ CheckTemplateArgumentInfo CTAI;
if (CheckTemplateArgumentList(VarTemplate, TemplateNameLoc, TemplateArgs,
- /*DefaultArgs=*/{}, false, SugaredConverted,
- CanonicalConverted,
+ /*DefaultArgs=*/{},
+ /*PartialTemplateArgs=*/false, CTAI,
/*UpdateArgsWithConversions=*/true))
return true;
@@ -4195,21 +4196,21 @@ DeclResult Sema::ActOnVarTemplateSpecialization(
if (IsPartialSpecialization) {
if (CheckTemplatePartialSpecializationArgs(TemplateNameLoc, VarTemplate,
TemplateArgs.size(),
- CanonicalConverted))
+ CTAI.CanonicalConverted))
return true;
- // FIXME: Move these checks to CheckTemplatePartialSpecializationArgs so we
- // also do them during instantiation.
+ // FIXME: Move these checks to CheckTemplatePartialSpecializationArgs so
+ // we also do them during instantiation.
if (!Name.isDependent() &&
!TemplateSpecializationType::anyDependentTemplateArguments(
- TemplateArgs, CanonicalConverted)) {
+ TemplateArgs, CTAI.CanonicalConverted)) {
Diag(TemplateNameLoc, diag::err_partial_spec_fully_specialized)
<< VarTemplate->getDeclName();
IsPartialSpecialization = false;
}
if (isSameAsPrimaryTemplate(VarTemplate->getTemplateParameters(),
- CanonicalConverted) &&
+ CTAI.CanonicalConverted) &&
(!Context.getLangOpts().CPlusPlus20 ||
!TemplateParams->hasAssociatedConstraints())) {
// C++ [temp.class.spec]p9b3:
@@ -4217,11 +4218,11 @@ DeclResult Sema::ActOnVarTemplateSpecialization(
// -- The argument list of the specialization shall not be identical
// to the implicit argument list of the primary template.
Diag(TemplateNameLoc, diag::err_partial_spec_args_match_primary_template)
- << /*variable template*/ 1
- << /*is definition*/(SC != SC_Extern && !CurContext->isRecord())
- << FixItHint::CreateRemoval(SourceRange(LAngleLoc, RAngleLoc));
- // FIXME: Recover from this by treating the declaration as a redeclaration
- // of the primary template.
+ << /*variable template*/ 1
+ << /*is definition*/ (SC != SC_Extern && !CurContext->isRecord())
+ << FixItHint::CreateRemoval(SourceRange(LAngleLoc, RAngleLoc));
+ // FIXME: Recover from this by treating the declaration as a
+ // redeclaration of the primary template.
return true;
}
}
@@ -4231,9 +4232,10 @@ DeclResult Sema::ActOnVarTemplateSpecialization(
if (IsPartialSpecialization)
PrevDecl = VarTemplate->findPartialSpecialization(
- CanonicalConverted, TemplateParams, InsertPos);
+ CTAI.CanonicalConverted, TemplateParams, InsertPos);
else
- PrevDecl = VarTemplate->findSpecialization(CanonicalConverted, InsertPos);
+ PrevDecl =
+ VarTemplate->findSpecialization(CTAI.CanonicalConverted, InsertPos);
VarTemplateSpecializationDecl *Specialization = nullptr;
@@ -4260,7 +4262,7 @@ DeclResult Sema::ActOnVarTemplateSpecialization(
VarTemplatePartialSpecializationDecl::Create(
Context, VarTemplate->getDeclContext(), TemplateKWLoc,
TemplateNameLoc, TemplateParams, VarTemplate, DI->getType(), DI, SC,
- CanonicalConverted);
+ CTAI.CanonicalConverted);
Partial->setTemplateArgsAsWritten(TemplateArgs);
if (!PrevPartial)
@@ -4278,7 +4280,7 @@ DeclResult Sema::ActOnVarTemplateSpecialization(
// this explicit specialization or friend declaration.
Specialization = VarTemplateSpecializationDecl::Create(
Context, VarTemplate->getDeclContext(), TemplateKWLoc, TemplateNameLoc,
- VarTemplate, DI->getType(), DI, SC, CanonicalConverted);
+ VarTemplate, DI->getType(), DI, SC, CTAI.CanonicalConverted);
Specialization->setTemplateArgsAsWritten(TemplateArgs);
if (!PrevDecl)
@@ -4350,25 +4352,25 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc,
assert(Template && "A variable template id without template?");
// Check that the template argument list is well-formed for this template.
- SmallVector<TemplateArgument, 4> SugaredConverted, CanonicalConverted;
+ CheckTemplateArgumentInfo CTAI;
if (CheckTemplateArgumentList(
Template, TemplateNameLoc,
const_cast<TemplateArgumentListInfo &>(TemplateArgs),
- /*DefaultArgs=*/{}, false, SugaredConverted, CanonicalConverted,
+ /*DefaultArgs=*/{}, /*PartialTemplateArgs=*/false, CTAI,
/*UpdateArgsWithConversions=*/true))
return true;
// Produce a placeholder value if the specialization is dependent.
if (Template->getDeclContext()->isDependentContext() ||
TemplateSpecializationType::anyDependentTemplateArguments(
- TemplateArgs, CanonicalConverted))
+ TemplateArgs, CTAI.CanonicalConverted))
return DeclResult();
// Find the variable template specialization declaration that
// corresponds to these arguments.
void *InsertPos = nullptr;
if (VarTemplateSpecializationDecl *Spec =
- Template->findSpecialization(CanonicalConverted, InsertPos)) {
+ Template->findSpecialization(CTAI.CanonicalConverted, InsertPos)) {
checkSpecializationReachability(TemplateNameLoc, Spec);
// If we already have a variable template specialization, return it.
return Spec;
@@ -4412,7 +4414,7 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc,
TemplateDeductionInfo Info(FailedCandidates.getLocation());
if (TemplateDeductionResult Result =
- DeduceTemplateArguments(Partial, SugaredConverted, Info);
+ DeduceTemplateArguments(Partial, CTAI.SugaredConverted, Info);
Result != TemplateDeductionResult::Success) {
// Store the failed-deduction information for use in diagnostics, later.
// TODO: Actually use the failed-deduction info?
@@ -4479,7 +4481,7 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc,
// FIXME: LateAttrs et al.?
VarTemplateSpecializationDecl *Decl = BuildVarTemplateInstantiation(
Template, InstantiationPattern, PartialSpecArgs, TemplateArgs,
- CanonicalConverted, TemplateNameLoc /*, LateAttrs, StartingScope*/);
+ CTAI.CanonicalConverted, TemplateNameLoc /*, LateAttrs, StartingScope*/);
if (!Decl)
return true;
@@ -4559,12 +4561,12 @@ Sema::CheckConceptTemplateId(const CXXScopeSpec &SS,
if (NamedConcept->isInvalidDecl())
return ExprError();
- llvm::SmallVector<TemplateArgument, 4> SugaredConverted, CanonicalConverted;
+ CheckTemplateArgumentInfo CTAI;
if (CheckTemplateArgumentList(
NamedConcept, ConceptNameInfo.getLoc(),
const_cast<TemplateArgumentListInfo &>(*TemplateArgs),
/*DefaultArgs=*/{},
- /*PartialTemplateArgs=*/false, SugaredConverted, CanonicalConverted,
+ /*PartialTemplateArgs=*/false, CTAI,
/*UpdateArgsWithConversions=*/false))
return ExprError();
@@ -4572,12 +4574,12 @@ Sema::CheckConceptTemplateId(const CXXScopeSpec &SS,
auto *CSD = ImplicitConceptSpecializationDecl::Create(
Context, NamedConcept->getDeclContext(), NamedConcept->getLocation(),
- CanonicalConverted);
+ CTAI.CanonicalConverted);
ConstraintSatisfaction Satisfaction;
bool AreArgsDependent =
TemplateSpecializationType::anyDependentTemplateArguments(
- *TemplateArgs, CanonicalConverted);
- MultiLevelTemplateArgumentList MLTAL(NamedConcept, CanonicalConverted,
+ *TemplateArgs, CTAI.CanonicalConverted);
+ MultiLevelTemplateArgumentList MLTAL(NamedConcept, CTAI.CanonicalConverted,
/*Final=*/false);
LocalInstantiationScope Scope(*this);
@@ -5198,18 +5200,17 @@ convertTypeTemplateArgumentToTemplate(ASTContext &Context, TypeLoc TLoc) {
return TemplateArgumentLoc();
}
-bool Sema::CheckTemplateArgument(
- NamedDecl *Param, TemplateArgumentLoc &ArgLoc, NamedDecl *Template,
- SourceLocation TemplateLoc, SourceLocation RAngleLoc,
- unsigned ArgumentPackIndex,
- SmallVectorImpl<TemplateArgument> &SugaredConverted,
- SmallVectorImpl<TemplateArgument> &CanonicalConverted,
- CheckTemplateArgumentKind CTAK, bool PartialOrdering,
- bool PartialOrderingTTP, bool *MatchedPackOnParmToNonPackOnArg) {
+bool Sema::CheckTemplateArgument(NamedDecl *Param, TemplateArgumentLoc &ArgLoc,
+ NamedDecl *Template,
+ SourceLocation TemplateLoc,
+ SourceLocation RAngleLoc,
+ unsigned ArgumentPackIndex,
+ CheckTemplateArgumentInfo &CTAI,
+ CheckTemplateArgumentKind CTAK) {
...
[truncated]
|
erichkeane
approved these changes
Jan 28, 2025
| @@ -11650,6 +11650,33 @@ class Sema final : public SemaBase { | |||
| CTAK_DeducedFromArrayBound | |||
| }; | |||
|
|
|||
| struct CheckTemplateArgumentInfo { | |||
| explicit CheckTemplateArgumentInfo(bool PartialOrdering = false, | |||
There was a problem hiding this comment.
This constructor doesn't make it completely better? We still end up having 2 very similar bools (actually, looks like 3?) that are pretty easily confused.
I DO like the documentation and pulling the converted together into 1 though. But it would be great if we had a better way to represent the bools that made construction less obtuse.
mizvekov
force-pushed
the
users/mizvekov/clang-fix-nullptr-nondeduced-mismatch
branch
from
e8ded39 to
0e3246e
Compare
mizvekov
force-pushed
the
users/mizvekov/clang-nfc-partial-ordering-cleanup
branch
from
4387c95 to
db745bb
Compare
Base automatically changed from
users/mizvekov/clang-fix-nullptr-nondeduced-mismatch
to
main
January 28, 2025 21:08
Endilll
reviewed
Jan 28, 2025
mizvekov
force-pushed
the
users/mizvekov/clang-nfc-partial-ordering-cleanup
branch
from
db745bb to
c6bfaae
Compare
No changes to test, that was just noise from GitHub auto-rebasing, sorry about that. |
mizvekov
deleted the
users/mizvekov/clang-nfc-partial-ordering-cleanup
branch
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/50/builds/9534 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/20/builds/8225 Here is the relevant piece of the build log for the reference |
shafik
reviewed
Jan 29, 2025
There was a problem hiding this comment.
The summary is really not acceptable. You did a lot of work and there is no description of it at all. At minimum you should have explained you created CheckTemplateArgumentInfo and then you replace that in the following functions. Explain in why this is an improvement and why it has not impact other than elegance.
shafik
reviewed
Jan 29, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.