[clang] [clang-format][NFC] Annotate attribute squares more effectively (PR #164052)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 17 23:26:04 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-format
Author: owenca (owenca)
<details>
<summary>Changes</summary>
Annotate left/right attribute squares distinctively and only annotate the outer pair of C++ attribute squares.
---
Full diff: https://github.com/llvm/llvm-project/pull/164052.diff
5 Files Affected:
- (modified) clang/lib/Format/ContinuationIndenter.cpp (+7-5)
- (modified) clang/lib/Format/DefinitionBlockSeparator.cpp (+1-1)
- (modified) clang/lib/Format/FormatToken.h (+2-1)
- (modified) clang/lib/Format/TokenAnnotator.cpp (+30-30)
- (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+23)
``````````diff
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
index cd4c1aabac971..d5e3765c9341f 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -433,7 +433,7 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
}
if ((startsNextParameter(Current, Style) || Previous.is(tok::semi) ||
(Previous.is(TT_TemplateCloser) && Current.is(TT_StartOfName) &&
- State.Line->First->isNot(TT_AttributeSquare) && Style.isCpp() &&
+ State.Line->First->isNot(TT_AttributeLSquare) && Style.isCpp() &&
// FIXME: This is a temporary workaround for the case where clang-format
// sets BreakBeforeParameter to avoid bin packing and this creates a
// completely unnecessary line break after a template type that isn't
@@ -1369,7 +1369,8 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
}
if (Current.is(TT_LambdaArrow) &&
Previous.isOneOf(tok::kw_noexcept, tok::kw_mutable, tok::kw_constexpr,
- tok::kw_consteval, tok::kw_static, TT_AttributeSquare)) {
+ tok::kw_consteval, tok::kw_static,
+ TT_AttributeRSquare)) {
return ContinuationIndent;
}
if ((Current.isOneOf(tok::r_brace, tok::r_square) ||
@@ -1494,9 +1495,10 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
Current.isNot(tok::l_paren) &&
!Current.endsSequence(TT_StartOfName, TT_AttributeMacro,
TT_PointerOrReference)) ||
- PreviousNonComment->isOneOf(
- TT_AttributeRParen, TT_AttributeSquare, TT_FunctionAnnotationRParen,
- TT_JavaAnnotation, TT_LeadingJavaAnnotation))) ||
+ PreviousNonComment->isOneOf(TT_AttributeRParen, TT_AttributeRSquare,
+ TT_FunctionAnnotationRParen,
+ TT_JavaAnnotation,
+ TT_LeadingJavaAnnotation))) ||
(!Style.IndentWrappedFunctionNames &&
NextNonComment->isOneOf(tok::kw_operator, TT_FunctionDeclarationName))) {
return std::max(CurrentState.LastSpace, CurrentState.Indent);
diff --git a/clang/lib/Format/DefinitionBlockSeparator.cpp b/clang/lib/Format/DefinitionBlockSeparator.cpp
index 3f4ce5fa3a428..855f2efad1e53 100644
--- a/clang/lib/Format/DefinitionBlockSeparator.cpp
+++ b/clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -169,7 +169,7 @@ void DefinitionBlockSeparator::separateBlocks(
}
}
- if (Style.isCSharp() && OperateLine->First->is(TT_AttributeSquare))
+ if (Style.isCSharp() && OperateLine->First->is(TT_AttributeLSquare))
return true;
return false;
};
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index f015d27bed6af..6f3d24aefc1ca 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -30,9 +30,10 @@ namespace format {
TYPE(ArraySubscriptLSquare) \
TYPE(AttributeColon) \
TYPE(AttributeLParen) \
+ TYPE(AttributeLSquare) \
TYPE(AttributeMacro) \
TYPE(AttributeRParen) \
- TYPE(AttributeSquare) \
+ TYPE(AttributeRSquare) \
TYPE(BinaryOperator) \
TYPE(BitFieldColon) \
TYPE(BlockComment) \
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 5b784eded4601..66e5a85941ba4 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -531,10 +531,6 @@ class AnnotatingParser {
OpeningParen.Previous->is(TT_LeadingJavaAnnotation)) {
CurrentToken->setType(TT_LeadingJavaAnnotation);
}
- if (OpeningParen.Previous &&
- OpeningParen.Previous->is(TT_AttributeSquare)) {
- CurrentToken->setType(TT_AttributeSquare);
- }
if (!HasMultipleLines)
OpeningParen.setPackingKind(PPK_Inconclusive);
@@ -722,9 +718,11 @@ class AnnotatingParser {
} else if (InsideInlineASM) {
Left->setType(TT_InlineASMSymbolicNameLSquare);
} else if (IsCpp11AttributeSpecifier) {
- Left->setType(TT_AttributeSquare);
- if (!IsInnerSquare && Left->Previous)
- Left->Previous->EndsCppAttributeGroup = false;
+ if (!IsInnerSquare) {
+ Left->setType(TT_AttributeLSquare);
+ if (Left->Previous)
+ Left->Previous->EndsCppAttributeGroup = false;
+ }
} else if (Style.isJavaScript() && Parent &&
Contexts.back().ContextKind == tok::l_brace &&
Parent->isOneOf(tok::l_brace, tok::comma)) {
@@ -733,7 +731,7 @@ class AnnotatingParser {
Parent && Parent->isOneOf(tok::l_brace, tok::comma)) {
Left->setType(TT_DesignatedInitializerLSquare);
} else if (IsCSharpAttributeSpecifier) {
- Left->setType(TT_AttributeSquare);
+ Left->setType(TT_AttributeLSquare);
} else if (CurrentToken->is(tok::r_square) && Parent &&
Parent->is(TT_TemplateCloser)) {
Left->setType(TT_ArraySubscriptLSquare);
@@ -797,13 +795,12 @@ class AnnotatingParser {
while (CurrentToken) {
if (CurrentToken->is(tok::r_square)) {
- if (IsCpp11AttributeSpecifier) {
- CurrentToken->setType(TT_AttributeSquare);
- if (!IsInnerSquare)
- CurrentToken->EndsCppAttributeGroup = true;
+ if (IsCpp11AttributeSpecifier && !IsInnerSquare) {
+ CurrentToken->setType(TT_AttributeRSquare);
+ CurrentToken->EndsCppAttributeGroup = true;
}
if (IsCSharpAttributeSpecifier) {
- CurrentToken->setType(TT_AttributeSquare);
+ CurrentToken->setType(TT_AttributeRSquare);
} else if (((CurrentToken->Next &&
CurrentToken->Next->is(tok::l_paren)) ||
(CurrentToken->Previous &&
@@ -1297,7 +1294,7 @@ class AnnotatingParser {
bool consumeToken() {
if (IsCpp) {
const auto *Prev = CurrentToken->getPreviousNonComment();
- if (Prev && Prev->is(tok::r_square) && Prev->is(TT_AttributeSquare) &&
+ if (Prev && Prev->is(TT_AttributeRSquare) &&
CurrentToken->isOneOf(tok::kw_if, tok::kw_switch, tok::kw_case,
tok::kw_default, tok::kw_for, tok::kw_while) &&
mustBreakAfterAttributes(*CurrentToken, Style)) {
@@ -2850,7 +2847,7 @@ class AnnotatingParser {
T = Tok->Previous;
continue;
}
- } else if (T->is(TT_AttributeSquare)) {
+ } else if (T->is(TT_AttributeRSquare)) {
// Handle `x = (foo *[[clang::foo]])&v;`:
if (T->MatchingParen && T->MatchingParen->Previous) {
T = T->MatchingParen->Previous;
@@ -3656,7 +3653,7 @@ static FormatToken *getFunctionName(const AnnotatedLine &Line,
for (FormatToken *Tok = Line.getFirstNonComment(), *Name = nullptr; Tok;
Tok = Tok->getNextNonComment()) {
// Skip C++11 attributes both before and after the function name.
- if (Tok->is(tok::l_square) && Tok->is(TT_AttributeSquare)) {
+ if (Tok->is(TT_AttributeLSquare)) {
Tok = Tok->MatchingParen;
if (!Tok)
break;
@@ -4328,7 +4325,7 @@ unsigned TokenAnnotator::splitPenalty(const AnnotatedLine &Line,
return 35;
if (Right.isNoneOf(TT_ObjCMethodExpr, TT_LambdaLSquare,
TT_ArrayInitializerLSquare,
- TT_DesignatedInitializerLSquare, TT_AttributeSquare)) {
+ TT_DesignatedInitializerLSquare, TT_AttributeLSquare)) {
return 500;
}
}
@@ -4808,7 +4805,7 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
if (Right.is(tok::l_square) &&
Right.isNoneOf(TT_ObjCMethodExpr, TT_LambdaLSquare,
TT_DesignatedInitializerLSquare,
- TT_StructuredBindingLSquare, TT_AttributeSquare) &&
+ TT_StructuredBindingLSquare, TT_AttributeLSquare) &&
Left.isNoneOf(tok::numeric_constant, TT_DictLiteral) &&
!(Left.isNot(tok::r_square) && Style.SpaceBeforeSquareBrackets &&
Right.is(TT_ArraySubscriptLSquare))) {
@@ -4826,7 +4823,7 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
// Space between template and attribute.
// e.g. template <typename T> [[nodiscard]] ...
- if (Left.is(TT_TemplateCloser) && Right.is(TT_AttributeSquare))
+ if (Left.is(TT_TemplateCloser) && Right.is(TT_AttributeLSquare))
return true;
// Space before parentheses common for all languages
if (Right.is(tok::l_paren)) {
@@ -4841,10 +4838,8 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
return Style.SpaceBeforeParensOptions.AfterRequiresInExpression ||
spaceRequiredBeforeParens(Right);
}
- if (Left.is(TT_AttributeRParen) ||
- (Left.is(tok::r_square) && Left.is(TT_AttributeSquare))) {
+ if (Left.isOneOf(TT_AttributeRParen, TT_AttributeRSquare))
return true;
- }
if (Left.is(TT_ForEachMacro)) {
return Style.SpaceBeforeParensOptions.AfterForeachMacros ||
spaceRequiredBeforeParens(Right);
@@ -5662,16 +5657,14 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
}
// Break after C# [...] and before public/protected/private/internal.
- if (Left.is(TT_AttributeSquare) && Left.is(tok::r_square) &&
+ if (Left.is(TT_AttributeRSquare) &&
(Right.isAccessSpecifier(/*ColonRequired=*/false) ||
Right.is(Keywords.kw_internal))) {
return true;
}
// Break between ] and [ but only when there are really 2 attributes.
- if (Left.is(TT_AttributeSquare) && Right.is(TT_AttributeSquare) &&
- Left.is(tok::r_square) && Right.is(tok::l_square)) {
+ if (Left.is(TT_AttributeRSquare) && Right.is(TT_AttributeLSquare))
return true;
- }
} else if (Style.isJavaScript()) {
// FIXME: This might apply to other languages and token kinds.
if (Right.is(tok::string_literal) && Left.is(tok::plus) && BeforeLeft &&
@@ -6090,6 +6083,11 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
return true;
}
+ if (Style.BreakAfterAttributes == FormatStyle::ABS_Leave &&
+ Left.is(TT_AttributeRSquare) && Right.NewlinesBefore > 0) {
+ return true;
+ }
+
return false;
}
@@ -6411,8 +6409,10 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
if (Right.isAttribute())
return true;
- if (Right.is(tok::l_square) && Right.is(TT_AttributeSquare))
- return Left.isNot(TT_AttributeSquare);
+ if (Right.is(TT_AttributeLSquare)) {
+ assert(Left.isNot(tok::l_square));
+ return true;
+ }
if (Left.is(tok::identifier) && Right.is(tok::string_literal))
return true;
@@ -6453,8 +6453,8 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
Left.getPrecedence() == prec::Assignment)) {
return true;
}
- if ((Left.is(TT_AttributeSquare) && Right.is(tok::l_square)) ||
- (Left.is(tok::r_square) && Right.is(TT_AttributeSquare))) {
+ if (Left.is(tok::r_square) && Right.is(TT_AttributeRSquare)) {
+ assert(Left.isNot(TT_AttributeRSquare));
return false;
}
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 1152466b0179f..f3637383a0a65 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -4232,6 +4232,29 @@ TEST_F(TokenAnnotatorTest, QtProperty) {
EXPECT_TOKEN(Tokens[12], tok::identifier, TT_QtProperty);
}
+TEST_F(TokenAnnotatorTest, AttributeSquares) {
+ auto Tokens = annotate("[[maybe_unused]] const int i;");
+ ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+ EXPECT_TOKEN(Tokens[0], tok::l_square, TT_AttributeLSquare);
+ EXPECT_TOKEN(Tokens[1], tok::l_square, TT_Unknown);
+ EXPECT_TOKEN(Tokens[3], tok::r_square, TT_Unknown);
+ EXPECT_TOKEN(Tokens[4], tok::r_square, TT_AttributeRSquare);
+ EXPECT_TRUE(Tokens[4]->EndsCppAttributeGroup);
+
+ Tokens = annotate("[[foo([[]])]] [[maybe_unused]] int j;");
+ ASSERT_EQ(Tokens.size(), 20u) << Tokens;
+ EXPECT_TOKEN(Tokens[0], tok::l_square, TT_AttributeLSquare);
+ EXPECT_TOKEN(Tokens[1], tok::l_square, TT_Unknown);
+ EXPECT_TOKEN(Tokens[9], tok::r_square, TT_Unknown);
+ EXPECT_TOKEN(Tokens[10], tok::r_square, TT_AttributeRSquare);
+ EXPECT_FALSE(Tokens[10]->EndsCppAttributeGroup);
+ EXPECT_TOKEN(Tokens[11], tok::l_square, TT_AttributeLSquare);
+ EXPECT_TOKEN(Tokens[12], tok::l_square, TT_Unknown);
+ EXPECT_TOKEN(Tokens[14], tok::r_square, TT_Unknown);
+ EXPECT_TOKEN(Tokens[15], tok::r_square, TT_AttributeRSquare);
+ EXPECT_TRUE(Tokens[15]->EndsCppAttributeGroup);
+}
+
} // namespace
} // namespace format
} // namespace clang
``````````
</details>
https://github.com/llvm/llvm-project/pull/164052
More information about the cfe-commits
mailing list