r330573 - Format closing braces when reformatting the line containing the opening brace.
Manuel Klimek via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 23 02:34:26 PDT 2018
Author: klimek
Date: Mon Apr 23 02:34:26 2018
New Revision: 330573
URL: http://llvm.org/viewvc/llvm-project?rev=330573&view=rev
Log:
Format closing braces when reformatting the line containing the opening brace.
This required a couple of yaks to be shaved:
1. MatchingOpeningBlockLineIndex was misused to also store the
closing index; instead, use a second variable, as this 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();
Differential Revision: https://reviews.llvm.org/D45726
Modified:
cfe/trunk/lib/Format/AffectedRangeManager.cpp
cfe/trunk/lib/Format/AffectedRangeManager.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
cfe/trunk/lib/Format/SortJavaScriptImports.cpp
cfe/trunk/lib/Format/TokenAnnotator.h
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/lib/Format/UnwrappedLineParser.h
cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
cfe/trunk/unittests/Format/FormatTestSelective.cpp
Modified: cfe/trunk/lib/Format/AffectedRangeManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/AffectedRangeManager.cpp?rev=330573&r1=330572&r2=330573&view=diff
==============================================================================
--- cfe/trunk/lib/Format/AffectedRangeManager.cpp (original)
+++ cfe/trunk/lib/Format/AffectedRangeManager.cpp Mon Apr 23 02:34:26 2018
@@ -21,8 +21,9 @@ namespace clang {
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 @@ bool AffectedRangeManager::computeAffect
continue;
}
- if (nonPPLineAffected(Line, PreviousLine))
+ if (nonPPLineAffected(Line, PreviousLine, Lines))
SomeLineAffected = true;
PreviousLine = Line;
@@ -99,10 +100,10 @@ void AffectedRangeManager::markAllAsAffe
}
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 @@ bool AffectedRangeManager::nonPPLineAffe
Line->First->NewlinesBefore < 2 && PreviousLine &&
PreviousLine->Affected && PreviousLine->Last->is(tok::comment);
+ bool IsAffectedClosingBrace =
+ Line->First->is(tok::r_brace) &&
+ Line->MatchingOpeningBlockLineIndex != UnwrappedLine::kInvalidIndex &&
+ Lines[Line->MatchingOpeningBlockLineIndex]->Affected;
+
if (SomeTokenAffected || SomeFirstChildAffected || LineMoved ||
- IsContinuedComment) {
+ IsContinuedComment || IsAffectedClosingBrace) {
Line->Affected = true;
SomeLineAffected = true;
}
Modified: cfe/trunk/lib/Format/AffectedRangeManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/AffectedRangeManager.h?rev=330573&r1=330572&r2=330573&view=diff
==============================================================================
--- cfe/trunk/lib/Format/AffectedRangeManager.h (original)
+++ cfe/trunk/lib/Format/AffectedRangeManager.h Mon Apr 23 02:34:26 2018
@@ -30,10 +30,9 @@ public:
: 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 @@ private:
// 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;
Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=330573&r1=330572&r2=330573&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Mon Apr 23 02:34:26 2018
@@ -1006,8 +1006,7 @@ public:
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 @@ public:
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 @@ public:
// 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);
Modified: cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp?rev=330573&r1=330572&r2=330573&view=diff
==============================================================================
--- cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp (original)
+++ cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp Mon Apr 23 02:34:26 2018
@@ -141,8 +141,7 @@ std::pair<tooling::Replacements, unsigne
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;
Modified: cfe/trunk/lib/Format/SortJavaScriptImports.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/SortJavaScriptImports.cpp?rev=330573&r1=330572&r2=330573&view=diff
==============================================================================
--- cfe/trunk/lib/Format/SortJavaScriptImports.cpp (original)
+++ cfe/trunk/lib/Format/SortJavaScriptImports.cpp Mon Apr 23 02:34:26 2018
@@ -128,8 +128,7 @@ public:
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;
Modified: cfe/trunk/lib/Format/TokenAnnotator.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.h?rev=330573&r1=330572&r2=330573&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.h (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.h Mon Apr 23 02:34:26 2018
@@ -40,6 +40,7 @@ public:
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 @@ public:
LineType Type;
unsigned Level;
size_t MatchingOpeningBlockLineIndex;
+ size_t MatchingClosingBlockLineIndex;
bool InPPDirective;
bool MustBeDeclaration;
bool MightBeFunctionDecl;
Modified: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp?rev=330573&r1=330572&r2=330573&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp Mon Apr 23 02:34:26 2018
@@ -252,9 +252,9 @@ private:
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
@@ -1033,9 +1033,12 @@ UnwrappedLineFormatter::format(const Sma
// 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;
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=330573&r1=330572&r2=330573&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Mon Apr 23 02:34:26 2018
@@ -570,7 +570,7 @@ void UnwrappedLineParser::parseBlock(boo
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;
}
}
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=330573&r1=330572&r2=330573&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.h (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.h Mon Apr 23 02:34:26 2018
@@ -53,7 +53,11 @@ struct UnwrappedLine {
/// \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 If this \c UnwrappedLine opens a block, stores the index of the
+ /// line with the corresponding closing brace.
+ size_t MatchingClosingBlockLineIndex = kInvalidIndex;
static const size_t kInvalidIndex = -1;
Modified: cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp?rev=330573&r1=330572&r2=330573&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp (original)
+++ cfe/trunk/lib/Format/UsingDeclarationsSorter.cpp Mon Apr 23 02:34:26 2018
@@ -187,8 +187,7 @@ std::pair<tooling::Replacements, unsigne
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) {
Modified: cfe/trunk/unittests/Format/FormatTestSelective.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestSelective.cpp?rev=330573&r1=330572&r2=330573&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestSelective.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestSelective.cpp Mon Apr 23 02:34:26 2018
@@ -177,6 +177,72 @@ TEST_F(FormatTestSelective, FormatsComme
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 @@ TEST_F(FormatTestSelective, StopFormatti
" if (a) {\n"
" g();\n"
" h();\n"
- "}\n"
+ " }\n"
"\n"
"void g() {\n"
"}",
More information about the cfe-commits
mailing list