Skip to content

[Clang][AST] Track whether template template parameters used the 'typename' keyword #88139

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

Merged
merged 3 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,9 @@ Improvements to Clang's diagnostics
Added the ``-Wtentative-definition-array`` warning group to cover this.
Fixes #GH87766

- Clang now uses the correct type-parameter-key (``class`` or ``typename``) when printing
template template parameter declarations.

Improvements to Clang's time-trace
----------------------------------

Expand Down Expand Up @@ -535,6 +538,7 @@ Bug Fixes to C++ Support
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
- Clang now properly preserves ``FoundDecls`` within a ``ConceptReference``. (#GH82628)
- The presence of the ``typename`` keyword is now stored in ``TemplateTemplateParmDecl``.

Miscellaneous Bug Fixes
^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
49 changes: 33 additions & 16 deletions clang/include/clang/AST/DeclTemplate.h
Original file line number Diff line number Diff line change
Expand Up @@ -1581,26 +1581,36 @@ 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.
LLVM_PREFERRED_TYPE(bool)
unsigned Typename : 1;

/// Whether this parameter is a parameter pack.
bool ParameterPack;
LLVM_PREFERRED_TYPE(bool)
unsigned 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;
LLVM_PREFERRED_TYPE(bool)
unsigned 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;
Expand All @@ -1613,14 +1623,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);
Expand All @@ -1634,6 +1643,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; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

So these should return the syntax of typename and class rather than a bool.

Copy link
Contributor

Choose a reason for hiding this comment

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

We do not have an enum for that yet afaik (ie this is consistent with ActOnTypeParameter).
I would not oppose using an enum everywhere


/// 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.
///
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -9064,7 +9064,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);

Expand Down
2 changes: 1 addition & 1 deletion clang/lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/AST/ASTImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
5 changes: 4 additions & 1 deletion clang/lib/AST/DeclPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 << " ...";
Expand Down
18 changes: 9 additions & 9 deletions clang/lib/AST/DeclTemplate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -805,10 +805,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(),
Expand All @@ -819,26 +819,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 *
Expand All @@ -847,7 +847,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;
}
Expand Down Expand Up @@ -1469,7 +1469,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
Expand Down
9 changes: 5 additions & 4 deletions clang/lib/Parse/ParseTemplate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
24 changes: 9 additions & 15 deletions clang/lib/Sema/SemaTemplate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
6 changes: 4 additions & 2 deletions clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Serialization/ASTReaderDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
1 change: 1 addition & 0 deletions clang/lib/Serialization/ASTWriterDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
3 changes: 1 addition & 2 deletions clang/unittests/AST/DeclPrinterTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down