Revert "[clang-format] Fix AlignConsecutive on PP blocks"

[sylvestre.ledru]

Summary

This reverts commit b2eb439317576ce718193763c12bff9fccdfc166.

Caused the regression:
https://bugs.llvm.org/show_bug.cgi?id=47589

[lebedev.ri]

clang/unittests/Format/FormatTest.cpp

12255

Passing-by comment: you probably want to keep the tests though?

[sylvestre.ledru]

clang/unittests/Format/FormatTest.cpp

12255

Sure, i will keep that
and add a test case to avoid breaking what we did

[MyDeveloperDay]

LGTM, only keep the tests if they remain passing, otherwise perhaps @JakeMerdichAMD can add them back in.

Could you add a test for the thing that broke to keep us honest?

@JakeMerdichAMD when you get time, swing back round again with your fix because I think this was a good change (apart of the issues)

[sylvestre.ledru]

Add the test which was failing + add the previous tests (but disabled)

[MyDeveloperDay]

LGTM

clang/lib/Format/FormatToken.h

 class TokenRole;
 class AnnotatedLine;
 
 /// A wrapper around a \c Token storing information about the
 /// whitespace characters preceding it.
 struct FormatToken {
   FormatToken()
       : HasUnescapedNewline(false), IsMultiline(false), IsFirst(false),
-        MustBreakBefore(false), MustBreakAlignBefore(false),
-        IsUnterminatedLiteral(false), CanBreakBefore(false),
-        ClosesTemplateDeclaration(false), StartsBinaryExpression(false),
-        EndsBinaryExpression(false), PartOfMultiVariableDeclStmt(false),
-        ContinuesLineCommentSection(false), Finalized(false),
-        BlockKind(BK_Unknown), Decision(FD_Unformatted),
+        MustBreakBefore(false), IsUnterminatedLiteral(false),
+        CanBreakBefore(false), ClosesTemplateDeclaration(false),
+        StartsBinaryExpression(false), EndsBinaryExpression(false),
+        PartOfMultiVariableDeclStmt(false), ContinuesLineCommentSection(false),
+        Finalized(false), BlockKind(BK_Unknown), Decision(FD_Unformatted),
         PackingKind(PPK_Inconclusive), Type(TT_Unknown) {}
 
   /// The \c Token.
   Token Tok;
 
   /// The raw text of the token.
   ///
   /// Contains the raw token text without leading whitespace and without leading
struct FormatToken {
   unsigned IsFirst : 1;
 
   /// Whether there must be a line break before this token.
   ///
   /// This happens for example when a preprocessor directive ended directly
   /// before the token.
   unsigned MustBreakBefore : 1;
 
-  /// Whether to not align across this token
-  ///
-  /// This happens for example when a preprocessor directive ended directly
-  /// before the token, but very rarely otherwise.
-  unsigned MustBreakAlignBefore : 1;
-
   /// Set to \c true if this token is an unterminated literal.
   unsigned IsUnterminatedLiteral : 1;
 
   /// \c true if it is allowed to break before this token.
   unsigned CanBreakBefore : 1;
 
   /// \c true if this is the ">" of "template<..>".
   unsigned ClosesTemplateDeclaration : 1;

clang/lib/Format/UnwrappedLineParser.cpp

while (FormatTok->getType() == TT_ConflictStart ||
         conditionalCompilationStart(/*Unreachable=*/false);
       } else if (FormatTok->getType() == TT_ConflictAlternative) {
         conditionalCompilationAlternative();
       } else if (FormatTok->getType() == TT_ConflictEnd) {
         conditionalCompilationEnd();
       }
       FormatTok = Tokens->getNextToken();
       FormatTok->MustBreakBefore = true;
-      FormatTok->MustBreakAlignBefore = true;
     }
 
     if (!PPStack.empty() && (PPStack.back().Kind == PP_Unreachable) &&
         !Line->InPPDirective) {
       continue;
     }
 
     if (!FormatTok->Tok.is(tok::comment)) {
       distributeComments(Comments, FormatTok);
       Comments.clear();
       return;
     }
 
     Comments.push_back(FormatTok);
   } while (!eof());
 
   distributeComments(Comments, nullptr);
   Comments.clear();
 }
 
 void UnwrappedLineParser::pushToken(FormatToken *Tok) {
   Line->Tokens.push_back(UnwrappedLineNode(Tok));
   if (MustBreakBeforeNextToken) {
     Line->Tokens.back().Tok->MustBreakBefore = true;
-    Line->Tokens.back().Tok->MustBreakAlignBefore = true;
     MustBreakBeforeNextToken = false;
   }
 }
 
 } // end namespace format
 } // end namespace clang

clang/lib/Format/WhitespaceManager.cpp

static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
   unsigned i = StartAt;
   for (unsigned e = Changes.size(); i != e; ++i) {
     if (Changes[i].indentAndNestingLevel() < IndentAndNestingLevel)
       break;
 
     if (Changes[i].NewlinesBefore != 0) {
       CommasBeforeMatch = 0;
       EndOfSequence = i;
-      // If there is a blank line, there is a forced-align-break (eg,
-      // preprocessor), or if the last line didn't contain any matching token,
-      // the sequence ends here.
-      if (Changes[i].NewlinesBefore > 1 ||
-          Changes[i].Tok->MustBreakAlignBefore || !FoundMatchOnLine)
+      // If there is a blank line, or if the last line didn't contain any
+      // matching token, the sequence ends here.
+      if (Changes[i].NewlinesBefore > 1 || !FoundMatchOnLine)
         AlignCurrentSequence();
 
       FoundMatchOnLine = false;
     }
 
     if (Changes[i].Tok->is(tok::comma)) {
       ++CommasBeforeMatch;
     } else if (Changes[i].indentAndNestingLevel() > IndentAndNestingLevel) {
void WhitespaceManager::alignTrailingComments() {
   unsigned MaxColumn = UINT_MAX;
   unsigned StartOfSequence = 0;
   bool BreakBeforeNext = false;
   unsigned Newlines = 0;
   for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
     if (Changes[i].StartOfBlockComment)
       continue;
     Newlines += Changes[i].NewlinesBefore;
-    if (Changes[i].Tok->MustBreakAlignBefore)
-      BreakBeforeNext = true;
     if (!Changes[i].IsTrailingComment)
       continue;
 
     unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn;
     unsigned ChangeMaxColumn;
 
     if (Style.ColumnLimit == 0)
       ChangeMaxColumn = UINT_MAX;

clang/unittests/Format/FormatTest.cpp

verifyFormat("int a   = 5;\n"
                Alignment);
   verifyFormat("int oneTwoThree = 123;\n"
                "int oneTwo      = 12;\n"
                "method();\n",
                Alignment);
   verifyFormat("int oneTwoThree = 123; // comment\n"
                "int oneTwo      = 12;  // comment",
                Alignment);
 
   // Bug 25167
+  /* Uncomment when fixed
     verifyFormat("#if A\n"
                  "#else\n"
                  "int aaaaaaaa = 12;\n"
                  "#endif\n"
                  "#if B\n"
                  "#else\n"
                  "int a = 12;\n"
                  "#endif\n",
                  Alignment);
     verifyFormat("enum foo {\n"
                  "#if A\n"
                  "#else\n"
                  "  aaaaaaaa = 12;\n"
                  "#endif\n"
                  "#if B\n"
                  "#else\n"
                  "  a = 12;\n"
                  "#endif\n"
                  "};\n",
                  Alignment);
+  */
 
   EXPECT_EQ("int a = 5;\n"
             "\n"
             "int oneTwoThree = 123;",
             format("int a       = 5;\n"
                    "\n"
                    "int oneTwoThree= 123;",
                    Alignment));

clang/unittests/Format/FormatTestComments.cpp

EXPECT_EQ("int a; // line about a\n"
             format("int a; // line about a\n"
                    "       // line 1 about b\n"
                    "       // line 2 about b\n"
                    "       long b;",
                    getLLVMStyleWithColumns(80)));
 
   // Checks an edge case in preprocessor handling.
   // These comments should *not* be aligned
-  EXPECT_EQ(
+  EXPECT_NE( // change for EQ when fixed
       "#if FOO\n"
       "#else\n"
       "long a; // Line about a\n"
       "#endif\n"
       "#if BAR\n"
       "#else\n"
       "long b_long_name; // Line about b\n"
       "#endif\n",
       format("#if FOO\n"
              "#else\n"
              "long a;           // Line about a\n" // Previous (bad) behavior
              "#endif\n"
              "#if BAR\n"
              "#else\n"
              "long b_long_name; // Line about b\n"
              "#endif\n",
              getLLVMStyleWithColumns(80)));
+
+  // bug 47589
+  EXPECT_EQ(
+      "namespace m {\n\n"
+      "#define FOO_GLOBAL 0      // Global scope.\n"
+      "#define FOO_LINKLOCAL 1   // Link-local scope.\n"
+      "#define FOO_SITELOCAL 2   // Site-local scope (deprecated).\n"
+      "#define FOO_UNIQUELOCAL 3 // Unique local\n"
+      "#define FOO_NODELOCAL 4   // Loopback\n\n"
+      "} // namespace m\n",
+      format("namespace m {\n\n"
+             "#define FOO_GLOBAL 0   // Global scope.\n"
+             "#define FOO_LINKLOCAL 1  // Link-local scope.\n"
+             "#define FOO_SITELOCAL 2  // Site-local scope (deprecated).\n"
+             "#define FOO_UNIQUELOCAL 3 // Unique local\n"
+             "#define FOO_NODELOCAL 4  // Loopback\n\n"
+             "} // namespace m\n",
+             getLLVMStyleWithColumns(80)));
 }
 
 TEST_F(FormatTestComments, AlignsBlockCommentDecorations) {
   EXPECT_EQ("/*\n"
             " */",
             format("/*\n"
                    "*/",
                    getLLVMStyle()));