Skip to content

Commit

Permalink
[clang-format] Fix a bug in RemoveParentheses: ReturnStatement (llvm#…
Browse files Browse the repository at this point in the history
…67911)

Don't remove the outermost parentheses surrounding a return statement
expression when inside a function/lambda that has the decltype(auto)
return type.

Fixed llvm#67892.

(cherry picked from commit 75441a6)
  • Loading branch information
owenca authored and tru committed Oct 16, 2023
1 parent be4016e commit 7a23a5d
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 0 deletions.
24 changes: 24 additions & 0 deletions clang/lib/Format/UnwrappedLineParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,12 @@ void UnwrappedLineParser::reset() {
CommentsBeforeNextToken.clear();
FormatTok = nullptr;
MustBreakBeforeNextToken = false;
IsDecltypeAutoFunction = false;
PreprocessorDirectives.clear();
CurrentLines = &Lines;
DeclarationScopeStack.clear();
NestedTooDeep.clear();
NestedLambdas.clear();
PPStack.clear();
Line->FirstStartColumn = FirstStartColumn;

Expand Down Expand Up @@ -1757,6 +1759,17 @@ void UnwrappedLineParser::parseStructuralElement(
if (parseStructLike())
return;
break;
case tok::kw_decltype:
nextToken();
if (FormatTok->is(tok::l_paren)) {
parseParens();
assert(FormatTok->Previous);
if (FormatTok->Previous->endsSequence(tok::r_paren, tok::kw_auto,
tok::l_paren)) {
Line->SeenDecltypeAuto = true;
}
}
break;
case tok::period:
nextToken();
// In Java, classes have an implicit static member "class".
Expand Down Expand Up @@ -1818,6 +1831,7 @@ void UnwrappedLineParser::parseStructuralElement(
if (NextLBracesType != TT_Unknown)
FormatTok->setFinalizedType(NextLBracesType);
if (!tryToParsePropertyAccessor() && !tryToParseBracedList()) {
IsDecltypeAutoFunction = Line->SeenDecltypeAuto;
// A block outside of parentheses must be the last part of a
// structural element.
// FIXME: Figure out cases where this is not true, and add projections
Expand All @@ -1835,6 +1849,7 @@ void UnwrappedLineParser::parseStructuralElement(
}
FormatTok->setFinalizedType(TT_FunctionLBrace);
parseBlock();
IsDecltypeAutoFunction = false;
addUnwrappedLine();
return;
}
Expand Down Expand Up @@ -2249,9 +2264,15 @@ bool UnwrappedLineParser::tryToParseLambda() {
return true;
}
}

FormatTok->setFinalizedType(TT_LambdaLBrace);
LSquare.setFinalizedType(TT_LambdaLSquare);

NestedLambdas.push_back(Line->SeenDecltypeAuto);
parseChildBlock();
assert(!NestedLambdas.empty());
NestedLambdas.pop_back();

return true;
}

Expand Down Expand Up @@ -2471,6 +2492,8 @@ bool UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
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 && !Blacklisted) || ReturnParens) {
Expand Down Expand Up @@ -4386,6 +4409,7 @@ void UnwrappedLineParser::addUnwrappedLine(LineLevel AdjustLevel) {
Line->MatchingOpeningBlockLineIndex = UnwrappedLine::kInvalidIndex;
Line->FirstStartColumn = 0;
Line->IsContinuation = false;
Line->SeenDecltypeAuto = false;

if (ClosesWhitesmithsBlock && AdjustLevel == LineLevel::Remove)
--Line->Level;
Expand Down
11 changes: 11 additions & 0 deletions clang/lib/Format/UnwrappedLineParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ struct UnwrappedLine {

bool MustBeDeclaration;

/// Whether the parser has seen \c decltype(auto) in this line.
bool SeenDecltypeAuto = false;

/// \c True if this line should be indented by ContinuationIndent in
/// addition to the normal indention level.
bool IsContinuation = false;
Expand Down Expand Up @@ -341,6 +344,14 @@ class UnwrappedLineParser {
// statement contains more than some predefined number of nested statements).
SmallVector<bool, 8> NestedTooDeep;

// Keeps a stack of the states of nested lambdas (true if the return type of
// the lambda is `decltype(auto)`).
SmallVector<bool, 4> NestedLambdas;

// Whether the parser is parsing the body of a function whose return type is
// `decltype(auto)`.
bool IsDecltypeAutoFunction = false;

// Represents preprocessor branch type, so we can find matching
// #if/#else/#endif directives.
enum PPBranchKind {
Expand Down
50 changes: 50 additions & 0 deletions clang/unittests/Format/FormatTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26305,6 +26305,56 @@ TEST_F(FormatTest, RemoveParentheses) {
verifyFormat("co_return 0;", "co_return ((0));", Style);
verifyFormat("return 0;", "return (((0)));", Style);
verifyFormat("return ({ 0; });", "return ((({ 0; })));", Style);
verifyFormat("inline decltype(auto) f() {\n"
" if (a) {\n"
" return (a);\n"
" }\n"
" return (b);\n"
"}",
"inline decltype(auto) f() {\n"
" if (a) {\n"
" return ((a));\n"
" }\n"
" return ((b));\n"
"}",
Style);
verifyFormat("auto g() {\n"
" decltype(auto) x = [] {\n"
" auto y = [] {\n"
" if (a) {\n"
" return a;\n"
" }\n"
" return b;\n"
" };\n"
" if (c) {\n"
" return (c);\n"
" }\n"
" return (d);\n"
" };\n"
" if (e) {\n"
" return e;\n"
" }\n"
" return f;\n"
"}",
"auto g() {\n"
" decltype(auto) x = [] {\n"
" auto y = [] {\n"
" if (a) {\n"
" return ((a));\n"
" }\n"
" return ((b));\n"
" };\n"
" if (c) {\n"
" return ((c));\n"
" }\n"
" return ((d));\n"
" };\n"
" if (e) {\n"
" return ((e));\n"
" }\n"
" return ((f));\n"
"}",
Style);

Style.ColumnLimit = 25;
verifyFormat("return (a + b) - (c + d);",
Expand Down

0 comments on commit 7a23a5d

Please sign in to comment.