russellmcc created this revision.
russellmcc added a reviewer: djasper.
Herald added a subscriber: cfe-commits.
When clang-format is set to always use tabs (with tab width 4), when asked to
format line 3 (using the `-lines=3:3` flag):
int f() {
int a;
foobar();
int b;
}
We would expect clang-format to not modify the leading whitespace on line 4.
However, it does - see the new test. This is because the whitespace manager is
called to replace the whitespace before the first token of line 4. This is
necessary to edit the number of new lines before line 4, and to edit the
trailing whitespace on line 3. I've added a flag to `replaceWhitespace` that
allows it to not edit the leading whitespace, and only edit the whitespace up
to the last newline.
We ran into this when trying to integrate clang-format into our CI to ensure no
new formatting diffs were introduced in a patch. With the behavior without
this patch, the number of affected lines grew each time clang-format was run,
so running clang-format locally was not enough to ensure that CI would pass.
Repository:
rC Clang
https://reviews.llvm.org/D54881
Files:
lib/Format/UnwrappedLineFormatter.cpp
lib/Format/UnwrappedLineFormatter.h
lib/Format/WhitespaceManager.cpp
lib/Format/WhitespaceManager.h
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -36,12 +36,12 @@
SC_DoNotCheck
};
- std::string format(llvm::StringRef Code,
- const FormatStyle &Style = getLLVMStyle(),
- StatusCheck CheckComplete = SC_ExpectComplete) {
+ std::string formatRange(llvm::StringRef Code, tooling::Range Range,
+ const FormatStyle &Style = getLLVMStyle(),
+ StatusCheck CheckComplete = SC_ExpectComplete) {
LLVM_DEBUG(llvm::errs() << "---\n");
LLVM_DEBUG(llvm::errs() << Code << "\n\n");
- std::vector<tooling::Range> Ranges(1, tooling::Range(0, Code.size()));
+ std::vector<tooling::Range> Ranges(1, std::move(Range));
FormattingAttemptStatus Status;
tooling::Replacements Replaces =
reformat(Style, Code, Ranges, "<stdin>", &Status);
@@ -57,6 +57,13 @@
return *Result;
}
+ std::string format(llvm::StringRef Code,
+ const FormatStyle &Style = getLLVMStyle(),
+ StatusCheck CheckComplete = SC_ExpectComplete) {
+ return formatRange(Code, tooling::Range(0, Code.size()), Style,
+ CheckComplete);
+ }
+
FormatStyle getStyleWithColumns(FormatStyle Style, unsigned ColumnLimit) {
Style.ColumnLimit = ColumnLimit;
return Style;
@@ -110,6 +117,14 @@
format(Code, Style, SC_DoNotCheck);
}
+ void verifyWithPrefixAndSuffix(llvm::StringRef Expected, llvm::StringRef Code,
+ llvm::StringRef prefix, llvm::StringRef suffix,
+ const FormatStyle &Style = getLLVMStyle()) {
+ EXPECT_EQ(llvm::Twine(prefix + Expected + suffix).str(),
+ formatRange(llvm::Twine(prefix + Code + suffix).str(),
+ tooling::Range(prefix.size(), Code.size()), Style));
+ }
+
int ReplacementCount;
};
@@ -9141,6 +9156,7 @@
"\t */\n"
"\t int i;\n"
"}"));
+
Tab.AlignConsecutiveAssignments = true;
Tab.AlignConsecutiveDeclarations = true;
Tab.TabWidth = 4;
@@ -9156,6 +9172,18 @@
Tab);
}
+TEST_F(FormatTest, FormattingIndentationDoesNotAffectOtherLines) {
+ FormatStyle Tab = getLLVMStyleWithColumns(42);
+ Tab.TabWidth = 4;
+ Tab.UseTab = FormatStyle::UT_Always;
+ verifyWithPrefixAndSuffix("\tfoobar();", " foobar();",
+ "int f() {\n int a;\n", "\n int b;\n}\n",
+ Tab);
+ Tab.UseTab = FormatStyle::UT_Never;
+ verifyWithPrefixAndSuffix(" foobar();", "\tfoobar();",
+ "int f() {\n\tint a;\n", "\n\tint b;\n}\n", Tab);
+}
+
TEST_F(FormatTest, CalculatesOriginalColumn) {
EXPECT_EQ("\"qqqqqqqqqqqqqqqqqqqqqqqqqq\\\n"
"q\"; /* some\n"
Index: lib/Format/WhitespaceManager.h
===================================================================
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -49,7 +49,8 @@
/// into tabs and spaces for some format styles.
void replaceWhitespace(FormatToken &Tok, unsigned Newlines, unsigned Spaces,
unsigned StartOfTokenColumn,
- bool InPPDirective = false);
+ bool InPPDirective = false,
+ bool OnlyUntilLastNewline = false);
/// Adds information about an unchangeable token's whitespace.
///
Index: lib/Format/WhitespaceManager.cpp
===================================================================
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -46,13 +46,23 @@
void WhitespaceManager::replaceWhitespace(FormatToken &Tok, unsigned Newlines,
unsigned Spaces,
unsigned StartOfTokenColumn,
- bool InPPDirective) {
+ bool InPPDirective,
+ bool OnlyUntilLastNewline) {
if (Tok.Finalized)
return;
Tok.Decision = (Newlines > 0) ? FD_Break : FD_Continue;
- Changes.push_back(Change(Tok, /*CreateReplacement=*/true, Tok.WhitespaceRange,
- Spaces, StartOfTokenColumn, Newlines, "", "",
- InPPDirective && !Tok.IsFirst,
+ auto OriginalWhitespaceRange = Tok.WhitespaceRange;
+
+ if (OnlyUntilLastNewline) {
+ OriginalWhitespaceRange.setEnd(
+ OriginalWhitespaceRange.getBegin().getLocWithOffset(
+ Tok.LastNewlineOffset));
+ Spaces = 0;
+ StartOfTokenColumn = 0;
+ }
+ Changes.push_back(Change(Tok, /*CreateReplacement=*/true,
+ OriginalWhitespaceRange, Spaces, StartOfTokenColumn,
+ Newlines, "", "", InPPDirective && !Tok.IsFirst,
/*IsInsideToken=*/false));
}
Index: lib/Format/UnwrappedLineFormatter.h
===================================================================
--- lib/Format/UnwrappedLineFormatter.h
+++ lib/Format/UnwrappedLineFormatter.h
@@ -51,7 +51,8 @@
void formatFirstToken(const AnnotatedLine &Line,
const AnnotatedLine *PreviousLine,
const SmallVectorImpl<AnnotatedLine *> &Lines,
- unsigned Indent, unsigned NewlineIndent);
+ unsigned Indent, unsigned NewlineIndent,
+ bool OnlyUntilLastNewline = false);
/// Returns the column limit for a line, taking into account whether we
/// need an escaped newline due to a continued preprocessor directive.
Index: lib/Format/UnwrappedLineFormatter.cpp
===================================================================
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -1113,7 +1113,7 @@
if (ReformatLeadingWhitespace)
formatFirstToken(TheLine, PreviousLine, Lines,
TheLine.First->OriginalColumn,
- TheLine.First->OriginalColumn);
+ TheLine.First->OriginalColumn, true);
else
Whitespaces->addUntouchableToken(*TheLine.First,
TheLine.InPPDirective);
@@ -1136,7 +1136,7 @@
void UnwrappedLineFormatter::formatFirstToken(
const AnnotatedLine &Line, const AnnotatedLine *PreviousLine,
const SmallVectorImpl<AnnotatedLine *> &Lines, unsigned Indent,
- unsigned NewlineIndent) {
+ unsigned NewlineIndent, bool OnlyUntilLastNewline) {
FormatToken &RootToken = *Line.First;
if (RootToken.is(tok::eof)) {
unsigned Newlines = std::min(RootToken.NewlinesBefore, 1u);
@@ -1188,7 +1188,8 @@
Whitespaces->replaceWhitespace(RootToken, Newlines, Indent, Indent,
Line.InPPDirective &&
- !RootToken.HasUnescapedNewline);
+ !RootToken.HasUnescapedNewline,
+ OnlyUntilLastNewline);
}
unsigned
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
