Skip to content

Commit 8c7b64b

Browse files
martinboehmemartinboehme
committed
[clang] Reject non-declaration C++11 attributes on declarations
For backwards compatiblity, we emit only a warning instead of an error if the attribute is one of the existing type attributes that we have historically allowed to "slide" to the `DeclSpec` just as if it had been specified in GNU syntax. (We will call these "legacy type attributes" below.) The high-level changes that achieve this are: - We introduce a new field `Declarator::DeclarationAttrs` (with appropriate accessors) to store C++11 attributes occurring in the attribute-specifier-seq at the beginning of a simple-declaration (and other similar declarations). Previously, these attributes were placed on the `DeclSpec`, which made it impossible to reconstruct later on whether the attributes had in fact been placed on the decl-specifier-seq or ahead of the declaration. - In the parser, we propgate declaration attributes and decl-specifier-seq attributes separately until we can place them in `Declarator::DeclarationAttrs` or `DeclSpec::Attrs`, respectively. - In `ProcessDeclAttributes()`, in addition to processing declarator attributes, we now also process the attributes from `Declarator::DeclarationAttrs` (except if they are legacy type attributes). - In `ConvertDeclSpecToType()`, in addition to processing `DeclSpec` attributes, we also process any legacy type attributes that occur in `Declarator::DeclarationAttrs` (and emit a warning). - We make `ProcessDeclAttribute` emit an error if it sees any non-declaration attributes in C++11 syntax, except in the following cases: - If it is being called for attributes on a `DeclSpec` or `DeclaratorChunk` - If the attribute is a legacy type attribute (in which case we only emit a warning) The standard justifies treating attributes at the beginning of a simple-declaration and attributes after a declarator-id the same. Here are some relevant parts of the standard: - The attribute-specifier-seq at the beginning of a simple-declaration "appertains to each of the entities declared by the declarators of the init-declarator-list" (https://eel.is/c++draft/dcl.dcl#dcl.pre-3) - "In the declaration for an entity, attributes appertaining to that entity can appear at the start of the declaration and after the declarator-id for that declaration." (https://eel.is/c++draft/dcl.dcl#dcl.pre-note-2) - "The optional attribute-specifier-seq following a declarator-id appertains to the entity that is declared." (https://eel.is/c++draft/dcl.dcl#dcl.meaning.general-1) The standard contains similar wording to that for a simple-declaration in other similar types of declarations, for example: - "The optional attribute-specifier-seq in a parameter-declaration appertains to the parameter." (https://eel.is/c++draft/dcl.fct#3) - "The optional attribute-specifier-seq in an exception-declaration appertains to the parameter of the catch clause" (https://eel.is/c++draft/except.pre#1) The new behavior is tested both on the newly added type attribute `annotate_type`, for which we emit errors, and for the legacy type attribute `address_space` (chosen somewhat randomly from the various legacy type attributes), for which we emit warnings. Depends On D111548 Reviewed By: aaron.ballman, rsmith Differential Revision: https://reviews.llvm.org/D126061
1 parent 7acc88b commit 8c7b64b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+965
-281
lines changed

‎clang/include/clang/Basic/AttributeCommonInfo.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,21 @@ class AttributeCommonInfo {
150150

151151
bool isAlignasAttribute() const {
152152
// FIXME: Use a better mechanism to determine this.
153+
// We use this in `isCXX11Attribute` below, so it _should_ only return
154+
// true for the `alignas` spelling, but it currently also returns true
155+
// for the `_Alignas` spelling, which only exists in C11. Distinguishing
156+
// between the two is important because they behave differently:
157+
// - `alignas` may only appear in the attribute-specifier-seq before
158+
// the decl-specifier-seq and is therefore associated with the
159+
// declaration.
160+
// - `_Alignas` may appear anywhere within the declaration-specifiers
161+
// and is therefore associated with the `DeclSpec`.
162+
// It's not clear how best to fix this:
163+
// - We have the necessary information in the form of the `SpellingIndex`,
164+
// but we would need to compare against AlignedAttr::Keyword_alignas,
165+
// and we can't depend on clang/AST/Attr.h here.
166+
// - We could test `getAttrName()->getName() == "alignas"`, but this is
167+
// inefficient.
153168
return getParsedKind() == AT_Aligned && isKeywordAttribute();
154169
}
155170

‎clang/include/clang/Basic/DiagnosticCommonKinds.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,8 @@ def err_opencl_unknown_type_specifier : Error<
163163

164164
def warn_unknown_attribute_ignored : Warning<
165165
"unknown attribute %0 ignored">, InGroup<UnknownAttributes>;
166+
def warn_attribute_ignored : Warning<"%0 attribute ignored">,
167+
InGroup<IgnoredAttributes>;
166168
def err_use_of_tag_name_without_tag : Error<
167169
"must use '%1' tag to refer to type %0%select{| in this scope}2">;
168170

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3354,8 +3354,6 @@ def warn_redeclaration_without_import_attribute : Warning<
33543354
def warn_dllimport_dropped_from_inline_function : Warning<
33553355
"%q0 redeclared inline; %1 attribute ignored">,
33563356
InGroup<IgnoredAttributes>;
3357-
def warn_attribute_ignored : Warning<"%0 attribute ignored">,
3358-
InGroup<IgnoredAttributes>;
33593357
def warn_nothrow_attribute_ignored : Warning<"'nothrow' attribute conflicts with"
33603358
" exception specification; attribute ignored">,
33613359
InGroup<IgnoredAttributes>;
@@ -3392,8 +3390,11 @@ def note_attribute_has_no_effect_on_compile_time_if_here : Note<
33923390
"annotating the 'if %select{constexpr|consteval}0' statement here">;
33933391
def err_decl_attribute_invalid_on_stmt : Error<
33943392
"%0 attribute cannot be applied to a statement">;
3395-
def err_stmt_attribute_invalid_on_decl : Error<
3393+
def err_attribute_invalid_on_decl : Error<
33963394
"%0 attribute cannot be applied to a declaration">;
3395+
def warn_type_attribute_deprecated_on_decl : Warning<
3396+
"applying attribute %0 to a declaration is deprecated; apply it to the type instead">,
3397+
InGroup<DeprecatedAttributes>;
33973398
def warn_declspec_attribute_ignored : Warning<
33983399
"attribute %0 is ignored, place it after "
33993400
"\"%select{class|struct|interface|union|enum}1\" to apply attribute to "

‎clang/include/clang/Parse/Parser.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2075,7 +2075,8 @@ class Parser : public CodeCompletionHandler {
20752075
SourceLocation *TrailingElseLoc = nullptr);
20762076
StmtResult ParseStatementOrDeclarationAfterAttributes(
20772077
StmtVector &Stmts, ParsedStmtContext StmtCtx,
2078-
SourceLocation *TrailingElseLoc, ParsedAttributes &Attrs);
2078+
SourceLocation *TrailingElseLoc, ParsedAttributes &DeclAttrs,
2079+
ParsedAttributes &DeclSpecAttrs);
20792080
StmtResult ParseExprStatement(ParsedStmtContext StmtCtx);
20802081
StmtResult ParseLabeledStatement(ParsedAttributes &Attrs,
20812082
ParsedStmtContext StmtCtx);
@@ -2324,15 +2325,18 @@ class Parser : public CodeCompletionHandler {
23242325

23252326
DeclGroupPtrTy ParseDeclaration(DeclaratorContext Context,
23262327
SourceLocation &DeclEnd,
2327-
ParsedAttributes &Attrs,
2328+
ParsedAttributes &DeclAttrs,
2329+
ParsedAttributes &DeclSpecAttrs,
23282330
SourceLocation *DeclSpecStart = nullptr);
23292331
DeclGroupPtrTy
23302332
ParseSimpleDeclaration(DeclaratorContext Context, SourceLocation &DeclEnd,
2331-
ParsedAttributes &Attrs, bool RequireSemi,
2333+
ParsedAttributes &DeclAttrs,
2334+
ParsedAttributes &DeclSpecAttrs, bool RequireSemi,
23322335
ForRangeInit *FRI = nullptr,
23332336
SourceLocation *DeclSpecStart = nullptr);
23342337
bool MightBeDeclarator(DeclaratorContext Context);
23352338
DeclGroupPtrTy ParseDeclGroup(ParsingDeclSpec &DS, DeclaratorContext Context,
2339+
ParsedAttributes &Attrs,
23362340
SourceLocation *DeclEnd = nullptr,
23372341
ForRangeInit *FRI = nullptr);
23382342
Decl *ParseDeclarationAfterDeclarator(Declarator &D,

‎clang/include/clang/Parse/RAIIObjectsForParser.h

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -201,9 +201,11 @@ namespace clang {
201201
ParsingDeclRAIIObject ParsingRAII;
202202

203203
public:
204-
ParsingDeclarator(Parser &P, const ParsingDeclSpec &DS, DeclaratorContext C)
205-
: Declarator(DS, C), ParsingRAII(P, &DS.getDelayedDiagnosticPool()) {
206-
}
204+
ParsingDeclarator(Parser &P, const ParsingDeclSpec &DS,
205+
const ParsedAttributes &DeclarationAttrs,
206+
DeclaratorContext C)
207+
: Declarator(DS, DeclarationAttrs, C),
208+
ParsingRAII(P, &DS.getDelayedDiagnosticPool()) {}
207209

208210
const ParsingDeclSpec &getDeclSpec() const {
209211
return static_cast<const ParsingDeclSpec&>(Declarator::getDeclSpec());
@@ -228,9 +230,10 @@ namespace clang {
228230
ParsingDeclRAIIObject ParsingRAII;
229231

230232
public:
231-
ParsingFieldDeclarator(Parser &P, const ParsingDeclSpec &DS)
232-
: FieldDeclarator(DS), ParsingRAII(P, &DS.getDelayedDiagnosticPool()) {
233-
}
233+
ParsingFieldDeclarator(Parser &P, const ParsingDeclSpec &DS,
234+
const ParsedAttributes &DeclarationAttrs)
235+
: FieldDeclarator(DS, DeclarationAttrs),
236+
ParsingRAII(P, &DS.getDelayedDiagnosticPool()) {}
234237

235238
const ParsingDeclSpec &getDeclSpec() const {
236239
return static_cast<const ParsingDeclSpec&>(D.getDeclSpec());

‎clang/include/clang/Sema/DeclSpec.h

Lines changed: 44 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1852,9 +1852,13 @@ class Declarator {
18521852
/// Indicates whether this declarator has an initializer.
18531853
unsigned HasInitializer : 1;
18541854

1855-
/// Attrs - Attributes.
1855+
/// Attributes attached to the declarator.
18561856
ParsedAttributes Attrs;
18571857

1858+
/// Attributes attached to the declaration. See also documentation for the
1859+
/// corresponding constructor parameter.
1860+
const ParsedAttributesView &DeclarationAttrs;
1861+
18581862
/// The asm label, if specified.
18591863
Expr *AsmLabel;
18601864

@@ -1893,16 +1897,40 @@ class Declarator {
18931897
friend struct DeclaratorChunk;
18941898

18951899
public:
1896-
Declarator(const DeclSpec &ds, DeclaratorContext C)
1897-
: DS(ds), Range(ds.getSourceRange()), Context(C),
1900+
/// `DS` and `DeclarationAttrs` must outlive the `Declarator`. In particular,
1901+
/// take care not to pass temporary objects for these parameters.
1902+
///
1903+
/// `DeclarationAttrs` contains [[]] attributes from the
1904+
/// attribute-specifier-seq at the beginning of a declaration, which appertain
1905+
/// to the declared entity itself. Attributes with other syntax (e.g. GNU)
1906+
/// should not be placed in this attribute list; if they occur at the
1907+
/// beginning of a declaration, they apply to the `DeclSpec` and should be
1908+
/// attached to that instead.
1909+
///
1910+
/// Here is an example of an attribute associated with a declaration:
1911+
///
1912+
/// [[deprecated]] int x, y;
1913+
///
1914+
/// This attribute appertains to all of the entities declared in the
1915+
/// declaration, i.e. `x` and `y` in this case.
1916+
Declarator(const DeclSpec &DS, const ParsedAttributesView &DeclarationAttrs,
1917+
DeclaratorContext C)
1918+
: DS(DS), Range(DS.getSourceRange()), Context(C),
18981919
InvalidType(DS.getTypeSpecType() == DeclSpec::TST_error),
18991920
GroupingParens(false), FunctionDefinition(static_cast<unsigned>(
19001921
FunctionDefinitionKind::Declaration)),
19011922
Redeclaration(false), Extension(false), ObjCIvar(false),
19021923
ObjCWeakProperty(false), InlineStorageUsed(false),
1903-
HasInitializer(false), Attrs(ds.getAttributePool().getFactory()),
1904-
AsmLabel(nullptr), TrailingRequiresClause(nullptr),
1905-
InventedTemplateParameterList(nullptr) {}
1924+
HasInitializer(false), Attrs(DS.getAttributePool().getFactory()),
1925+
DeclarationAttrs(DeclarationAttrs), AsmLabel(nullptr),
1926+
TrailingRequiresClause(nullptr),
1927+
InventedTemplateParameterList(nullptr) {
1928+
assert(llvm::all_of(DeclarationAttrs,
1929+
[](const ParsedAttr &AL) {
1930+
return AL.isStandardAttributeSyntax();
1931+
}) &&
1932+
"DeclarationAttrs may only contain [[]] attributes");
1933+
}
19061934

19071935
~Declarator() {
19081936
clear();
@@ -2531,9 +2559,14 @@ class Declarator {
25312559
const ParsedAttributes &getAttributes() const { return Attrs; }
25322560
ParsedAttributes &getAttributes() { return Attrs; }
25332561

2562+
const ParsedAttributesView &getDeclarationAttributes() const {
2563+
return DeclarationAttrs;
2564+
}
2565+
25342566
/// hasAttributes - do we contain any attributes?
25352567
bool hasAttributes() const {
2536-
if (!getAttributes().empty() || getDeclSpec().hasAttributes())
2568+
if (!getAttributes().empty() || !getDeclarationAttributes().empty() ||
2569+
getDeclSpec().hasAttributes())
25372570
return true;
25382571
for (unsigned i = 0, e = getNumTypeObjects(); i != e; ++i)
25392572
if (!getTypeObject(i).getAttrs().empty())
@@ -2615,8 +2648,10 @@ class Declarator {
26152648
struct FieldDeclarator {
26162649
Declarator D;
26172650
Expr *BitfieldSize;
2618-
explicit FieldDeclarator(const DeclSpec &DS)
2619-
: D(DS, DeclaratorContext::Member), BitfieldSize(nullptr) {}
2651+
explicit FieldDeclarator(const DeclSpec &DS,
2652+
const ParsedAttributes &DeclarationAttrs)
2653+
: D(DS, DeclarationAttrs, DeclaratorContext::Member),
2654+
BitfieldSize(nullptr) {}
26202655
};
26212656

26222657
/// Represents a C++11 virt-specifier-seq.

‎clang/include/clang/Sema/ParsedAttr.h

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,18 @@ class ParsedAttr final
651651
bool isKnownToGCC() const;
652652
bool isSupportedByPragmaAttribute() const;
653653

654+
/// Returns whether a [[]] attribute, if specified ahead of a declaration,
655+
/// should be applied to the decl-specifier-seq instead (i.e. whether it
656+
/// "slides" to the decl-specifier-seq).
657+
///
658+
/// By the standard, attributes specified before the declaration always
659+
/// appertain to the declaration, but historically we have allowed some of
660+
/// these attributes to slide to the decl-specifier-seq, so we need to keep
661+
/// supporting this behavior.
662+
///
663+
/// This may only be called if isStandardAttributeSyntax() returns true.
664+
bool slidesFromDeclToDeclSpecLegacyBehavior() const;
665+
654666
/// If the parsed attribute has a semantic equivalent, and it would
655667
/// have a semantic Spelling enumeration (due to having semantically-distinct
656668
/// spelling variations), return the value of that semantic spelling. If the
@@ -901,6 +913,12 @@ class ParsedAttributesView {
901913

902914
public:
903915
SourceRange Range;
916+
917+
static const ParsedAttributesView &none() {
918+
static const ParsedAttributesView Attrs;
919+
return Attrs;
920+
}
921+
904922
bool empty() const { return AttrList.empty(); }
905923
SizeType size() const { return AttrList.size(); }
906924
ParsedAttr &operator[](SizeType pos) { return *AttrList[pos]; }
@@ -1103,6 +1121,11 @@ class ParsedAttributes : public ParsedAttributesView {
11031121
mutable AttributePool pool;
11041122
};
11051123

1124+
/// Consumes the attributes from `First` and `Second` and concatenates them into
1125+
/// `Result`. Sets `Result.Range` to the combined range of `First` and `Second`.
1126+
void takeAndConcatenateAttrs(ParsedAttributes &First, ParsedAttributes &Second,
1127+
ParsedAttributes &Result);
1128+
11061129
/// These constants match the enumerated choices of
11071130
/// err_attribute_argument_n_type and err_attribute_argument_type.
11081131
enum AttributeArgumentNType {

‎clang/include/clang/Sema/Sema.h

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4429,8 +4429,38 @@ class Sema final {
44294429
// Helper for delayed processing of attributes.
44304430
void ProcessDeclAttributeDelayed(Decl *D,
44314431
const ParsedAttributesView &AttrList);
4432-
void ProcessDeclAttributeList(Scope *S, Decl *D, const ParsedAttributesView &AL,
4433-
bool IncludeCXX11Attributes = true);
4432+
4433+
// Options for ProcessDeclAttributeList().
4434+
struct ProcessDeclAttributeOptions {
4435+
ProcessDeclAttributeOptions()
4436+
: IncludeCXX11Attributes(true), IgnoreTypeAttributes(false) {}
4437+
4438+
ProcessDeclAttributeOptions WithIncludeCXX11Attributes(bool Val) {
4439+
ProcessDeclAttributeOptions Result = *this;
4440+
Result.IncludeCXX11Attributes = Val;
4441+
return Result;
4442+
}
4443+
4444+
ProcessDeclAttributeOptions WithIgnoreTypeAttributes(bool Val) {
4445+
ProcessDeclAttributeOptions Result = *this;
4446+
Result.IgnoreTypeAttributes = Val;
4447+
return Result;
4448+
}
4449+
4450+
// Should C++11 attributes be processed?
4451+
bool IncludeCXX11Attributes;
4452+
4453+
// Should any type attributes encountered be ignored?
4454+
// If this option is false, a diagnostic will be emitted for any type
4455+
// attributes of a kind that does not "slide" from the declaration to
4456+
// the decl-specifier-seq.
4457+
bool IgnoreTypeAttributes;
4458+
};
4459+
4460+
void ProcessDeclAttributeList(Scope *S, Decl *D,
4461+
const ParsedAttributesView &AttrList,
4462+
const ProcessDeclAttributeOptions &Options =
4463+
ProcessDeclAttributeOptions());
44344464
bool ProcessAccessDeclAttributeList(AccessSpecDecl *ASDecl,
44354465
const ParsedAttributesView &AttrList);
44364466

0 commit comments

Comments
 (0)