Format closing braces when reformatting the line containing the opening brace.

This required a couple of yaks to be shaved:
1. MatchingOpeningBlockLineIndex was misused to also store the
   closing index; instead, use a second variable, as this doesn't
   work correctly for "} else {".
2. We needed to change the API of AffectedRangeManager to not
   use iterators; we always passed in begin / end for the whole
   container before, so there was no mismatch in generality.
3. We need an extra check to discontinue formatting at the top
   level, as we now sometimes change the indent of the closing
   brace, but want to bail out immediately afterwards, for
   example:
     void f() {
       if (a) {
     }
     void g();
   Previously:
     void f() {
       if (a) {
     }
     void g();
   Now:
     void f() {
       if (a) {
       }
     void g();

Differential Revision: https://reviews.llvm.org/D45726

llvm-svn: 330573

11 files changed, 104 insertions, 30 deletions

diff --git a/clang/lib/Format/AffectedRangeManager.cpp b/clang/lib/Format/AffectedRangeManager.cpp
index 5d4df194120..02e9f5e46f1 100644
--- a/clang/lib/Format/AffectedRangeManager.cpp
+++ b/clang/lib/Format/AffectedRangeManager.cpp
@@ -21,8 +21,9 @@ namespace clang {
 namespace format {
 
 bool AffectedRangeManager::computeAffectedLines(
-    SmallVectorImpl<AnnotatedLine *>::iterator I,
-    SmallVectorImpl<AnnotatedLine *>::iterator E) {
+    SmallVectorImpl<AnnotatedLine *> &Lines) {
+  SmallVectorImpl<AnnotatedLine *>::iterator I = Lines.begin();
+  SmallVectorImpl<AnnotatedLine *>::iterator E = Lines.end();
   bool SomeLineAffected = false;
   const AnnotatedLine *PreviousLine = nullptr;
   while (I != E) {
@@ -48,7 +49,7 @@ bool AffectedRangeManager::computeAffectedLines(
       continue;
     }
 
-    if (nonPPLineAffected(Line, PreviousLine))
+    if (nonPPLineAffected(Line, PreviousLine, Lines))
       SomeLineAffected = true;
 
     PreviousLine = Line;
@@ -99,10 +100,10 @@ void AffectedRangeManager::markAllAsAffected(
 }
 
 bool AffectedRangeManager::nonPPLineAffected(
-    AnnotatedLine *Line, const AnnotatedLine *PreviousLine) {
+    AnnotatedLine *Line, const AnnotatedLine *PreviousLine,
+    SmallVectorImpl<AnnotatedLine *> &Lines) {
   bool SomeLineAffected = false;
-  Line->ChildrenAffected =
-      computeAffectedLines(Line->Children.begin(), Line->Children.end());
+  Line->ChildrenAffected = computeAffectedLines(Line->Children);
   if (Line->ChildrenAffected)
     SomeLineAffected = true;
 
@@ -138,8 +139,13 @@ bool AffectedRangeManager::nonPPLineAffected(
       Line->First->NewlinesBefore < 2 && PreviousLine &&
       PreviousLine->Affected && PreviousLine->Last->is(tok::comment);
 
+  bool IsAffectedClosingBrace =
+      Line->First->is(tok::r_brace) &&
+      Line->MatchingOpeningBlockLineIndex != UnwrappedLine::kInvalidIndex &&
+      Lines[Line->MatchingOpeningBlockLineIndex]->Affected;
+
   if (SomeTokenAffected || SomeFirstChildAffected || LineMoved ||
-      IsContinuedComment) {
+      IsContinuedComment || IsAffectedClosingBrace) {
     Line->Affected = true;
     SomeLineAffected = true;
   }
diff --git a/clang/lib/Format/AffectedRangeManager.h b/clang/lib/Format/AffectedRangeManager.h
index d8d5ee55acd..b9a0cadd40a 100644
--- a/clang/lib/Format/AffectedRangeManager.h
+++ b/clang/lib/Format/AffectedRangeManager.h
@@ -30,10 +30,9 @@ public:
       : SourceMgr(SourceMgr), Ranges(Ranges.begin(), Ranges.end()) {}
 
   // Determines which lines are affected by the SourceRanges given as input.
-  // Returns \c true if at least one line between I and E or one of their
+  // Returns \c true if at least one line in \p Lines or one of their
   // children is affected.
-  bool computeAffectedLines(SmallVectorImpl<AnnotatedLine *>::iterator I,
-                            SmallVectorImpl<AnnotatedLine *>::iterator E);
+  bool computeAffectedLines(SmallVectorImpl<AnnotatedLine *> &Lines);
 
   // Returns true if 'Range' intersects with one of the input ranges.
   bool affectsCharSourceRange(const CharSourceRange &Range);
@@ -54,8 +53,8 @@ private:
 
   // Determines whether 'Line' is affected by the SourceRanges given as input.
   // Returns \c true if line or one if its children is affected.
-  bool nonPPLineAffected(AnnotatedLine *Line,
-                         const AnnotatedLine *PreviousLine);
+  bool nonPPLineAffected(AnnotatedLine *Line, const AnnotatedLine *PreviousLine,
+                         SmallVectorImpl<AnnotatedLine *> &Lines);
 
   const SourceManager &SourceMgr;
   const SmallVector<CharSourceRange, 8> Ranges;
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 98b2656ee29..e2d25657c72 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1006,8 +1006,7 @@ public:
   analyze(TokenAnnotator &Annotator,
           SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
           FormatTokenLexer &Tokens) override {
-    AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(),
-                                          AnnotatedLines.end());
+    AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
     tooling::Replacements Result;
     requoteJSStringLiteral(AnnotatedLines, Result);
     return {Result, 0};
@@ -1097,8 +1096,7 @@ public:
           FormatTokenLexer &Tokens) override {
     tooling::Replacements Result;
     deriveLocalStyle(AnnotatedLines);
-    AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(),
-                                          AnnotatedLines.end());
+    AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
     for (unsigned i = 0, e = AnnotatedLines.size(); i != e; ++i) {
       Annotator.calculateFormattingInformation(*AnnotatedLines[i]);
     }
@@ -1222,8 +1220,7 @@ public:
     // To determine if some redundant code is actually introduced by
     // replacements(e.g. deletions), we need to come up with a more
     // sophisticated way of computing affected ranges.
-    AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(),
-                                          AnnotatedLines.end());
+    AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
 
     checkEmptyNamespace(AnnotatedLines);
 
diff --git a/clang/lib/Format/NamespaceEndCommentsFixer.cpp b/clang/lib/Format/NamespaceEndCommentsFixer.cpp
index f5832a443fd..6311c058aed 100644
--- a/clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ b/clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -141,8 +141,7 @@ std::pair<tooling::Replacements, unsigned> NamespaceEndCommentsFixer::analyze(
     TokenAnnotator &Annotator, SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
     FormatTokenLexer &Tokens) {
   const SourceManager &SourceMgr = Env.getSourceManager();
-  AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(),
-                                        AnnotatedLines.end());
+  AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
   tooling::Replacements Fixes;
   std::string AllNamespaceNames = "";
   size_t StartLineIndex = SIZE_MAX;
diff --git a/clang/lib/Format/SortJavaScriptImports.cpp b/clang/lib/Format/SortJavaScriptImports.cpp
index d0b979e100d..b598a26f8cc 100644
--- a/clang/lib/Format/SortJavaScriptImports.cpp
+++ b/clang/lib/Format/SortJavaScriptImports.cpp
@@ -128,8 +128,7 @@ public:
           SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
           FormatTokenLexer &Tokens) override {
     tooling::Replacements Result;
-    AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(),
-                                          AnnotatedLines.end());
+    AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
 
     const AdditionalKeywords &Keywords = Tokens.getKeywords();
     SmallVector<JsModuleReference, 16> References;
diff --git a/clang/lib/Format/TokenAnnotator.h b/clang/lib/Format/TokenAnnotator.h
index 04a18d45b82..7be0753c20a 100644
--- a/clang/lib/Format/TokenAnnotator.h
+++ b/clang/lib/Format/TokenAnnotator.h
@@ -40,6 +40,7 @@ public:
   AnnotatedLine(const UnwrappedLine &Line)
       : First(Line.Tokens.front().Tok), Level(Line.Level),
         MatchingOpeningBlockLineIndex(Line.MatchingOpeningBlockLineIndex),
+        MatchingClosingBlockLineIndex(Line.MatchingClosingBlockLineIndex),
         InPPDirective(Line.InPPDirective),
         MustBeDeclaration(Line.MustBeDeclaration), MightBeFunctionDecl(false),
         IsMultiVariableDeclStmt(false), Affected(false),
@@ -112,6 +113,7 @@ public:
   LineType Type;
   unsigned Level;
   size_t MatchingOpeningBlockLineIndex;
+  size_t MatchingClosingBlockLineIndex;
   bool InPPDirective;
   bool MustBeDeclaration;
   bool MightBeFunctionDecl;
diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 953a5d370c5..45ddc1cc638 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -252,9 +252,9 @@ private:
     if (Style.CompactNamespaces) {
       if (isNamespaceDeclaration(TheLine)) {
         int i = 0;
-        unsigned closingLine = TheLine->MatchingOpeningBlockLineIndex - 1;
+        unsigned closingLine = TheLine->MatchingClosingBlockLineIndex - 1;
         for (; I + 1 + i != E && isNamespaceDeclaration(I[i + 1]) &&
-               closingLine == I[i + 1]->MatchingOpeningBlockLineIndex &&
+               closingLine == I[i + 1]->MatchingClosingBlockLineIndex &&
                I[i + 1]->Last->TotalLength < Limit;
              i++, closingLine--) {
           // No extra indent for compacted namespaces
@@ -1033,9 +1033,12 @@ UnwrappedLineFormatter::format(const SmallVectorImpl<AnnotatedLine *> &Lines,
     // scope was added. However, we need to carefully stop doing this when we
     // exit the scope of affected lines to prevent indenting a the entire
     // remaining file if it currently missing a closing brace.
+    bool PreviousRBrace =
+        PreviousLine && PreviousLine->startsWith(tok::r_brace);
     bool ContinueFormatting =
         TheLine.Level > RangeMinLevel ||
-        (TheLine.Level == RangeMinLevel && !TheLine.startsWith(tok::r_brace));
+        (TheLine.Level == RangeMinLevel && !PreviousRBrace &&
+         !TheLine.startsWith(tok::r_brace));
 
     bool FixIndentation = (FixBadIndentation || ContinueFormatting) &&
                           Indent != TheLine.First->OriginalColumn;
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index b61ffb21d84..b5bc80ba095 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -570,7 +570,7 @@ void UnwrappedLineParser::parseBlock(bool MustBeDeclaration, bool AddLevel,
     Line->MatchingOpeningBlockLineIndex = OpeningLineIndex;
     if (OpeningLineIndex != UnwrappedLine::kInvalidIndex) {
       // Update the opening line to add the forward reference as well
-      (*CurrentLines)[OpeningLineIndex].MatchingOpeningBlockLineIndex =
+      (*CurrentLines)[OpeningLineIndex].MatchingClosingBlockLineIndex =
           CurrentLines->size() - 1;
     }
   }
diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index b876735ec79..da7529c2ade 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -53,7 +53,11 @@ struct UnwrappedLine {
   /// \c MatchingOpeningBlockLineIndex stores the index of the corresponding
   /// opening line. Otherwise, \c MatchingOpeningBlockLineIndex must be
   /// \c kInvalidIndex.
-  size_t MatchingOpeningBlockLineIndex;
+  size_t MatchingOpeningBlockLineIndex = kInvalidIndex;
+
+  /// \brief If this \c UnwrappedLine opens a block, stores the index of the
+  /// line with the corresponding closing brace.
+  size_t MatchingClosingBlockLineIndex = kInvalidIndex;
 
   static const size_t kInvalidIndex = -1;
 
diff --git a/clang/lib/Format/UsingDeclarationsSorter.cpp b/clang/lib/Format/UsingDeclarationsSorter.cpp
index ef0c7a7d5a4..d7ab4f31d27 100644
--- a/clang/lib/Format/UsingDeclarationsSorter.cpp
+++ b/clang/lib/Format/UsingDeclarationsSorter.cpp
@@ -187,8 +187,7 @@ std::pair<tooling::Replacements, unsigned> UsingDeclarationsSorter::analyze(
     TokenAnnotator &Annotator, SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
     FormatTokenLexer &Tokens) {
   const SourceManager &SourceMgr = Env.getSourceManager();
-  AffectedRangeMgr.computeAffectedLines(AnnotatedLines.begin(),
-                                        AnnotatedLines.end());
+  AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
   tooling::Replacements Fixes;
   SmallVector<UsingDeclaration, 4> UsingDeclarations;
   for (size_t I = 0, E = AnnotatedLines.size(); I != E; ++I) {
diff --git a/clang/unittests/Format/FormatTestSelective.cpp b/clang/unittests/Format/FormatTestSelective.cpp
index 182218fe96d..57d5daf14c4 100644
--- a/clang/unittests/Format/FormatTestSelective.cpp
+++ b/clang/unittests/Format/FormatTestSelective.cpp
@@ -177,6 +177,72 @@ TEST_F(FormatTestSelective, FormatsCommentsLocally) {
                    0, 0));
 }
 
+TEST_F(FormatTestSelective, ContinueReindenting) {
+  // When we change an indent, we continue formatting as long as following
+  // lines are not indented correctly.
+  EXPECT_EQ("int   i;\n"
+            "int b;\n"
+            "int c;\n"
+            "int d;\n"
+            "int e;\n"
+            "  int f;\n",
+            format("int   i;\n"
+                   "  int b;\n"
+                   " int   c;\n"
+                   "  int d;\n"
+                   "int e;\n"
+                   "  int f;\n",
+                   11, 0));
+}
+
+TEST_F(FormatTestSelective, ReindentClosingBrace) {
+  EXPECT_EQ("int   i;\n"
+            "int f() {\n"
+            "  int a;\n"
+            "  int b;\n"
+            "}\n"
+            " int c;\n",
+            format("int   i;\n"
+                   "  int f(){\n"
+                   "int a;\n"
+                   "int b;\n"
+                   "  }\n"
+                   " int c;\n",
+                   11, 0));
+  EXPECT_EQ("void f() {\n"
+            "  if (foo) {\n"
+            "    b();\n"
+            "  } else {\n"
+            "    c();\n"
+            "  }\n"
+            "int d;\n"
+            "}\n",
+            format("void f() {\n"
+                   "  if (foo) {\n"
+                   "b();\n"
+                   "}else{\n"
+                   "c();\n"
+                   "}\n"
+                   "int d;\n"
+                   "}\n",
+                   13, 0));
+  EXPECT_EQ("int i = []() {\n"
+            "  class C {\n"
+            "    int a;\n"
+            "    int b;\n"
+            "  };\n"
+            "  int c;\n"
+            "};\n",
+            format("int i = []() {\n"
+                   "  class C{\n"
+                   "int a;\n"
+                   "int b;\n"
+                   "};\n"
+                   "int c;\n"
+                   "  };\n",
+                   17, 0));
+}
+
 TEST_F(FormatTestSelective, IndividualStatementsOfNestedBlocks) {
   EXPECT_EQ("DEBUG({\n"
             "  int i;\n"
@@ -503,7 +569,7 @@ TEST_F(FormatTestSelective, StopFormattingWhenLeavingScope) {
       "  if (a) {\n"
       "    g();\n"
       "    h();\n"
-      "}\n"
+      "  }\n"
       "\n"
       "void g() {\n"
       "}",