Skip to content

Commit

Permalink
[clang-format] Add an option to remove redundant parentheses
Browse files Browse the repository at this point in the history
Differential Revision: https://reviews.llvm.org/D154484
  • Loading branch information
owenca committed Jul 11, 2023
1 parent 392b306 commit 3a6a070
Show file tree
Hide file tree
Showing 8 changed files with 239 additions and 5 deletions.
44 changes: 44 additions & 0 deletions clang/docs/ClangFormatStyleOptions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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>`
Expand Down
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
--------
Expand Down
37 changes: 37 additions & 0 deletions clang/include/clang/Format/Format.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 &&
Expand Down
65 changes: 65 additions & 0 deletions clang/lib/Format/Format.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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;
Expand Down
38 changes: 34 additions & 4 deletions clang/lib/Format/UnwrappedLineParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2432,22 +2432,50 @@ bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
/// \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;
Expand All @@ -2463,6 +2491,7 @@ void UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
}
break;
case tok::equal:
SeenEqual = true;
if (Style.isCSharp() && FormatTok->is(TT_FatArrow))
tryToParseChildBlock();
else
Expand Down Expand Up @@ -2499,6 +2528,7 @@ void UnwrappedLineParser::parseParens(TokenType AmpAmpTokenType) {
break;
}
} while (!eof());
return SeenEqual;
}

void UnwrappedLineParser::parseSquare(bool LambdaIntroducer) {
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Format/UnwrappedLineParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 7 additions & 0 deletions clang/unittests/Format/ConfigParseTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
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 @@ -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
Expand Down

0 comments on commit 3a6a070

Please sign in to comment.