[clang] 3a6a070 - [clang-format] Add an option to remove redundant parentheses
Owen Pan via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 11 16:33:27 PDT 2023
Author: Owen Pan
Date: 2023-07-11T16:33:19-07:00
New Revision: 3a6a0702c2a4c2290f0b55b0451a9f97d7592baf
URL: https://github.com/llvm/llvm-project/commit/3a6a0702c2a4c2290f0b55b0451a9f97d7592baf
DIFF: https://github.com/llvm/llvm-project/commit/3a6a0702c2a4c2290f0b55b0451a9f97d7592baf.diff
LOG: [clang-format] Add an option to remove redundant parentheses
Differential Revision: https://reviews.llvm.org/D154484
Added:
Modified:
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/lib/Format/UnwrappedLineParser.h
clang/unittests/Format/ConfigParseTest.cpp
clang/unittests/Format/FormatTest.cpp
Removed:
################################################################################
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 866bf0910898b3..8965b20e62c641 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -4357,6 +4357,50 @@ the configuration (without a prefix: ``Auto``).
}
}
+.. _RemoveParentheses:
+
+**RemoveParentheses** (``RemoveParenthesesStyle``) :versionbadge:`clang-format 17` :ref:`¶ <RemoveParentheses>`
+ 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:`¶ <RemoveSemicolon>`
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1f025097babc95..b7babe3ddc3f41 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -821,6 +821,7 @@ clang-format
- 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
index 3992dd89fb950d..71948027fbe3ed 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -3383,6 +3383,42 @@ struct FormatStyle {
/// \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 @@ struct FormatStyle {
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
index fd46dfec21d8de..c71139d26ff80d 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -502,6 +502,16 @@ struct ScalarEnumerationTraits<FormatStyle::ReferenceAlignmentStyle> {
}
};
+template <>
+struct ScalarEnumerationTraits<FormatStyle::RemoveParenthesesStyle> {
+ 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<FormatStyle::RequiresClausePositionStyle> {
static void enumeration(IO &IO,
@@ -989,6 +999,7 @@ template <> struct MappingTraits<FormatStyle> {
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 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
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 @@ FormatStyle::GetLanguageStyle(FormatStyle::LanguageKind Language) const {
namespace {
+class ParensRemover : public TokenAnalyzer {
+public:
+ ParensRemover(const Environment &Env, const FormatStyle &Style)
+ : TokenAnalyzer(Env, Style) {}
+
+ std::pair<tooling::Replacements, unsigned>
+ analyze(TokenAnnotator &Annotator,
+ SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
+ FormatTokenLexer &Tokens) override {
+ AffectedRangeMgr.computeAffectedLines(AnnotatedLines);
+ tooling::Replacements Result;
+ removeParens(AnnotatedLines, Result);
+ return {Result, 0};
+ }
+
+private:
+ void removeParens(SmallVectorImpl<AnnotatedLine *> &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 @@ reformat(const FormatStyle &Style, StringRef Code,
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 @@ reformat(const FormatStyle &Style, StringRef Code,
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.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index e9da1453917041..737ba52a1fb1b4 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2432,22 +2432,50 @@ bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
/// \brief Parses a pair of parentheses (and everything between them).
/// \param AmpAmpTokenType If
diff erent 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 @@ void UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
}
break;
case tok::equal:
+ SeenEqual = true;
if (Style.isCSharp() && FormatTok->is(TT_FatArrow))
tryToParseChildBlock();
else
@@ -2499,6 +2528,7 @@ void UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
break;
}
} while (!eof());
+ return SeenEqual;
}
void UnwrappedLineParser::parseSquare(bool LambdaIntroducer) {
diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h
index 311d879b0a0881..57515af64a3e95 100644
--- a/clang/lib/Format/UnwrappedLineParser.h
+++ b/clang/lib/Format/UnwrappedLineParser.h
@@ -156,7 +156,7 @@ class UnwrappedLineParser {
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/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index 6c720ec22054cf..7903db12225f8c 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -917,6 +917,13 @@ TEST(ConfigParseTest, ParsesConfiguration) {
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
index 7e8476f4082d31..e01db1501e2d39 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -25973,6 +25973,56 @@ TEST_F(FormatTest, PreprocessorOverlappingRegions) {
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
More information about the cfe-commits
mailing list