From f4ab9ef34cf3ab19baa7fc16332b9fc6d1423ba6 Mon Sep 17 00:00:00 2001 From: Manuel Klimek Date: Fri, 11 Jan 2013 17:54:10 +0000 Subject: [PATCH] Implements pulling simple blocks into a single line. void f() { return 42; } The final change that implements the feature. llvm-svn: 172225 --- clang/lib/Format/Format.cpp | 99 +++++++++++++++++++---- clang/test/Index/comment-objc-decls.m | 6 +- clang/unittests/Format/FormatTest.cpp | 112 +++++++++++--------------- 3 files changed, 134 insertions(+), 83 deletions(-) diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp index 38d39bca6354..df5b4cbb9fd9 100644 --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -1279,28 +1279,27 @@ public: I != E; ++I) { const UnwrappedLine &TheLine = *I; if (touchesRanges(TheLine)) { - TokenAnnotator Annotator(TheLine, Style, SourceMgr, Lex); - if (!Annotator.annotate()) + llvm::OwningPtr 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 @@ public: } 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 &AnnotatedLine, + std::vector::iterator &I, + std::vector::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 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 @@ private: 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()); diff --git a/clang/test/Index/comment-objc-decls.m b/clang/test/Index/comment-objc-decls.m index ae3b0bbf415d..58e65e4ca52e 100644 --- a/clang/test/Index/comment-objc-decls.m +++ b/clang/test/Index/comment-objc-decls.m @@ -45,7 +45,7 @@ id IvarNSObject; } @end -// CHECK: Declaration>@interface NSObject {\n id IvarNSObject;\n}\n@end +// CHECK: Declaration>@interface NSObject { id IvarNSObject; }\n@end // CHECK: id IvarNSObject /** @@ -73,7 +73,7 @@ */ @property (copy) id PropertyMyClass; @end -// CHECK: @interface MyClass : NSObject <MyProto> {\n id IvarMyClass;\n}\n@end +// CHECK: @interface MyClass : NSObject <MyProto> { id IvarMyClass; }\n@end // CHECK: id IvarMyClass // CHECK: - (id)MethodMyClass; // CHECK: + (id)ClassMethodMyClass; @@ -90,7 +90,7 @@ id IvarMyClassExtension; } @end -// CHECK: @interface MyClass () {\n id IvarMyClassExtension;\n}\n@end +// CHECK: @interface MyClass () { id IvarMyClassExtension; }\n@end // CHECK: id IvarMyClassExtension diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index cb634fc7e18e..a16f66cd086b 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -362,30 +362,24 @@ TEST_F(FormatTest, FormatsEnum) { 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 @@ TEST_F(FormatTest, FormatTryCatch) { "}"); // 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, FormatTryCatch) { } 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, IndentPreprocessorDirectivesAtZero) { } 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, LayoutBlockInsideParens) { } 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 @@ TEST_F(FormatTest, IncorrectAccessSpecifier) { verifyFormat("public\n" "{}"); verifyFormat("public\n" - "B {\n" - " int x;\n" - "}"); + "B { int x; }"); } TEST_F(FormatTest, IncorrectCodeUnbalancedBraces) { @@ -1166,6 +1148,30 @@ TEST_F(FormatTest, LayoutTokensFollowingBlockInParentheses) { " 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 @@ TEST_F(FormatTest, FormatObjCInterface) { "+(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 {\n" - " int _i;\n" - "}\n" + verifyFormat("@interface Foo : Bar { 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) {\n" - " int _i;\n" - "}\n" + verifyFormat("@interface Foo (HackStuff) { int _i; }\n" "+ (id)init;\n" "@end"); } @@ -1361,9 +1355,7 @@ TEST_F(FormatTest, FormatObjCImplementation) { " 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 @@ TEST_F(FormatTest, FormatObjCImplementation) { "+ (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, ObjCAt) { 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_;"); -- GitLab