r345242 - [clang-format] Break before next parameter after a formatted multiline raw string parameter
Krasimir Georgiev via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 25 00:39:30 PDT 2018
Author: krasimir
Date: Thu Oct 25 00:39:30 2018
New Revision: 345242
URL: http://llvm.org/viewvc/llvm-project?rev=345242&view=rev
Log:
[clang-format] Break before next parameter after a formatted multiline raw string parameter
Summary:
Currently clang-format breaks before the next parameter after multiline parameters (also recursively for the parent expressions of multiline parameters). However, it fails to do so for formatted multiline raw string literals:
```
$ cat test.cc
// Examples
// Regular multiline tokens
int x = f(R"(multi
line)", 2);
}
int y = g(h(R"(multi
line)"), 2);
// Formatted multiline tokens
int z = f(R"pb(multi: 1 #
line: 2)pb", 2);
int w = g(h(R"pb(multi: 1 #
line: 2)pb"), 2);
$ clang-format -style=google test.cc
// Examples
// Regular multiline tokens
int x = f(R"(multi
line)",
2);
}
int y = g(h(R"(multi
line)"),
2);
// Formatted multiline tokens
int z = f(R"pb(multi: 1 #
line: 2)pb", 2);
int w = g(h(R"pb(multi: 1 #
line: 2)pb"), 2);
```
This patch addresses this inconsistency by forcing breaking after multiline formatted raw string literals. This requires a little tweak to the indentation chosen for the contents of a formatted raw string literal: in case when that's a parameter and not the last one, the indentation is based off of the uniform indentation of all of the parameters.
Reviewers: sammccall
Reviewed By: sammccall
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D52448
Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/unittests/Format/FormatTestRawStrings.cpp
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=345242&r1=345241&r2=345242&view=diff
==============================================================================
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Thu Oct 25 00:39:30 2018
@@ -1502,10 +1502,25 @@ unsigned ContinuationIndenter::reformatR
// violate the rectangle rule and visually flows within the surrounding
// source.
bool ContentStartsOnNewline = Current.TokenText[OldPrefixSize] == '\n';
- unsigned NextStartColumn =
- ContentStartsOnNewline
- ? State.Stack.back().NestedBlockIndent + Style.IndentWidth
- : FirstStartColumn;
+ // If this token is the last parameter (checked by looking if it's followed by
+ // `)`, the base the indent off the line's nested block indent. Otherwise,
+ // base the indent off the arguments indent, so we can achieve:
+ // fffffffffff(1, 2, 3, R"pb(
+ // key1: 1 #
+ // key2: 2)pb");
+ //
+ // fffffffffff(1, 2, 3,
+ // R"pb(
+ // key1: 1 #
+ // key2: 2
+ // )pb",
+ // 5);
+ unsigned CurrentIndent = (Current.Next && Current.Next->is(tok::r_paren))
+ ? State.Stack.back().NestedBlockIndent
+ : State.Stack.back().Indent;
+ unsigned NextStartColumn = ContentStartsOnNewline
+ ? CurrentIndent + Style.IndentWidth
+ : FirstStartColumn;
// The last start column is the column the raw string suffix starts if it is
// put on a newline.
@@ -1517,7 +1532,7 @@ unsigned ContinuationIndenter::reformatR
// indent.
unsigned LastStartColumn = Current.NewlinesBefore
? FirstStartColumn - NewPrefixSize
- : State.Stack.back().NestedBlockIndent;
+ : CurrentIndent;
std::pair<tooling::Replacements, unsigned> Fixes = internal::reformat(
RawStringStyle, RawText, {tooling::Range(0, RawText.size())},
@@ -1527,8 +1542,7 @@ unsigned ContinuationIndenter::reformatR
auto NewCode = applyAllReplacements(RawText, Fixes.first);
tooling::Replacements NoFixes;
if (!NewCode) {
- State.Column += Current.ColumnWidth;
- return 0;
+ return addMultilineToken(Current, State);
}
if (!DryRun) {
if (NewDelimiter != OldDelimiter) {
@@ -1577,6 +1591,13 @@ unsigned ContinuationIndenter::reformatR
unsigned PrefixExcessCharacters =
StartColumn + NewPrefixSize > Style.ColumnLimit ?
StartColumn + NewPrefixSize - Style.ColumnLimit : 0;
+ bool IsMultiline =
+ ContentStartsOnNewline || (NewCode->find('\n') != std::string::npos);
+ if (IsMultiline) {
+ // Break before further function parameters on all levels.
+ for (unsigned i = 0, e = State.Stack.size(); i != e; ++i)
+ State.Stack[i].BreakBeforeParameter = true;
+ }
return Fixes.second + PrefixExcessCharacters * Style.PenaltyExcessCharacter;
}
Modified: cfe/trunk/unittests/Format/FormatTestRawStrings.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestRawStrings.cpp?rev=345242&r1=345241&r2=345242&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTestRawStrings.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestRawStrings.cpp Thu Oct 25 00:39:30 2018
@@ -576,10 +576,13 @@ auto S =
auto S = R"pb(item_1:1 item_2:2)pb"+rest;)test",
getRawStringPbStyleWithColumns(40)));
+ // `rest` fits on the line after )pb", but forced on newline since the raw
+ // string literal is multiline.
expect_eq(R"test(
auto S = R"pb(item_1: 1,
item_2: 2,
- item_3: 3)pb" + rest;)test",
+ item_3: 3)pb" +
+ rest;)test",
format(R"test(
auto S = R"pb(item_1:1,item_2:2,item_3:3)pb"+rest;)test",
getRawStringPbStyleWithColumns(40)));
@@ -615,7 +618,8 @@ auto S = first+R"pb(item_1:1 item_2:2)pb
expect_eq(R"test(
auto S = R"pb(item_1: 1,
item_2: 2,
- item_3: 3)pb" + rest;)test",
+ item_3: 3)pb" +
+ rest;)test",
format(R"test(
auto S = R"pb(item_1:1,item_2:2,item_3:3)pb"+rest;)test",
getRawStringPbStyleWithColumns(40)));
@@ -889,6 +893,95 @@ item {
Style));
}
+TEST_F(FormatTestRawStrings,
+ BreaksBeforeNextParamAfterMultilineRawStringParam) {
+ FormatStyle Style = getRawStringPbStyleWithColumns(60);
+ expect_eq(R"test(
+int f() {
+ int a = g(x, R"pb(
+ key: 1 #
+ key: 2
+ )pb",
+ 3, 4);
+}
+)test",
+ format(R"test(
+int f() {
+ int a = g(x, R"pb(
+ key: 1 #
+ key: 2
+ )pb", 3, 4);
+}
+)test",
+ Style));
+
+ // Breaks after a parent of a multiline param.
+ expect_eq(R"test(
+int f() {
+ int a = g(x, h(R"pb(
+ key: 1 #
+ key: 2
+ )pb"),
+ 3, 4);
+}
+)test",
+ format(R"test(
+int f() {
+ int a = g(x, h(R"pb(
+ key: 1 #
+ key: 2
+ )pb"), 3, 4);
+}
+)test",
+ Style));
+
+ expect_eq(R"test(
+int f() {
+ int a = g(x,
+ h(R"pb(
+ key: 1 #
+ key: 2
+ )pb",
+ 2),
+ 3, 4);
+}
+)test",
+ format(R"test(
+int f() {
+ int a = g(x, h(R"pb(
+ key: 1 #
+ key: 2
+ )pb", 2), 3, 4);
+}
+)test",
+ Style));
+ // Breaks if formatting introduces a multiline raw string.
+ expect_eq(R"test(
+int f() {
+ int a = g(x, R"pb(key1: value111111111
+ key2: value2222222222)pb",
+ 3, 4);
+}
+)test",
+ format(R"test(
+int f() {
+ int a = g(x, R"pb(key1: value111111111 key2: value2222222222)pb", 3, 4);
+}
+)test",
+ Style));
+ // Does not force a break after an original multiline param that is
+ // reformatterd as on single line.
+ expect_eq(R"test(
+int f() {
+ int a = g(R"pb(key: 1)pb", 2);
+})test",
+ format(R"test(
+int f() {
+ int a = g(R"pb(key:
+ 1)pb", 2);
+})test", Style));
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
More information about the cfe-commits
mailing list