klimek created this revision.
klimek added a reviewer: krasimir.
...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();
Format closing braces.
Repository:
rC Clang
https://reviews.llvm.org/D45726
Files:
lib/Format/AffectedRangeManager.cpp
lib/Format/AffectedRangeManager.h
lib/Format/Format.cpp
lib/Format/NamespaceEndCommentsFixer.cpp
lib/Format/SortJavaScriptImports.cpp
lib/Format/TokenAnnotator.h
lib/Format/UnwrappedLineFormatter.cpp
lib/Format/UnwrappedLineParser.cpp
lib/Format/UnwrappedLineParser.h
lib/Format/UsingDeclarationsSorter.cpp
unittests/Format/FormatTestSelective.cpp
Index: unittests/Format/FormatTestSelective.cpp
===================================================================
--- unittests/Format/FormatTestSelective.cpp
+++ unittests/Format/FormatTestSelective.cpp
@@ -177,6 +177,72 @@
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 @@
" if (a) {\n"
" g();\n"
" h();\n"
- "}\n"
+ " }\n"
"\n"
"void g() {\n"
"}",
Index: lib/Format/UsingDeclarationsSorter.cpp
===================================================================
--- lib/Format/UsingDeclarationsSorter.cpp
+++ lib/Format/UsingDeclarationsSorter.cpp
@@ -187,8 +187,7 @@
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) {
Index: lib/Format/UnwrappedLineParser.h
===================================================================
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -53,7 +53,10 @@
/// \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 The corresponding closing line to MatchingOpeningBlockLineIndex.
+ size_t MatchingClosingBlockLineIndex = kInvalidIndex;
static const size_t kInvalidIndex = -1;
Index: lib/Format/UnwrappedLineParser.cpp
===================================================================
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -570,7 +570,7 @@
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;
}
}
Index: lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -251,9 +251,9 @@
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
@@ -1032,9 +1032,12 @@
// 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;
Index: lib/Format/TokenAnnotator.h
===================================================================
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -40,6 +40,7 @@
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 @@
LineType Type;
unsigned Level;
size_t MatchingOpeningBlockLineIndex;
+ size_t MatchingClosingBlockLineIndex;
bool InPPDirective;
bool MustBeDeclaration;
bool MightBeFunctionDecl;
Index: lib/Format/SortJavaScriptImports.cpp
===================================================================
--- lib/Format/SortJavaScriptImports.cpp
+++ lib/Format/SortJavaScriptImports.cpp
@@ -128,8 +128,7 @@
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;
Index: lib/Format/NamespaceEndCommentsFixer.cpp
===================================================================
--- lib/Format/NamespaceEndCommentsFixer.cpp
+++ lib/Format/NamespaceEndCommentsFixer.cpp
@@ -141,8 +141,7 @@
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;
Index: lib/Format/Format.cpp
===================================================================
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1006,8 +1006,7 @@
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 @@
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 @@
// 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);
Index: lib/Format/AffectedRangeManager.h
===================================================================
--- lib/Format/AffectedRangeManager.h
+++ lib/Format/AffectedRangeManager.h
@@ -30,10 +30,9 @@
: 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 @@
// 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;
Index: lib/Format/AffectedRangeManager.cpp
===================================================================
--- lib/Format/AffectedRangeManager.cpp
+++ lib/Format/AffectedRangeManager.cpp
@@ -21,8 +21,9 @@
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 @@
continue;
}
- if (nonPPLineAffected(Line, PreviousLine))
+ if (nonPPLineAffected(Line, PreviousLine, Lines))
SomeLineAffected = true;
PreviousLine = Line;
@@ -99,10 +100,10 @@
}
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 @@
Line->First->NewlinesBefore < 2 && PreviousLine &&
PreviousLine->Affected && PreviousLine->Last->is(tok::comment);
+ bool IsAffectedClosingBrace =
+ Line->First->is(tok::r_brace) &&
+ Line->MatchingOpeningBlockLineIndex != -1 &&
+ Lines[Line->MatchingOpeningBlockLineIndex]->Affected;
+
if (SomeTokenAffected || SomeFirstChildAffected || LineMoved ||
- IsContinuedComment) {
+ IsContinuedComment || IsAffectedClosingBrace) {
Line->Affected = true;
SomeLineAffected = true;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
