-
Notifications
You must be signed in to change notification settings - Fork 15.1k
clang-format: Add splitting for strings with user-defined suffixes #167150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -252,6 +252,8 @@ class BreakableStringLiteral : public BreakableToken { | |||||
| /// after formatting. | ||||||
| BreakableStringLiteral(const FormatToken &Tok, unsigned StartColumn, | ||||||
| StringRef Prefix, StringRef Postfix, | ||||||
| StringRef ContinuationPrefix, | ||||||
| StringRef ContinuationPostfix, | ||||||
| unsigned UnbreakableTailLength, bool InPPDirective, | ||||||
| encoding::Encoding Encoding, const FormatStyle &Style); | ||||||
|
|
||||||
|
|
@@ -274,15 +276,21 @@ class BreakableStringLiteral : public BreakableToken { | |||||
| protected: | ||||||
| // The column in which the token starts. | ||||||
| unsigned StartColumn; | ||||||
| // The prefix a line needs after a break in the token. | ||||||
| // The prefix a line needs at the start | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| StringRef Prefix; | ||||||
| // The postfix a line needs before introducing a break. | ||||||
| // The postfix a line needs at the end | ||||||
| StringRef Postfix; | ||||||
| // The prefix every line except the first line needs | ||||||
| StringRef ContinuationPrefix; | ||||||
| // The postfix every line except the last line needs | ||||||
| StringRef ContinuationPostfix; | ||||||
| // The token text excluding the prefix and postfix. | ||||||
| StringRef Line; | ||||||
| // Length of the sequence of tokens after this string literal that cannot | ||||||
| // contain line breaks. | ||||||
| unsigned UnbreakableTailLength; | ||||||
| // Whether the string prefix and postfix should be repeated on each line | ||||||
| // when breaking the string. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leftover? |
||||||
| }; | ||||||
|
|
||||||
| class BreakableStringLiteralUsingOperators : public BreakableStringLiteral { | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2540,22 +2540,46 @@ ContinuationIndenter::createBreakableToken(const FormatToken &Current, | |||||||||
|
|
||||||||||
| StringRef Prefix; | ||||||||||
| StringRef Postfix; | ||||||||||
|
|
||||||||||
| // FIXME: Handle whitespace between '_T', '(', '"..."', and ')'. | ||||||||||
| // FIXME: Store Prefix and Suffix (or PrefixLength and SuffixLength to | ||||||||||
| // reduce the overhead) for each FormatToken, which is a string, so that we | ||||||||||
| // don't run multiple checks here on the hot path. | ||||||||||
| if ((Text.ends_with(Postfix = "\"") && | ||||||||||
| (Text.starts_with(Prefix = "@\"") || Text.starts_with(Prefix = "\"") || | ||||||||||
| Text.starts_with(Prefix = "u\"") || | ||||||||||
| Text.starts_with(Prefix = "U\"") || | ||||||||||
| Text.starts_with(Prefix = "u8\"") || | ||||||||||
| Text.starts_with(Prefix = "L\""))) || | ||||||||||
| (Text.starts_with(Prefix = "_T(\"") && | ||||||||||
| Text.ends_with(Postfix = "\")"))) { | ||||||||||
| if (Text.starts_with(Prefix = "_T(\"") && Text.ends_with(Postfix = "\")")) { | ||||||||||
| // We need to put `_T("` and `")` on each line because it is a macro | ||||||||||
| llvm::StringRef ContinuationPrefix = Prefix; | ||||||||||
| llvm::StringRef ContinuationPostfix = Postfix; | ||||||||||
|
|
||||||||||
| return std::make_unique<BreakableStringLiteral>( | ||||||||||
| Current, StartColumn, Prefix, Postfix, UnbreakableTailLength, | ||||||||||
| State.Line->InPPDirective, Encoding, Style); | ||||||||||
| Current, StartColumn, Prefix, Postfix, ContinuationPrefix, | ||||||||||
| ContinuationPostfix, UnbreakableTailLength, State.Line->InPPDirective, | ||||||||||
| Encoding, Style); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| static const auto PostfixRegex = | ||||||||||
| llvm::Regex(R"("(_[a-zA-Z_][a-zA-Z0-9_]*)?$)"); | ||||||||||
| llvm::SmallVector<llvm::StringRef, 1> Matches; | ||||||||||
|
|
||||||||||
| if (PostfixRegex.match(Text, &Matches)) { | ||||||||||
|
Comment on lines
+2561
to
+2563
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| Postfix = Matches.front(); | ||||||||||
|
|
||||||||||
| if ((Text.starts_with(Prefix = "@\"") || | ||||||||||
| Text.starts_with(Prefix = "\"") || | ||||||||||
| Text.starts_with(Prefix = "u\"") || | ||||||||||
| Text.starts_with(Prefix = "U\"") || | ||||||||||
| Text.starts_with(Prefix = "u8\"") || | ||||||||||
| Text.starts_with(Prefix = "L\""))) { | ||||||||||
|
|
||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Drop empty line. |
||||||||||
| // Repeat the prefix on every line but don't repeat the suffix | ||||||||||
| llvm::StringRef ContinuationPrefix = Prefix; | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| llvm::StringRef ContinuationPostfix = "\""; | ||||||||||
| return std::make_unique<BreakableStringLiteral>( | ||||||||||
| Current, StartColumn, Prefix, Postfix, ContinuationPrefix, | ||||||||||
| ContinuationPostfix, UnbreakableTailLength, | ||||||||||
| State.Line->InPPDirective, Encoding, Style); | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Drop |
||||||||||
| } else if (Current.is(TT_BlockComment)) { | ||||||||||
| if (Style.ReflowComments == FormatStyle::RCS_Never || | ||||||||||
| // If a comment token switches formatting, like | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15838,6 +15838,10 @@ TEST_F(FormatTest, BreaksWideAndNSStringLiterals) { | |||||||||||||
| "@\"NSString literal\";", getGoogleStyleWithColumns(19)); | ||||||||||||||
| verifyFormat(R"(NSString *s = @"那那那那";)", getLLVMStyleWithColumns(26)); | ||||||||||||||
|
|
||||||||||||||
| EXPECT_EQ("L\"suffixed \"\n" | ||||||||||||||
| "L\"string\"_s;", | ||||||||||||||
| format("L\"suffixed string\"_s;", getLLVMStyleWithColumns(19))); | ||||||||||||||
|
Comment on lines
+15841
to
+15843
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
|
|
||||||||||||||
| // This input makes clang-format try to split the incomplete unicode escape | ||||||||||||||
| // sequence, which used to lead to a crasher. | ||||||||||||||
| verifyNoCrash( | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.