Skip to content

[Clang][Parse] Diagnose member template declarations with multiple declarators #78243

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.

Sign up for GitHub

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 1 commit into from
Feb 1, 2024
Merged

[Clang][Parse] Diagnose member template declarations with multiple declarators #78243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ Improvements to Clang's diagnostics
- Clang now applies syntax highlighting to the code snippets it
prints.

- Clang now diagnoses member template declarations with multiple declarators.

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

Expand Down
32 changes: 16 additions & 16 deletions clang/include/clang/Parse/Parser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2423,6 +2423,7 @@ class Parser : public CodeCompletionHandler {
bool MightBeDeclarator(DeclaratorContext Context);
DeclGroupPtrTy ParseDeclGroup(ParsingDeclSpec &DS, DeclaratorContext Context,
ParsedAttributes &Attrs,
ParsedTemplateInfo &TemplateInfo,
SourceLocation *DeclEnd = nullptr,
ForRangeInit *FRI = nullptr);
Decl *ParseDeclarationAfterDeclarator(Declarator &D,
Expand Down Expand Up @@ -3615,16 +3616,15 @@ class Parser : public CodeCompletionHandler {
// C++ 14: Templates [temp]

// C++ 14.1: Template Parameters [temp.param]
Decl *ParseDeclarationStartingWithTemplate(DeclaratorContext Context,
SourceLocation &DeclEnd,
ParsedAttributes &AccessAttrs,
AccessSpecifier AS = AS_none);
Decl *ParseTemplateDeclarationOrSpecialization(DeclaratorContext Context,
SourceLocation &DeclEnd,
ParsedAttributes &AccessAttrs,
AccessSpecifier AS);
Decl *ParseSingleDeclarationAfterTemplate(
DeclaratorContext Context, const ParsedTemplateInfo &TemplateInfo,
DeclGroupPtrTy
ParseDeclarationStartingWithTemplate(DeclaratorContext Context,
SourceLocation &DeclEnd,
ParsedAttributes &AccessAttrs);
DeclGroupPtrTy ParseTemplateDeclarationOrSpecialization(
DeclaratorContext Context, SourceLocation &DeclEnd,
ParsedAttributes &AccessAttrs, AccessSpecifier AS);
DeclGroupPtrTy ParseDeclarationAfterTemplate(
DeclaratorContext Context, ParsedTemplateInfo &TemplateInfo,
ParsingDeclRAIIObject &DiagsFromParams, SourceLocation &DeclEnd,
ParsedAttributes &AccessAttrs, AccessSpecifier AS = AS_none);
bool ParseTemplateParameters(MultiParseScope &TemplateScopes, unsigned Depth,
Expand Down Expand Up @@ -3673,12 +3673,12 @@ class Parser : public CodeCompletionHandler {
TemplateTy Template, SourceLocation OpenLoc);
ParsedTemplateArgument ParseTemplateTemplateArgument();
ParsedTemplateArgument ParseTemplateArgument();
Decl *ParseExplicitInstantiation(DeclaratorContext Context,
SourceLocation ExternLoc,
SourceLocation TemplateLoc,
SourceLocation &DeclEnd,
ParsedAttributes &AccessAttrs,
AccessSpecifier AS = AS_none);
DeclGroupPtrTy ParseExplicitInstantiation(DeclaratorContext Context,
SourceLocation ExternLoc,
SourceLocation TemplateLoc,
SourceLocation &DeclEnd,
ParsedAttributes &AccessAttrs,
AccessSpecifier AS = AS_none);
// C++2a: Template, concept definition [temp]
Decl *
ParseConceptDefinition(const ParsedTemplateInfo &TemplateInfo,
Expand Down
97 changes: 84 additions & 13 deletions clang/lib/Parse/ParseDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1916,9 +1916,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclaration(DeclaratorContext Context,
case tok::kw_export:
ProhibitAttributes(DeclAttrs);
ProhibitAttributes(DeclSpecAttrs);
SingleDecl =
ParseDeclarationStartingWithTemplate(Context, DeclEnd, DeclAttrs);
break;
return ParseDeclarationStartingWithTemplate(Context, DeclEnd, DeclAttrs);
case tok::kw_inline:
// Could be the start of an inline namespace. Allowed as an ext in C++03.
if (getLangOpts().CPlusPlus && NextToken().is(tok::kw_namespace)) {
Expand Down Expand Up @@ -1994,8 +1992,9 @@ Parser::DeclGroupPtrTy Parser::ParseSimpleDeclaration(
ParsingDeclSpec DS(*this);
DS.takeAttributesFrom(DeclSpecAttrs);

ParsedTemplateInfo TemplateInfo;
DeclSpecContext DSContext = getDeclSpecContextFromDeclaratorContext(Context);
ParseDeclarationSpecifiers(DS, ParsedTemplateInfo(), AS_none, DSContext);
ParseDeclarationSpecifiers(DS, TemplateInfo, AS_none, DSContext);

// If we had a free-standing type definition with a missing semicolon, we
// may get this far before the problem becomes obvious.
Expand Down Expand Up @@ -2027,7 +2026,7 @@ Parser::DeclGroupPtrTy Parser::ParseSimpleDeclaration(
if (DeclSpecStart)
DS.SetRangeStart(*DeclSpecStart);

return ParseDeclGroup(DS, Context, DeclAttrs, &DeclEnd, FRI);
return ParseDeclGroup(DS, Context, DeclAttrs, TemplateInfo, &DeclEnd, FRI);
}

/// Returns true if this might be the start of a declarator, or a common typo
Expand Down Expand Up @@ -2184,6 +2183,7 @@ void Parser::SkipMalformedDecl() {
Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
DeclaratorContext Context,
ParsedAttributes &Attrs,
ParsedTemplateInfo &TemplateInfo,
SourceLocation *DeclEnd,
ForRangeInit *FRI) {
// Parse the first declarator.
Expand All @@ -2193,8 +2193,19 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
ParsedAttributes LocalAttrs(AttrFactory);
LocalAttrs.takeAllFrom(Attrs);
ParsingDeclarator D(*this, DS, LocalAttrs, Context);
if (TemplateInfo.TemplateParams)
D.setTemplateParameterLists(*TemplateInfo.TemplateParams);

bool IsTemplateSpecOrInst =
(TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation ||
TemplateInfo.Kind == ParsedTemplateInfo::ExplicitSpecialization);
SuppressAccessChecks SAC(*this, IsTemplateSpecOrInst);

ParseDeclarator(D);

if (IsTemplateSpecOrInst)
SAC.done();

// Bail out if the first declarator didn't seem well-formed.
if (!D.hasName() && !D.mayOmitIdentifier()) {
SkipMalformedDecl();
Expand Down Expand Up @@ -2262,15 +2273,54 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
// need to handle the file scope definition case.
if (Context == DeclaratorContext::File) {
if (isStartOfFunctionDefinition(D)) {
// C++23 [dcl.typedef] p1:
// The typedef specifier shall not be [...], and it shall not be
// used in the decl-specifier-seq of a parameter-declaration nor in
// the decl-specifier-seq of a function-definition.
if (DS.getStorageClassSpec() == DeclSpec::SCS_typedef) {
Diag(Tok, diag::err_function_declared_typedef);

// Recover by treating the 'typedef' as spurious.
// If the user intended to write 'typename', we should have already
// suggested adding it elsewhere. In any case, recover by ignoring
// 'typedef' and suggest removing it.
Diag(DS.getStorageClassSpecLoc(),
diag::err_function_declared_typedef)
<< FixItHint::CreateRemoval(DS.getStorageClassSpecLoc());
DS.ClearStorageClassSpecs();
}
Decl *TheDecl = nullptr;

if (TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation) {
if (D.getName().getKind() != UnqualifiedIdKind::IK_TemplateId) {
// If the declarator-id is not a template-id, issue a diagnostic
// and recover by ignoring the 'template' keyword.
Diag(Tok, diag::err_template_defn_explicit_instantiation) << 0;
TheDecl = ParseFunctionDefinition(D, ParsedTemplateInfo(),
&LateParsedAttrs);
} else {
SourceLocation LAngleLoc =
PP.getLocForEndOfToken(TemplateInfo.TemplateLoc);
Diag(D.getIdentifierLoc(),
diag::err_explicit_instantiation_with_definition)
<< SourceRange(TemplateInfo.TemplateLoc)
<< FixItHint::CreateInsertion(LAngleLoc, "<>");

// Recover as if it were an explicit specialization.
TemplateParameterLists FakedParamLists;
FakedParamLists.push_back(Actions.ActOnTemplateParameterList(
0, SourceLocation(), TemplateInfo.TemplateLoc, LAngleLoc,
std::nullopt, LAngleLoc, nullptr));

TheDecl = ParseFunctionDefinition(
D,
ParsedTemplateInfo(&FakedParamLists,
/*isSpecialization=*/true,
/*lastParameterListWasEmpty=*/true),
&LateParsedAttrs);
}
} else {
TheDecl =
ParseFunctionDefinition(D, TemplateInfo, &LateParsedAttrs);
}

Decl *TheDecl = ParseFunctionDefinition(D, ParsedTemplateInfo(),
&LateParsedAttrs);
return Actions.ConvertDeclToDeclGroup(TheDecl);
}

Expand Down Expand Up @@ -2360,8 +2410,8 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
}

SmallVector<Decl *, 8> DeclsInGroup;
Decl *FirstDecl = ParseDeclarationAfterDeclaratorAndAttributes(
D, ParsedTemplateInfo(), FRI);
Decl *FirstDecl =
ParseDeclarationAfterDeclaratorAndAttributes(D, TemplateInfo, FRI);
if (LateParsedAttrs.size() > 0)
ParseLexedAttributeList(LateParsedAttrs, FirstDecl, true, false);
D.complete(FirstDecl);
Expand All @@ -2384,6 +2434,16 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
break;
}

// C++23 [temp.pre]p5:
// In a template-declaration, explicit specialization, or explicit
// instantiation the init-declarator-list in the declaration shall
// contain at most one declarator.
if (TemplateInfo.Kind != ParsedTemplateInfo::NonTemplate &&
D.isFirstDeclarator()) {
Diag(CommaLoc, diag::err_multiple_template_declarators)
<< TemplateInfo.Kind;
}

// Parse the next declarator.
D.clear();
D.setCommaLoc(CommaLoc);
Expand Down Expand Up @@ -2413,7 +2473,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
// declarator requires-clause
if (Tok.is(tok::kw_requires))
ParseTrailingRequiresClause(D);
Decl *ThisDecl = ParseDeclarationAfterDeclarator(D);
Decl *ThisDecl = ParseDeclarationAfterDeclarator(D, TemplateInfo);
D.complete(ThisDecl);
if (ThisDecl)
DeclsInGroup.push_back(ThisDecl);
Expand Down Expand Up @@ -6526,6 +6586,17 @@ void Parser::ParseDirectDeclarator(Declarator &D) {
/*ObjectHasErrors=*/false, EnteringContext);
}

// C++23 [basic.scope.namespace]p1:
// For each non-friend redeclaration or specialization whose target scope
// is or is contained by the scope, the portion after the declarator-id,
// class-head-name, or enum-head-name is also included in the scope.
// C++23 [basic.scope.class]p1:
// For each non-friend redeclaration or specialization whose target scope
// is or is contained by the scope, the portion after the declarator-id,
// class-head-name, or enum-head-name is also included in the scope.
//
// FIXME: We should not be doing this for friend declarations; they have
// their own special lookup semantics specified by [basic.lookup.unqual]p6.
if (D.getCXXScopeSpec().isValid()) {
if (Actions.ShouldEnterDeclaratorScope(getCurScope(),
D.getCXXScopeSpec()))
Expand Down
35 changes: 31 additions & 4 deletions clang/lib/Parse/ParseDeclCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2855,7 +2855,7 @@ Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS,
}

// static_assert-declaration. A templated static_assert declaration is
// diagnosed in Parser::ParseSingleDeclarationAfterTemplate.
// diagnosed in Parser::ParseDeclarationAfterTemplate.
if (!TemplateInfo.Kind &&
Tok.isOneOf(tok::kw_static_assert, tok::kw__Static_assert)) {
SourceLocation DeclEnd;
Expand All @@ -2868,9 +2868,8 @@ Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS,
"Nested template improperly parsed?");
ObjCDeclContextSwitch ObjCDC(*this);
SourceLocation DeclEnd;
return DeclGroupPtrTy::make(
DeclGroupRef(ParseTemplateDeclarationOrSpecialization(
DeclaratorContext::Member, DeclEnd, AccessAttrs, AS)));
return ParseTemplateDeclarationOrSpecialization(DeclaratorContext::Member,
DeclEnd, AccessAttrs, AS);
}

// Handle: member-declaration ::= '__extension__' member-declaration
Expand Down Expand Up @@ -3279,6 +3278,16 @@ Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS,
break;
}

// C++23 [temp.pre]p5:
// In a template-declaration, explicit specialization, or explicit
// instantiation the init-declarator-list in the declaration shall
// contain at most one declarator.
if (TemplateInfo.Kind != ParsedTemplateInfo::NonTemplate &&
DeclaratorInfo.isFirstDeclarator()) {
Diag(CommaLoc, diag::err_multiple_template_declarators)
<< TemplateInfo.Kind;
}

// Parse the next declarator.
DeclaratorInfo.clear();
VS.clear();
Expand Down Expand Up @@ -4228,6 +4237,24 @@ void Parser::ParseTrailingRequiresClause(Declarator &D) {

SourceLocation RequiresKWLoc = ConsumeToken();

// C++23 [basic.scope.namespace]p1:
// For each non-friend redeclaration or specialization whose target scope
// is or is contained by the scope, the portion after the declarator-id,
// class-head-name, or enum-head-name is also included in the scope.
// C++23 [basic.scope.class]p1:
// For each non-friend redeclaration or specialization whose target scope
// is or is contained by the scope, the portion after the declarator-id,
// class-head-name, or enum-head-name is also included in the scope.
//
// FIXME: We should really be calling ParseTrailingRequiresClause in
// ParseDirectDeclarator, when we are already in the declarator scope.
// This would also correctly suppress access checks for specializations
// and explicit instantiations, which we currently do not do.
CXXScopeSpec &SS = D.getCXXScopeSpec();
DeclaratorScopeObj DeclScopeObj(*this, SS);
if (SS.isValid() && Actions.ShouldEnterDeclaratorScope(getCurScope(), SS))
DeclScopeObj.EnterDeclaratorScope();

ExprResult TrailingRequiresClause;
ParseScope ParamScope(this, Scope::DeclScope |
Scope::FunctionDeclarationScope |
Expand Down
Loading