[clang-format] Revert breaking stream operators to previous default#89016
Conversation
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
left a comment
There was a problem hiding this comment.
I'd be happy to see this, I'm sure you can come up with a better choice of options than I did.
|
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 |
…ious default (llvm#89016) Backport 29ecd6d
…ious default (llvm#89016) Backport 29ecd6d
…ious default (llvm#89016) Backport 29ecd6d
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.