[Format] Fix for bug 35641

[kadircet]

Summary

Bug was caused due to comments at the start of scope. For a code like:

int func() { //
  int b;
  int c;
}

the comment at the first line gets IndentAndNestingLevel (1,1) whereas
the following declarations get only (0,1) which prevents them from insertion
of a new scope. So, I changed the AlignTokenSequence to look at previous
*non-comment* token when deciding whether to introduce a new scope into
stack or not.

[djasper]

Thanks for the fix.

unittests/Format/FormatTest.cpp

9453 ↗

(On Diff #134264)

Make this "See llvm.org/PR35641".

9457 ↗

(On Diff #134264)

Might be useful to use different types here to verify that alignment is actually happening, e.g. "int" and "unsigned".

[kadircet]

Changed description of unit test to include direct link to bug.
Used different variable types to make sure alignment happens.

[kadircet]

Btw, I cannot commit the change myself, don't have commit rights.

[lebedev.ri]

The diff looks wrong, the lib/Format/WhitespaceManager.cpp change gone missing.
You need to upload the diff to the git master/svn trunk/etc, *NOT* to the previous commit (previous diff).

[kadircet]

Rebased and uploaded diff to the master. Sorry for the inconvenience.

[kadircet]

ping

cfe/trunk/lib/Format/WhitespaceManager.cpp

AlignTokenSequence(unsigned Start, unsigned End, unsigned Column, F &&Matches,
   SmallVector<unsigned, 16> ScopeStack;
 
   for (unsigned i = Start; i != End; ++i) {
     if (ScopeStack.size() != 0 &&
         Changes[i].indentAndNestingLevel() <
             Changes[ScopeStack.back()].indentAndNestingLevel())
       ScopeStack.pop_back();
 
+    // Compare current token to previous non-comment token to ensure whether
+    // it is in a deeper scope or not.
+    unsigned PreviousNonComment = i - 1;
+    while (PreviousNonComment > Start &&
+           Changes[PreviousNonComment].Tok->is(tok::comment))
+      PreviousNonComment--;
     if (i != Start && Changes[i].indentAndNestingLevel() >
-                          Changes[i - 1].indentAndNestingLevel())
+                          Changes[PreviousNonComment].indentAndNestingLevel())
       ScopeStack.push_back(i);
 
     bool InsideNestedScope = ScopeStack.size() != 0;
 
     if (Changes[i].NewlinesBefore > 0 && !InsideNestedScope) {
       Shift = 0;
       FoundMatchOnLine = false;
     }

cfe/trunk/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, AlignConsecutiveDeclarations) {
   Alignment.PointerAlignment = FormatStyle::PAS_Middle;
   verifyFormat(
       "auto found = range::find_if(vsProducts, [&](auto * aProduct) {\n"
       "  static const Version verVs2017;\n"
       "  return true;\n"
       "});\n",
       Alignment);
   Alignment.PointerAlignment = FormatStyle::PAS_Right;
+
+  // See llvm.org/PR35641
+  Alignment.AlignConsecutiveDeclarations = true;
+  verifyFormat("int func() { //\n"
+               "  int      b;\n"
+               "  unsigned c;\n"
+               "}",
+               Alignment);
 }
 
 TEST_F(FormatTest, LinuxBraceBreaking) {
   FormatStyle LinuxBraceStyle = getLLVMStyle();
   LinuxBraceStyle.BreakBeforeBraces = FormatStyle::BS_Linux;
   verifyFormat("namespace a\n"
                "{\n"
                "class A\n"