Skip to content

Commit

Permalink
[clang-format] Handle variable declarations in BreakAfterAttributes (#…
Browse files Browse the repository at this point in the history
…71755)

Also cleaned up some old test cases.

Fixes #71563.
  • Loading branch information
owenca committed Nov 10, 2023
1 parent fdbff88 commit 5c36f43
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 28 deletions.
13 changes: 11 additions & 2 deletions clang/docs/ClangFormatStyleOptions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2049,8 +2049,8 @@ the configuration (without a prefix: ``Auto``).
.. _BreakAfterAttributes:

**BreakAfterAttributes** (``AttributeBreakingStyle``) :versionbadge:`clang-format 16` :ref:`<BreakAfterAttributes>`
Break after a group of C++11 attributes before a function
declaration/definition name.
Break after a group of C++11 attributes before a variable/function
(including constructor/destructor) declaration/definition name.

Possible values:

Expand All @@ -2059,6 +2059,10 @@ the configuration (without a prefix: ``Auto``).

.. code-block:: c++

[[maybe_unused]]
const int i;
[[gnu::const]] [[maybe_unused]]
int j;
[[nodiscard]]
inline int f();
[[gnu::const]] [[nodiscard]]
Expand All @@ -2069,6 +2073,9 @@ the configuration (without a prefix: ``Auto``).

.. code-block:: c++

[[maybe_unused]] const int i;
[[gnu::const]] [[maybe_unused]]
int j;
[[nodiscard]] inline int f();
[[gnu::const]] [[nodiscard]]
int g();
Expand All @@ -2078,6 +2085,8 @@ the configuration (without a prefix: ``Auto``).

.. code-block:: c++

[[maybe_unused]] const int i;
[[gnu::const]] [[maybe_unused]] int j;
[[nodiscard]] inline int f();
[[gnu::const]] [[nodiscard]] int g();

Expand Down
13 changes: 11 additions & 2 deletions clang/include/clang/Format/Format.h
Original file line number Diff line number Diff line change
Expand Up @@ -1428,6 +1428,10 @@ struct FormatStyle {
enum AttributeBreakingStyle : int8_t {
/// Always break after attributes.
/// \code
/// [[maybe_unused]]
/// const int i;
/// [[gnu::const]] [[maybe_unused]]
/// int j;
/// [[nodiscard]]
/// inline int f();
/// [[gnu::const]] [[nodiscard]]
Expand All @@ -1436,21 +1440,26 @@ struct FormatStyle {
ABS_Always,
/// Leave the line breaking after attributes as is.
/// \code
/// [[maybe_unused]] const int i;
/// [[gnu::const]] [[maybe_unused]]
/// int j;
/// [[nodiscard]] inline int f();
/// [[gnu::const]] [[nodiscard]]
/// int g();
/// \endcode
ABS_Leave,
/// Never break after attributes.
/// \code
/// [[maybe_unused]] const int i;
/// [[gnu::const]] [[maybe_unused]] int j;
/// [[nodiscard]] inline int f();
/// [[gnu::const]] [[nodiscard]] int g();
/// \endcode
ABS_Never,
};

/// Break after a group of C++11 attributes before a function
/// declaration/definition name.
/// Break after a group of C++11 attributes before a variable/function
/// (including constructor/destructor) declaration/definition name.
/// \version 16
AttributeBreakingStyle BreakAfterAttributes;

Expand Down
27 changes: 16 additions & 11 deletions clang/lib/Format/TokenAnnotator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2000,6 +2000,10 @@ class AnnotatingParser {
(!Line.MightBeFunctionDecl || Current.NestingLevel != 0)) {
Contexts.back().FirstStartOfName = &Current;
Current.setType(TT_StartOfName);
if (auto *PrevNonComment = Current.getPreviousNonComment();
PrevNonComment && PrevNonComment->is(TT_StartOfName)) {
PrevNonComment->setType(TT_Unknown);
}
} else if (Current.is(tok::semi)) {
// Reset FirstStartOfName after finding a semicolon so that a for loop
// with multiple increment statements is not confused with a for loop
Expand Down Expand Up @@ -3258,7 +3262,7 @@ static bool isFunctionDeclarationName(bool IsCpp, const FormatToken &Current,
if (Current.is(TT_FunctionDeclarationName))
return true;

if (!Current.Tok.getIdentifierInfo())
if (!Current.Tok.getIdentifierInfo() || Current.is(TT_CtorDtorDeclName))
return false;

auto skipOperatorName = [](const FormatToken *Next) -> const FormatToken * {
Expand Down Expand Up @@ -3441,29 +3445,30 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) const {
if (AlignArrayOfStructures)
calculateArrayInitializerColumnList(Line);

const bool IsCpp = Style.isCpp();
bool LineIsFunctionDeclaration = false;
FormatToken *ClosingParen = nullptr;
for (FormatToken *Tok = Current, *AfterLastAttribute = nullptr; Tok;
Tok = Tok->Next) {
if (Tok->Previous->EndsCppAttributeGroup)
AfterLastAttribute = Tok;
if (const bool IsCtorOrDtor = Tok->is(TT_CtorDtorDeclName);
IsCtorOrDtor ||
isFunctionDeclarationName(Style.isCpp(), *Tok, Line, ClosingParen)) {
if (!IsCtorOrDtor) {
LineIsFunctionDeclaration = true;
Tok->setFinalizedType(TT_FunctionDeclarationName);
}
if (AfterLastAttribute &&
if (isFunctionDeclarationName(IsCpp, *Tok, Line, ClosingParen)) {
LineIsFunctionDeclaration = true;
Tok->setFinalizedType(TT_FunctionDeclarationName);
}
if (LineIsFunctionDeclaration ||
Tok->isOneOf(TT_CtorDtorDeclName, TT_StartOfName)) {
if (IsCpp && AfterLastAttribute &&
mustBreakAfterAttributes(*AfterLastAttribute, Style)) {
AfterLastAttribute->MustBreakBefore = true;
Line.ReturnTypeWrapped = true;
if (LineIsFunctionDeclaration)
Line.ReturnTypeWrapped = true;
}
break;
}
}

if (Style.isCpp()) {
if (IsCpp) {
if (!LineIsFunctionDeclaration) {
// Annotate */&/&& in `operator` function calls as binary operators.
for (const auto *Tok = Line.First; Tok; Tok = Tok->Next) {
Expand Down
41 changes: 28 additions & 13 deletions clang/unittests/Format/FormatTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8479,18 +8479,25 @@ TEST_F(FormatTest, BreaksFunctionDeclarationsWithTrailingTokens) {
" aaaaaaaaaaaaaaaaaaaaaaaaa));");
verifyFormat("bool aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
" __attribute__((unused));");
verifyGoogleFormat(

Style = getGoogleStyle();
Style.AttributeMacros.push_back("GUARDED_BY");
verifyFormat(
"bool aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
" GUARDED_BY(aaaaaaaaaaaa);");
verifyGoogleFormat(
" GUARDED_BY(aaaaaaaaaaaa);",
Style);
verifyFormat(
"bool aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
" GUARDED_BY(aaaaaaaaaaaa);");
verifyGoogleFormat(
" GUARDED_BY(aaaaaaaaaaaa);",
Style);
verifyFormat(
"bool aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa GUARDED_BY(aaaaaaaaaaaa) =\n"
" aaaaaaaa::aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa;");
verifyGoogleFormat(
" aaaaaaaa::aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa;",
Style);
verifyFormat(
"bool aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa GUARDED_BY(aaaaaaaaaaaa) =\n"
" aaaaaaaaaaaaaaaaaaaaaaaaa;");
" aaaaaaaaaaaaaaaaaaaaaaaaa;",
Style);
}

TEST_F(FormatTest, FunctionAnnotations) {
Expand Down Expand Up @@ -26192,9 +26199,10 @@ TEST_F(FormatTest, RemoveSemicolon) {
}

TEST_F(FormatTest, BreakAfterAttributes) {
FormatStyle Style = getLLVMStyle();

constexpr StringRef Code("[[nodiscard]] inline int f(int &i);\n"
constexpr StringRef Code("[[maybe_unused]] const int i;\n"
"[[foo([[]])]] [[maybe_unused]]\n"
"int j;\n"
"[[nodiscard]] inline int f(int &i);\n"
"[[foo([[]])]] [[nodiscard]]\n"
"int g(int &i);\n"
"[[nodiscard]]\n"
Expand All @@ -26207,11 +26215,14 @@ TEST_F(FormatTest, BreakAfterAttributes) {
" return 1;\n"
"}");

FormatStyle Style = getLLVMStyle();
EXPECT_EQ(Style.BreakAfterAttributes, FormatStyle::ABS_Leave);
verifyNoChange(Code, Style);

Style.BreakAfterAttributes = FormatStyle::ABS_Never;
verifyFormat("[[nodiscard]] inline int f(int &i);\n"
verifyFormat("[[maybe_unused]] const int i;\n"
"[[foo([[]])]] [[maybe_unused]] int j;\n"
"[[nodiscard]] inline int f(int &i);\n"
"[[foo([[]])]] [[nodiscard]] int g(int &i);\n"
"[[nodiscard]] inline int f(int &i) {\n"
" i = 1;\n"
Expand All @@ -26224,7 +26235,11 @@ TEST_F(FormatTest, BreakAfterAttributes) {
Code, Style);

Style.BreakAfterAttributes = FormatStyle::ABS_Always;
verifyFormat("[[nodiscard]]\n"
verifyFormat("[[maybe_unused]]\n"
"const int i;\n"
"[[foo([[]])]] [[maybe_unused]]\n"
"int j;\n"
"[[nodiscard]]\n"
"inline int f(int &i);\n"
"[[foo([[]])]] [[nodiscard]]\n"
"int g(int &i);\n"
Expand Down

0 comments on commit 5c36f43

Please sign in to comment.