arichardson created this revision.
arichardson added reviewers: JakeMerdichAMD, MyDeveloperDay.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
arichardson requested review of this revision.

While writing tests for D88227 <https://reviews.llvm.org/D88227>, I noticed 
that multi-variable declarations
such as `SomeType *qualified *a = NULL, *const *b;` were being formatted
weirdly for Left and Middle pointer alignment. This change attempts to make
this more consistent and ensures there is always a space between the */&
and the variable name.

Before (with PAS_Left):
SomeType * const *a = NULL, * const *b;
After:
SomeType* const* a = NULL, * const* b;


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88239

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6629,14 +6629,30 @@
   Style.PointerAlignment = FormatStyle::PAS_Left;
   Style.DerivePointerAlignment = false;
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
-               "    *aaaaaaaaaaaaaaaaaaaaaaaaaaaaa = aaaaaaaaaaaaaaaaaaa,\n"
-               "    *b = bbbbbbbbbbbbbbbbbbb;",
+               "    * aaaaaaaaaaaaaaaaaaaaaaaaaaaaa = aaaaaaaaaaaaaaaaaaa,\n"
+               "      * b = bbbbbbbbbbbbbbbbbbb;",
                Style);
-  verifyFormat("aaaaaaaaa *a = aaaaaaaaaaaaaaaaaaa, *b = bbbbbbbbbbbbbbbbbbb,\n"
-               "          *b = bbbbbbbbbbbbbbbbbbb, *d = ddddddddddddddddddd;",
+  verifyFormat("aaaaaaaaa* a = aaaaaaaaaaaaaaaaaaa, * b = bbbbbbbbbbbbbbbbbb,\n"
+               "           * b = bbbbbbbbbbbbbbbbbbb, * d = dddddddddddddddd;",
                Style);
   verifyFormat("vector<int*> a, b;", Style);
+  verifyFormat("for (int* p, * q; p != q; p = p->next) {\n}", Style);
+  verifyFormat("char* x, * const* y = NULL;", Style);
+  verifyFormat("char** x, ** y = NULL;", Style);
+  verifyFormat("int x, * xp = &x, & xr = x, *& xpr = xp, * const& xcpr = xp;",
+               Style);
+  Style.PointerAlignment = FormatStyle::PAS_Right;
   verifyFormat("for (int *p, *q; p != q; p = p->next) {\n}", Style);
+  verifyFormat("char *x, *const *y = NULL;", Style);
+  verifyFormat("char **x, **y = NULL;", Style);
+  verifyFormat("int x, *xp = &x, &xr = x, *&xpr = xp, *const &xcpr = xp;",
+               Style);
+  Style.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("for (int * p, * q; p != q; p = p->next) {\n}", Style);
+  verifyFormat("char * x, * const * y = NULL;", Style);
+  verifyFormat("char ** x, ** y = NULL;", Style);
+  verifyFormat("int x, * xp = &x, & xr = x, *& xpr = xp, * const & xcpr = xp;",
+               Style);
 }
 
 TEST_F(FormatTest, ConditionalExpressionsInBrackets) {
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -914,13 +914,31 @@
       }
       break;
     case tok::pipe:
-    case tok::amp:
       // | and & in declarations/type expressions represent union and
       // intersection types, respectively.
       if (Style.Language == FormatStyle::LK_JavaScript &&
           !Contexts.back().IsExpression)
         Tok->setType(TT_JsTypeOperator);
       break;
+    case tok::amp:
+      if (Style.Language == FormatStyle::LK_JavaScript &&
+          !Contexts.back().IsExpression) {
+        Tok->setType(TT_JsTypeOperator);
+        break;
+      }
+      LLVM_FALLTHROUGH; // Handle multi-variable declarations of reference type
+    case tok::star:
+      if (Style.isCpp() && Contexts.back().FirstStartOfName &&
+          Contexts.back().FirstStartOfName->PartOfMultiVariableDeclStmt) {
+        // '*' or '&' after a comma is always a pointer or reference if
+        // we are in a context declaring multiple variables.
+        FormatToken *Prev = Tok->getPreviousNonComment();
+        if (Prev && Prev->is(tok::comma)) {
+          Tok->setType(TT_PointerOrReference);
+          Line.IsMultiVariableDeclStmt = true;
+        }
+      }
+      break;
     case tok::kw_if:
     case tok::kw_while:
       if (Tok->is(tok::kw_if) && CurrentToken &&
@@ -2859,12 +2877,16 @@
       if (!TokenBeforeMatchingParen || !Left.is(TT_TypeDeclarationParen))
         return true;
     }
-    return (Left.Tok.isLiteral() ||
-            (!Left.isOneOf(TT_PointerOrReference, tok::l_paren) &&
-             (Style.PointerAlignment != FormatStyle::PAS_Left ||
-              (Line.IsMultiVariableDeclStmt &&
-               (Left.NestingLevel == 0 ||
-                (Left.NestingLevel == 1 && Line.First->is(tok::kw_for)))))));
+    if (Left.Tok.isLiteral())
+      return true;
+    // We group multiple '*'/'&' tokens for all pointer alignment settings.
+    // TODO: why is the l_paren needed?
+    if (Left.isOneOf(TT_PointerOrReference, tok::l_paren))
+      return false;
+    // We always place a space after the comma in multi-variable declarations.
+    if (Line.IsMultiVariableDeclStmt && Left.is(tok::comma))
+      return true;
+    return Style.PointerAlignment != FormatStyle::PAS_Left;
   }
   if (Right.is(TT_FunctionTypeLParen) && Left.isNot(tok::l_paren) &&
       (!Left.is(TT_PointerOrReference) ||
@@ -2880,9 +2902,7 @@
            (Right.is(tok::l_brace) && Right.is(BK_Block)) ||
            (!Right.isOneOf(TT_PointerOrReference, TT_ArraySubscriptLSquare,
                            tok::l_paren) &&
-            (Style.PointerAlignment != FormatStyle::PAS_Right &&
-             !Line.IsMultiVariableDeclStmt) &&
-            Left.Previous &&
+            Style.PointerAlignment != FormatStyle::PAS_Right && Left.Previous &&
             !Left.Previous->isOneOf(tok::l_paren, tok::coloncolon,
                                     tok::l_square));
   // Ensure right pointer alignement with ellipsis e.g. int *...P
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D88239: [clan... Alexander Richardson via Phabricator via cfe-commits

Reply via email to