Skip to content

Commit

Permalink
[Clang] Fix: Restore warning inadvertently removed by D126061.
Browse files Browse the repository at this point in the history
Before D126061, Clang would warn about this code

```
struct X {
    [[deprecated]] struct Y {};
};
```

with the warning

    attribute 'deprecated' is ignored, place it after "struct" to apply attribute to type declaration

D126061 inadvertently caused this warning to no longer be emitted. This patch
restores the previous behavior.

The reason for the bug is that after D126061, C++11 attributes applied to a
member declaration are no longer placed in `DS.getAttributes()` but are instead
tracked in a separate list (`DeclAttrs`). In the case of a free-standing
decl-specifier-seq, we would simply ignore the contents of this list. Instead,
we now pass the list on to `Sema::ParsedFreeStandingDeclSpec()` so that it can
issue the appropriate warning.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D128499
  • Loading branch information
martinboehme committed Jun 28, 2022
1 parent 527ef8c commit 8686610
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 20 deletions.
2 changes: 2 additions & 0 deletions clang/include/clang/Sema/Sema.h
Expand Up @@ -3117,8 +3117,10 @@ class Sema final {
void ActOnTranslationUnitScope(Scope *S);

Decl *ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, DeclSpec &DS,
const ParsedAttributesView &DeclAttrs,
RecordDecl *&AnonRecord);
Decl *ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, DeclSpec &DS,
const ParsedAttributesView &DeclAttrs,
MultiTemplateParamsArg TemplateParams,
bool IsExplicitInstantiation,
RecordDecl *&AnonRecord);
Expand Down
8 changes: 4 additions & 4 deletions clang/lib/Parse/ParseDecl.cpp
Expand Up @@ -1863,8 +1863,8 @@ Parser::DeclGroupPtrTy Parser::ParseSimpleDeclaration(
DeclEnd = Tok.getLocation();
if (RequireSemi) ConsumeToken();
RecordDecl *AnonRecord = nullptr;
Decl *TheDecl = Actions.ParsedFreeStandingDeclSpec(getCurScope(), AS_none,
DS, AnonRecord);
Decl *TheDecl = Actions.ParsedFreeStandingDeclSpec(
getCurScope(), AS_none, DS, ParsedAttributesView::none(), AnonRecord);
DS.complete(TheDecl);
if (AnonRecord) {
Decl* decls[] = {AnonRecord, TheDecl};
Expand Down Expand Up @@ -4358,8 +4358,8 @@ void Parser::ParseStructDeclaration(
// declarator list is omitted."
ProhibitAttributes(Attrs);
RecordDecl *AnonRecord = nullptr;
Decl *TheDecl = Actions.ParsedFreeStandingDeclSpec(getCurScope(), AS_none,
DS, AnonRecord);
Decl *TheDecl = Actions.ParsedFreeStandingDeclSpec(
getCurScope(), AS_none, DS, ParsedAttributesView::none(), AnonRecord);
assert(!AnonRecord && "Did not expect anonymous struct or union here");
DS.complete(TheDecl);
return;
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Parse/ParseDeclCXX.cpp
Expand Up @@ -2766,7 +2766,7 @@ Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS,

RecordDecl *AnonRecord = nullptr;
Decl *TheDecl = Actions.ParsedFreeStandingDeclSpec(
getCurScope(), AS, DS, TemplateParams, false, AnonRecord);
getCurScope(), AS, DS, DeclAttrs, TemplateParams, false, AnonRecord);
DS.complete(TheDecl);
if (AnonRecord) {
Decl* decls[] = {AnonRecord, TheDecl};
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Parse/ParseTemplate.cpp
Expand Up @@ -231,7 +231,7 @@ Decl *Parser::ParseSingleDeclarationAfterTemplate(
DeclEnd = ConsumeToken();
RecordDecl *AnonRecord = nullptr;
Decl *Decl = Actions.ParsedFreeStandingDeclSpec(
getCurScope(), AS, DS,
getCurScope(), AS, DS, ParsedAttributesView::none(),
TemplateInfo.TemplateParams ? *TemplateInfo.TemplateParams
: MultiTemplateParamsArg(),
TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation,
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Parse/Parser.cpp
Expand Up @@ -1113,8 +1113,8 @@ Parser::DeclGroupPtrTy Parser::ParseDeclOrFunctionDefInternal(
ProhibitAttributes(Attrs, CorrectLocationForAttributes);
ConsumeToken();
RecordDecl *AnonRecord = nullptr;
Decl *TheDecl = Actions.ParsedFreeStandingDeclSpec(getCurScope(), AS_none,
DS, AnonRecord);
Decl *TheDecl = Actions.ParsedFreeStandingDeclSpec(
getCurScope(), AS_none, DS, ParsedAttributesView::none(), AnonRecord);
DS.complete(TheDecl);
if (AnonRecord) {
Decl* decls[] = {AnonRecord, TheDecl};
Expand Down
27 changes: 16 additions & 11 deletions clang/lib/Sema/SemaDecl.cpp
Expand Up @@ -4629,11 +4629,12 @@ bool Sema::checkVarDeclRedefinition(VarDecl *Old, VarDecl *New) {

/// ParsedFreeStandingDeclSpec - This method is invoked when a declspec with
/// no declarator (e.g. "struct foo;") is parsed.
Decl *
Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, DeclSpec &DS,
RecordDecl *&AnonRecord) {
return ParsedFreeStandingDeclSpec(S, AS, DS, MultiTemplateParamsArg(), false,
AnonRecord);
Decl *Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS,
DeclSpec &DS,
const ParsedAttributesView &DeclAttrs,
RecordDecl *&AnonRecord) {
return ParsedFreeStandingDeclSpec(
S, AS, DS, DeclAttrs, MultiTemplateParamsArg(), false, AnonRecord);
}

// The MS ABI changed between VS2013 and VS2015 with regard to numbers used to
Expand Down Expand Up @@ -4846,11 +4847,12 @@ static unsigned GetDiagnosticTypeSpecifierID(DeclSpec::TST T) {
/// ParsedFreeStandingDeclSpec - This method is invoked when a declspec with
/// no declarator (e.g. "struct foo;") is parsed. It also accepts template
/// parameters to cope with template friend declarations.
Decl *
Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, DeclSpec &DS,
MultiTemplateParamsArg TemplateParams,
bool IsExplicitInstantiation,
RecordDecl *&AnonRecord) {
Decl *Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS,
DeclSpec &DS,
const ParsedAttributesView &DeclAttrs,
MultiTemplateParamsArg TemplateParams,
bool IsExplicitInstantiation,
RecordDecl *&AnonRecord) {
Decl *TagD = nullptr;
TagDecl *Tag = nullptr;
if (DS.getTypeSpecType() == DeclSpec::TST_class ||
Expand Down Expand Up @@ -5089,7 +5091,7 @@ Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, DeclSpec &DS,
// Warn about ignored type attributes, for example:
// __attribute__((aligned)) struct A;
// Attributes should be placed after tag to apply to type declaration.
if (!DS.getAttributes().empty()) {
if (!DS.getAttributes().empty() || !DeclAttrs.empty()) {
DeclSpec::TST TypeSpecType = DS.getTypeSpecType();
if (TypeSpecType == DeclSpec::TST_class ||
TypeSpecType == DeclSpec::TST_struct ||
Expand All @@ -5099,6 +5101,9 @@ Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, DeclSpec &DS,
for (const ParsedAttr &AL : DS.getAttributes())
Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
for (const ParsedAttr &AL : DeclAttrs)
Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
}
}

Expand Down
38 changes: 37 additions & 1 deletion clang/test/SemaCXX/attr-declspec-ignored.cpp
Expand Up @@ -6,14 +6,50 @@ namespace test1 {
__attribute__((visibility("hidden"))) __attribute__((aligned)) struct B; // expected-warning{{attribute 'visibility' is ignored, place it after "struct" to apply attribute to type declaration}} \
// expected-warning{{attribute 'aligned' is ignored, place it after "struct" to apply attribute to type declaration}}
__attribute__((visibility("hidden"))) __attribute__((aligned)) union C; // expected-warning{{attribute 'visibility' is ignored, place it after "union" to apply attribute to type declaration}} \
// expected-warning{{attribute 'aligned' is ignored, place it after "union" to apply attribute to type declaration}}
// expected-warning{{attribute 'aligned' is ignored, place it after "union" to apply attribute to type declaration}}
__attribute__((visibility("hidden"))) __attribute__((aligned)) enum D {D}; // expected-warning{{attribute 'visibility' is ignored, place it after "enum" to apply attribute to type declaration}} \
// expected-warning{{attribute 'aligned' is ignored, place it after "enum" to apply attribute to type declaration}}

// Test that we get the same warnings for type declarations nested in a record.
struct X {
__attribute__((visibility("hidden"))) __attribute__((aligned)) class A; // expected-warning{{attribute 'visibility' is ignored, place it after "class" to apply attribute to type declaration}} \
// expected-warning{{attribute 'aligned' is ignored, place it after "class" to apply attribute to type declaration}}
__attribute__((visibility("hidden"))) __attribute__((aligned)) struct B; // expected-warning{{attribute 'visibility' is ignored, place it after "struct" to apply attribute to type declaration}} \
// expected-warning{{attribute 'aligned' is ignored, place it after "struct" to apply attribute to type declaration}}
__attribute__((visibility("hidden"))) __attribute__((aligned)) union C; // expected-warning{{attribute 'visibility' is ignored, place it after "union" to apply attribute to type declaration}} \
// expected-warning{{attribute 'aligned' is ignored, place it after "union" to apply attribute to type declaration}}
__attribute__((visibility("hidden"))) __attribute__((aligned)) enum D {D}; // expected-warning{{attribute 'visibility' is ignored, place it after "enum" to apply attribute to type declaration}} \
// expected-warning{{attribute 'aligned' is ignored, place it after "enum" to apply attribute to type declaration}}

// Also test [[]] attribute syntax. (On a non-nested declaration, these
// generate a hard "misplaced attributes" error, which we test for
// elsewhere.)
[[gnu::visibility("hidden")]] [[gnu::aligned]] class E; // expected-warning{{attribute 'visibility' is ignored, place it after "class" to apply attribute to type declaration}} \
// expected-warning{{attribute 'aligned' is ignored, place it after "class" to apply attribute to type declaration}}
[[gnu::visibility("hidden")]] [[gnu::aligned]] struct F; // expected-warning{{attribute 'visibility' is ignored, place it after "struct" to apply attribute to type declaration}} \
// expected-warning{{attribute 'aligned' is ignored, place it after "struct" to apply attribute to type declaration}}
[[gnu::visibility("hidden")]] [[gnu::aligned]] union G; // expected-warning{{attribute 'visibility' is ignored, place it after "union" to apply attribute to type declaration}} \
// expected-warning{{attribute 'aligned' is ignored, place it after "union" to apply attribute to type declaration}}
[[gnu::visibility("hidden")]] [[gnu::aligned]] enum H {H}; // expected-warning{{attribute 'visibility' is ignored, place it after "enum" to apply attribute to type declaration}} \
// expected-warning{{attribute 'aligned' is ignored, place it after "enum" to apply attribute to type declaration}}
};
}

namespace test2 {
__attribute__((visibility("hidden"))) __attribute__((aligned)) class A {} a;
__attribute__((visibility("hidden"))) __attribute__((aligned)) struct B {} b;
__attribute__((visibility("hidden"))) __attribute__((aligned)) union C {} c;
__attribute__((visibility("hidden"))) __attribute__((aligned)) enum D {D} d;

struct X {
__attribute__((visibility("hidden"))) __attribute__((aligned)) class A {} a;
__attribute__((visibility("hidden"))) __attribute__((aligned)) struct B {} b;
__attribute__((visibility("hidden"))) __attribute__((aligned)) union C {} c;
__attribute__((visibility("hidden"))) __attribute__((aligned)) enum D {D} d;

[[gnu::visibility("hidden")]] [[gnu::aligned]] class E {} e;
[[gnu::visibility("hidden")]] [[gnu::aligned]] struct F {} f;
[[gnu::visibility("hidden")]] [[gnu::aligned]] union G {} g;
[[gnu::visibility("hidden")]] [[gnu::aligned]] enum H {H} h;
};
}

0 comments on commit 8686610

Please sign in to comment.