[clang] d6daca2 - [clang-format] Fix BreakBeforeBinaryOperators with TemplateCloser on the lhs
Björn Schäpers via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 1 12:55:41 PST 2022
Author: Björn Schäpers
Date: 2022-03-01T21:55:32+01:00
New Revision: d6daca21738aed64837b825c32d4f1b4f54cec04
URL: https://github.com/llvm/llvm-project/commit/d6daca21738aed64837b825c32d4f1b4f54cec04
DIFF: https://github.com/llvm/llvm-project/commit/d6daca21738aed64837b825c32d4f1b4f54cec04.diff
LOG: [clang-format] Fix BreakBeforeBinaryOperators with TemplateCloser on the lhs
In the presence of pp branches we parse the token stream multiple times.
Thus the token already has the type set. It's best just not to assert on
any type in the parser.
Fixes https://github.com/llvm/llvm-project/issues/54019
Differential Revision: https://reviews.llvm.org/D120621
Added:
Modified:
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/FormatTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index d4ce01e327844..3015cd2be8879 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -4414,6 +4414,14 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
return false;
if (Left.is(TT_TemplateCloser) && Right.is(TT_TemplateOpener))
return true;
+ if ((Left.is(tok::greater) && Right.is(tok::greater)) ||
+ (Left.is(tok::less) && Right.is(tok::less)))
+ return false;
+ if (Right.is(TT_BinaryOperator) &&
+ Style.BreakBeforeBinaryOperators != FormatStyle::BOS_None &&
+ (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_All ||
+ Right.getPrecedence() != prec::Assignment))
+ return true;
if (Left.isOneOf(TT_TemplateCloser, TT_UnaryOperator) ||
Left.is(tok::kw_operator))
return false;
@@ -4487,14 +4495,6 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine &Line,
if (Right.is(TT_InheritanceComma) &&
Style.BreakInheritanceList == FormatStyle::BILS_BeforeComma)
return true;
- if ((Left.is(tok::greater) && Right.is(tok::greater)) ||
- (Left.is(tok::less) && Right.is(tok::less)))
- return false;
- if (Right.is(TT_BinaryOperator) &&
- Style.BreakBeforeBinaryOperators != FormatStyle::BOS_None &&
- (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_All ||
- Right.getPrecedence() != prec::Assignment))
- return true;
if (Left.is(TT_ArrayInitializerLSquare))
return true;
if (Right.is(tok::kw_typename) && Left.isNot(tok::kw_const))
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 64807e144ae10..5fa56a9048b03 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -6440,6 +6440,43 @@ TEST_F(FormatTest, AllowBinPackingInsideArguments) {
Style);
}
+TEST_F(FormatTest, BreakBinaryOperatorsInPresenceOfTemplates) {
+ auto Style = getLLVMStyleWithColumns(45);
+ EXPECT_EQ(Style.BreakBeforeBinaryOperators, FormatStyle::BOS_None);
+ verifyFormat("bool b =\n"
+ " is_default_constructible_v<hash<T>> and\n"
+ " is_copy_constructible_v<hash<T>> and\n"
+ " is_move_constructible_v<hash<T>> and\n"
+ " is_copy_assignable_v<hash<T>> and\n"
+ " is_move_assignable_v<hash<T>> and\n"
+ " is_destructible_v<hash<T>> and\n"
+ " is_swappable_v<hash<T>> and\n"
+ " is_callable_v<hash<T>(T)>;",
+ Style);
+
+ Style.BreakBeforeBinaryOperators = FormatStyle::BOS_NonAssignment;
+ verifyFormat("bool b = is_default_constructible_v<hash<T>>\n"
+ " and is_copy_constructible_v<hash<T>>\n"
+ " and is_move_constructible_v<hash<T>>\n"
+ " and is_copy_assignable_v<hash<T>>\n"
+ " and is_move_assignable_v<hash<T>>\n"
+ " and is_destructible_v<hash<T>>\n"
+ " and is_swappable_v<hash<T>>\n"
+ " and is_callable_v<hash<T>(T)>;",
+ Style);
+
+ Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+ verifyFormat("bool b = is_default_constructible_v<hash<T>>\n"
+ " and is_copy_constructible_v<hash<T>>\n"
+ " and is_move_constructible_v<hash<T>>\n"
+ " and is_copy_assignable_v<hash<T>>\n"
+ " and is_move_assignable_v<hash<T>>\n"
+ " and is_destructible_v<hash<T>>\n"
+ " and is_swappable_v<hash<T>>\n"
+ " and is_callable_v<hash<T>(T)>;",
+ Style);
+}
+
TEST_F(FormatTest, ConstructorInitializers) {
verifyFormat("Constructor() : Initializer(FitsOnTheLine) {}");
verifyFormat("Constructor() : Inttializer(FitsOnTheLine) {}",
@@ -24054,6 +24091,40 @@ TEST_F(FormatTest, RequiresClauses) {
" }\n"
"};");
+ auto Style = getLLVMStyle();
+
+ verifyFormat(
+ "template <typename T>\n"
+ " requires is_default_constructible_v<hash<T>> and\n"
+ " is_copy_constructible_v<hash<T>> and\n"
+ " is_move_constructible_v<hash<T>> and\n"
+ " is_copy_assignable_v<hash<T>> and "
+ "is_move_assignable_v<hash<T>> and\n"
+ " is_destructible_v<hash<T>> and is_swappable_v<hash<T>> and\n"
+ " is_callable_v<hash<T>(T)> and\n"
+ " is_same_v<size_t, decltype(hash<T>(declval<T>()))> and\n"
+ " is_same_v<size_t, decltype(hash<T>(declval<T &>()))> and\n"
+ " is_same_v<size_t, decltype(hash<T>(declval<const T &>()))>\n"
+ "struct S {};",
+ Style);
+
+ Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
+ verifyFormat(
+ "template <typename T>\n"
+ " requires is_default_constructible_v<hash<T>>\n"
+ " and is_copy_constructible_v<hash<T>>\n"
+ " and is_move_constructible_v<hash<T>>\n"
+ " and is_copy_assignable_v<hash<T>> and "
+ "is_move_assignable_v<hash<T>>\n"
+ " and is_destructible_v<hash<T>> and is_swappable_v<hash<T>>\n"
+ " and is_callable_v<hash<T>(T)>\n"
+ " and is_same_v<size_t, decltype(hash<T>(declval<T>()))>\n"
+ " and is_same_v<size_t, decltype(hash<T>(declval<T &>()))>\n"
+ " and is_same_v<size_t, decltype(hash<T>(declval<const T "
+ "&>()))>\n"
+ "struct S {};",
+ Style);
+
// Not a clause, but we once hit an assert.
verifyFormat("#if 0\n"
"#else\n"
More information about the cfe-commits
mailing list