https://github.com/jmr updated https://github.com/llvm/llvm-project/pull/200210
>From b75447a5e14667466194b2301e0b9959a4dad855 Mon Sep 17 00:00:00 2001 From: Jesse Rosenstock <[email protected]> Date: Tue, 16 Jun 2026 20:36:01 +0200 Subject: [PATCH 1/3] [Format] Configure ASSIGN_OR_RETURN macros for Google style --- clang/lib/Format/Format.cpp | 14 +++ clang/unittests/Format/ConfigParseTest.cpp | 15 ++- .../Format/FormatTestMacroExpansion.cpp | 119 +++++++++++++++++- 3 files changed, 143 insertions(+), 5 deletions(-) diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index a29d62c99bb95..fbc514d36b9a2 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -2101,6 +2101,20 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) { GoogleStyle.IncludeStyle.IncludeIsMainRegex = "([-_](test|unittest))?$"; GoogleStyle.IndentCaseLabels = true; GoogleStyle.KeepEmptyLines.AtStartOfBlock = false; + + GoogleStyle.Macros.push_back("ASSIGN_OR_RETURN(a, b)=a = (b)"); + GoogleStyle.Macros.push_back( + "ASSIGN_OR_RETURN(a, b, c)=a = (b); if (x) return c"); + GoogleStyle.Macros.push_back("RETURN_IF_ERROR(expr)=if (x) return expr"); + GoogleStyle.Macros.push_back( + "ASSERT_OK_AND_ASSIGN(lhs, rexpr)=lhs = (rexpr)"); + GoogleStyle.Macros.push_back("ABSL_ASSIGN_OR_RETURN(a, b)=a = (b)"); + GoogleStyle.Macros.push_back( + "ABSL_ASSIGN_OR_RETURN(a, b, c)=a = (b); if (x) return c"); + GoogleStyle.Macros.push_back("ABSL_RETURN_IF_ERROR(expr)=if (x) return expr"); + GoogleStyle.Macros.push_back( + "ABSL_ASSERT_OK_AND_ASSIGN(lhs, rexpr)=lhs = (rexpr)"); + GoogleStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Never; GoogleStyle.ObjCSpaceAfterProperty = false; GoogleStyle.ObjCSpaceBeforeProtocolList = true; diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index eeaf5d3f66d96..7f420eab68d83 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -1099,6 +1099,20 @@ TEST(ConfigParseTest, ParsesConfiguration) { StatementAttributeLikeMacros, std::vector<std::string>({"emit", "Q_EMIT"})); + Style.Macros.clear(); + CHECK_PARSE("{Macros: [foo]}", Macros, std::vector<std::string>({"foo"})); + std::vector<std::string> GoogleMacros; + GoogleMacros.push_back("ASSIGN_OR_RETURN(a, b)=a = (b)"); + GoogleMacros.push_back("ASSIGN_OR_RETURN(a, b, c)=a = (b); if (x) return c"); + GoogleMacros.push_back("RETURN_IF_ERROR(expr)=if (x) return expr"); + GoogleMacros.push_back("ASSERT_OK_AND_ASSIGN(lhs, rexpr)=lhs = (rexpr)"); + GoogleMacros.push_back("ABSL_ASSIGN_OR_RETURN(a, b)=a = (b)"); + GoogleMacros.push_back( + "ABSL_ASSIGN_OR_RETURN(a, b, c)=a = (b); if (x) return c"); + GoogleMacros.push_back("ABSL_RETURN_IF_ERROR(expr)=if (x) return expr"); + GoogleMacros.push_back("ABSL_ASSERT_OK_AND_ASSIGN(lhs, rexpr)=lhs = (rexpr)"); + CHECK_PARSE("BasedOnStyle: Google", Macros, GoogleMacros); + Style.StatementMacros.clear(); CHECK_PARSE("StatementMacros: [QUNUSED]", StatementMacros, std::vector<std::string>{"QUNUSED"}); @@ -1106,7 +1120,6 @@ TEST(ConfigParseTest, ParsesConfiguration) { std::vector<std::string>({"QUNUSED", "QT_REQUIRE_VERSION"})); CHECK_PARSE_LIST(JavaImportGroups); - CHECK_PARSE_LIST(Macros); CHECK_PARSE_LIST(MacrosSkippedByRemoveParentheses); CHECK_PARSE_LIST(NamespaceMacros); CHECK_PARSE_LIST(ObjCPropertyAttributeOrder); diff --git a/clang/unittests/Format/FormatTestMacroExpansion.cpp b/clang/unittests/Format/FormatTestMacroExpansion.cpp index d391fe3d715c3..1232885536b64 100644 --- a/clang/unittests/Format/FormatTestMacroExpansion.cpp +++ b/clang/unittests/Format/FormatTestMacroExpansion.cpp @@ -58,10 +58,18 @@ TEST_F(FormatTestMacroExpansion, UnexpandConfiguredMacros) { verifyFormat("ASSIGN_OR_RETURN(MySomewhatLongType *variable,\n" " MySomewhatLongFunction(SomethingElse()));", Style); - verifyFormat("ASSIGN_OR_RETURN(MySomewhatLongType *variable,\n" - " MySomewhatLongFunction(SomethingElse()), " - "ReturnMe());", - Style); + verifyFormat( + "ASSIGN_OR_RETURN(MySomewhatLongType *variable,\n" + " MySomewhatLongFunction(SomethingElse()), RetMe());", + Style); + + verifyFormat( + "void f() {\n" + " ASSIGN_OR_RETURN(MySomewhatLongType* variable,\n" + " MySomewhatLongFunction(SomethingElse()));\n" + " ASSIGN_OR_RETURN(MySomewhatLongType* variable,\n" + " MySomewhatLongFunction(SomethingElse()), RetMe());", + getGoogleStyle()); verifyFormat(R"( #define MACRO(a, b) ID(a + b) @@ -301,6 +309,109 @@ TEST_F(FormatTestMacroExpansion, IndentChildrenWithinMacroCall) { Style); } +// clang-format off +TEST_F(FormatTestMacroExpansion, NestedMacrosInLambdas) { + // These are tests for regressions reported in + // https://github.com/llvm/llvm-project/pull/169037#issuecomment-4056423543 + verifyFormat( + "void f() {\n" + " RETURN_IF_ERROR(Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " a,\n" + " []() {\n" + " if (z()) {\n" + " ASSIGN_OR_RETURN(w, Wwwwwwwwwwwwwwwwwwwwwwwwwwwwwwww());\n" + " }\n" + " }));\n" + "}", + getGoogleStyle()); + + verifyFormat( + "void g() {\n" + " RETURN_IF_ERROR(q(\n" + " w,\n" + " []() {\n" + " if (z()) {\n" + " ASSIGN_OR_RETURN(w, Wwwwwwwwwwwwwwwwwwwwwwwwwwwwwwww());\n" + " }\n" + " }));\n" + "}", + getGoogleStyle()); + + verifyFormat( + "void f() {\n" + " ABSL_RETURN_IF_ERROR(\n" + " Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" + " a,\n" + " []() {\n" + " if (z()) {\n" + " ABSL_ASSIGN_OR_RETURN(w, Wwwwwwwwwwwwwwwwwwwwwwwwwwwwwwww());\n" + " }\n" + " }));\n" + "}", + getGoogleStyle()); + + verifyFormat( + "void g() {\n" + " ABSL_RETURN_IF_ERROR(q(\n" + " w,\n" + " []() {\n" + " if (z()) {\n" + " ABSL_ASSIGN_OR_RETURN(w, Wwwwwwwwwwwwwwwwwwwwwwwwwwwwwwww());\n" + " }\n" + " }));\n" + "}", + getGoogleStyle()); +} + +TEST_F(FormatTestMacroExpansion, PredefinedGoogleMacros) { + verifyFormat( + "void f() {\n" + " ASSIGN_OR_RETURN(MySomewhatLongType* variable,\n" + " MySomewhatLongFunction(SomethingElse()));\n" + " ASSIGN_OR_RETURN(MySomewhatLongType* variable,\n" + " MySomewhatLongFunction(SomethingElse()), RetMe());\n" + "}", + getGoogleStyle()); + + verifyFormat( + "void f() {\n" + " ABSL_ASSIGN_OR_RETURN(MySomewhatLongType* variable,\n" + " MySomewhatLongFunction(SomethingElse()));\n" + " ABSL_ASSIGN_OR_RETURN(MySomewhatLongType* variable,\n" + " MySomewhatLongFunction(SomethingElse()), RetMe());\n" + "}", + getGoogleStyle()); + + verifyFormat( + "void f() {\n" + " RETURN_IF_ERROR(\n" + " MySomewhatLongFunction(SomethingElse(WithManyArguments, AndSomeMore)));\n" + "}", + getGoogleStyle()); + + verifyFormat( + "void f() {\n" + " ABSL_RETURN_IF_ERROR(\n" + " MySomewhatLongFunction(SomethingElse(WithManyArguments, AndSomeMore)));\n" + "}", + getGoogleStyle()); + + verifyFormat( + "void f() {\n" + " ASSERT_OK_AND_ASSIGN(MySomewhatLongType* variable,\n" + " MySomewhatLongFunction(SomethingElse()));\n" + "}", + getGoogleStyle()); + + verifyFormat( + "void f() {\n" + " ABSL_ASSERT_OK_AND_ASSIGN(MySomewhatLongType* variable,\n" + " MySomewhatLongFunction(SomethingElse()));\n" + "}", + getGoogleStyle()); +} +// clang-format on + } // namespace } // namespace test } // namespace format >From d0567e7b83abec8e3e65581d212a3878a779c5d1 Mon Sep 17 00:00:00 2001 From: Jesse Rosenstock <[email protected]> Date: Thu, 28 May 2026 15:09:08 +0200 Subject: [PATCH 2/3] Address review comments - Initialize GoogleStyle.Macros with an initializer list instead of multiple push_back() calls. - Simplify ConfigParseTest by using CHECK_PARSE_LIST for Macros and removing the duplicate hardcoded Google macros test. Adds a Style.Macros.clear() prior to the check to prevent tests from bleeding state. - Move clang-format off/on comments inside the test functions in FormatTestMacroExpansion.cpp to avoid disabling formatting for the TEST_F macros themselves. Assisted-by: Gemini --- clang/lib/Format/Format.cpp | 22 +++++++++---------- clang/unittests/Format/ConfigParseTest.cpp | 18 ++++----------- .../Format/FormatTestMacroExpansion.cpp | 6 +++-- 3 files changed, 18 insertions(+), 28 deletions(-) diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index fbc514d36b9a2..6be166810613c 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -2102,18 +2102,16 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) { GoogleStyle.IndentCaseLabels = true; GoogleStyle.KeepEmptyLines.AtStartOfBlock = false; - GoogleStyle.Macros.push_back("ASSIGN_OR_RETURN(a, b)=a = (b)"); - GoogleStyle.Macros.push_back( - "ASSIGN_OR_RETURN(a, b, c)=a = (b); if (x) return c"); - GoogleStyle.Macros.push_back("RETURN_IF_ERROR(expr)=if (x) return expr"); - GoogleStyle.Macros.push_back( - "ASSERT_OK_AND_ASSIGN(lhs, rexpr)=lhs = (rexpr)"); - GoogleStyle.Macros.push_back("ABSL_ASSIGN_OR_RETURN(a, b)=a = (b)"); - GoogleStyle.Macros.push_back( - "ABSL_ASSIGN_OR_RETURN(a, b, c)=a = (b); if (x) return c"); - GoogleStyle.Macros.push_back("ABSL_RETURN_IF_ERROR(expr)=if (x) return expr"); - GoogleStyle.Macros.push_back( - "ABSL_ASSERT_OK_AND_ASSIGN(lhs, rexpr)=lhs = (rexpr)"); + GoogleStyle.Macros = { + "ASSIGN_OR_RETURN(a, b)=a = (b)", + "ASSIGN_OR_RETURN(a, b, c)=a = (b); if (x) return c", + "RETURN_IF_ERROR(expr)=if (x) return expr", + "ASSERT_OK_AND_ASSIGN(lhs, rexpr)=lhs = (rexpr)", + "ABSL_ASSIGN_OR_RETURN(a, b)=a = (b)", + "ABSL_ASSIGN_OR_RETURN(a, b, c)=a = (b); if (x) return c", + "ABSL_RETURN_IF_ERROR(expr)=if (x) return expr", + "ABSL_ASSERT_OK_AND_ASSIGN(lhs, rexpr)=lhs = (rexpr)", + }; GoogleStyle.ObjCBinPackProtocolList = FormatStyle::BPS_Never; GoogleStyle.ObjCSpaceAfterProperty = false; diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp index 7f420eab68d83..a333ba91aa383 100644 --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -1099,20 +1099,6 @@ TEST(ConfigParseTest, ParsesConfiguration) { StatementAttributeLikeMacros, std::vector<std::string>({"emit", "Q_EMIT"})); - Style.Macros.clear(); - CHECK_PARSE("{Macros: [foo]}", Macros, std::vector<std::string>({"foo"})); - std::vector<std::string> GoogleMacros; - GoogleMacros.push_back("ASSIGN_OR_RETURN(a, b)=a = (b)"); - GoogleMacros.push_back("ASSIGN_OR_RETURN(a, b, c)=a = (b); if (x) return c"); - GoogleMacros.push_back("RETURN_IF_ERROR(expr)=if (x) return expr"); - GoogleMacros.push_back("ASSERT_OK_AND_ASSIGN(lhs, rexpr)=lhs = (rexpr)"); - GoogleMacros.push_back("ABSL_ASSIGN_OR_RETURN(a, b)=a = (b)"); - GoogleMacros.push_back( - "ABSL_ASSIGN_OR_RETURN(a, b, c)=a = (b); if (x) return c"); - GoogleMacros.push_back("ABSL_RETURN_IF_ERROR(expr)=if (x) return expr"); - GoogleMacros.push_back("ABSL_ASSERT_OK_AND_ASSIGN(lhs, rexpr)=lhs = (rexpr)"); - CHECK_PARSE("BasedOnStyle: Google", Macros, GoogleMacros); - Style.StatementMacros.clear(); CHECK_PARSE("StatementMacros: [QUNUSED]", StatementMacros, std::vector<std::string>{"QUNUSED"}); @@ -1120,6 +1106,10 @@ TEST(ConfigParseTest, ParsesConfiguration) { std::vector<std::string>({"QUNUSED", "QT_REQUIRE_VERSION"})); CHECK_PARSE_LIST(JavaImportGroups); + // Clear Macros populated as a side effect of the BasedOnStyle: Google test + // for AttributeMacros above. + Style.Macros.clear(); + CHECK_PARSE_LIST(Macros); CHECK_PARSE_LIST(MacrosSkippedByRemoveParentheses); CHECK_PARSE_LIST(NamespaceMacros); CHECK_PARSE_LIST(ObjCPropertyAttributeOrder); diff --git a/clang/unittests/Format/FormatTestMacroExpansion.cpp b/clang/unittests/Format/FormatTestMacroExpansion.cpp index 1232885536b64..5fbcda060a69b 100644 --- a/clang/unittests/Format/FormatTestMacroExpansion.cpp +++ b/clang/unittests/Format/FormatTestMacroExpansion.cpp @@ -309,10 +309,10 @@ TEST_F(FormatTestMacroExpansion, IndentChildrenWithinMacroCall) { Style); } -// clang-format off TEST_F(FormatTestMacroExpansion, NestedMacrosInLambdas) { // These are tests for regressions reported in // https://github.com/llvm/llvm-project/pull/169037#issuecomment-4056423543 + // clang-format off verifyFormat( "void f() {\n" " RETURN_IF_ERROR(Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n" @@ -361,9 +361,11 @@ TEST_F(FormatTestMacroExpansion, NestedMacrosInLambdas) { " }));\n" "}", getGoogleStyle()); + // clang-format on } TEST_F(FormatTestMacroExpansion, PredefinedGoogleMacros) { + // clang-format off verifyFormat( "void f() {\n" " ASSIGN_OR_RETURN(MySomewhatLongType* variable,\n" @@ -409,8 +411,8 @@ TEST_F(FormatTestMacroExpansion, PredefinedGoogleMacros) { " MySomewhatLongFunction(SomethingElse()));\n" "}", getGoogleStyle()); + // clang-format on } -// clang-format on } // namespace } // namespace test >From d75aee9286d8eb3c0b3b4386c57e32347848bdb6 Mon Sep 17 00:00:00 2001 From: Jesse Rosenstock <[email protected]> Date: Tue, 16 Jun 2026 21:55:41 +0200 Subject: [PATCH 3/3] Split PredefinedGoogleMacros test into focused tests Split the monolithic PredefinedGoogleMacros test into two targeted tests to make the test coverage clearer: - GoogleMacrosFormatsAsPtrDecl: Verifies that the predefined macros cause `*` to be treated as a pointer declaration rather than multiplication (e.g., `Type*` instead of `Type *`). - GoogleMacrosShortFunctionFormattingIsBroken: Documents a pre-existing clang-format bug where macros whose expansion contains multiple statements or control flow (e.g., `if`) prevent short function merging but do not force `{` to a new line, producing `{ code\n}`. Remove clang-format off/on by using short names that fit 80 columns. Assisted-by: Gemini --- .../Format/FormatTestMacroExpansion.cpp | 71 +++++++------------ 1 file changed, 24 insertions(+), 47 deletions(-) diff --git a/clang/unittests/Format/FormatTestMacroExpansion.cpp b/clang/unittests/Format/FormatTestMacroExpansion.cpp index 5fbcda060a69b..8a01d33bc4cf4 100644 --- a/clang/unittests/Format/FormatTestMacroExpansion.cpp +++ b/clang/unittests/Format/FormatTestMacroExpansion.cpp @@ -364,54 +364,31 @@ TEST_F(FormatTestMacroExpansion, NestedMacrosInLambdas) { // clang-format on } -TEST_F(FormatTestMacroExpansion, PredefinedGoogleMacros) { - // clang-format off - verifyFormat( - "void f() {\n" - " ASSIGN_OR_RETURN(MySomewhatLongType* variable,\n" - " MySomewhatLongFunction(SomethingElse()));\n" - " ASSIGN_OR_RETURN(MySomewhatLongType* variable,\n" - " MySomewhatLongFunction(SomethingElse()), RetMe());\n" - "}", - getGoogleStyle()); - - verifyFormat( - "void f() {\n" - " ABSL_ASSIGN_OR_RETURN(MySomewhatLongType* variable,\n" - " MySomewhatLongFunction(SomethingElse()));\n" - " ABSL_ASSIGN_OR_RETURN(MySomewhatLongType* variable,\n" - " MySomewhatLongFunction(SomethingElse()), RetMe());\n" - "}", - getGoogleStyle()); - - verifyFormat( - "void f() {\n" - " RETURN_IF_ERROR(\n" - " MySomewhatLongFunction(SomethingElse(WithManyArguments, AndSomeMore)));\n" - "}", - getGoogleStyle()); - - verifyFormat( - "void f() {\n" - " ABSL_RETURN_IF_ERROR(\n" - " MySomewhatLongFunction(SomethingElse(WithManyArguments, AndSomeMore)));\n" - "}", - getGoogleStyle()); - - verifyFormat( - "void f() {\n" - " ASSERT_OK_AND_ASSIGN(MySomewhatLongType* variable,\n" - " MySomewhatLongFunction(SomethingElse()));\n" - "}", - getGoogleStyle()); +// Verify that the predefined Google macros cause `*` to be treated as a +// pointer declaration rather than multiplication. +TEST_F(FormatTestMacroExpansion, GoogleMacrosFormatsAsPtrDecl) { + verifyFormat("void f() { ASSIGN_OR_RETURN(Type* x, F()); }", + getGoogleStyle()); + verifyFormat("void f() { ABSL_ASSIGN_OR_RETURN(Type* x, F()); }", + getGoogleStyle()); + verifyFormat("void f() { ASSERT_OK_AND_ASSIGN(Type* x, F()); }", + getGoogleStyle()); + verifyFormat("void f() { ABSL_ASSERT_OK_AND_ASSIGN(Type* x, F()); }", + getGoogleStyle()); +} - verifyFormat( - "void f() {\n" - " ABSL_ASSERT_OK_AND_ASSIGN(MySomewhatLongType* variable,\n" - " MySomewhatLongFunction(SomethingElse()));\n" - "}", - getGoogleStyle()); - // clang-format on +// Macros whose expansion contains multiple statements or control flow (e.g., +// `if`) prevent short function merging but don't force `{` to a new line, +// resulting in `{ code\n}` instead of `{ code }` or `{\n code\n}`. +TEST_F(FormatTestMacroExpansion, GoogleMacrosShortFunctionFormattingIsBroken) { + // RETURN_IF_ERROR expands to `if (x) return expr`. + verifyFormat("void f() { RETURN_IF_ERROR(F());\n}", getGoogleStyle()); + verifyFormat("void f() { ABSL_RETURN_IF_ERROR(F());\n}", getGoogleStyle()); + // 3-arg ASSIGN_OR_RETURN expands to `a = (b); if (x) return c`. + verifyFormat("void f() { ASSIGN_OR_RETURN(auto x, F(), R());\n}", + getGoogleStyle()); + verifyFormat("void f() { ABSL_ASSIGN_OR_RETURN(auto x, F(), R());\n}", + getGoogleStyle()); } } // namespace _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
