[clang-format] Google Style: disable DerivePointerAlignment.

[jyknight]

The Google C++ Style Guide is being changed to specify that spaces should go after the asterisk/ampersand, rather than permitting either before or after on a file-by-file basis.

The new requirement is:

When referring to a pointer or reference (variable declarations or
definitions, arguments, return types, template parameters, etc.),
you must not place a space before the asterisk/ampersand. Use a
space to separate the type from the declared name (if present).

The Google ObjC style is silent on this matter, but the de-facto style is not being modified at this time. So, keep DerivePointerAlignment enabled for ObjC language mode.

[llvmbot]

@llvm/pr-subscribers-clang-format

Author: James Y Knight (jyknight)

Changes

The Google C++ Style Guide is being changed to specify that spaces should go after the asterisk/ampersand, rather than permitting either before or after on a file-by-file basis.

The Google ObjC style is not being modified at this time, so keep DerivePointerAlignment enabled for ObjC language mode.

---

Full diff: https://github.com/llvm/llvm-project/pull/149602.diff

2 Files Affected:

• (modified) clang/lib/Format/Format.cpp (+2-1)
• (modified) clang/unittests/Format/FormatTest.cpp (+13-9)

diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 62feb3db0ed5e..c8462891f8290 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1753,7 +1753,7 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
   GoogleStyle.AttributeMacros.push_back("absl_nullable");
   GoogleStyle.AttributeMacros.push_back("absl_nullability_unknown");
   GoogleStyle.BreakTemplateDeclarations = FormatStyle::BTDS_Yes;
-  GoogleStyle.DerivePointerAlignment = true;
+  GoogleStyle.DerivePointerAlignment = false;
   GoogleStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Regroup;
   GoogleStyle.IncludeStyle.IncludeCategories = {{"^<ext/.*\\.h>", 2, 0, false},
                                                 {"^<.*\\.h>", 1, 0, false},
@@ -1862,6 +1862,7 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
   } else if (Language == FormatStyle::LK_ObjC) {
     GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
     GoogleStyle.ColumnLimit = 100;
+    GoogleStyle.DerivePointerAlignment = true;
     // "Regroup" doesn't work well for ObjC yet (main header heuristic,
     // relationship between ObjC standard library headers and other heades,
     // #imports, etc.)
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 0bc1c6d45656e..195d045573b4d 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -8571,10 +8571,10 @@ TEST_F(FormatTest, BreaksFunctionDeclarations) {
                "operator<<(const SomeLooooooooooooooooooooooooogType &other);");
   verifyGoogleFormat(
       "SomeLoooooooooooooooooooooooooooooogType operator>>(\n"
-      "    const SomeLooooooooogType &a, const SomeLooooooooogType &b);");
+      "    const SomeLooooooooogType& a, const SomeLooooooooogType& b);");
   verifyGoogleFormat(
       "SomeLoooooooooooooooooooooooooooooogType operator<<(\n"
-      "    const SomeLooooooooogType &a, const SomeLooooooooogType &b);");
+      "    const SomeLooooooooogType& a, const SomeLooooooooogType& b);");
 
   verifyFormat("void aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
                "    int aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa = 1);");
@@ -8583,7 +8583,7 @@ TEST_F(FormatTest, BreaksFunctionDeclarations) {
   verifyGoogleFormat(
       "typename aaaaaaaaaa<aaaaaa>::aaaaaaaaaaa\n"
       "aaaaaaaaaa<aaaaaa>::aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(\n"
-      "    bool *aaaaaaaaaaaaaaaaaa, bool *aa) {}");
+      "    bool* aaaaaaaaaaaaaaaaaa, bool* aa) {}");
   verifyGoogleFormat("template <typename T>\n"
                      "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
                      "aaaaaaaaaaaaaaaaaaaaaaa<T>::aaaaaaaaaaaaa(\n"
@@ -12287,6 +12287,7 @@ TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
       "    aaaaaaaaaaaaaaaaaaaaaaaaaaaa, *aaaaaaaaaaaaaaaaaaaaaaaaaaaaa);");
 
   verifyGoogleFormat("int const* a = &b;");
+  verifyFormat("int const* a = &b;", "int const *a = &b;", getGoogleStyle());
   verifyGoogleFormat("**outparam = 1;");
   verifyGoogleFormat("*outparam = a * b;");
   verifyGoogleFormat("int main(int argc, char** argv) {}");
@@ -12891,27 +12892,30 @@ TEST_F(FormatTest, UnderstandsEllipsis) {
 }
 
 TEST_F(FormatTest, AdaptivelyFormatsPointersAndReferences) {
+  FormatStyle Style = getLLVMStyle();
+  Style.DerivePointerAlignment = true;
+
   verifyFormat("int *a;\n"
                "int *a;\n"
                "int *a;",
                "int *a;\n"
                "int* a;\n"
                "int *a;",
-               getGoogleStyle());
+               Style);
   verifyFormat("int* a;\n"
                "int* a;\n"
                "int* a;",
                "int* a;\n"
                "int* a;\n"
                "int *a;",
-               getGoogleStyle());
+               Style);
   verifyFormat("int *a;\n"
                "int *a;\n"
                "int *a;",
                "int *a;\n"
                "int * a;\n"
                "int *  a;",
-               getGoogleStyle());
+               Style);
   verifyFormat("auto x = [] {\n"
                "  int *a;\n"
                "  int *a;\n"
@@ -12920,7 +12924,7 @@ TEST_F(FormatTest, AdaptivelyFormatsPointersAndReferences) {
                "auto x=[]{int *a;\n"
                "int * a;\n"
                "int *  a;};",
-               getGoogleStyle());
+               Style);
 }
 
 TEST_F(FormatTest, UnderstandsRvalueReferences) {
@@ -13056,7 +13060,7 @@ TEST_F(FormatTest, FormatsCasts) {
   verifyFormat("virtual void foo(char &) const;");
   verifyFormat("virtual void foo(int *a, char *) const;");
   verifyFormat("int a = sizeof(int *) + b;");
-  verifyGoogleFormat("int a = alignof(int *) + b;");
+  verifyGoogleFormat("int a = alignof(int*) + b;");
   verifyFormat("bool b = f(g<int>) && c;");
   verifyFormat("typedef void (*f)(int i) func;");
   verifyFormat("void operator++(int) noexcept;");
@@ -25419,7 +25423,7 @@ TEST_F(FormatTest, AtomicQualifier) {
   verifyFormat("struct foo {\n"
                "  int a1;\n"
                "  _Atomic(a) a2;\n"
-               "  _Atomic(_Atomic(int) *const) a3;\n"
+               "  _Atomic(_Atomic(int)* const) a3;\n"
                "};",
                Google);
   verifyFormat("_Atomic(uint64_t) a;");

[owenca]

The Google C++ Style Guide is being changed to specify that spaces should go after the asterisk/ampersand, rather than permitting either before or after on a file-by-file basis.

Can you add a link to the relevant section of the style guide?

The Google ObjC style is not being modified at this time, so keep DerivePointerAlignment enabled for ObjC language mode.

What about other languages?

[jyknight]

Can you add a link to the relevant section of the style guide?

The new policy is not yet published, but I added a link to the section which will be changed.

What about other languages?

So far as I know, this control is not relevant for languages other than C/C++/ObjC/ObjC++?

[owenca]

Can you add a link to the relevant section of the style guide?

The new policy is not yet published, but I added a link to the section which will be changed.

This patch looks good now, but IMO we should wait until the new style is published.

[jyknight]

Style Guide has been updated internally. I don't know how long it takes to make its way to the public site, so I'd like to proceed. I've pasted the updated text into the commit description above.

[owenca]

Style Guide has been updated internally. I don't know how long it takes to make its way to the public site, so I'd like to proceed. I've pasted the updated text into the commit description above.

I still think we should wait until it becomes official. WDYT @mydeveloperday @AaronBallman ?

[AaronBallman]

LGTM! Style guide changes don't seem to be live now, but I think it's fine to land the changes anyway.