enyquist updated this revision to Diff 93092.
enyquist added a comment.

Option implemented completely in WhitespaceManager.cpp, so no overhead if the 
option isn't used. Also some minor style fixes.


Repository:
  rL LLVM

https://reviews.llvm.org/D28462

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/WhitespaceManager.cpp
  lib/Format/WhitespaceManager.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7518,8 +7518,78 @@
   verifyFormat("a or_eq 8;", Spaces);
 }
 
+TEST_F(FormatTest, AlignConsecutiveMacros) {
+  FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveAssignments = true;
+  Alignment.AlignConsecutiveDeclarations = true;
+  Alignment.AlignConsecutiveMacros = false;
+
+  verifyFormat("#define a 3\n"
+               "#define bbbb 4\n"
+               "#define ccc (5)",
+               Alignment);
+
+  verifyFormat("#define f(x) (x * x)\n"
+               "#define fff(x, y, z) (x * y + z)\n"
+               "#define ffff(x, y) (x - y)",
+               Alignment);
+
+  verifyFormat("#define foo(x, y) (x + y)\n"
+               "#define bar (5, 6)(2 + 2)",
+               Alignment);
+
+  verifyFormat("#define a 3\n"
+               "#define bbbb 4\n"
+               "#define ccc (5)\n"
+               "#define f(x) (x * x)\n"
+               "#define fff(x, y, z) (x * y + z)\n"
+               "#define ffff(x, y) (x - y)",
+               Alignment);
+
+  Alignment.AlignConsecutiveMacros = true;
+  verifyFormat("#define a    3\n"
+               "#define bbbb 4\n"
+               "#define ccc  (5)",
+               Alignment);
+
+  verifyFormat("#define f(x)         (x * x)\n"
+               "#define fff(x, y, z) (x * y + z)\n"
+               "#define ffff(x, y)   (x - y)",
+               Alignment);
+
+  verifyFormat("#define foo(x, y) (x + y)\n"
+               "#define bar       (5, 6)(2 + 2)",
+               Alignment);
+
+  verifyFormat("#define a            3\n"
+               "#define bbbb         4\n"
+               "#define ccc          (5)\n"
+               "#define f(x)         (x * x)\n"
+               "#define fff(x, y, z) (x * y + z)\n"
+               "#define ffff(x, y)   (x - y)",
+               Alignment);
+
+  verifyFormat("#define a         5\n"
+               "#define foo(x, y) (x + y)\n"
+               "#define CCC       (6)\n"
+               "auto lambda = []() {\n"
+               "  auto  ii = 0;\n"
+               "  float j  = 0;\n"
+               "  return 0;\n"
+               "};\n"
+               "int   i  = 0;\n"
+               "float i2 = 0;\n"
+               "auto  v  = type{\n"
+               "    i = 1,   //\n"
+               "    (i = 2), //\n"
+               "    i = 3    //\n"
+               "};",
+               Alignment);
+}
+
 TEST_F(FormatTest, AlignConsecutiveAssignments) {
   FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
   Alignment.AlignConsecutiveAssignments = false;
   verifyFormat("int a = 5;\n"
                "int oneTwoThree = 123;",
@@ -7660,7 +7730,10 @@
                "int j = 2;",
                Alignment);
 
-  verifyFormat("auto lambda = []() {\n"
+  verifyFormat("#define a         5\n"
+               "#define foo(x, y) (x + y)\n"
+               "#define CCC       (6)\n"
+               "auto lambda = []() {\n"
                "  auto i = 0;\n"
                "  return 0;\n"
                "};\n"
@@ -7702,6 +7775,7 @@
 
 TEST_F(FormatTest, AlignConsecutiveDeclarations) {
   FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
   Alignment.AlignConsecutiveDeclarations = false;
   verifyFormat("float const a = 5;\n"
                "int oneTwoThree = 123;",
@@ -7933,7 +8007,10 @@
                Alignment);
 
   Alignment.AlignConsecutiveAssignments = true;
-  verifyFormat("auto lambda = []() {\n"
+  verifyFormat("#define a         5\n"
+               "#define foo(x, y) (x + y)\n"
+               "#define CCC       (6)\n"
+               "auto lambda = []() {\n"
                "  auto  ii = 0;\n"
                "  float j  = 0;\n"
                "  return 0;\n"
@@ -8659,6 +8736,7 @@
   CHECK_PARSE_BOOL(AlignEscapedNewlinesLeft);
   CHECK_PARSE_BOOL(AlignOperands);
   CHECK_PARSE_BOOL(AlignTrailingComments);
+  CHECK_PARSE_BOOL(AlignConsecutiveMacros);
   CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
   CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
Index: lib/Format/WhitespaceManager.h
===================================================================
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -165,6 +165,9 @@
   /// \c EscapedNewlineColumn for the first tokens or token parts in a line.
   void calculateLineBreakInformation();
 
+  /// \brief Align consecutive C/C++ preprocessor macros over all \c Changes.
+  void alignConsecutiveMacros();
+
   /// \brief Align consecutive assignments over all \c Changes.
   void alignConsecutiveAssignments();
 
Index: lib/Format/WhitespaceManager.cpp
===================================================================
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -87,6 +87,7 @@
 
   std::sort(Changes.begin(), Changes.end(), Change::IsBeforeInFile(SourceMgr));
   calculateLineBreakInformation();
+  alignConsecutiveMacros();
   alignConsecutiveDeclarations();
   alignConsecutiveAssignments();
   alignTrailingComments();
@@ -189,7 +190,8 @@
 template <typename F>
 static void
 AlignTokenSequence(unsigned Start, unsigned End, unsigned Column, F &&Matches,
-                   SmallVector<WhitespaceManager::Change, 16> &Changes) {
+                   SmallVector<WhitespaceManager::Change, 16> &Changes,
+                   bool ConsiderScope) {
   bool FoundMatchOnLine = false;
   int Shift = 0;
 
@@ -207,34 +209,37 @@
   SmallVector<unsigned, 16> ScopeStack;
 
   for (unsigned i = Start; i != End; ++i) {
-    if (ScopeStack.size() != 0 &&
+    if (ConsiderScope && ScopeStack.size() != 0 &&
         Changes[i].nestingAndIndentLevel() <
             Changes[ScopeStack.back()].nestingAndIndentLevel())
       ScopeStack.pop_back();
 
-    if (i != Start && Changes[i].nestingAndIndentLevel() >
-                          Changes[i - 1].nestingAndIndentLevel())
+    if (ConsiderScope && i != Start &&
+        Changes[i].nestingAndIndentLevel() >
+            Changes[i - 1].nestingAndIndentLevel())
       ScopeStack.push_back(i);
 
     bool InsideNestedScope = ScopeStack.size() != 0;
 
-    if (Changes[i].NewlinesBefore > 0 && !InsideNestedScope) {
+    if (Changes[i].NewlinesBefore > 0 &&
+        (!InsideNestedScope || !ConsiderScope)) {
       Shift = 0;
       FoundMatchOnLine = false;
     }
 
     // If this is the first matching token to be aligned, remember by how many
     // spaces it has to be shifted, so the rest of the changes on the line are
     // shifted by the same amount
-    if (!FoundMatchOnLine && !InsideNestedScope && Matches(Changes[i])) {
+    if (!FoundMatchOnLine && (!ConsiderScope || !InsideNestedScope) &&
+        Matches(Changes[i])) {
       FoundMatchOnLine = true;
       Shift = Column - Changes[i].StartOfTokenColumn;
       Changes[i].Spaces += Shift;
     }
 
     // This is for function parameters that are split across multiple lines,
     // as mentioned in the ScopeStack comment.
-    if (InsideNestedScope && Changes[i].NewlinesBefore > 0) {
+    if (ConsiderScope && InsideNestedScope && Changes[i].NewlinesBefore > 0) {
       unsigned ScopeStart = ScopeStack.back();
       if (Changes[ScopeStart - 1].Tok->is(TT_FunctionDeclarationName) ||
           (ScopeStart > Start + 1 &&
@@ -279,6 +284,7 @@
 template <typename F>
 static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
                             SmallVector<WhitespaceManager::Change, 16> &Changes,
+                            bool ConsiderScope, bool ConsiderCommas,
                             unsigned StartAt) {
   unsigned MinColumn = 0;
   unsigned MaxColumn = UINT_MAX;
@@ -293,9 +299,9 @@
                                    ? Changes[StartAt].nestingAndIndentLevel()
                                    : std::pair<unsigned, unsigned>(0, 0);
 
-  // Keep track of the number of commas before the matching tokens, we will only
-  // align a sequence of matching tokens if they are preceded by the same number
-  // of commas.
+  // Keep track of the number of commas before the matching tokens. If
+  // ConsiderCommas is true, we will only align a sequence of matching tokens
+  // if they are preceded by the same number of commas.
   unsigned CommasBeforeLastMatch = 0;
   unsigned CommasBeforeMatch = 0;
 
@@ -312,7 +318,7 @@
   auto AlignCurrentSequence = [&] {
     if (StartOfSequence > 0 && StartOfSequence < EndOfSequence)
       AlignTokenSequence(StartOfSequence, EndOfSequence, MinColumn, Matches,
-                         Changes);
+                         Changes, ConsiderScope);
     MinColumn = 0;
     MaxColumn = UINT_MAX;
     StartOfSequence = 0;
@@ -337,9 +343,11 @@
 
     if (Changes[i].Tok->is(tok::comma)) {
       ++CommasBeforeMatch;
-    } else if (Changes[i].nestingAndIndentLevel() > NestingAndIndentLevel) {
+    } else if (ConsiderScope &&
+               (Changes[i].nestingAndIndentLevel() > NestingAndIndentLevel)) {
       // Call AlignTokens recursively, skipping over this scope block.
-      unsigned StoppedAt = AlignTokens(Style, Matches, Changes, i);
+      unsigned StoppedAt = AlignTokens(Style, Matches, Changes, ConsiderScope,
+                                       ConsiderCommas, i);
       i = StoppedAt - 1;
       continue;
     }
@@ -349,12 +357,12 @@
 
     // If there is more than one matching token per line, or if the number of
     // preceding commas, do not match anymore, end the sequence.
-    if (FoundMatchOnLine || CommasBeforeMatch != CommasBeforeLastMatch)
+    if (FoundMatchOnLine ||
+        (ConsiderCommas && (CommasBeforeMatch != CommasBeforeLastMatch)))
       AlignCurrentSequence();
 
     CommasBeforeLastMatch = CommasBeforeMatch;
     FoundMatchOnLine = true;
-
     if (StartOfSequence == 0)
       StartOfSequence = i;
 
@@ -366,7 +374,7 @@
 
     // If we are restricted by the maximum column width, end the sequence.
     if (ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn ||
-        CommasBeforeLastMatch != CommasBeforeMatch) {
+        (ConsiderCommas && (CommasBeforeLastMatch != CommasBeforeMatch))) {
       AlignCurrentSequence();
       StartOfSequence = i;
     }
@@ -380,6 +388,63 @@
   return i;
 }
 
+// This function heuristically determines whether 'Current' is the identifier
+// following the 'define' keyword in a simple PP macro (i.e. no parameters)
+static bool endsMacroIdentifier(const FormatToken *Current) {
+  const FormatToken *Keyword = Current->Previous;
+
+  if (!Keyword || !Current->is(tok::identifier))
+    return false;
+
+  if (!Current->Next || !Current->Next->SpacesRequiredBefore)
+    return false;
+
+  return Keyword->is(tok::pp_define);
+}
+
+// This function heuristically determines whether 'Current' is the closing
+// r_paren token for the parameter list of a function-like PP macro
+static bool endsMacroWithArgsIdentifier(const FormatToken *Current) {
+  const FormatToken *Keyword, *Param = Current->Previous;
+
+  if (!Param || !Current->is(tok::r_paren))
+    return false;
+
+  while (Param && !Param->is(tok::l_paren)) {
+    if (!Param->is(tok::identifier) && !Param->is(tok::comma))
+      return false;
+    if (!Param->Previous)
+      return false;
+
+    Param = Param->Previous;
+  }
+
+  if (!Param || Param->SpacesRequiredBefore)
+    return false;
+  if (!Param->Previous || !Param->Previous->is(tok::identifier))
+    return false;
+
+  Keyword = Param->Previous->Previous;
+  return Keyword && Keyword->is(tok::pp_define);
+}
+
+static bool endsPPIdentifier(const FormatToken *Current) {
+  return endsMacroIdentifier(Current) || endsMacroWithArgsIdentifier(Current);
+}
+
+void WhitespaceManager::alignConsecutiveMacros() {
+  if (!Style.AlignConsecutiveMacros)
+    return;
+
+  AlignTokens(Style, [&](const Change &C) {
+                       if (&C == &Changes.front())
+                         return false;
+
+                       return endsPPIdentifier((&C - 1)->Tok);
+                     },
+              Changes, false, false, /*StartAt=*/0);
+}
+
 void WhitespaceManager::alignConsecutiveAssignments() {
   if (!Style.AlignConsecutiveAssignments)
     return;
@@ -396,7 +461,7 @@
 
                 return C.Tok->is(tok::equal);
               },
-              Changes, /*StartAt=*/0);
+              Changes, true, true, /*StartAt=*/0);
 }
 
 void WhitespaceManager::alignConsecutiveDeclarations() {
@@ -417,7 +482,7 @@
                        C.Tok->is(TT_FunctionDeclarationName) ||
                        C.Tok->is(tok::kw_operator);
               },
-              Changes, /*StartAt=*/0);
+              Changes, true, true, /*StartAt=*/0);
 }
 
 void WhitespaceManager::alignTrailingComments() {
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -243,6 +243,7 @@
 
     IO.mapOptional("AccessModifierOffset", Style.AccessModifierOffset);
     IO.mapOptional("AlignAfterOpenBracket", Style.AlignAfterOpenBracket);
+    IO.mapOptional("AlignConsecutiveMacros", Style.AlignConsecutiveMacros);
     IO.mapOptional("AlignConsecutiveAssignments",
                    Style.AlignConsecutiveAssignments);
     IO.mapOptional("AlignConsecutiveDeclarations",
@@ -502,6 +503,7 @@
   LLVMStyle.AlignAfterOpenBracket = FormatStyle::BAS_Align;
   LLVMStyle.AlignOperands = true;
   LLVMStyle.AlignTrailingComments = true;
+  LLVMStyle.AlignConsecutiveMacros = false;
   LLVMStyle.AlignConsecutiveAssignments = false;
   LLVMStyle.AlignConsecutiveDeclarations = false;
   LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true;
Index: include/clang/Format/Format.h
===================================================================
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -76,6 +76,19 @@
   /// brackets.
   BracketAlignmentStyle AlignAfterOpenBracket;
 
+  /// \brief If ``true``, aligns consecutive C/C++ preprocessor macros.
+  ///
+  /// This will align C/C++ preprocessor macros of consecutive lines.
+  /// Will result in formattings like
+  /// \code
+  ///   #define SHORT_NAME       42
+  ///   #define LONGER_NAME      0x007f
+  ///   #define EVEN_LONGER_NAME (2)
+  ///   #define foo(x)           (x * x)
+  ///   #define bar(y, z)        (y + z)
+  /// \endcode
+  bool AlignConsecutiveMacros;
+
   /// \brief If ``true``, aligns consecutive assignments.
   ///
   /// This will align the assignment operators of consecutive lines. This
Index: docs/ClangFormatStyleOptions.rst
===================================================================
--- docs/ClangFormatStyleOptions.rst
+++ docs/ClangFormatStyleOptions.rst
@@ -185,6 +185,20 @@
 
 
 
+**AlignConsecutiveMacros** (``bool``)
+  If ``true``, aligns consecutive C/C++ preprocessor macros.
+
+  This will align the C/C++ preprocessor macros of consecutive lines. This
+  will result in formattings like
+
+  .. code-block:: c++
+
+    #define SHORT_NAME       42
+    #define LONGER_NAME      0x007f
+    #define EVEN_LONGER_NAME (2)
+    #define foo(x)           (x * x)
+    #define bar(y, z)        (y + z)
+
 **AlignConsecutiveAssignments** (``bool``)
   If ``true``, aligns consecutive assignments.
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to