r181770 - Fix clang-format bug in unwrapped-line merging.

Daniel Jasper djasper at google.com
Tue May 14 02:30:02 PDT 2013


Author: djasper
Date: Tue May 14 04:30:02 2013
New Revision: 181770

URL: http://llvm.org/viewvc/llvm-project?rev=181770&view=rev
Log:
Fix clang-format bug in unwrapped-line merging.

Before (in styles that allow it), clang-format would not merge an
if statement onto a single line, if only the second line was format
(e.g. in an editor integration):

if (a)
  return; // clang-format invoked on this line.

With this patch, this gets properly merged to:

if (a) return; // ...

Modified:
    cfe/trunk/lib/Format/Format.cpp
    cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=181770&r1=181769&r2=181770&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Tue May 14 04:30:02 2013
@@ -44,7 +44,7 @@ struct ScalarEnumerationTraits<clang::fo
   }
 };
 
-template<>
+template <>
 struct ScalarEnumerationTraits<clang::format::FormatStyle::BraceBreakingStyle> {
   static void
   enumeration(IO &IO, clang::format::FormatStyle::BraceBreakingStyle &Value) {
@@ -1193,9 +1193,15 @@ public:
           (touchesLine(TheLine) || touchesPPDirective(I + 1, E)))
         FormatPPDirective = true;
 
+      // Determine indent and try to merge multiple unwrapped lines.
       while (IndentForLevel.size() <= TheLine.Level)
         IndentForLevel.push_back(-1);
       IndentForLevel.resize(TheLine.Level + 1);
+      unsigned Indent = getIndent(IndentForLevel, TheLine.Level);
+      if (static_cast<int>(Indent) + Offset >= 0)
+        Indent += Offset;
+      tryFitMultipleLinesInOne(Indent, I, E);
+
       bool WasMoved = PreviousLineWasTouched && FirstTok.NewlinesBefore == 0;
       if (TheLine.First.is(tok::eof)) {
         if (PreviousLineWasTouched) {
@@ -1206,9 +1212,6 @@ public:
       } else if (TheLine.Type != LT_Invalid &&
                  (WasMoved || FormatPPDirective || touchesLine(TheLine))) {
         unsigned LevelIndent = getIndent(IndentForLevel, TheLine.Level);
-        unsigned Indent = LevelIndent;
-        if (static_cast<int>(Indent) + Offset >= 0)
-          Indent += Offset;
         if (FirstTok.WhiteSpaceStart.isValid() &&
             // Insert a break even if there is a structural error in case where
             // we break apart a line consisting of multiple unwrapped lines.
@@ -1219,7 +1222,6 @@ public:
           Indent = LevelIndent =
               SourceMgr.getSpellingColumnNumber(FirstTok.Tok.getLocation()) - 1;
         }
-        tryFitMultipleLinesInOne(Indent, I, E);
         UnwrappedLineFormatter Formatter(Style, SourceMgr, TheLine, Indent,
                                          TheLine.First, Whitespaces);
         PreviousEndOfLineColumn =
@@ -1228,18 +1230,17 @@ public:
         PreviousLineWasTouched = true;
       } else {
         if (FirstTok.NewlinesBefore > 0 || FirstTok.IsFirst) {
-          unsigned Indent =
+          unsigned LevelIndent =
               SourceMgr.getSpellingColumnNumber(FirstTok.Tok.getLocation()) - 1;
-          unsigned LevelIndent = Indent;
+          // Remove trailing whitespace of the previous line if it was touched.
+          if (PreviousLineWasTouched || touchesEmptyLineBefore(TheLine))
+            formatFirstToken(TheLine.First, PreviousLineLastToken, LevelIndent,
+                             TheLine.InPPDirective, PreviousEndOfLineColumn);
+
           if (static_cast<int>(LevelIndent) - Offset >= 0)
             LevelIndent -= Offset;
           if (TheLine.First.isNot(tok::comment))
             IndentForLevel[TheLine.Level] = LevelIndent;
-
-          // Remove trailing whitespace of the previous line if it was touched.
-          if (PreviousLineWasTouched || touchesEmptyLineBefore(TheLine))
-            formatFirstToken(TheLine.First, PreviousLineLastToken, Indent,
-                             TheLine.InPPDirective, PreviousEndOfLineColumn);
         }
         // If we did not reformat this unwrapped line, the column at the end of
         // the last token is unchanged - thus, we can calculate the end of the
@@ -1327,8 +1328,6 @@ private:
   ///
   /// This will change \c Line and \c AnnotatedLine to contain the merged line,
   /// if possible; note that \c I will be incremented when lines are merged.
-  ///
-  /// Returns whether the resulting \c Line can fit in a single line.
   void tryFitMultipleLinesInOne(unsigned Indent,
                                 std::vector<AnnotatedLine>::iterator &I,
                                 std::vector<AnnotatedLine>::iterator E) {
@@ -1352,7 +1351,6 @@ private:
                                     I->First.FormatTok.IsFirst)) {
       tryMergeSimplePPDirective(I, E, Limit);
     }
-    return;
   }
 
   void tryMergeSimplePPDirective(std::vector<AnnotatedLine>::iterator &I,

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=181770&r1=181769&r2=181770&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue May 14 04:30:02 2013
@@ -245,6 +245,8 @@ TEST_F(FormatTest, FormatIfWithoutCompou
                "}",
                AllowsMergedIf);
 
+  EXPECT_EQ("if (a) return;", format("if(a)\nreturn;", 7, 1, AllowsMergedIf));
+
   AllowsMergedIf.ColumnLimit = 14;
   verifyFormat("if (a) return;", AllowsMergedIf);
   verifyFormat("if (aaaaaaaaa)\n"
@@ -3810,7 +3812,7 @@ TEST_F(FormatTest, ReformatRegionAdjusts
             "a;\n"
             "}\n"
             "{\n"
-            "  b;\n"
+            "  b; //\n"
             "}\n"
             "}",
             format("{\n"
@@ -3818,15 +3820,15 @@ TEST_F(FormatTest, ReformatRegionAdjusts
                    "a;\n"
                    "}\n"
                    "{\n"
-                   "           b;\n"
+                   "           b; //\n"
                    "}\n"
                    "}",
                    22, 2, getLLVMStyle()));
   EXPECT_EQ("  {\n"
-            "    a;\n"
+            "    a; //\n"
             "  }",
             format("  {\n"
-                   "a;\n"
+                   "a; //\n"
                    "  }",
                    4, 2, getLLVMStyle()));
   EXPECT_EQ("void f() {}\n"





More information about the cfe-commits mailing list