Skip to content

Commit

Permalink
[Clang][Parse] Diagnose member template declarations with multiple de…
Browse files Browse the repository at this point in the history 
…clarators (llvm#78243)

According to [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. 

A member-declaration that is a template-declaration or explicit-specialization contains a declaration, even though it declares a member. This means it _will_ contain an init-declarator-list (not a member-declarator-list), so [temp.pre] p5 applies.

This diagnoses declarations such as:
```
struct A
{
    template<typename T>
    static const int x = 0, f(); // error: a template declaration can only declare a single entity

    template<typename T>
    static const int g(), y = 0; // error: a template declaration can only declare a single entity
};
```
The diagnostic messages are the same as those of the equivalent namespace scope declarations.

Note: since we currently do not diagnose declarations with multiple abbreviated function template declarators at namespace scope e.g., `void f(auto), g(auto);`, so this patch does not add diagnostics for the equivalent member declarations.

This patch also refactors `ParseSingleDeclarationAfterTemplate` (now named `ParseDeclarationAfterTemplate`) to call `ParseDeclGroup` and return the resultant `DeclGroup`.
  • Loading branch information
@sdkrystian @smithp35
sdkrystian authored and smithp35 committed Feb 1, 2024
1 parent c6af36d commit c8c44cc
Show file tree
Hide file tree
Showing 8 changed files with 185 additions and 205 deletions.
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
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
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
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
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

0 comments on commit c8c44cc

Please sign in to comment.