Skip to content

Commit a75f8d9

Browse files
mydeveloperdaymydeveloperday
committed
[clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions
Summary: https://bugs.llvm.org/show_bug.cgi?id=36294 Addressing bug related to returning after return type not being honoured for some operator types. ``` $ bin/clang-format --style="{BasedOnStyle: llvm, AlwaysBreakAfterReturnType: TopLevelDefinitions}" /tmp/foo.cpp class Foo { public: bool operator!() const; bool operator<(Foo const &) const; bool operator*() const; bool operator->() const; bool operator+() const; bool operator-() const; bool f() const; }; bool Foo::operator!() const { return true; } bool Foo::operator<(Foo const &) const { return true; } bool Foo::operator*() const { return true; } bool Foo::operator->() const { return true; } bool Foo::operator+() const { return true; } bool Foo::operator-() const { return true; } bool Foo::f() const { return true; } ``` Reviewers: mitchell-stellar, klimek, owenpan, sammccall, rianquinn Reviewed By: sammccall Subscribers: merge_guards_bot, cfe-commits Tags: #clang-format, #clang-tools-extra, #clang Differential Revision: https://reviews.llvm.org/D69573
1 parent 874b649 commit a75f8d9

File tree

2 files changed

+117
-7
lines changed

2 files changed

+117
-7
lines changed

‎clang/lib/Format/TokenAnnotator.cpp

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -950,9 +950,9 @@ class AnnotatingParser {
950950
if (CurrentToken->isOneOf(tok::star, tok::amp))
951951
CurrentToken->Type = TT_PointerOrReference;
952952
consumeToken();
953-
if (CurrentToken &&
954-
CurrentToken->Previous->isOneOf(TT_BinaryOperator, TT_UnaryOperator,
955-
tok::comma))
953+
if (CurrentToken && CurrentToken->Previous->isOneOf(
954+
TT_BinaryOperator, TT_UnaryOperator, tok::comma,
955+
tok::star, tok::arrow, tok::amp, tok::ampamp))
956956
CurrentToken->Previous->Type = TT_OverloadedOperator;
957957
}
958958
if (CurrentToken) {
@@ -1344,8 +1344,12 @@ class AnnotatingParser {
13441344
Contexts.back().IsExpression = false;
13451345
} else if (Current.is(tok::kw_new)) {
13461346
Contexts.back().CanBeExpression = false;
1347-
} else if (Current.isOneOf(tok::semi, tok::exclaim)) {
1347+
} else if (Current.is(tok::semi) ||
1348+
(Current.is(tok::exclaim) && Current.Previous &&
1349+
!Current.Previous->is(tok::kw_operator))) {
13481350
// This should be the condition or increment in a for-loop.
1351+
// But not operator !() (can't use TT_OverloadedOperator here as its not
1352+
// been annotated yet).
13491353
Contexts.back().IsExpression = true;
13501354
}
13511355
}
@@ -2155,11 +2159,22 @@ static bool isFunctionDeclarationName(const FormatToken &Current,
21552159
continue;
21562160
if (Next->isOneOf(tok::kw_new, tok::kw_delete)) {
21572161
// For 'new[]' and 'delete[]'.
2158-
if (Next->Next && Next->Next->is(tok::l_square) && Next->Next->Next &&
2159-
Next->Next->Next->is(tok::r_square))
2162+
if (Next->Next &&
2163+
Next->Next->startsSequence(tok::l_square, tok::r_square))
21602164
Next = Next->Next->Next;
21612165
continue;
21622166
}
2167+
if (Next->startsSequence(tok::l_square, tok::r_square)) {
2168+
// For operator[]().
2169+
Next = Next->Next;
2170+
continue;
2171+
}
2172+
if ((Next->isSimpleTypeSpecifier() || Next->is(tok::identifier)) &&
2173+
Next->Next && Next->Next->isOneOf(tok::star, tok::amp, tok::ampamp)) {
2174+
// For operator void*(), operator char*(), operator Foo*().
2175+
Next = Next->Next;
2176+
continue;
2177+
}
21632178

21642179
break;
21652180
}
@@ -2673,6 +2688,13 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
26732688
tok::l_square));
26742689
if (Right.is(tok::star) && Left.is(tok::l_paren))
26752690
return false;
2691+
if (Right.isOneOf(tok::star, tok::amp, tok::ampamp) &&
2692+
(Left.is(tok::identifier) || Left.isSimpleTypeSpecifier()) &&
2693+
Left.Previous && Left.Previous->is(tok::kw_operator))
2694+
// Space between the type and the *
2695+
// operator void*(), operator char*(), operator Foo*() dependant
2696+
// on PointerAlignment style.
2697+
return (Style.PointerAlignment == FormatStyle::PAS_Right);
26762698
const auto SpaceRequiredForArrayInitializerLSquare =
26772699
[](const FormatToken &LSquareTok, const FormatStyle &Style) {
26782700
return Style.SpacesInContainerLiterals ||

‎clang/unittests/Format/FormatTest.cpp

Lines changed: 89 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6134,7 +6134,48 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) {
61346134
"void\n"
61356135
"A::operator>>() {}\n"
61366136
"void\n"
6137-
"A::operator+() {}\n",
6137+
"A::operator+() {}\n"
6138+
"void\n"
6139+
"A::operator*() {}\n"
6140+
"void\n"
6141+
"A::operator->() {}\n"
6142+
"void\n"
6143+
"A::operator void *() {}\n"
6144+
"void\n"
6145+
"A::operator void &() {}\n"
6146+
"void\n"
6147+
"A::operator void &&() {}\n"
6148+
"void\n"
6149+
"A::operator char *() {}\n"
6150+
"void\n"
6151+
"A::operator[]() {}\n"
6152+
"void\n"
6153+
"A::operator!() {}\n",
6154+
Style);
6155+
verifyFormat("constexpr auto\n"
6156+
"operator()() const -> reference {}\n"
6157+
"constexpr auto\n"
6158+
"operator>>() const -> reference {}\n"
6159+
"constexpr auto\n"
6160+
"operator+() const -> reference {}\n"
6161+
"constexpr auto\n"
6162+
"operator*() const -> reference {}\n"
6163+
"constexpr auto\n"
6164+
"operator->() const -> reference {}\n"
6165+
"constexpr auto\n"
6166+
"operator++() const -> reference {}\n"
6167+
"constexpr auto\n"
6168+
"operator void *() const -> reference {}\n"
6169+
"constexpr auto\n"
6170+
"operator void &() const -> reference {}\n"
6171+
"constexpr auto\n"
6172+
"operator void &&() const -> reference {}\n"
6173+
"constexpr auto\n"
6174+
"operator char *() const -> reference {}\n"
6175+
"constexpr auto\n"
6176+
"operator!() const -> reference {}\n"
6177+
"constexpr auto\n"
6178+
"operator[]() const -> reference {}\n",
61386179
Style);
61396180
verifyFormat("void *operator new(std::size_t s);", // No break here.
61406181
Style);
@@ -14755,6 +14796,53 @@ TEST_F(FormatTest, STLWhileNotDefineChed) {
1475514796
"#endif // while");
1475614797
}
1475714798

14799+
TEST_F(FormatTest, OperatorSpacing) {
14800+
FormatStyle Style = getLLVMStyle();
14801+
Style.PointerAlignment = FormatStyle::PAS_Right;
14802+
verifyFormat("Foo::operator*();", Style);
14803+
verifyFormat("Foo::operator void *();", Style);
14804+
verifyFormat("Foo::operator()(void *);", Style);
14805+
verifyFormat("Foo::operator*(void *);", Style);
14806+
verifyFormat("Foo::operator*();", Style);
14807+
verifyFormat("operator*(int (*)(), class Foo);", Style);
14808+
14809+
verifyFormat("Foo::operator&();", Style);
14810+
verifyFormat("Foo::operator void &();", Style);
14811+
verifyFormat("Foo::operator()(void &);", Style);
14812+
verifyFormat("Foo::operator&(void &);", Style);
14813+
verifyFormat("Foo::operator&();", Style);
14814+
verifyFormat("operator&(int (&)(), class Foo);", Style);
14815+
14816+
verifyFormat("Foo::operator&&();", Style);
14817+
verifyFormat("Foo::operator void &&();", Style);
14818+
verifyFormat("Foo::operator()(void &&);", Style);
14819+
verifyFormat("Foo::operator&&(void &&);", Style);
14820+
verifyFormat("Foo::operator&&();", Style);
14821+
verifyFormat("operator&&(int(&&)(), class Foo);", Style);
14822+
14823+
Style.PointerAlignment = FormatStyle::PAS_Left;
14824+
verifyFormat("Foo::operator*();", Style);
14825+
verifyFormat("Foo::operator void*();", Style);
14826+
verifyFormat("Foo::operator()(void*);", Style);
14827+
verifyFormat("Foo::operator*(void*);", Style);
14828+
verifyFormat("Foo::operator*();", Style);
14829+
verifyFormat("operator*(int (*)(), class Foo);", Style);
14830+
14831+
verifyFormat("Foo::operator&();", Style);
14832+
verifyFormat("Foo::operator void&();", Style);
14833+
verifyFormat("Foo::operator()(void&);", Style);
14834+
verifyFormat("Foo::operator&(void&);", Style);
14835+
verifyFormat("Foo::operator&();", Style);
14836+
verifyFormat("operator&(int (&)(), class Foo);", Style);
14837+
14838+
verifyFormat("Foo::operator&&();", Style);
14839+
verifyFormat("Foo::operator void&&();", Style);
14840+
verifyFormat("Foo::operator()(void&&);", Style);
14841+
verifyFormat("Foo::operator&&(void&&);", Style);
14842+
verifyFormat("Foo::operator&&();", Style);
14843+
verifyFormat("operator&&(int(&&)(), class Foo);", Style);
14844+
}
14845+
1475814846
} // namespace
1475914847
} // namespace format
1476014848
} // namespace clang

0 commit comments

Comments
 (0)