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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@330573 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Format/AffectedRangeManager.cpp b/lib/Format/AffectedRangeManager.cpp
index 5d4df1941209a895dffcf69ba9c150fde6be13f1..02e9f5e46f1154d7fb93a8242de8f261f5a45d5f 100644 (file)
--- a/lib/Format/AffectedRangeManager.cpp
+++ b/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/lib/Format/AffectedRangeManager.h b/lib/Format/AffectedRangeManager.h
index d8d5ee55acd8b1b410e8d8bab848dc9eb89726fc..b9a0cadd40a3366a0dba9d228069f715c3c7ca75 100644 (file)
--- a/lib/Format/AffectedRangeManager.h
+++ b/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/lib/Format/Format.cpp b/lib/Format/Format.cpp
index 98b2656ee2996d992d4e33c4b82dcc417b284e09..e2d25657c72f3462990c4763e7f90ed096991dd6 100644 (file)
--- a/lib/Format/Format.cpp
+++ b/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/lib/Format/NamespaceEndCommentsFixer.cpp b/lib/Format/NamespaceEndCommentsFixer.cpp
index f5832a443fd839216b9779ddaf4fe41c79289484..6311c058aed2cd5fff1832bda181360a0ec0a84b 100644 (file)
--- a/lib/Format/NamespaceEndCommentsFixer.cpp
+++ b/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/lib/Format/SortJavaScriptImports.cpp b/lib/Format/SortJavaScriptImports.cpp
index d0b979e100d5dd3245ae4cb689696a9349c6b863..b598a26f8cc7538d76f1fe228e2dc46747a5c682 100644 (file)
--- a/lib/Format/SortJavaScriptImports.cpp
+++ b/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/lib/Format/TokenAnnotator.h b/lib/Format/TokenAnnotator.h
index 04a18d45b82e5218db405477816d94af531a16c4..7be0753c20aee4cbc8fd7c1f58a1fa34b71af188 100644 (file)
--- a/lib/Format/TokenAnnotator.h
+++ b/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/lib/Format/UnwrappedLineFormatter.cpp b/lib/Format/UnwrappedLineFormatter.cpp
index 953a5d370c54984f8f2c662a83aa60334939e8fd..45ddc1cc63842937f57a72cfbaba31b4c592055d 100644 (file)
--- a/lib/Format/UnwrappedLineFormatter.cpp
+++ b/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/lib/Format/UnwrappedLineParser.cpp b/lib/Format/UnwrappedLineParser.cpp
index b61ffb21d845d20ad1bf909ecbfd8d43800c89fa..b5bc80ba09570e706a6a64d915e2ce61e3aac1c8 100644 (file)
--- a/lib/Format/UnwrappedLineParser.cpp
+++ b/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/lib/Format/UnwrappedLineParser.h b/lib/Format/UnwrappedLineParser.h
index b876735ec79a6c6428868242bd9985bc79b15eca..da7529c2ade39225648edd6b5d03e5e6d96f6380 100644 (file)
--- a/lib/Format/UnwrappedLineParser.h
+++ b/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/lib/Format/UsingDeclarationsSorter.cpp b/lib/Format/UsingDeclarationsSorter.cpp
index ef0c7a7d5a45e809893f244647664fccf3c1834a..d7ab4f31d2725c41cbf219976f96e0a643864d0c 100644 (file)
--- a/lib/Format/UsingDeclarationsSorter.cpp
+++ b/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/unittests/Format/FormatTestSelective.cpp b/unittests/Format/FormatTestSelective.cpp
index 182218fe96dbea731674f60d5fc650abddc51a14..57d5daf14c494691e7cf09c40f0f3901be7433ae 100644 (file)
--- a/unittests/Format/FormatTestSelective.cpp
+++ b/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"
       "}",