Skip to content

[Sema] Keep attribute lists in the order the attributes were parsed #162714

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 9 commits into from
Oct 10, 2025
Merged

[Sema] Keep attribute lists in the order the attributes were parsed #162714

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
dfb1520
[attrs] Rename allAll, takeAllFrom etc. for clarity (NFC)
hnrklssn Oct 9, 2025
8c99cbb
[sema] Append attributes parsed later, instead of prepending
hnrklssn Oct 9, 2025
9b08278
format
hnrklssn Oct 9, 2025
9d171f9
update doc comment
hnrklssn Oct 9, 2025
cc15fc4
Rename attribute prepend/append functions (NFC)
hnrklssn Oct 9, 2025
63703ab
Rename functions again (NFC)
hnrklssn Oct 9, 2025
ecf29ba
format
hnrklssn Oct 9, 2025
285412a
Rename functions (NFC)
hnrklssn Oct 9, 2025
4e2a069
parsed-attributes -> ParsedAttributes
hnrklssn Oct 9, 2025
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
8 changes: 4 additions & 4 deletions clang/include/clang/Parse/Parser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2173,7 +2173,7 @@ class Parser : public CodeCompletionHandler {
if (Tok.is(tok::kw___attribute)) {
ParsedAttributes Attrs(AttrFactory);
ParseGNUAttributes(Attrs, LateAttrs, &D);
D.takeAttributes(Attrs);
D.takeAttributesAppend(Attrs);
}
}

Expand Down Expand Up @@ -2272,7 +2272,7 @@ class Parser : public CodeCompletionHandler {
if (isAllowedCXX11AttributeSpecifier()) {
ParsedAttributes Attrs(AttrFactory);
ParseCXX11Attributes(Attrs);
D.takeAttributes(Attrs);
D.takeAttributesAppend(Attrs);
}
}

Expand All @@ -2292,7 +2292,7 @@ class Parser : public CodeCompletionHandler {
ParsedAttributes AttrsWithRange(AttrFactory);
ParseMicrosoftAttributes(AttrsWithRange);
AttrsParsed = !AttrsWithRange.empty();
Attrs.takeAllFrom(AttrsWithRange);
Attrs.takeAllAppendingFrom(AttrsWithRange);
}
return AttrsParsed;
}
Expand Down Expand Up @@ -5175,7 +5175,7 @@ class Parser : public CodeCompletionHandler {
if (Tok.is(tok::colon)) {
ParsedAttributes Attrs(AttrFactory);
ParseHLSLAnnotations(Attrs, EndLoc, CouldBeBitField);
D.takeAttributes(Attrs);
D.takeAttributesAppend(Attrs);
return true;
}
return false;
Expand Down
14 changes: 7 additions & 7 deletions clang/include/clang/Sema/DeclSpec.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -835,16 +835,16 @@ class DeclSpec {
/// \endcode
///
void addAttributes(const ParsedAttributesView &AL) {
Attrs.addAll(AL.begin(), AL.end());
Attrs.prependAll(AL.begin(), AL.end());
}

bool hasAttributes() const { return !Attrs.empty(); }

ParsedAttributes &getAttributes() { return Attrs; }
const ParsedAttributes &getAttributes() const { return Attrs; }

void takeAttributesFrom(ParsedAttributes &attrs) {
Attrs.takeAllFrom(attrs);
void takeAttributesAppendingFrom(ParsedAttributes &attrs) {
Attrs.takeAllAppendingFrom(attrs);
}

/// Finish - This does final analysis of the declspec, issuing diagnostics for
Expand Down Expand Up @@ -2327,7 +2327,7 @@ class Declarator {
void AddTypeInfo(const DeclaratorChunk &TI, ParsedAttributes &&attrs,
SourceLocation EndLoc) {
DeclTypeInfo.push_back(TI);
DeclTypeInfo.back().getAttrs().addAll(attrs.begin(), attrs.end());
DeclTypeInfo.back().getAttrs().prependAll(attrs.begin(), attrs.end());
getAttributePool().takeAllFrom(attrs.getPool());

if (!EndLoc.isInvalid())
Expand Down Expand Up @@ -2638,7 +2638,7 @@ class Declarator {
return InventedTemplateParameterList;
}

/// takeAttributes - Takes attributes from the given parsed-attributes
/// takeAttributesAppend - Takes attributes from the given parsed-attributes
/// set and add them to this declarator.
///
/// These examples both add 3 attributes to "var":
Expand All @@ -2647,8 +2647,8 @@ class Declarator {
/// __attribute__((common,deprecated));
///
/// Also extends the range of the declarator.
void takeAttributes(ParsedAttributes &attrs) {
Attrs.takeAllFrom(attrs);
void takeAttributesAppend(ParsedAttributes &attrs) {
Attrs.takeAllAppendingFrom(attrs);

if (attrs.Range.getEnd().isValid())
SetRangeEnd(attrs.Range.getEnd());
Expand Down
16 changes: 8 additions & 8 deletions clang/include/clang/Sema/ParsedAttr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -856,19 +856,19 @@ class ParsedAttributesView {
friend class ParsedAttributesView;
};

void addAll(iterator B, iterator E) {
void prependAll(iterator B, iterator E) {
AttrList.insert(AttrList.begin(), B.I, E.I);
}

void addAll(const_iterator B, const_iterator E) {
void prependAll(const_iterator B, const_iterator E) {
AttrList.insert(AttrList.begin(), B.I, E.I);
}

void addAllAtEnd(iterator B, iterator E) {
void appendAll(iterator B, iterator E) {
AttrList.insert(AttrList.end(), B.I, E.I);
}

void addAllAtEnd(const_iterator B, const_iterator E) {
void appendAll(const_iterator B, const_iterator E) {
AttrList.insert(AttrList.end(), B.I, E.I);
}

Expand Down Expand Up @@ -943,18 +943,18 @@ class ParsedAttributes : public ParsedAttributesView {

AttributePool &getPool() const { return pool; }

void takeAllFrom(ParsedAttributes &Other) {
void takeAllPrependingFrom(ParsedAttributes &Other) {
assert(&Other != this &&
"ParsedAttributes can't take attributes from itself");
addAll(Other.begin(), Other.end());
prependAll(Other.begin(), Other.end());
Other.clearListOnly();
pool.takeAllFrom(Other.pool);
}

void takeAllAtEndFrom(ParsedAttributes &Other) {
void takeAllAppendingFrom(ParsedAttributes &Other) {
assert(&Other != this &&
"ParsedAttributes can't take attributes from itself");
addAllAtEnd(Other.begin(), Other.end());
appendAll(Other.begin(), Other.end());
Other.clearListOnly();
pool.takeAllFrom(Other.pool);
}
Expand Down
22 changes: 11 additions & 11 deletions clang/lib/Parse/ParseDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1934,12 +1934,12 @@ Parser::DeclGroupPtrTy Parser::ParseSimpleDeclaration(
bool RequireSemi, ForRangeInit *FRI, SourceLocation *DeclSpecStart) {
// Need to retain these for diagnostics before we add them to the DeclSepc.
ParsedAttributesView OriginalDeclSpecAttrs;
OriginalDeclSpecAttrs.addAll(DeclSpecAttrs.begin(), DeclSpecAttrs.end());
OriginalDeclSpecAttrs.prependAll(DeclSpecAttrs.begin(), DeclSpecAttrs.end());
OriginalDeclSpecAttrs.Range = DeclSpecAttrs.Range;

// Parse the common declaration-specifiers piece.
ParsingDeclSpec DS(*this);
DS.takeAttributesFrom(DeclSpecAttrs);
DS.takeAttributesAppendingFrom(DeclSpecAttrs);

ParsedTemplateInfo TemplateInfo;
DeclSpecContext DSContext = getDeclSpecContextFromDeclaratorContext(Context);
Expand Down Expand Up @@ -2135,7 +2135,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
// list. This ensures that we will not attempt to interpret them as statement
// attributes higher up the callchain.
ParsedAttributes LocalAttrs(AttrFactory);
LocalAttrs.takeAllFrom(Attrs);
LocalAttrs.takeAllPrependingFrom(Attrs);
ParsingDeclarator D(*this, DS, LocalAttrs, Context);
if (TemplateInfo.TemplateParams)
D.setTemplateParameterLists(*TemplateInfo.TemplateParams);
Expand Down Expand Up @@ -3462,7 +3462,7 @@ void Parser::ParseDeclarationSpecifiers(
PA.setInvalid();
}

DS.takeAttributesFrom(attrs);
DS.takeAttributesAppendingFrom(attrs);
}

// If this is not a declaration specifier token, we're done reading decl
Expand Down Expand Up @@ -3689,7 +3689,7 @@ void Parser::ParseDeclarationSpecifiers(
if (ParseImplicitInt(DS, &SS, TemplateInfo, AS, DSContext, Attrs)) {
if (!Attrs.empty()) {
AttrsLastTime = true;
attrs.takeAllFrom(Attrs);
attrs.takeAllAppendingFrom(Attrs);
}
continue;
}
Expand Down Expand Up @@ -3854,7 +3854,7 @@ void Parser::ParseDeclarationSpecifiers(
if (ParseImplicitInt(DS, nullptr, TemplateInfo, AS, DSContext, Attrs)) {
if (!Attrs.empty()) {
AttrsLastTime = true;
attrs.takeAllFrom(Attrs);
attrs.takeAllAppendingFrom(Attrs);
}
continue;
}
Expand Down Expand Up @@ -4463,7 +4463,7 @@ void Parser::ParseDeclarationSpecifiers(
// take them over and handle them here.
if (!Attributes.empty()) {
AttrsLastTime = true;
attrs.takeAllFrom(Attributes);
attrs.takeAllAppendingFrom(Attributes);
}
continue;
}
Expand Down Expand Up @@ -4830,7 +4830,7 @@ void Parser::ParseLexedCAttribute(LateParsedAttribute &LA, bool EnterScope,
ConsumeAnyToken();

if (OutAttrs) {
OutAttrs->takeAllFrom(Attrs);
OutAttrs->takeAllAppendingFrom(Attrs);
}
}

Expand Down Expand Up @@ -6122,7 +6122,7 @@ void Parser::ParseTypeQualifierListOpt(
isAllowedCXX11AttributeSpecifier()) {
ParsedAttributes Attrs(AttrFactory);
ParseCXX11Attributes(Attrs);
DS.takeAttributesFrom(Attrs);
DS.takeAttributesAppendingFrom(Attrs);
}

SourceLocation EndLoc;
Expand Down Expand Up @@ -7483,7 +7483,7 @@ void Parser::ParseParameterDeclarationClause(
// Take them so that we only apply the attributes to the first parameter.
// We have already started parsing the decl-specifier sequence, so don't
// parse any parameter-declaration pieces that precede it.
ArgDeclSpecAttrs.takeAllFrom(FirstArgAttrs);
ArgDeclSpecAttrs.takeAllPrependingFrom(FirstArgAttrs);
} else {
// Parse any C++11 attributes.
MaybeParseCXX11Attributes(ArgDeclAttrs);
Expand All @@ -7505,7 +7505,7 @@ void Parser::ParseParameterDeclarationClause(
DeclSpecContext::DSC_normal,
/*LateAttrs=*/nullptr, AllowImplicitTypename);

DS.takeAttributesFrom(ArgDeclSpecAttrs);
DS.takeAttributesAppendingFrom(ArgDeclSpecAttrs);

// Parse the declarator. This is "PrototypeContext" or
// "LambdaExprParameterContext", because we must accept either
Expand Down
8 changes: 4 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 @@ -739,7 +739,7 @@ Parser::DeclGroupPtrTy Parser::ParseUsingDeclaration(
<< FixItHint::CreateInsertionFromRange(
Tok.getLocation(), CharSourceRange::getTokenRange(Range))
<< FixItHint::CreateRemoval(Range);
Attrs.takeAllFrom(MisplacedAttrs);
Attrs.takeAllPrependingFrom(MisplacedAttrs);
}

// Maybe this is an alias-declaration.
Expand Down Expand Up @@ -787,7 +787,7 @@ Parser::DeclGroupPtrTy Parser::ParseUsingDeclaration(
// Parse (optional) attributes.
MaybeParseAttributes(PAKM_GNU | PAKM_CXX11, Attrs);
DiagnoseCXX11AttributeExtension(Attrs);
Attrs.addAll(PrefixAttrs.begin(), PrefixAttrs.end());
Attrs.prependAll(PrefixAttrs.begin(), PrefixAttrs.end());

if (InvalidDeclarator)
SkipUntil(tok::comma, tok::semi, StopBeforeMatch);
Expand Down Expand Up @@ -1948,7 +1948,7 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,

// Recover by adding misplaced attributes to the attribute list
// of the class so they can be applied on the class later.
attrs.takeAllFrom(Attributes);
attrs.takeAllAppendingFrom(Attributes);
}
}

Expand Down Expand Up @@ -2842,7 +2842,7 @@ Parser::DeclGroupPtrTy Parser::ParseCXXClassMemberDeclaration(
// decl-specifier-seq:
// Parse the common declaration-specifiers piece.
ParsingDeclSpec DS(*this, TemplateDiags);
DS.takeAttributesFrom(DeclSpecAttrs);
DS.takeAttributesAppendingFrom(DeclSpecAttrs);

if (MalformedTypeSpec)
DS.SetTypeSpecError();
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Parse/ParseExprCXX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1244,7 +1244,7 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer(
break;
}

D.takeAttributes(Attributes);
D.takeAttributesAppend(Attributes);
}

MultiParseScope TemplateParamScope(*this);
Expand Down
12 changes: 6 additions & 6 deletions clang/lib/Parse/ParseObjc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ void Parser::MaybeSkipAttributes(tok::ObjCKeywordKind Kind) {
Parser::DeclGroupPtrTy
Parser::ParseObjCAtDirectives(ParsedAttributes &DeclAttrs,
ParsedAttributes &DeclSpecAttrs) {
DeclAttrs.takeAllFrom(DeclSpecAttrs);
DeclAttrs.takeAllPrependingFrom(DeclSpecAttrs);

SourceLocation AtLoc = ConsumeToken(); // the "@"

Expand Down Expand Up @@ -1065,8 +1065,8 @@ void Parser::ParseObjCTypeQualifierList(ObjCDeclSpec &DS,

/// Take all the decl attributes out of the given list and add
/// them to the given attribute set.
static void takeDeclAttributes(ParsedAttributesView &attrs,
ParsedAttributesView &from) {
static void takeDeclAttributesAppend(ParsedAttributesView &attrs,
ParsedAttributesView &from) {
for (auto &AL : llvm::reverse(from)) {
if (!AL.isUsedAsTypeAttr()) {
from.remove(&AL);
Expand All @@ -1088,10 +1088,10 @@ static void takeDeclAttributes(ParsedAttributes &attrs,
attrs.getPool().takeAllFrom(D.getDeclSpec().getAttributePool());

// Now actually move the attributes over.
takeDeclAttributes(attrs, D.getMutableDeclSpec().getAttributes());
takeDeclAttributes(attrs, D.getAttributes());
takeDeclAttributesAppend(attrs, D.getMutableDeclSpec().getAttributes());
takeDeclAttributesAppend(attrs, D.getAttributes());
for (unsigned i = 0, e = D.getNumTypeObjects(); i != e; ++i)
takeDeclAttributes(attrs, D.getTypeObject(i).getAttrs());
takeDeclAttributesAppend(attrs, D.getTypeObject(i).getAttrs());
}

ParsedType Parser::ParseObjCTypeName(ObjCDeclSpec &DS,
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Parse/ParseStmt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,7 @@ StmtResult Parser::ParseLabeledStatement(ParsedAttributes &Attrs,
// and followed by a semicolon, GCC will reject (it appears to parse the
// attributes as part of a statement in that case). That looks like a bug.
if (!getLangOpts().CPlusPlus || Tok.is(tok::semi))
Attrs.takeAllFrom(TempAttrs);
Attrs.takeAllAppendingFrom(TempAttrs);
else {
StmtVector Stmts;
ParsedAttributes EmptyCXX11Attrs(AttrFactory);
Expand Down Expand Up @@ -2407,7 +2407,7 @@ StmtResult Parser::ParsePragmaLoopHint(StmtVector &Stmts,
Stmts, StmtCtx, TrailingElseLoc, Attrs, EmptyDeclSpecAttrs,
PrecedingLabel);

Attrs.takeAllFrom(TempAttrs);
Attrs.takeAllPrependingFrom(TempAttrs);

// Start of attribute range may already be set for some invalid input.
// See PR46336.
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Parse/ParseTemplate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclarationAfterTemplate(
ParsingDeclSpec DS(*this, &DiagsFromTParams);
DS.SetRangeStart(DeclSpecAttrs.Range.getBegin());
DS.SetRangeEnd(DeclSpecAttrs.Range.getEnd());
DS.takeAttributesFrom(DeclSpecAttrs);
DS.takeAttributesAppendingFrom(DeclSpecAttrs);

ParseDeclarationSpecifiers(DS, TemplateInfo, AS,
getDeclSpecContextFromDeclaratorContext(Context));
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Parse/Parser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1083,7 +1083,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclOrFunctionDefInternal(
"expected uninitialised source range");
DS.SetRangeStart(DeclSpecAttrs.Range.getBegin());
DS.SetRangeEnd(DeclSpecAttrs.Range.getEnd());
DS.takeAttributesFrom(DeclSpecAttrs);
DS.takeAttributesAppendingFrom(DeclSpecAttrs);

ParsedTemplateInfo TemplateInfo;
MaybeParseMicrosoftAttributes(DS.getAttributes());
Expand Down Expand Up @@ -1155,7 +1155,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclOrFunctionDefInternal(
}

DS.abort();
DS.takeAttributesFrom(Attrs);
DS.takeAttributesAppendingFrom(Attrs);

const char *PrevSpec = nullptr;
unsigned DiagID;
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/DeclSpec.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto,
[&](DeclSpec::TQ TypeQual, StringRef PrintName, SourceLocation SL) {
I.Fun.MethodQualifiers->SetTypeQual(TypeQual, SL);
});
I.Fun.MethodQualifiers->getAttributes().takeAllFrom(attrs);
I.Fun.MethodQualifiers->getAttributes().takeAllPrependingFrom(attrs);
I.Fun.MethodQualifiers->getAttributePool().takeAllFrom(attrs.getPool());
}

Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/ParsedAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ bool ParsedAttr::checkAtMostNumArgs(Sema &S, unsigned Num) const {
void clang::takeAndConcatenateAttrs(ParsedAttributes &First,
ParsedAttributes &&Second) {

First.takeAllAtEndFrom(Second);
First.takeAllAppendingFrom(Second);

if (!First.Range.getBegin().isValid())
First.Range.setBegin(Second.Range.getBegin());
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14637,7 +14637,7 @@ StmtResult Sema::ActOnCXXForRangeIdentifier(Scope *S, SourceLocation IdentLoc,

Declarator D(DS, ParsedAttributesView::none(), DeclaratorContext::ForInit);
D.SetIdentifier(Ident, IdentLoc);
D.takeAttributes(Attrs);
D.takeAttributesAppend(Attrs);

D.AddTypeInfo(DeclaratorChunk::getReference(0, IdentLoc, /*lvalue*/ false),
IdentLoc);
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Sema/internal_linkage.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ struct __attribute__((internal_linkage)) S { // expected-warning{{'internal_link
__attribute__((internal_linkage("foo"))) int g(void) {} // expected-error{{'internal_linkage' attribute takes no arguments}}

int var6 [[clang::internal_linkage]];
int var7 [[clang::internal_linkage]] __attribute__((common)); // expected-error{{'clang::internal_linkage' and 'common' attributes are not compatible}} \
int var7 [[clang::internal_linkage]] __attribute__((common)); // expected-error{{'common' and 'clang::internal_linkage' attributes are not compatible}} \
Copy link
Member

Choose a reason for hiding this comment

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

I think that the original order was better.

Copy link
Member Author

Choose a reason for hiding this comment

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

For context this is what the diagnostic looks like in the terminal:

internal_linkage.c:23:53: error: 'common' and 'clang::internal_linkage' attributes are not compatible
   23 | int var7 [[clang::internal_linkage]] __attribute__((common));
      |                                                     ^
internal_linkage.c:23:12: note: conflicting attribute is here
   23 | int var7 [[clang::internal_linkage]] __attribute__((common));
      |            ^

I think it makes sense to emit the error on the last occurring attribute, and I also think it makes sense to mention the erroring attribute first in the diagnostics message. This is also what this diagnostic almost always did already.

Would you prefer that the diagnostic was rephrased? Maybe something like attribute 'common' not compatible with attribute 'clang::internal_linkage'?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think that the rephrasing of the warning would absolutely help!

// expected-note{{conflicting attribute is here}}
__attribute__((common)) int var8 [[clang::internal_linkage]]; // expected-error{{'clang::internal_linkage' and 'common' attributes are not compatible}} \
// expected-note{{conflicting attribute is here}}
Loading