diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst --- a/clang/docs/ClangFormatStyleOptions.rst +++ b/clang/docs/ClangFormatStyleOptions.rst @@ -4357,6 +4357,50 @@ } } +.. _RemoveParentheses: + +**RemoveParentheses** (``RemoveParenthesesStyle``) :versionbadge:`clang-format 17` :ref:`¶ ` + Remove redundant parentheses. + + .. warning:: + + Setting this option to any value other than ``Leave`` could lead to + incorrect code formatting due to clang-format's lack of complete semantic + information. As such, extra care should be taken to review code changes + made by this option. + + Possible values: + + * ``RPS_Leave`` (in configuration: ``Leave``) + Do not remove parentheses. + + .. code-block:: c++ + + class __declspec((dllimport)) X {}; + co_return (((0))); + return ((a + b) - ((c + d))); + + * ``RPS_MultipleParentheses`` (in configuration: ``MultipleParentheses``) + Replace multiple parentheses with single parentheses. + + .. code-block:: c++ + + class __declspec(dllimport) X {}; + co_return (0); + return ((a + b) - (c + d)); + + * ``RPS_ReturnStatement`` (in configuration: ``ReturnStatement``) + Also remove parentheses enclosing the expression in a + ``return``/``co_return`` statement. + + .. code-block:: c++ + + class __declspec(dllimport) X {}; + co_return 0; + return (a + b) - (c + d); + + + .. _RemoveSemicolon: **RemoveSemicolon** (``Boolean``) :versionbadge:`clang-format 16` :ref:`¶ ` diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -821,6 +821,7 @@ - Add ``BracedInitializerIndentWidth`` which can be used to configure the indentation level of the contents of braced init lists. - Add ``KeepEmptyLinesAtEOF`` to keep empty lines at end of file. +- Add ``RemoveParentheses`` to remove redundant parentheses. libclang -------- diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h --- a/clang/include/clang/Format/Format.h +++ b/clang/include/clang/Format/Format.h @@ -3383,6 +3383,42 @@ /// \version 14 bool RemoveBracesLLVM; + /// Types of redundant parentheses to remove. + enum RemoveParenthesesStyle : int8_t { + /// Do not remove parentheses. + /// \code + /// class __declspec((dllimport)) X {}; + /// co_return (((0))); + /// return ((a + b) - ((c + d))); + /// \endcode + RPS_Leave, + /// Replace multiple parentheses with single parentheses. + /// \code + /// class __declspec(dllimport) X {}; + /// co_return (0); + /// return ((a + b) - (c + d)); + /// \endcode + RPS_MultipleParentheses, + /// Also remove parentheses enclosing the expression in a + /// ``return``/``co_return`` statement. + /// \code + /// class __declspec(dllimport) X {}; + /// co_return 0; + /// return (a + b) - (c + d); + /// \endcode + RPS_ReturnStatement, + }; + + /// Remove redundant parentheses. + /// \warning + /// Setting this option to any value other than ``Leave`` could lead to + /// incorrect code formatting due to clang-format's lack of complete semantic + /// information. As such, extra care should be taken to review code changes + /// made by this option. + /// \endwarning + /// \version 17 + RemoveParenthesesStyle RemoveParentheses; + /// Remove semicolons after the closing brace of a non-empty function. /// \warning /// Setting this option to `true` could lead to incorrect code formatting due @@ -4416,6 +4452,7 @@ RawStringFormats == R.RawStringFormats && ReferenceAlignment == R.ReferenceAlignment && RemoveBracesLLVM == R.RemoveBracesLLVM && + RemoveParentheses == R.RemoveParentheses && RemoveSemicolon == R.RemoveSemicolon && RequiresClausePosition == R.RequiresClausePosition && RequiresExpressionIndentation == R.RequiresExpressionIndentation && diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp --- a/clang/lib/Format/Format.cpp +++ b/clang/lib/Format/Format.cpp @@ -502,6 +502,16 @@ } }; +template <> +struct ScalarEnumerationTraits { + static void enumeration(IO &IO, FormatStyle::RemoveParenthesesStyle &Value) { + IO.enumCase(Value, "Leave", FormatStyle::RPS_Leave); + IO.enumCase(Value, "MultipleParentheses", + FormatStyle::RPS_MultipleParentheses); + IO.enumCase(Value, "ReturnStatement", FormatStyle::RPS_ReturnStatement); + } +}; + template <> struct ScalarEnumerationTraits { static void enumeration(IO &IO, @@ -989,6 +999,7 @@ IO.mapOptional("ReferenceAlignment", Style.ReferenceAlignment); IO.mapOptional("ReflowComments", Style.ReflowComments); IO.mapOptional("RemoveBracesLLVM", Style.RemoveBracesLLVM); + IO.mapOptional("RemoveParentheses", Style.RemoveParentheses); IO.mapOptional("RemoveSemicolon", Style.RemoveSemicolon); IO.mapOptional("RequiresClausePosition", Style.RequiresClausePosition); IO.mapOptional("RequiresExpressionIndentation", @@ -1429,6 +1440,7 @@ LLVMStyle.ReferenceAlignment = FormatStyle::RAS_Pointer; LLVMStyle.ReflowComments = true; LLVMStyle.RemoveBracesLLVM = false; + LLVMStyle.RemoveParentheses = FormatStyle::RPS_Leave; LLVMStyle.RemoveSemicolon = false; LLVMStyle.RequiresClausePosition = FormatStyle::RCPS_OwnLine; LLVMStyle.RequiresExpressionIndentation = FormatStyle::REI_OuterScope; @@ -1982,6 +1994,50 @@ namespace { +class ParensRemover : public TokenAnalyzer { +public: + ParensRemover(const Environment &Env, const FormatStyle &Style) + : TokenAnalyzer(Env, Style) {} + + std::pair + analyze(TokenAnnotator &Annotator, + SmallVectorImpl &AnnotatedLines, + FormatTokenLexer &Tokens) override { + AffectedRangeMgr.computeAffectedLines(AnnotatedLines); + tooling::Replacements Result; + removeParens(AnnotatedLines, Result); + return {Result, 0}; + } + +private: + void removeParens(SmallVectorImpl &Lines, + tooling::Replacements &Result) { + const auto &SourceMgr = Env.getSourceManager(); + for (auto *Line : Lines) { + removeParens(Line->Children, Result); + if (!Line->Affected) + continue; + for (const auto *Token = Line->First; Token && !Token->Finalized; + Token = Token->Next) { + if (!Token->Optional || !Token->isOneOf(tok::l_paren, tok::r_paren)) + continue; + auto *Next = Token->Next; + assert(Next && Next->isNot(tok::eof)); + SourceLocation Start; + if (Next->NewlinesBefore == 0) { + Start = Token->Tok.getLocation(); + Next->WhitespaceRange = Token->WhitespaceRange; + } else { + Start = Token->WhitespaceRange.getBegin(); + } + const auto &Range = + CharSourceRange::getCharRange(Start, Token->Tok.getEndLoc()); + cantFail(Result.add(tooling::Replacement(SourceMgr, Range, " "))); + } + } + } +}; + class BracesInserter : public TokenAnalyzer { public: BracesInserter(const Environment &Env, const FormatStyle &Style) @@ -3428,6 +3484,7 @@ expandPresetsSpaceBeforeParens(Expanded); Expanded.InsertBraces = false; Expanded.RemoveBracesLLVM = false; + Expanded.RemoveParentheses = FormatStyle::RPS_Leave; Expanded.RemoveSemicolon = false; switch (Expanded.RequiresClausePosition) { case FormatStyle::RCPS_SingleLine: @@ -3483,6 +3540,14 @@ if (Style.QualifierAlignment != FormatStyle::QAS_Leave) addQualifierAlignmentFixerPasses(Expanded, Passes); + if (Style.RemoveParentheses != FormatStyle::RPS_Leave) { + FormatStyle S = Expanded; + S.RemoveParentheses = Style.RemoveParentheses; + Passes.emplace_back([&, S = std::move(S)](const Environment &Env) { + return ParensRemover(Env, S).process(/*SkipAnnotation=*/true); + }); + } + if (Style.InsertBraces) { FormatStyle S = Expanded; S.InsertBraces = true; diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h --- a/clang/lib/Format/UnwrappedLineParser.h +++ b/clang/lib/Format/UnwrappedLineParser.h @@ -156,7 +156,7 @@ bool tryToParseBracedList(); bool parseBracedList(bool ContinueOnSemicolons = false, bool IsEnum = false, tok::TokenKind ClosingBraceKind = tok::r_brace); - void parseParens(TokenType AmpAmpTokenType = TT_Unknown); + bool parseParens(TokenType AmpAmpTokenType = TT_Unknown); void parseSquare(bool LambdaIntroducer = false); void keepAncestorBraces(); void parseUnbracedBody(bool CheckEOF = false); diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -2432,22 +2432,50 @@ /// \brief Parses a pair of parentheses (and everything between them). /// \param AmpAmpTokenType If different than TT_Unknown sets this type for all /// double ampersands. This applies for all nested scopes as well. -void UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) { +/// +/// Returns whether there is a `=` token between the parentheses. +bool UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) { assert(FormatTok->is(tok::l_paren) && "'(' expected."); + auto *LeftParen = FormatTok; + bool SeenEqual = false; + const bool MightBeStmtExpr = Tokens->peekNextToken()->is(tok::l_brace); nextToken(); do { switch (FormatTok->Tok.getKind()) { case tok::l_paren: - parseParens(AmpAmpTokenType); + if (parseParens(AmpAmpTokenType)) + SeenEqual = true; if (Style.Language == FormatStyle::LK_Java && FormatTok->is(tok::l_brace)) parseChildBlock(); break; case tok::r_paren: + if (!MightBeStmtExpr && + Style.RemoveParentheses > FormatStyle::RPS_Leave) { + const auto *Prev = LeftParen->Previous; + const auto *Next = Tokens->peekNextToken(); + const bool DoubleParens = + Prev && Prev->is(tok::l_paren) && Next && Next->is(tok::r_paren); + const auto *PrevPrev = Prev ? Prev->getPreviousNonComment() : nullptr; + const bool Blacklisted = + PrevPrev && + (PrevPrev->is(tok::kw___attribute) || + (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 && + Prev && Prev->isOneOf(tok::kw_return, tok::kw_co_return) && Next && + Next->is(tok::semi); + if ((DoubleParens && !Blacklisted) || ReturnParens) { + LeftParen->Optional = true; + FormatTok->Optional = true; + } + } nextToken(); - return; + return SeenEqual; case tok::r_brace: // A "}" inside parenthesis is an error if there wasn't a matching "{". - return; + return SeenEqual; case tok::l_square: tryToParseLambda(); break; @@ -2463,6 +2491,7 @@ } break; case tok::equal: + SeenEqual = true; if (Style.isCSharp() && FormatTok->is(TT_FatArrow)) tryToParseChildBlock(); else @@ -2499,6 +2528,7 @@ break; } } while (!eof()); + return SeenEqual; } void UnwrappedLineParser::parseSquare(bool LambdaIntroducer) { diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp --- a/clang/unittests/Format/ConfigParseTest.cpp +++ b/clang/unittests/Format/ConfigParseTest.cpp @@ -917,6 +917,13 @@ LineEnding, FormatStyle::LE_CRLF); Style.LineEnding = DefaultLineEnding; CHECK_PARSE("UseCRLF: true", LineEnding, FormatStyle::LE_DeriveCRLF); + + CHECK_PARSE("RemoveParentheses: MultipleParentheses", RemoveParentheses, + FormatStyle::RPS_MultipleParentheses); + CHECK_PARSE("RemoveParentheses: ReturnStatement", RemoveParentheses, + FormatStyle::RPS_ReturnStatement); + CHECK_PARSE("RemoveParentheses: Leave", RemoveParentheses, + FormatStyle::RPS_Leave); } TEST(ConfigParseTest, ParsesConfigurationWithLanguages) { diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -25973,6 +25973,56 @@ getGoogleStyle()); } +TEST_F(FormatTest, RemoveParentheses) { + FormatStyle Style = getLLVMStyle(); + EXPECT_EQ(Style.RemoveParentheses, FormatStyle::RPS_Leave); + + Style.RemoveParentheses = FormatStyle::RPS_MultipleParentheses; + verifyFormat("int x __attribute__((aligned(16))) = 0;", Style); + verifyFormat("class __declspec(dllimport) X {};", + "class __declspec((dllimport)) X {};", Style); + verifyFormat("int x = (({ 0; }));", "int x = ((({ 0; })));", Style); + verifyFormat("while (a)\n" + " b;", + "while (((a)))\n" + " b;", + Style); + verifyFormat("while ((a = b))\n" + " c;", + "while (((a = b)))\n" + " c;", + Style); + verifyFormat("if (a)\n" + " b;", + "if (((a)))\n" + " b;", + Style); + verifyFormat("if constexpr ((a = b))\n" + " c;", + "if constexpr (((a = b)))\n" + " c;", + Style); + verifyFormat("if (({ a; }))\n" + " b;", + "if ((({ a; })))\n" + " b;", + Style); + verifyFormat("return (0);", "return (((0)));", Style); + verifyFormat("return (({ 0; }));", "return ((({ 0; })));", Style); + + Style.RemoveParentheses = FormatStyle::RPS_ReturnStatement; + verifyFormat("return 0;", "return (0);", Style); + verifyFormat("co_return 0;", "co_return ((0));", Style); + verifyFormat("return 0;", "return (((0)));", Style); + verifyFormat("return ({ 0; });", "return ((({ 0; })));", Style); + + Style.ColumnLimit = 25; + verifyFormat("return (a + b) - (c + d);", + "return (((a + b)) -\n" + " ((c + d)));", + Style); +} + } // namespace } // namespace test } // namespace format