Skip to content

release/20.x: [clang-format] Don't remove parentheses separated from ellipsis by comma (#130471) #130702

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

Merged
merged 1 commit into from
Mar 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 51 additions & 35 deletions clang/lib/Format/UnwrappedLineParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2562,12 +2562,12 @@ bool UnwrappedLineParser::parseBracedList(bool IsAngleBracket, bool IsEnum) {
/// Returns whether there is a `=` token between the parentheses.
bool UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
assert(FormatTok->is(tok::l_paren) && "'(' expected.");
auto *LeftParen = FormatTok;
auto *LParen = FormatTok;
bool SeenComma = false;
bool SeenEqual = false;
bool MightBeFoldExpr = false;
const bool MightBeStmtExpr = Tokens->peekNextToken()->is(tok::l_brace);
nextToken();
const bool MightBeStmtExpr = FormatTok->is(tok::l_brace);
do {
switch (FormatTok->Tok.getKind()) {
case tok::l_paren:
Expand All @@ -2577,44 +2577,60 @@ bool UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
parseChildBlock();
break;
case tok::r_paren: {
auto *Prev = LeftParen->Previous;
if (!MightBeStmtExpr && !MightBeFoldExpr && !Line->InMacroBody &&
Style.RemoveParentheses > FormatStyle::RPS_Leave) {
const auto *Next = Tokens->peekNextToken();
const bool DoubleParens =
Prev && Prev->is(tok::l_paren) && Next && Next->is(tok::r_paren);
const bool CommaSeparated =
!DoubleParens && Prev && Prev->isOneOf(tok::l_paren, tok::comma) &&
Next && Next->isOneOf(tok::comma, tok::r_paren);
const auto *PrevPrev = Prev ? Prev->getPreviousNonComment() : nullptr;
const bool Excluded =
PrevPrev &&
(PrevPrev->isOneOf(tok::kw___attribute, tok::kw_decltype) ||
SeenComma ||
(SeenEqual &&
(PrevPrev->isOneOf(tok::kw_if, tok::kw_while) ||
PrevPrev->endsSequence(tok::kw_constexpr, tok::kw_if))));
const bool ReturnParens =
Style.RemoveParentheses == FormatStyle::RPS_ReturnStatement &&
((NestedLambdas.empty() && !IsDecltypeAutoFunction) ||
(!NestedLambdas.empty() && !NestedLambdas.back())) &&
Prev && Prev->isOneOf(tok::kw_return, tok::kw_co_return) && Next &&
Next->is(tok::semi);
if ((DoubleParens && !Excluded) || (CommaSeparated && !SeenComma) ||
ReturnParens) {
LeftParen->Optional = true;
FormatTok->Optional = true;
}
}
auto *Prev = LParen->Previous;
auto *RParen = FormatTok;
nextToken();
if (Prev) {
auto OptionalParens = [&] {
if (MightBeStmtExpr || MightBeFoldExpr || Line->InMacroBody ||
SeenComma || Style.RemoveParentheses == FormatStyle::RPS_Leave) {
return false;
}
const bool DoubleParens =
Prev->is(tok::l_paren) && FormatTok->is(tok::r_paren);
if (DoubleParens) {
const auto *PrevPrev = Prev->getPreviousNonComment();
const bool Excluded =
PrevPrev &&
(PrevPrev->isOneOf(tok::kw___attribute, tok::kw_decltype) ||
(SeenEqual &&
(PrevPrev->isOneOf(tok::kw_if, tok::kw_while) ||
PrevPrev->endsSequence(tok::kw_constexpr, tok::kw_if))));
if (!Excluded)
return true;
} else {
const bool CommaSeparated =
Prev->isOneOf(tok::l_paren, tok::comma) &&
FormatTok->isOneOf(tok::comma, tok::r_paren);
if (CommaSeparated &&
// LParen is not preceded by ellipsis, comma.
!Prev->endsSequence(tok::comma, tok::ellipsis) &&
// RParen is not followed by comma, ellipsis.
!(FormatTok->is(tok::comma) &&
Tokens->peekNextToken()->is(tok::ellipsis))) {
return true;
}
const bool ReturnParens =
Style.RemoveParentheses == FormatStyle::RPS_ReturnStatement &&
((NestedLambdas.empty() && !IsDecltypeAutoFunction) ||
(!NestedLambdas.empty() && !NestedLambdas.back())) &&
Prev->isOneOf(tok::kw_return, tok::kw_co_return) &&
FormatTok->is(tok::semi);
if (ReturnParens)
return true;
}
return false;
};
if (Prev->is(TT_TypenameMacro)) {
LeftParen->setFinalizedType(TT_TypeDeclarationParen);
FormatTok->setFinalizedType(TT_TypeDeclarationParen);
} else if (Prev->is(tok::greater) && FormatTok->Previous == LeftParen) {
LParen->setFinalizedType(TT_TypeDeclarationParen);
RParen->setFinalizedType(TT_TypeDeclarationParen);
} else if (Prev->is(tok::greater) && RParen->Previous == LParen) {
Prev->setFinalizedType(TT_TemplateCloser);
} else if (OptionalParens()) {
LParen->Optional = true;
RParen->Optional = true;
}
}
nextToken();
return SeenEqual;
}
case tok::r_brace:
Expand Down
4 changes: 4 additions & 0 deletions clang/unittests/Format/FormatTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27881,6 +27881,10 @@ TEST_F(FormatTest, RemoveParentheses) {
verifyFormat("foo((a, b));", "foo(((a), b));", Style);
verifyFormat("foo((a, b));", "foo((a, (b)));", Style);
verifyFormat("foo((a, b, c));", "foo((a, ((b)), c));", Style);
verifyFormat("(..., (hash_a = hash_combine(hash_a, hash_b)));",
"(..., ((hash_a = hash_combine(hash_a, hash_b))));", Style);
verifyFormat("((hash_a = hash_combine(hash_a, hash_b)), ...);",
"(((hash_a = hash_combine(hash_a, hash_b))), ...);", Style);
verifyFormat("return (0);", "return (((0)));", Style);
verifyFormat("return (({ 0; }));", "return ((({ 0; })));", Style);
verifyFormat("return ((... && std::is_convertible_v<TArgsLocal, TArgs>));",
Expand Down
Loading