r195925 - clang-format: Improve selective formatting of nested statements.
Daniel Jasper
djasper at google.com
Thu Nov 28 07:58:56 PST 2013
Author: djasper
Date: Thu Nov 28 09:58:55 2013
New Revision: 195925
URL: http://llvm.org/viewvc/llvm-project?rev=195925&view=rev
Log:
clang-format: Improve selective formatting of nested statements.
Previously, clang-format could create quite corrupt formattings if
individual lines of nested blocks (e.g. in "DEBUG({})" or lambdas) were
used. With this patch, it tries to extend the formatted regions to leave
around some reasonable format without always formatting the entire
surrounding statement.
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.h
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=195925&r1=195924&r2=195925&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Thu Nov 28 09:58:55 2013
@@ -560,7 +560,7 @@ public:
Joiner(Style) {}
unsigned format(const SmallVectorImpl<AnnotatedLine *> &Lines, bool DryRun,
- int AdditionalIndent = 0) {
+ int AdditionalIndent = 0, bool FixBadIndentation = false) {
assert(!Lines.empty());
unsigned Penalty = 0;
std::vector<int> IndentForLevel;
@@ -589,6 +589,9 @@ public:
}
I += MergedLines;
+ unsigned LevelIndent = getIndent(IndentForLevel, TheLine.Level);
+ bool FixIndentation =
+ FixBadIndentation && (LevelIndent != FirstTok->OriginalColumn);
if (TheLine.First->is(tok::eof)) {
if (PreviousLine && PreviousLine->Affected && !DryRun) {
// Remove the file's trailing whitespace.
@@ -597,8 +600,8 @@ public:
/*IndentLevel=*/0, /*Spaces=*/0,
/*TargetColumn=*/0);
}
- } else if (TheLine.Type != LT_Invalid && TheLine.Affected) {
- unsigned LevelIndent = getIndent(IndentForLevel, TheLine.Level);
+ } else if (TheLine.Type != LT_Invalid &&
+ (TheLine.Affected || FixIndentation)) {
if (FirstTok->WhitespaceRange.isValid()) {
if (!DryRun)
formatFirstToken(*TheLine.First, PreviousLine, TheLine.Level,
@@ -620,6 +623,7 @@ public:
while (State.NextToken != NULL)
Indenter->addTokenToState(State, /*Newline=*/false, DryRun);
} else if (Style.ColumnLimit == 0) {
+ // FIXME: Implement nested blocks for ColumnLimit = 0.
NoColumnLimitFormatter Formatter(Indenter);
if (!DryRun)
Formatter.format(Indent, &TheLine);
@@ -628,6 +632,8 @@ public:
}
IndentForLevel[TheLine.Level] = LevelIndent;
+ } else if (TheLine.ChildrenAffected) {
+ format(TheLine.Children, DryRun);
} else {
// Format the first token if necessary, and notify the WhitespaceManager
// about the unchanged whitespace.
@@ -636,7 +642,7 @@ public:
(Tok->NewlinesBefore > 0 || Tok->IsFirst)) {
unsigned LevelIndent = Tok->OriginalColumn;
if (!DryRun) {
- // Remove trailing whitespace of the previous line if.
+ // Remove trailing whitespace of the previous line.
if ((PreviousLine && PreviousLine->Affected) ||
TheLine.LeadingEmptyLinesAffected) {
formatFirstToken(*Tok, PreviousLine, TheLine.Level, LevelIndent,
@@ -924,7 +930,8 @@ private:
if (NewLine) {
int AdditionalIndent = State.Stack.back().Indent -
Previous.Children[0]->Level * Style.IndentWidth;
- Penalty += format(Previous.Children, DryRun, AdditionalIndent);
+ Penalty += format(Previous.Children, DryRun, AdditionalIndent,
+ /*FixBadIndentation=*/true);
return true;
}
@@ -1263,7 +1270,8 @@ public:
private:
// Determines which lines are affected by the SourceRanges given as input.
- // Returns \c true if at least one line between I and E was affected.
+ // Returns \c true if at least one line between I and E or one of their
+ // children is affected.
bool computeAffectedLines(SmallVectorImpl<AnnotatedLine *>::iterator I,
SmallVectorImpl<AnnotatedLine *>::iterator E) {
bool SomeLineAffected = false;
@@ -1291,29 +1299,58 @@ private:
continue;
}
- bool SomeTokenAffected = false;
- for (FormatToken *Tok = Line->First; Tok; Tok = Tok->Next) {
- bool IncludeLeadingNewlines = Tok != Line->First;
- if (affectsTokenRange(*Tok, *Tok, IncludeLeadingNewlines)) {
- SomeTokenAffected = true;
- break;
- }
- }
- bool SomeChildAffected =
- computeAffectedLines(Line->Children.begin(), Line->Children.end());
- bool LineMoved = PreviousLineAffected && Line->First->NewlinesBefore == 0;
- if (SomeTokenAffected || SomeChildAffected || LineMoved) {
- Line->Affected = true;
+ if (nonPPLineAffected(Line, &PreviousLineAffected))
SomeLineAffected = true;
- PreviousLineAffected = true;
- } else {
- PreviousLineAffected = false;
- }
++I;
}
return SomeLineAffected;
}
+
+ // 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, bool *PreviousLineAffected) {
+ bool SomeLineAffected = false;
+ Line->ChildrenAffected =
+ computeAffectedLines(Line->Children.begin(), Line->Children.end());
+ if (Line->ChildrenAffected)
+ SomeLineAffected = true;
+
+ // Stores whether one of the line's tokens is directly affected.
+ bool SomeTokenAffected = false;
+ // Stores whether we need to look at the leading newlines of the next token
+ // in order to determine whether it was affected.
+ bool IncludeLeadingNewlines = false;
+
+ // Stores whether the first child line of any of this line's tokens is
+ // affected.
+ bool SomeFirstChildAffected = false;
+
+ for (FormatToken *Tok = Line->First; Tok; Tok = Tok->Next) {
+ // Determine whether 'Tok' was affected.
+ if (affectsTokenRange(*Tok, *Tok, IncludeLeadingNewlines))
+ SomeTokenAffected = true;
+
+ // Determine whether the first child of 'Tok' was affected.
+ if (!Tok->Children.empty() && Tok->Children.front()->Affected)
+ SomeFirstChildAffected = true;
+
+ IncludeLeadingNewlines = Tok->Children.empty();
+ }
+
+ // Was this line moved, i.e. has it previously been on the same line as an
+ // affected line?
+ bool LineMoved = *PreviousLineAffected && Line->First->NewlinesBefore == 0;
+
+ if (SomeTokenAffected || SomeFirstChildAffected || LineMoved) {
+ Line->Affected = true;
+ *PreviousLineAffected = true;
+ SomeLineAffected = true;
+ } else {
+ *PreviousLineAffected = false;
+ }
+ return SomeLineAffected;
+ }
// Marks all lines between I and E as well as all their children as affected.
void markAllAsAffected(SmallVectorImpl<AnnotatedLine *>::iterator I,
Modified: cfe/trunk/lib/Format/TokenAnnotator.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.h?rev=195925&r1=195924&r2=195925&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.h (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.h Thu Nov 28 09:58:55 2013
@@ -42,7 +42,7 @@ public:
InPPDirective(Line.InPPDirective),
MustBeDeclaration(Line.MustBeDeclaration), MightBeFunctionDecl(false),
StartsDefinition(false), Affected(false),
- LeadingEmptyLinesAffected(false) {
+ LeadingEmptyLinesAffected(false), ChildrenAffected(false) {
assert(!Line.Tokens.empty());
// Calculate Next and Previous for all tokens. Note that we must overwrite
@@ -96,6 +96,9 @@ public:
/// input ranges.
bool LeadingEmptyLinesAffected;
+ /// \c True if a one of this line's children intersects with an input range.
+ bool ChildrenAffected;
+
private:
// Disallow copying.
AnnotatedLine(const AnnotatedLine &) LLVM_DELETED_FUNCTION;
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=195925&r1=195924&r2=195925&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Nov 28 09:58:55 2013
@@ -2511,6 +2511,36 @@ TEST_F(FormatTest, LayoutNestedBlocks) {
" return;\n"
" },\n"
" a);", Style);
+}
+
+TEST_F(FormatTest, IndividualStatementsOfNestedBlocks) {
+ EXPECT_EQ("DEBUG({\n"
+ " int i;\n"
+ " int j;\n"
+ "});",
+ format("DEBUG( {\n"
+ " int i;\n"
+ " int j;\n"
+ "} ) ;",
+ 20, 1, getLLVMStyle()));
+ EXPECT_EQ("DEBUG( {\n"
+ " int i;\n"
+ " int j;\n"
+ "} ) ;",
+ format("DEBUG( {\n"
+ " int i;\n"
+ " int j;\n"
+ "} ) ;",
+ 41, 1, getLLVMStyle()));
+ EXPECT_EQ("DEBUG({\n"
+ " int i;\n"
+ " int j;\n"
+ "});",
+ format("DEBUG( {\n"
+ " int i;\n"
+ " int j;\n"
+ "} ) ;",
+ 20, 1, getLLVMStyle()));
EXPECT_EQ("Debug({\n"
" if (aaaaaaaaaaaaaaaaaaaaaaaa)\n"
@@ -2523,18 +2553,26 @@ TEST_F(FormatTest, LayoutNestedBlocks) {
" },\n"
" a);",
50, 1, getLLVMStyle()));
-}
-
-TEST_F(FormatTest, IndividualStatementsOfNestedBlocks) {
EXPECT_EQ("DEBUG({\n"
- " int i;\n"
- " int j;\n"
+ " DEBUG({\n"
+ " int a;\n"
+ " int b;\n"
+ " }) ;\n"
"});",
- format("DEBUG( {\n"
- " int i;\n"
- " int j;\n"
- "} ) ;",
- 40, 1, getLLVMStyle()));
+ format("DEBUG({\n"
+ " DEBUG({\n"
+ " int a;\n"
+ " int b;\n" // Format this line only.
+ " }) ;\n" // Don't touch this line.
+ "});",
+ 35, 0, getLLVMStyle()));
+ EXPECT_EQ("DEBUG({\n"
+ " int a; //\n"
+ "});",
+ format("DEBUG({\n"
+ " int a; //\n"
+ "});",
+ 0, 0, getLLVMStyle()));
}
TEST_F(FormatTest, PutEmptyBlocksIntoOneLine) {
More information about the cfe-commits
mailing list