[clang] [Format] Configure ASSIGN_OR_RETURN macros for Google style (PR #200210)
Jesse Rosenstock via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 17 00:12:09 PDT 2026
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 <jmr at google.com>
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 <jmr at google.com>
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 <jmr at google.com>
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
More information about the cfe-commits
mailing list