[Sema] Keep attribute lists in the order the attributes were parsed #162714
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This renames functions that prepend or append lists of attributes, to be explicit wrt whether they prepend or append. This should make it easier to spot mistakes, and no longer makes the prepend option seem like the default.
To make sure attributes occur the order they are parsed (for consistent diagnostics), the most recently parsed attributes should be appended rather than prepended.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Henrik G. Olsson (hnrklssn) ChangesThis renames some attribute list related functions, to make callers think about whether they want to append or prepend to the list, instead of defaulting to prepending which is often not the desired behaviour (for the cases where it matters, sometimes we're just adding to an empty list). Then it adjusts some of these calls to append where they were previously prepending. This has the effect of making Full diff: https://github.com/llvm/llvm-project/pull/162714.diff 14 Files Affected:
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index e301cf1080977..5fc70ebb39930 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -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);
}
}
@@ -2272,7 +2272,7 @@ class Parser : public CodeCompletionHandler {
if (isAllowedCXX11AttributeSpecifier()) {
ParsedAttributes Attrs(AttrFactory);
ParseCXX11Attributes(Attrs);
- D.takeAttributes(Attrs);
+ D.takeAttributesAppend(Attrs);
}
}
@@ -2292,7 +2292,7 @@ class Parser : public CodeCompletionHandler {
ParsedAttributes AttrsWithRange(AttrFactory);
ParseMicrosoftAttributes(AttrsWithRange);
AttrsParsed = !AttrsWithRange.empty();
- Attrs.takeAllFrom(AttrsWithRange);
+ Attrs.takeAllFromAppend(AttrsWithRange);
}
return AttrsParsed;
}
@@ -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;
diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h
index c1a99a1fddc80..4b52fc02088a0 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -835,7 +835,7 @@ class DeclSpec {
/// \endcode
///
void addAttributes(const ParsedAttributesView &AL) {
- Attrs.addAll(AL.begin(), AL.end());
+ Attrs.addAllPrepend(AL.begin(), AL.end());
}
bool hasAttributes() const { return !Attrs.empty(); }
@@ -843,8 +843,8 @@ class DeclSpec {
ParsedAttributes &getAttributes() { return Attrs; }
const ParsedAttributes &getAttributes() const { return Attrs; }
- void takeAttributesFrom(ParsedAttributes &attrs) {
- Attrs.takeAllFrom(attrs);
+ void takeAttributesFromAppend(ParsedAttributes &attrs) {
+ Attrs.takeAllFromAppend(attrs);
}
/// Finish - This does final analysis of the declspec, issuing diagnostics for
@@ -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().addAllPrepend(attrs.begin(), attrs.end());
getAttributePool().takeAllFrom(attrs.getPool());
if (!EndLoc.isInvalid())
@@ -2638,7 +2638,7 @@ class Declarator {
return InventedTemplateParameterList;
}
- /// takeAttributes - Takes attributes from the given parsed-attributes
+ /// takeAttributesPrepend - Takes attributes from the given parsed-attributes
/// set and add them to this declarator.
///
/// These examples both add 3 attributes to "var":
@@ -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.takeAllFromAppend(attrs);
if (attrs.Range.getEnd().isValid())
SetRangeEnd(attrs.Range.getEnd());
diff --git a/clang/include/clang/Sema/ParsedAttr.h b/clang/include/clang/Sema/ParsedAttr.h
index 2edee7eaa19a1..85b75742808a9 100644
--- a/clang/include/clang/Sema/ParsedAttr.h
+++ b/clang/include/clang/Sema/ParsedAttr.h
@@ -856,19 +856,19 @@ class ParsedAttributesView {
friend class ParsedAttributesView;
};
- void addAll(iterator B, iterator E) {
+ void addAllPrepend(iterator B, iterator E) {
AttrList.insert(AttrList.begin(), B.I, E.I);
}
- void addAll(const_iterator B, const_iterator E) {
+ void addAllPrepend(const_iterator B, const_iterator E) {
AttrList.insert(AttrList.begin(), B.I, E.I);
}
- void addAllAtEnd(iterator B, iterator E) {
+ void addAllAppend(iterator B, iterator E) {
AttrList.insert(AttrList.end(), B.I, E.I);
}
- void addAllAtEnd(const_iterator B, const_iterator E) {
+ void addAllAppend(const_iterator B, const_iterator E) {
AttrList.insert(AttrList.end(), B.I, E.I);
}
@@ -943,18 +943,18 @@ class ParsedAttributes : public ParsedAttributesView {
AttributePool &getPool() const { return pool; }
- void takeAllFrom(ParsedAttributes &Other) {
+ void takeAllFromPrepend(ParsedAttributes &Other) {
assert(&Other != this &&
"ParsedAttributes can't take attributes from itself");
- addAll(Other.begin(), Other.end());
+ addAllPrepend(Other.begin(), Other.end());
Other.clearListOnly();
pool.takeAllFrom(Other.pool);
}
- void takeAllAtEndFrom(ParsedAttributes &Other) {
+ void takeAllFromAppend(ParsedAttributes &Other) {
assert(&Other != this &&
"ParsedAttributes can't take attributes from itself");
- addAllAtEnd(Other.begin(), Other.end());
+ addAllAppend(Other.begin(), Other.end());
Other.clearListOnly();
pool.takeAllFrom(Other.pool);
}
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index d6cd7eb8c2c3d..a6dd47ba85409 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -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.addAllPrepend(DeclSpecAttrs.begin(), DeclSpecAttrs.end());
OriginalDeclSpecAttrs.Range = DeclSpecAttrs.Range;
// Parse the common declaration-specifiers piece.
ParsingDeclSpec DS(*this);
- DS.takeAttributesFrom(DeclSpecAttrs);
+ DS.takeAttributesFromAppend(DeclSpecAttrs);
ParsedTemplateInfo TemplateInfo;
DeclSpecContext DSContext = getDeclSpecContextFromDeclaratorContext(Context);
@@ -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.takeAllFromPrepend(Attrs);
ParsingDeclarator D(*this, DS, LocalAttrs, Context);
if (TemplateInfo.TemplateParams)
D.setTemplateParameterLists(*TemplateInfo.TemplateParams);
@@ -3462,7 +3462,7 @@ void Parser::ParseDeclarationSpecifiers(
PA.setInvalid();
}
- DS.takeAttributesFrom(attrs);
+ DS.takeAttributesFromAppend(attrs);
}
// If this is not a declaration specifier token, we're done reading decl
@@ -3689,7 +3689,7 @@ void Parser::ParseDeclarationSpecifiers(
if (ParseImplicitInt(DS, &SS, TemplateInfo, AS, DSContext, Attrs)) {
if (!Attrs.empty()) {
AttrsLastTime = true;
- attrs.takeAllFrom(Attrs);
+ attrs.takeAllFromAppend(Attrs);
}
continue;
}
@@ -3854,7 +3854,7 @@ void Parser::ParseDeclarationSpecifiers(
if (ParseImplicitInt(DS, nullptr, TemplateInfo, AS, DSContext, Attrs)) {
if (!Attrs.empty()) {
AttrsLastTime = true;
- attrs.takeAllFrom(Attrs);
+ attrs.takeAllFromAppend(Attrs);
}
continue;
}
@@ -4463,7 +4463,7 @@ void Parser::ParseDeclarationSpecifiers(
// take them over and handle them here.
if (!Attributes.empty()) {
AttrsLastTime = true;
- attrs.takeAllFrom(Attributes);
+ attrs.takeAllFromAppend(Attributes);
}
continue;
}
@@ -4830,7 +4830,7 @@ void Parser::ParseLexedCAttribute(LateParsedAttribute &LA, bool EnterScope,
ConsumeAnyToken();
if (OutAttrs) {
- OutAttrs->takeAllFrom(Attrs);
+ OutAttrs->takeAllFromAppend(Attrs);
}
}
@@ -6122,7 +6122,7 @@ void Parser::ParseTypeQualifierListOpt(
isAllowedCXX11AttributeSpecifier()) {
ParsedAttributes Attrs(AttrFactory);
ParseCXX11Attributes(Attrs);
- DS.takeAttributesFrom(Attrs);
+ DS.takeAttributesFromAppend(Attrs);
}
SourceLocation EndLoc;
@@ -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.takeAllFromPrepend(FirstArgAttrs);
} else {
// Parse any C++11 attributes.
MaybeParseCXX11Attributes(ArgDeclAttrs);
@@ -7505,7 +7505,7 @@ void Parser::ParseParameterDeclarationClause(
DeclSpecContext::DSC_normal,
/*LateAttrs=*/nullptr, AllowImplicitTypename);
- DS.takeAttributesFrom(ArgDeclSpecAttrs);
+ DS.takeAttributesFromAppend(ArgDeclSpecAttrs);
// Parse the declarator. This is "PrototypeContext" or
// "LambdaExprParameterContext", because we must accept either
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 19f94122f151e..9c7e3a9e56901 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -739,7 +739,7 @@ Parser::DeclGroupPtrTy Parser::ParseUsingDeclaration(
<< FixItHint::CreateInsertionFromRange(
Tok.getLocation(), CharSourceRange::getTokenRange(Range))
<< FixItHint::CreateRemoval(Range);
- Attrs.takeAllFrom(MisplacedAttrs);
+ Attrs.takeAllFromPrepend(MisplacedAttrs);
}
// Maybe this is an alias-declaration.
@@ -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.addAllPrepend(PrefixAttrs.begin(), PrefixAttrs.end());
if (InvalidDeclarator)
SkipUntil(tok::comma, tok::semi, StopBeforeMatch);
@@ -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.takeAllFromAppend(Attributes);
}
}
@@ -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.takeAttributesFromAppend(DeclSpecAttrs);
if (MalformedTypeSpec)
DS.SetTypeSpecError();
diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index a2c69578d5087..cb0d2a000994a 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -1244,7 +1244,7 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer(
break;
}
- D.takeAttributes(Attributes);
+ D.takeAttributesAppend(Attributes);
}
MultiParseScope TemplateParamScope(*this);
diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp
index a64fb02294c5a..c2dbc4edf0c3c 100644
--- a/clang/lib/Parse/ParseObjc.cpp
+++ b/clang/lib/Parse/ParseObjc.cpp
@@ -43,7 +43,7 @@ void Parser::MaybeSkipAttributes(tok::ObjCKeywordKind Kind) {
Parser::DeclGroupPtrTy
Parser::ParseObjCAtDirectives(ParsedAttributes &DeclAttrs,
ParsedAttributes &DeclSpecAttrs) {
- DeclAttrs.takeAllFrom(DeclSpecAttrs);
+ DeclAttrs.takeAllFromPrepend(DeclSpecAttrs);
SourceLocation AtLoc = ConsumeToken(); // the "@"
@@ -1065,7 +1065,7 @@ 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,
+static void takeDeclAttributesAppend(ParsedAttributesView &attrs,
ParsedAttributesView &from) {
for (auto &AL : llvm::reverse(from)) {
if (!AL.isUsedAsTypeAttr()) {
@@ -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,
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 2e7af1219547e..7ac0d335a6555 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -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.takeAllFromAppend(TempAttrs);
else {
StmtVector Stmts;
ParsedAttributes EmptyCXX11Attrs(AttrFactory);
@@ -2407,7 +2407,7 @@ StmtResult Parser::ParsePragmaLoopHint(StmtVector &Stmts,
Stmts, StmtCtx, TrailingElseLoc, Attrs, EmptyDeclSpecAttrs,
PrecedingLabel);
- Attrs.takeAllFrom(TempAttrs);
+ Attrs.takeAllFromPrepend(TempAttrs);
// Start of attribute range may already be set for some invalid input.
// See PR46336.
diff --git a/clang/lib/Parse/ParseTemplate.cpp b/clang/lib/Parse/ParseTemplate.cpp
index 74aff0b804057..b9f691597a736 100644
--- a/clang/lib/Parse/ParseTemplate.cpp
+++ b/clang/lib/Parse/ParseTemplate.cpp
@@ -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.takeAttributesFromAppend(DeclSpecAttrs);
ParseDeclarationSpecifiers(DS, TemplateInfo, AS,
getDeclSpecContextFromDeclaratorContext(Context));
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index a17398b84c6a6..e13bfacadd5c2 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -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.takeAttributesFromAppend(DeclSpecAttrs);
ParsedTemplateInfo TemplateInfo;
MaybeParseMicrosoftAttributes(DS.getAttributes());
@@ -1155,7 +1155,7 @@ Parser::DeclGroupPtrTy Parser::ParseDeclOrFunctionDefInternal(
}
DS.abort();
- DS.takeAttributesFrom(Attrs);
+ DS.takeAttributesFromAppend(Attrs);
const char *PrevSpec = nullptr;
unsigned DiagID;
diff --git a/clang/lib/Sema/DeclSpec.cpp b/clang/lib/Sema/DeclSpec.cpp
index 8756ce5f0d850..f086c5d489f4c 100644
--- a/clang/lib/Sema/DeclSpec.cpp
+++ b/clang/lib/Sema/DeclSpec.cpp
@@ -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().takeAllFromPrepend(attrs);
I.Fun.MethodQualifiers->getAttributePool().takeAllFrom(attrs.getPool());
}
diff --git a/clang/lib/Sema/ParsedAttr.cpp b/clang/lib/Sema/ParsedAttr.cpp
index 294f88eae931c..42fa2af65b04f 100644
--- a/clang/lib/Sema/ParsedAttr.cpp
+++ b/clang/lib/Sema/ParsedAttr.cpp
@@ -304,7 +304,7 @@ bool ParsedAttr::checkAtMostNumArgs(Sema &S, unsigned Num) const {
void clang::takeAndConcatenateAttrs(ParsedAttributes &First,
ParsedAttributes &&Second) {
- First.takeAllAtEndFrom(Second);
+ First.takeAllFromAppend(Second);
if (!First.Range.getBegin().isValid())
First.Range.setBegin(Second.Range.getBegin());
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 6eaf7b9435491..24d80c3c08a96 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -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);
diff --git a/clang/test/Sema/internal_linkage.c b/clang/test/Sema/internal_linkage.c
index a1bff73fb6620..4ea8599039c14 100644
--- a/clang/test/Sema/internal_linkage.c
+++ b/clang/test/Sema/internal_linkage.c
@@ -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}} \
// 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}}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
compnerd
reviewed
Oct 9, 2025
|
|
||
| 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}} \ |
There was a problem hiding this comment.
I think that the original order was better.
There was a problem hiding this comment.
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'?
There was a problem hiding this comment.
Yeah, I think that the rephrasing of the warning would absolutely help!
clang/include/clang/Sema/DeclSpec.h
Outdated
|
|
||
| void takeAttributesFrom(ParsedAttributes &attrs) { | ||
| Attrs.takeAllFrom(attrs); | ||
| void takeAttributesFromAppend(ParsedAttributes &attrs) { |
There was a problem hiding this comment.
Does it make sense to inline the operation? i.e. takeAttributesAppendingFrom
There was a problem hiding this comment.
Sure. Also renamed addAllAppend and addAllPrepend to appendAll and prependAll.
addAll[Prepend/Append] -> [prepend/append]All takeAllFrom[Prepend/Append] -> takeAll[Prepend/Append]From takeAttributesFromAppend -> takeAttributesAppendFrom
Rename from AppendFrom/PrependFrom to AppendingFrom/PrependingFrom.
compnerd
reviewed
Oct 9, 2025
takeAttributesAppend -> takeAttributesAppending prependAll -> prepend appendAll -> append
compnerd
approved these changes
Oct 9, 2025
|
Failures in |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/37503 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/15169 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/140/builds/32182 Here is the relevant piece of the build log for the reference |
|
On a closer look, it seems to have been related after all. Fixing! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/46/builds/24641 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/3/builds/23209 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/17303 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/25800 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/25723 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/28787 Here is the relevant piece of the build log for the reference |
|
Fix here: #162785 |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/108/builds/18599 Here is the relevant piece of the build log for the reference |
hnrklssn
added a commit
that referenced
this pull request
Oct 10, 2025
This test was broken by #162714. Previously the `[[clang::opencl_private]]` attribute was applied first, resulting in the clashing `__global` attribute being ignored. Now that `__global` is applied first, it results in an error, and then `[[clang::opencl_private]]` is applied, resulting in another error.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/55/builds/18431 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/23/builds/14548 Here is the relevant piece of the build log for the reference |
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Oct 10, 2025
This test was broken by llvm/llvm-project#162714. Previously the `[[clang::opencl_private]]` attribute was applied first, resulting in the clashing `__global` attribute being ignored. Now that `__global` is applied first, it results in an error, and then `[[clang::opencl_private]]` is applied, resulting in another error.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/85/builds/14387 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/199/builds/6448 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/41/builds/9510 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/24/builds/13513 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/15820 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/94/builds/11625 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This renames some attribute list related functions, to make callers think about whether they want to append or prepend to the list, instead of defaulting to prepending which is often not the desired behaviour (for the cases where it matters, sometimes we're just adding to an empty list). Then it adjusts some of these calls to append where they were previously prepending. This has the effect of making
err_attributes_are_not_compatibleconsistent in emitting diagnostics as<new-attr> and <existing-attr> are not compatible, regardless of the syntax used to apply the attributes.