[Clang][AST] Track whether template template parameters used the 'typename' keyword #88139
Conversation
This file contains hidden or 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-modules @llvm/pr-subscribers-clang Author: Krystian Stasiowski (sdkrystian) ChangesThis patch adds a Full diff: https://github.com/llvm/llvm-project/pull/88139.diff 12 Files Affected:
diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h
index e3b6a7efb1127a..8c679a8db0b7ab 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -1581,26 +1581,33 @@ class TemplateTemplateParmDecl final
DefaultArgStorage<TemplateTemplateParmDecl, TemplateArgumentLoc *>;
DefArgStorage DefaultArgument;
+ /// Whether this template template parameter was declaration with
+ /// the 'typename' keyword.
+ ///
+ /// If false, it was declared with the 'class' keyword.
+ bool Typename : 1;
+
/// Whether this parameter is a parameter pack.
- bool ParameterPack;
+ bool ParameterPack : 1;
/// Whether this template template parameter is an "expanded"
/// parameter pack, meaning that it is a pack expansion and we
/// already know the set of template parameters that expansion expands to.
- bool ExpandedParameterPack = false;
+ bool ExpandedParameterPack : 1;
/// The number of parameters in an expanded parameter pack.
unsigned NumExpandedParams = 0;
- TemplateTemplateParmDecl(DeclContext *DC, SourceLocation L,
- unsigned D, unsigned P, bool ParameterPack,
- IdentifierInfo *Id, TemplateParameterList *Params)
+ TemplateTemplateParmDecl(DeclContext *DC, SourceLocation L, unsigned D,
+ unsigned P, bool ParameterPack, IdentifierInfo *Id,
+ bool Typename, TemplateParameterList *Params)
: TemplateDecl(TemplateTemplateParm, DC, L, Id, Params),
- TemplateParmPosition(D, P), ParameterPack(ParameterPack) {}
+ TemplateParmPosition(D, P), Typename(Typename),
+ ParameterPack(ParameterPack), ExpandedParameterPack(false) {}
- TemplateTemplateParmDecl(DeclContext *DC, SourceLocation L,
- unsigned D, unsigned P,
- IdentifierInfo *Id, TemplateParameterList *Params,
+ TemplateTemplateParmDecl(DeclContext *DC, SourceLocation L, unsigned D,
+ unsigned P, IdentifierInfo *Id, bool Typename,
+ TemplateParameterList *Params,
ArrayRef<TemplateParameterList *> Expansions);
void anchor() override;
@@ -1613,14 +1620,13 @@ class TemplateTemplateParmDecl final
static TemplateTemplateParmDecl *Create(const ASTContext &C, DeclContext *DC,
SourceLocation L, unsigned D,
unsigned P, bool ParameterPack,
- IdentifierInfo *Id,
+ IdentifierInfo *Id, bool Typename,
TemplateParameterList *Params);
- static TemplateTemplateParmDecl *Create(const ASTContext &C, DeclContext *DC,
- SourceLocation L, unsigned D,
- unsigned P,
- IdentifierInfo *Id,
- TemplateParameterList *Params,
- ArrayRef<TemplateParameterList *> Expansions);
+ static TemplateTemplateParmDecl *
+ Create(const ASTContext &C, DeclContext *DC, SourceLocation L, unsigned D,
+ unsigned P, IdentifierInfo *Id, bool Typename,
+ TemplateParameterList *Params,
+ ArrayRef<TemplateParameterList *> Expansions);
static TemplateTemplateParmDecl *CreateDeserialized(ASTContext &C,
unsigned ID);
@@ -1634,6 +1640,14 @@ class TemplateTemplateParmDecl final
using TemplateParmPosition::setPosition;
using TemplateParmPosition::getIndex;
+ /// Whether this template template parameter was declared with
+ /// the 'typename' keyword.
+ bool wasDeclaredWithTypename() const { return Typename; }
+
+ /// Set whether this template template parameter was declared with
+ /// the 'typename' or 'class' keyword.
+ void setDeclaredWithTypename(bool withTypename) { Typename = withTypename; }
+
/// Whether this template template parameter is a template
/// parameter pack.
///
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index f311f9f3743454..e59bd6872c4bdc 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9069,7 +9069,7 @@ class Sema final : public SemaBase {
Expr *DefaultArg);
NamedDecl *ActOnTemplateTemplateParameter(
Scope *S, SourceLocation TmpLoc, TemplateParameterList *Params,
- SourceLocation EllipsisLoc, IdentifierInfo *ParamName,
+ bool Typename, SourceLocation EllipsisLoc, IdentifierInfo *ParamName,
SourceLocation ParamNameLoc, unsigned Depth, unsigned Position,
SourceLocation EqualLoc, ParsedTemplateArgument DefaultArg);
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index f7f55dc4e7a9f4..ca18cd3b31d213 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -799,7 +799,7 @@ ASTContext::getCanonicalTemplateTemplateParmDecl(
TemplateTemplateParmDecl *CanonTTP = TemplateTemplateParmDecl::Create(
*this, getTranslationUnitDecl(), SourceLocation(), TTP->getDepth(),
- TTP->getPosition(), TTP->isParameterPack(), nullptr,
+ TTP->getPosition(), TTP->isParameterPack(), nullptr, /*Typename=*/false,
TemplateParameterList::Create(*this, SourceLocation(), SourceLocation(),
CanonParams, SourceLocation(),
/*RequiresClause=*/nullptr));
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 45d4c9600537be..a300fc6f5746dc 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -5952,7 +5952,8 @@ ASTNodeImporter::VisitTemplateTemplateParmDecl(TemplateTemplateParmDecl *D) {
ToD, D, Importer.getToContext(),
Importer.getToContext().getTranslationUnitDecl(), *LocationOrErr,
D->getDepth(), D->getPosition(), D->isParameterPack(),
- (*NameOrErr).getAsIdentifierInfo(), *TemplateParamsOrErr))
+ (*NameOrErr).getAsIdentifierInfo(), D->wasDeclaredWithTypename(),
+ *TemplateParamsOrErr))
return ToD;
if (D->hasDefaultArgument()) {
diff --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index 6afdb6cfccb142..c66774dd1df151 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -1218,7 +1218,10 @@ void DeclPrinter::VisitTemplateDecl(const TemplateDecl *D) {
if (const TemplateTemplateParmDecl *TTP =
dyn_cast<TemplateTemplateParmDecl>(D)) {
- Out << "class";
+ if (TTP->wasDeclaredWithTypename())
+ Out << "typename";
+ else
+ Out << "class";
if (TTP->isParameterPack())
Out << " ...";
diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index 3c217d6a6a5ae3..cd5bd794344d0f 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -807,10 +807,10 @@ void TemplateTemplateParmDecl::anchor() {}
TemplateTemplateParmDecl::TemplateTemplateParmDecl(
DeclContext *DC, SourceLocation L, unsigned D, unsigned P,
- IdentifierInfo *Id, TemplateParameterList *Params,
+ IdentifierInfo *Id, bool Typename, TemplateParameterList *Params,
ArrayRef<TemplateParameterList *> Expansions)
: TemplateDecl(TemplateTemplateParm, DC, L, Id, Params),
- TemplateParmPosition(D, P), ParameterPack(true),
+ TemplateParmPosition(D, P), Typename(Typename), ParameterPack(true),
ExpandedParameterPack(true), NumExpandedParams(Expansions.size()) {
if (!Expansions.empty())
std::uninitialized_copy(Expansions.begin(), Expansions.end(),
@@ -821,26 +821,26 @@ TemplateTemplateParmDecl *
TemplateTemplateParmDecl::Create(const ASTContext &C, DeclContext *DC,
SourceLocation L, unsigned D, unsigned P,
bool ParameterPack, IdentifierInfo *Id,
- TemplateParameterList *Params) {
+ bool Typename, TemplateParameterList *Params) {
return new (C, DC) TemplateTemplateParmDecl(DC, L, D, P, ParameterPack, Id,
- Params);
+ Typename, Params);
}
TemplateTemplateParmDecl *
TemplateTemplateParmDecl::Create(const ASTContext &C, DeclContext *DC,
SourceLocation L, unsigned D, unsigned P,
- IdentifierInfo *Id,
+ IdentifierInfo *Id, bool Typename,
TemplateParameterList *Params,
ArrayRef<TemplateParameterList *> Expansions) {
return new (C, DC,
additionalSizeToAlloc<TemplateParameterList *>(Expansions.size()))
- TemplateTemplateParmDecl(DC, L, D, P, Id, Params, Expansions);
+ TemplateTemplateParmDecl(DC, L, D, P, Id, Typename, Params, Expansions);
}
TemplateTemplateParmDecl *
TemplateTemplateParmDecl::CreateDeserialized(ASTContext &C, unsigned ID) {
return new (C, ID) TemplateTemplateParmDecl(nullptr, SourceLocation(), 0, 0,
- false, nullptr, nullptr);
+ false, nullptr, false, nullptr);
}
TemplateTemplateParmDecl *
@@ -849,7 +849,7 @@ TemplateTemplateParmDecl::CreateDeserialized(ASTContext &C, unsigned ID,
auto *TTP =
new (C, ID, additionalSizeToAlloc<TemplateParameterList *>(NumExpansions))
TemplateTemplateParmDecl(nullptr, SourceLocation(), 0, 0, nullptr,
- nullptr, std::nullopt);
+ false, nullptr, std::nullopt);
TTP->NumExpandedParams = NumExpansions;
return TTP;
}
@@ -1471,7 +1471,7 @@ createMakeIntegerSeqParameterList(const ASTContext &C, DeclContext *DC) {
// template <typename T, ...Ints> class IntSeq
auto *TemplateTemplateParm = TemplateTemplateParmDecl::Create(
C, DC, SourceLocation(), /*Depth=*/0, /*Position=*/0,
- /*ParameterPack=*/false, /*Id=*/nullptr, TPL);
+ /*ParameterPack=*/false, /*Id=*/nullptr, /*Typename=*/false, TPL);
TemplateTemplateParm->setImplicit(true);
// typename T
diff --git a/clang/lib/Parse/ParseTemplate.cpp b/clang/lib/Parse/ParseTemplate.cpp
index d4897f8f66072e..42f26640f9cc5d 100644
--- a/clang/lib/Parse/ParseTemplate.cpp
+++ b/clang/lib/Parse/ParseTemplate.cpp
@@ -805,10 +805,12 @@ NamedDecl *Parser::ParseTemplateTemplateParameter(unsigned Depth,
// identifier, comma, or greater. Provide a fixit if the identifier, comma,
// or greater appear immediately or after 'struct'. In the latter case,
// replace the keyword with 'class'.
+ bool TypenameKeyword = false;
if (!TryConsumeToken(tok::kw_class)) {
bool Replace = Tok.isOneOf(tok::kw_typename, tok::kw_struct);
const Token &Next = Tok.is(tok::kw_struct) ? NextToken() : Tok;
if (Tok.is(tok::kw_typename)) {
+ TypenameKeyword = true;
Diag(Tok.getLocation(),
getLangOpts().CPlusPlus17
? diag::warn_cxx14_compat_template_template_param_typename
@@ -878,10 +880,9 @@ NamedDecl *Parser::ParseTemplateTemplateParameter(unsigned Depth,
}
}
- return Actions.ActOnTemplateTemplateParameter(getCurScope(), TemplateLoc,
- ParamList, EllipsisLoc,
- ParamName, NameLoc, Depth,
- Position, EqualLoc, DefaultArg);
+ return Actions.ActOnTemplateTemplateParameter(
+ getCurScope(), TemplateLoc, ParamList, TypenameKeyword, EllipsisLoc,
+ ParamName, NameLoc, Depth, Position, EqualLoc, DefaultArg);
}
/// ParseNonTypeTemplateParameter - Handle the parsing of non-type
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 2013799b5eb816..7b96335ce3d7a9 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -1630,26 +1630,20 @@ NamedDecl *Sema::ActOnNonTypeTemplateParameter(Scope *S, Declarator &D,
/// ActOnTemplateTemplateParameter - Called when a C++ template template
/// parameter (e.g. T in template <template \<typename> class T> class array)
/// has been parsed. S is the current scope.
-NamedDecl *Sema::ActOnTemplateTemplateParameter(Scope* S,
- SourceLocation TmpLoc,
- TemplateParameterList *Params,
- SourceLocation EllipsisLoc,
- IdentifierInfo *Name,
- SourceLocation NameLoc,
- unsigned Depth,
- unsigned Position,
- SourceLocation EqualLoc,
- ParsedTemplateArgument Default) {
+NamedDecl *Sema::ActOnTemplateTemplateParameter(
+ Scope *S, SourceLocation TmpLoc, TemplateParameterList *Params,
+ bool Typename, SourceLocation EllipsisLoc, IdentifierInfo *Name,
+ SourceLocation NameLoc, unsigned Depth, unsigned Position,
+ SourceLocation EqualLoc, ParsedTemplateArgument Default) {
assert(S->isTemplateParamScope() &&
"Template template parameter not in template parameter scope!");
// Construct the parameter object.
bool IsParameterPack = EllipsisLoc.isValid();
- TemplateTemplateParmDecl *Param =
- TemplateTemplateParmDecl::Create(Context, Context.getTranslationUnitDecl(),
- NameLoc.isInvalid()? TmpLoc : NameLoc,
- Depth, Position, IsParameterPack,
- Name, Params);
+ TemplateTemplateParmDecl *Param = TemplateTemplateParmDecl::Create(
+ Context, Context.getTranslationUnitDecl(),
+ NameLoc.isInvalid() ? TmpLoc : NameLoc, Depth, Position, IsParameterPack,
+ Name, Typename, Params);
Param->setAccess(AS_public);
if (Param->isParameterPack())
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 8248b10814fea5..707446e132094f 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3233,12 +3233,14 @@ TemplateDeclInstantiator::VisitTemplateTemplateParmDecl(
Param = TemplateTemplateParmDecl::Create(
SemaRef.Context, Owner, D->getLocation(),
D->getDepth() - TemplateArgs.getNumSubstitutedLevels(),
- D->getPosition(), D->getIdentifier(), InstParams, ExpandedParams);
+ D->getPosition(), D->getIdentifier(), D->wasDeclaredWithTypename(),
+ InstParams, ExpandedParams);
else
Param = TemplateTemplateParmDecl::Create(
SemaRef.Context, Owner, D->getLocation(),
D->getDepth() - TemplateArgs.getNumSubstitutedLevels(),
- D->getPosition(), D->isParameterPack(), D->getIdentifier(), InstParams);
+ D->getPosition(), D->isParameterPack(), D->getIdentifier(),
+ D->wasDeclaredWithTypename(), InstParams);
if (D->hasDefaultArgument() && !D->defaultArgumentWasInherited()) {
NestedNameSpecifierLoc QualifierLoc =
D->getDefaultArgument().getTemplateQualifierLoc();
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 78448855fba09c..3bb31ceb03bf87 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2715,6 +2715,7 @@ void ASTDeclReader::VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) {
void ASTDeclReader::VisitTemplateTemplateParmDecl(TemplateTemplateParmDecl *D) {
VisitTemplateDecl(D);
+ D->setDeclaredWithTypename(Record.readBool());
// TemplateParmPosition.
D->setDepth(Record.readInt());
D->setPosition(Record.readInt());
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 86f64bf2a24250..87e773be54fae9 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1921,6 +1921,7 @@ void ASTDeclWriter::VisitTemplateTemplateParmDecl(TemplateTemplateParmDecl *D) {
Record.push_back(D->getNumExpansionTemplateParameters());
VisitTemplateDecl(D);
+ Record.push_back(D->wasDeclaredWithTypename());
// TemplateParmPosition.
Record.push_back(D->getDepth());
Record.push_back(D->getPosition());
diff --git a/clang/unittests/AST/DeclPrinterTest.cpp b/clang/unittests/AST/DeclPrinterTest.cpp
index f2b027a25621ce..c24e442621c923 100644
--- a/clang/unittests/AST/DeclPrinterTest.cpp
+++ b/clang/unittests/AST/DeclPrinterTest.cpp
@@ -1220,8 +1220,7 @@ TEST(DeclPrinter, TestTemplateTemplateParameterWrittenWithTypename) {
ASSERT_TRUE(PrintedDeclCXX17Matches(
"template <template <typename> typename Z> void A();",
functionTemplateDecl(hasName("A")).bind("id"),
- "template <template <typename> class Z> void A()"));
- // WRONG: We should use typename if the parameter was written with it.
+ "template <template <typename> typename Z> void A()"));
}
TEST(DeclPrinter, TestTemplateArgumentList1) {
|
sdkrystian
force-pushed
the
temp-temp-param-kw
branch
from
5bc0f03 to
a3a9dd9
Compare
erichkeane
reviewed
Apr 9, 2024
| /// the 'typename' keyword. | ||
| /// | ||
| /// If false, it was declared with the 'class' keyword. | ||
| bool Typename : 1; |
There was a problem hiding this comment.
I like the bitfield nature here, but would prefer we do an 'unsigned' for each, and just 'treat' them as a bool later (it is a bit of a convention here).
Additionally, Typename should have an enum value that it gets compared to, rather than true meaning typename, false meaning class.
There was a problem hiding this comment.
Yup, i believe using bool is still causing issues for msvc
There was a problem hiding this comment.
Corentin's point is relevant here, you can't really use 'bool' in these (and should use the PREFERRED_TYPE or whatever attribute) thanks to some versions of MSVC we support.
| @@ -1634,6 +1640,14 @@ class TemplateTemplateParmDecl final | |||
| using TemplateParmPosition::setPosition; | |||
| using TemplateParmPosition::getIndex; | |||
|
|
|||
| /// Whether this template template parameter was declared with | |||
| /// the 'typename' keyword. | |||
| bool wasDeclaredWithTypename() const { return Typename; } | |||
There was a problem hiding this comment.
So these should return the syntax of typename and class rather than a bool.
There was a problem hiding this comment.
We do not have an enum for that yet afaik (ie this is consistent with ActOnTypeParameter).
I would not oppose using an enum everywhere
|
@erichkeane I added a release note. Regarding the addition of an enumeration type, how about: enum class TypeParmKeyword {
Class,
Typename
};(I used |
That enum name is fine to me, I think that makes sense. 'None' and using this for TypeTemplateParm is also perhaps valuable, but not necessary for this patch, feel free to change it if you'd like, but it would make sense in a followup. |
|
@erichkeane I'm thinking of keeping the use of |
I can live with that. |
erichkeane
reviewed
Apr 11, 2024
| /// the 'typename' keyword. | ||
| /// | ||
| /// If false, it was declared with the 'class' keyword. | ||
| bool Typename : 1; |
There was a problem hiding this comment.
Corentin's point is relevant here, you can't really use 'bool' in these (and should use the PREFERRED_TYPE or whatever attribute) thanks to some versions of MSVC we support.
erichkeane
approved these changes
Apr 11, 2024
|
What category should the release note go under? |
|
sdkrystian
force-pushed
the
temp-temp-param-kw
branch
from
6419cd3 to
9ebabf4
Compare
sdkrystian
added a commit
to sdkrystian/llvm-project
that referenced
this pull request
Apr 11, 2024
|
Seems like there is a stray call to |
sdkrystian
added a commit
that referenced
this pull request
Apr 11, 2024
cor3ntin
added a commit
to cor3ntin/llvm-project
that referenced
this pull request
Apr 16, 2024
…the 'typename' keyword (llvm#88139)" This reverts commit 4e6d18f.
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.
This patch adds a
Typenamebit-field toTemplateTemplateParmDeclwhich stores whether the template template parameter was declared with thetypenamekeyword.