[clang-format] Revert breaking stream operators to previous default #89016
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reverts commit d68826d, which changes the previous default behavior of always breaking before a stream insertion operator `<<` if both operands are string literals. Also reverts the related commits 27f5479 and bf05be5. See the discussion in llvm#88483.
|
@llvm/pr-subscribers-clang-format Author: Owen Pan (owenca) ChangesReverts commit d68826d, which changes the previous default behavior of always breaking before a stream insertion operator Also reverts the related commits 27f5479 and bf05be5. See the discussion in #88483. Full diff: https://github.com/llvm/llvm-project/pull/89016.diff 5 Files Affected:
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 628f70417866c3..80e5605fb5778d 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -5595,12 +5595,8 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
return true;
if (Left.IsUnterminatedLiteral)
return true;
- // FIXME: Breaking after newlines seems useful in general. Turn this into an
- // option and recognize more cases like endl etc, and break independent of
- // what comes after operator lessless.
- if (Right.is(tok::lessless) && Right.Next &&
- Right.Next->is(tok::string_literal) && Left.is(tok::string_literal) &&
- Left.TokenText.ends_with("\\n\"")) {
+ if (Right.is(tok::lessless) && Right.Next && Left.is(tok::string_literal) &&
+ Right.Next->is(tok::string_literal)) {
return true;
}
if (Right.is(TT_RequiresClause)) {
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 4906b3350b5b22..bc61b9c089e922 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -27339,12 +27339,6 @@ TEST_F(FormatTest, PPDirectivesAndCommentsInBracedInit) {
getLLVMStyleWithColumns(30));
}
-TEST_F(FormatTest, StreamOutputOperator) {
- verifyFormat("std::cout << \"foo\" << \"bar\" << baz;");
- verifyFormat("std::cout << \"foo\\n\"\n"
- " << \"bar\";");
-}
-
TEST_F(FormatTest, BreakAdjacentStringLiterals) {
constexpr StringRef Code{
"return \"Code\" \"\\0\\52\\26\\55\\55\\0\" \"x013\" \"\\02\\xBA\";"};
@@ -27359,6 +27353,7 @@ TEST_F(FormatTest, BreakAdjacentStringLiterals) {
Style.BreakAdjacentStringLiterals = false;
verifyFormat(Code, Style);
}
+
} // namespace
} // namespace test
} // namespace format
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index da02ced8c7a949..a71b6806476956 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2841,15 +2841,6 @@ TEST_F(TokenAnnotatorTest, BraceKind) {
EXPECT_BRACE_KIND(Tokens[16], BK_BracedInit);
}
-TEST_F(TokenAnnotatorTest, StreamOperator) {
- auto Tokens = annotate("\"foo\\n\" << aux << \"foo\\n\" << \"foo\";");
- ASSERT_EQ(Tokens.size(), 9u) << Tokens;
- EXPECT_FALSE(Tokens[1]->MustBreakBefore);
- EXPECT_FALSE(Tokens[3]->MustBreakBefore);
- // Only break between string literals if the former ends with \n.
- EXPECT_TRUE(Tokens[5]->MustBreakBefore);
-}
-
TEST_F(TokenAnnotatorTest, UnderstandsElaboratedTypeSpecifier) {
auto Tokens = annotate("auto foo() -> enum En {}");
ASSERT_EQ(Tokens.size(), 10u) << Tokens;
diff --git a/polly/lib/Analysis/DependenceInfo.cpp b/polly/lib/Analysis/DependenceInfo.cpp
index 9ee004fbac9e53..a530fa7b58fa6e 100644
--- a/polly/lib/Analysis/DependenceInfo.cpp
+++ b/polly/lib/Analysis/DependenceInfo.cpp
@@ -951,8 +951,8 @@ class DependenceInfoPrinterLegacyPass final : public ScopPass {
bool runOnScop(Scop &S) override {
DependenceInfo &P = getAnalysis<DependenceInfo>();
- OS << "Printing analysis '" << P.getPassName() << "' for " << "region: '"
- << S.getRegion().getNameStr() << "' in function '"
+ OS << "Printing analysis '" << P.getPassName() << "' for "
+ << "region: '" << S.getRegion().getNameStr() << "' in function '"
<< S.getFunction().getName() << "':\n";
P.printScop(OS, S);
diff --git a/polly/lib/Analysis/ScopBuilder.cpp b/polly/lib/Analysis/ScopBuilder.cpp
index 214e4d360d6bbb..a34f2931527255 100644
--- a/polly/lib/Analysis/ScopBuilder.cpp
+++ b/polly/lib/Analysis/ScopBuilder.cpp
@@ -2715,9 +2715,10 @@ void ScopBuilder::addUserContext() {
if (NameContext != NameUserContext) {
std::string SpaceStr = stringFromIslObj(Space, "null");
errs() << "Error: the name of dimension " << i
- << " provided in -polly-context " << "is '" << NameUserContext
- << "', but the name in the computed " << "context is '"
- << NameContext << "'. Due to this name mismatch, "
+ << " provided in -polly-context "
+ << "is '" << NameUserContext << "', but the name in the computed "
+ << "context is '" << NameContext
+ << "'. Due to this name mismatch, "
<< "the -polly-context option is ignored. Please provide "
<< "the context in the parameter space: " << SpaceStr << ".\n";
return;
|
You can test this locally with the following command:git-clang-format --diff 5964c944bfe74cee2872cddb66eff22866cdb6ee 930f422681b3a4f248afc094d3af98952cd7d386 -- clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTest.cpp clang/unittests/Format/TokenAnnotatorTest.cpp polly/lib/Analysis/DependenceInfo.cpp polly/lib/Analysis/ScopBuilder.cppView the diff from clang-format here.diff --git a/polly/lib/Analysis/DependenceInfo.cpp b/polly/lib/Analysis/DependenceInfo.cpp
index a530fa7b58..9ee004fbac 100644
--- a/polly/lib/Analysis/DependenceInfo.cpp
+++ b/polly/lib/Analysis/DependenceInfo.cpp
@@ -951,8 +951,8 @@ public:
bool runOnScop(Scop &S) override {
DependenceInfo &P = getAnalysis<DependenceInfo>();
- OS << "Printing analysis '" << P.getPassName() << "' for "
- << "region: '" << S.getRegion().getNameStr() << "' in function '"
+ OS << "Printing analysis '" << P.getPassName() << "' for " << "region: '"
+ << S.getRegion().getNameStr() << "' in function '"
<< S.getFunction().getName() << "':\n";
P.printScop(OS, S);
diff --git a/polly/lib/Analysis/ScopBuilder.cpp b/polly/lib/Analysis/ScopBuilder.cpp
index a34f293152..214e4d360d 100644
--- a/polly/lib/Analysis/ScopBuilder.cpp
+++ b/polly/lib/Analysis/ScopBuilder.cpp
@@ -2715,10 +2715,9 @@ void ScopBuilder::addUserContext() {
if (NameContext != NameUserContext) {
std::string SpaceStr = stringFromIslObj(Space, "null");
errs() << "Error: the name of dimension " << i
- << " provided in -polly-context "
- << "is '" << NameUserContext << "', but the name in the computed "
- << "context is '" << NameContext
- << "'. Due to this name mismatch, "
+ << " provided in -polly-context " << "is '" << NameUserContext
+ << "', but the name in the computed " << "context is '"
+ << NameContext << "'. Due to this name mismatch, "
<< "the -polly-context option is ignored. Please provide "
<< "the context in the parameter space: " << SpaceStr << ".\n";
return;
|
mydeveloperday
approved these changes
Apr 17, 2024
|
Do you think we should push this into the 18 branch? |
|
Yep. |
|
/cherry-pick 29ecd6d |
|
Failed to cherry-pick: 29ecd6d https://github.com/llvm/llvm-project/actions/runs/8756865337 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
owenca
added a commit
to owenca/llvm-project
that referenced
this pull request
Apr 20, 2024
…ious default (llvm#89016) Backport 29ecd6d
owenca
added a commit
to owenca/llvm-project
that referenced
this pull request
Apr 25, 2024
…ious default (llvm#89016) Backport 29ecd6d
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reverts commit d68826d, which changes the previous default behavior of always breaking before a stream insertion operator
<<if both operands are string literals.Also reverts the related commits 27f5479 and bf05be5.
See the discussion in #88483.