[cfe-commits] r172225 - in /cfe/trunk: lib/Format/Format.cpp test/Index/comment-objc-decls.m unittests/Format/FormatTest.cpp

Manuel Klimek klimek at google.com
Fri Jan 11 09:54:10 PST 2013


Author: klimek
Date: Fri Jan 11 11:54:10 2013
New Revision: 172225

URL: http://llvm.org/viewvc/llvm-project?rev=172225&view=rev
Log:
Implements pulling simple blocks into a single line.

void f() { return 42; }

The final change that implements the feature.

Modified:
    cfe/trunk/lib/Format/Format.cpp
    cfe/trunk/test/Index/comment-objc-decls.m
    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=172225&r1=172224&r2=172225&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Fri Jan 11 11:54:10 2013
@@ -1279,28 +1279,27 @@
          I != E; ++I) {
       const UnwrappedLine &TheLine = *I;
       if (touchesRanges(TheLine)) {
-        TokenAnnotator Annotator(TheLine, Style, SourceMgr, Lex);
-        if (!Annotator.annotate())
+        llvm::OwningPtr<TokenAnnotator> AnnotatedLine(
+            new TokenAnnotator(TheLine, Style, SourceMgr, Lex));
+        if (!AnnotatedLine->annotate())
           break;
-        unsigned Indent = formatFirstToken(Annotator.getRootToken(),
+        unsigned Indent = formatFirstToken(AnnotatedLine->getRootToken(),
                                            TheLine.Level, TheLine.InPPDirective,
                                            PreviousEndOfLineColumn);
-        unsigned Limit = Style.ColumnLimit - (TheLine.InPPDirective ? 1 : 0) -
-                         Indent;
-        // Check whether the UnwrappedLine can be put onto a single line. If
-        // so, this is bound to be the optimal solution (by definition) and we
-        // don't need to analyze the entire solution space.
-        bool FitsOnALine = fitsIntoLimit(Annotator.getRootToken(), Limit);
-        UnwrappedLineFormatter Formatter(Style, SourceMgr, TheLine, Indent,
-                                         FitsOnALine, Annotator.getLineType(),
-                                         Annotator.getRootToken(), Replaces,
-                                         StructuralError);
+
+        UnwrappedLine Line(TheLine);
+        bool FitsOnALine = tryFitMultipleLinesInOne(Indent, Line, AnnotatedLine,
+                                                    I, E);
+        UnwrappedLineFormatter Formatter(
+            Style, SourceMgr, Line, Indent, FitsOnALine,
+            AnnotatedLine->getLineType(), AnnotatedLine->getRootToken(),
+            Replaces, StructuralError);
         PreviousEndOfLineColumn = Formatter.format();
       } else {
         // 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
         // last token, and return the result.
-        const FormatToken *Last = getLastLine(TheLine);
+        const FormatToken *Last = getLastInLine(TheLine);
         PreviousEndOfLineColumn =
             SourceMgr.getSpellingColumnNumber(Last->Tok.getLocation()) +
             Lex.MeasureTokenLength(Last->Tok.getLocation(), SourceMgr,
@@ -1312,7 +1311,75 @@
   }
 
 private:
-  const FormatToken *getLastLine(const UnwrappedLine &TheLine) {
+  /// \brief Tries to merge lines into one.
+  ///
+  /// 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.
+  bool tryFitMultipleLinesInOne(unsigned Indent, UnwrappedLine &Line,
+                                llvm::OwningPtr<TokenAnnotator> &AnnotatedLine,
+                                std::vector<UnwrappedLine>::iterator &I,
+                                std::vector<UnwrappedLine>::iterator E) {
+    unsigned Limit = Style.ColumnLimit - (I->InPPDirective ? 1 : 0) - Indent;
+
+    // Check whether the UnwrappedLine can be put onto a single line. If
+    // so, this is bound to be the optimal solution (by definition) and we
+    // don't need to analyze the entire solution space.
+    bool FitsOnALine = fitsIntoLimit(AnnotatedLine->getRootToken(), Limit);
+    if (!FitsOnALine || I + 1 == E || I + 2 == E)
+      return FitsOnALine;
+
+    // Try to merge the next two lines if possible.
+    UnwrappedLine Combined(Line);
+
+    // First, check that the current line allows merging. This is the case if
+    // we're not in a control flow statement and the last token is an opening
+    // brace.
+    FormatToken *Last = &Combined.RootToken;
+    bool AllowedTokens =
+        !Last->Tok.is(tok::kw_if) && !Last->Tok.is(tok::kw_while) &&
+        !Last->Tok.is(tok::kw_do) && !Last->Tok.is(tok::r_brace) &&
+        !Last->Tok.is(tok::kw_else) && !Last->Tok.is(tok::kw_try) &&
+        !Last->Tok.is(tok::kw_catch) && !Last->Tok.is(tok::kw_for);
+    while (!Last->Children.empty())
+      Last = &Last->Children.back();
+    if (!Last->Tok.is(tok::l_brace))
+      return FitsOnALine;
+
+    // Second, check that the next line does not contain any braces - if it
+    // does, readability declines when putting it into a single line.
+    const FormatToken *Next = &(I + 1)->RootToken;
+    while (Next) {
+      AllowedTokens = AllowedTokens && !Next->Tok.is(tok::l_brace) &&
+                      !Next->Tok.is(tok::r_brace);
+      Last->Children.push_back(*Next);
+      Last = &Last->Children[0];
+      Last->Children.clear();
+      Next = Next->Children.empty() ? NULL : &Next->Children.back();
+    }
+
+    // Last, check that the third line contains a single closing brace.
+    Next = &(I + 2)->RootToken;
+    AllowedTokens = AllowedTokens && Next->Tok.is(tok::r_brace);
+    if (!Next->Children.empty() || !AllowedTokens)
+      return FitsOnALine;
+    Last->Children.push_back(*Next);
+
+    llvm::OwningPtr<TokenAnnotator> CombinedAnnotator(
+        new TokenAnnotator(Combined, Style, SourceMgr, Lex));
+    if (CombinedAnnotator->annotate() &&
+        fitsIntoLimit(CombinedAnnotator->getRootToken(), Limit)) {
+      // If the merged line fits, we use that instead and skip the next two
+      // lines.
+      AnnotatedLine.reset(CombinedAnnotator.take());
+      Line = Combined;
+      I += 2;
+    }
+    return FitsOnALine;
+  }
+
+  const FormatToken *getLastInLine(const UnwrappedLine &TheLine) {
     const FormatToken *Last = &TheLine.RootToken;
     while (!Last->Children.empty())
       Last = &Last->Children.back();
@@ -1321,7 +1388,7 @@
 
   bool touchesRanges(const UnwrappedLine &TheLine) {
     const FormatToken *First = &TheLine.RootToken;
-    const FormatToken *Last = getLastLine(TheLine);
+    const FormatToken *Last = getLastInLine(TheLine);
     CharSourceRange LineRange = CharSourceRange::getTokenRange(
                                     First->Tok.getLocation(),
                                     Last->Tok.getLocation());

Modified: cfe/trunk/test/Index/comment-objc-decls.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/comment-objc-decls.m?rev=172225&r1=172224&r2=172225&view=diff
==============================================================================
--- cfe/trunk/test/Index/comment-objc-decls.m (original)
+++ cfe/trunk/test/Index/comment-objc-decls.m Fri Jan 11 11:54:10 2013
@@ -45,7 +45,7 @@
   id IvarNSObject;
 }
 @end
-// CHECK: Declaration>@interface NSObject {\n  id IvarNSObject;\n}\n at end</Declaration>
+// CHECK: Declaration>@interface NSObject { id IvarNSObject; }\n at end</Declaration>
 // CHECK: <Declaration>id IvarNSObject</Declaration>
 
 /**
@@ -73,7 +73,7 @@
 */
 @property (copy) id PropertyMyClass;
 @end
-// CHECK: <Declaration>@interface MyClass : NSObject <MyProto> {\n    id IvarMyClass;\n}\n at end</Declaration>
+// CHECK: <Declaration>@interface MyClass : NSObject <MyProto> { id IvarMyClass; }\n at end</Declaration>
 // CHECK: <Declaration>id IvarMyClass</Declaration>
 // CHECK: <Declaration>- (id)MethodMyClass;</Declaration>
 // CHECK: <Declaration>+ (id)ClassMethodMyClass;</Declaration>
@@ -90,7 +90,7 @@
   id IvarMyClassExtension;
 }
 @end
-// CHECK: <Declaration>@interface MyClass () {\n  id IvarMyClassExtension;\n}\n at end</Declaration>
+// CHECK: <Declaration>@interface MyClass () { id IvarMyClassExtension; }\n at end</Declaration>
 // CHECK: <Declaration>id IvarMyClassExtension</Declaration>
 
 

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=172225&r1=172224&r2=172225&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Fri Jan 11 11:54:10 2013
@@ -362,30 +362,24 @@
 TEST_F(FormatTest, FormatsNamespaces) {
   verifyFormat("namespace some_namespace {\n"
                "class A {};\n"
-               "void f() {\n"
-               "  f();\n"
-               "}\n"
+               "void f() { f(); }\n"
                "}");
   verifyFormat("namespace {\n"
                "class A {};\n"
-               "void f() {\n"
-               "  f();\n"
-               "}\n"
+               "void f() { f(); }\n"
                "}");
   verifyFormat("inline namespace X {\n"
                "class A {};\n"
-               "void f() {\n"
-               "  f();\n"
-               "}\n"
+               "void f() { f(); }\n"
                "}");
   verifyFormat("using namespace some_namespace;\n"
                "class A {};\n"
-               "void f() {\n"
-               "  f();\n"
-               "}");
+               "void f() { f(); }");
 }
 
 TEST_F(FormatTest, FormatTryCatch) {
+  // FIXME: Handle try-catch explicitly in the UnwrappedLineParser, then we'll
+  // also not create single-line-blocks.
   verifyFormat("try {\n"
                "  throw a * b;\n"
                "}\n"
@@ -397,9 +391,7 @@
                "}");
 
   // Function-level try statements.
-  verifyFormat("int f() try {\n"
-               "  return 4;\n"
-               "}\n"
+  verifyFormat("int f() try { return 4; }\n"
                "catch (...) {\n"
                "  return 5;\n"
                "}");
@@ -413,15 +405,9 @@
 }
 
 TEST_F(FormatTest, FormatObjCTryCatch) {
-  verifyFormat("@try {\n"
-               "  f();\n"
-               "}\n"
-               "@catch (NSException e) {\n"
-               "  @throw;\n"
-               "}\n"
-               "@finally {\n"
-               "  exit(42);\n"
-               "}");
+  verifyFormat("@try { f(); }\n"
+               "@catch (NSException e) { @throw; }\n"
+               "@finally { exit(42); }");
 }
 
 TEST_F(FormatTest, StaticInitializers) {
@@ -546,7 +532,7 @@
 }
 
 TEST_F(FormatTest, FormatHashIfNotAtStartOfLine) {
-  verifyFormat("{\n  {\n    a #c;\n  }\n}");
+  verifyFormat("{\n  { a #c; }\n}");
 }
 
 TEST_F(FormatTest, FormatUnbalancedStructuralElements) {
@@ -609,9 +595,7 @@
 }
 
 TEST_F(FormatTest, LayoutBlockInsideStatement) {
-  EXPECT_EQ("SOME_MACRO {\n"
-            "  int i;\n"
-            "}\n"
+  EXPECT_EQ("SOME_MACRO { int i; }\n"
             "int i;", format("  SOME_MACRO  {int i;}  int i;"));
 }
 
@@ -1106,9 +1090,7 @@
   verifyFormat("public\n"
                "{}");
   verifyFormat("public\n"
-               "B {\n"
-               "  int x;\n"
-               "}");
+               "B { int x; }");
 }
 
 TEST_F(FormatTest, IncorrectCodeUnbalancedBraces) {
@@ -1166,6 +1148,30 @@
       "                                    ccccccccccccccccc));");
 }
 
+TEST_F(FormatTest, PullTrivialFunctionDefinitionsIntoSingleLine) {
+  verifyFormat("void f() { return 42; }");
+  verifyFormat("void f() {\n"
+               "  // Comment\n"
+               "}");
+  verifyFormat("{\n"
+               "#error {\n"
+               "  int a;\n"
+               "}");
+  verifyFormat("{\n"
+               "  int a;\n"
+               "#error {\n"
+               "}");
+}
+
+// FIXME: This breaks the order of the unwrapped lines:
+// TEST_F(FormatTest, OrderUnwrappedLines) {
+//   verifyFormat("{\n"
+//                "  bool a; //\n"
+//                "#error {\n"
+//                "  int a;\n"
+//                "}");
+// }
+
 //===----------------------------------------------------------------------===//
 // Objective-C tests.
 //===----------------------------------------------------------------------===//
@@ -1291,39 +1297,27 @@
                      "+(id) init;\n"
                      "@end");
 
-  verifyFormat("@interface Foo {\n"
-               "  int _i;\n"
-               "}\n"
+  verifyFormat("@interface Foo { int _i; }\n"
                "+ (id)init;\n"
                "@end");
 
-  verifyFormat("@interface Foo : Bar {\n"
-               "  int _i;\n"
-               "}\n"
+  verifyFormat("@interface Foo : Bar { int _i; }\n"
                "+ (id)init;\n"
                "@end");
 
-  verifyFormat("@interface Foo : Bar <Baz, Quux> {\n"
-               "  int _i;\n"
-               "}\n"
+  verifyFormat("@interface Foo : Bar <Baz, Quux> { int _i; }\n"
                "+ (id)init;\n"
                "@end");
 
-  verifyFormat("@interface Foo (HackStuff) {\n"
-               "  int _i;\n"
-               "}\n"
+  verifyFormat("@interface Foo (HackStuff) { int _i; }\n"
                "+ (id)init;\n"
                "@end");
 
-  verifyFormat("@interface Foo () {\n"
-               "  int _i;\n"
-               "}\n"
+  verifyFormat("@interface Foo () { int _i; }\n"
                "+ (id)init;\n"
                "@end");
 
-  verifyFormat("@interface Foo (HackStuff) <MyProtocol> {\n"
-               "  int _i;\n"
-               "}\n"
+  verifyFormat("@interface Foo (HackStuff) <MyProtocol> { int _i; }\n"
                "+ (id)init;\n"
                "@end");
 }
@@ -1361,9 +1355,7 @@
                "    return nil;\n"
                "}\n"
                "// Look, a comment!\n"
-               "- (int)answerWith:(int)i {\n"
-               "  return i;\n"
-               "}\n"
+               "- (int)answerWith:(int)i { return i; }\n"
                "@end");
 
   verifyFormat("@implementation Foo\n"
@@ -1375,15 +1367,11 @@
                "+ (id)init {}\n"
                "@end");
 
-  verifyFormat("@implementation Foo {\n"
-               "  int _i;\n"
-               "}\n"
+  verifyFormat("@implementation Foo { int _i; }\n"
                "+ (id)init {}\n"
                "@end");
 
-  verifyFormat("@implementation Foo : Bar {\n"
-               "  int _i;\n"
-               "}\n"
+  verifyFormat("@implementation Foo : Bar { int _i; }\n"
                "+ (id)init {}\n"
                "@end");
 
@@ -1473,18 +1461,14 @@
 
 TEST_F(FormatTest, ObjCSnippets) {
   // FIXME: Make the uncommented lines below pass.
-  verifyFormat("@autoreleasepool {\n"
-               "  foo();\n"
-               "}");
+  verifyFormat("@autoreleasepool { foo(); }");
   verifyFormat("@class Foo, Bar;");
   verifyFormat("@compatibility_alias AliasName ExistingClass;");
   verifyFormat("@dynamic textColor;");
   //verifyFormat("char *buf1 = @encode(int **);");
   verifyFormat("Protocol *proto = @protocol(p1);");
   //verifyFormat("SEL s = @selector(foo:);");
-  verifyFormat("@synchronized(self) {\n"
-               "  f();\n"
-               "}");
+  verifyFormat("@synchronized(self) { f(); }");
 
   verifyFormat("@synthesize dropArrowPosition = dropArrowPosition_;");
   verifyGoogleFormat("@synthesize dropArrowPosition = dropArrowPosition_;");





More information about the cfe-commits mailing list