[clang-format] Add an option to remove redundant parentheses

[MyDeveloperDay]

A thousand times yes!

clang/docs/ClangFormatStyleOptions.rst

4354

Should this be Leave, meaning "Don't touch anything?" vs None which could be confused with "No Parentheses"

4363

Honestly I'm fine with this as it is but should this be? to describe what it will become?

e.g. "SingleParentheses"

[MyDeveloperDay]

Here is a potential failure scenario

(test.cpp)

void *bar();

void foo() {
  void *p;
  int i = 0;

  while ((p = bar())) {
    i++;
  }
}

(.clang-format)

Language: Cpp
RemoveParentheses: ReturnStatement

clang-format test.cpp > test2.cpp

(test2.cpp)

void *bar();

void foo() {
  void *p;
  int i = 0;

  while (p = bar()) {
    i++;
  }
}

clang test2.cpp

test2.cpp:7:12: warning: using the result of an assignment as a condition
      without parentheses [-Wparentheses]
  while (p = bar()) {
         ~~^~~~~~~
test2.cpp:7:12: note: place parentheses around the assignment to silence this
      warning
  while (p = bar()) {
           ^
         (        )
test2.cpp:7:12: note: use '==' to turn this assignment into an equality
      comparison
  while (p = bar()) {
           ^
           ==
1 warning generated.

[owenpan]

Yep! After reformating clang/lib/Format/ and rebuilding clang-format, I got the same warning:

WhitespaceManager.cpp:233:45: warning: using the result of an assignment as a condition without parentheses [-Wparentheses]
      } else if (Change.StartOfBlockComment = LastBlockComment) {
                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
/Users/Owen/llvm-project/clang/lib/Format/WhitespaceManager.cpp:233:45: note: place parentheses around the assignment to silence this warning
      } else if (Change.StartOfBlockComment = LastBlockComment) {
                                            ^
                 (                                            )
/Users/Owen/llvm-project/clang/lib/Format/WhitespaceManager.cpp:233:45: note: use '==' to turn this assignment into an equality comparison
      } else if (Change.StartOfBlockComment = LastBlockComment) {
                                            ^
                                            ==
1 warning generated.

[MyDeveloperDay]

I reckon you could detect the = condition between the matching paren don't you? I ran this on my quite large code base and this was one of the only issues I've seen thus far..

This is a great feature, for my codebase this generated changes in about 10% of all files (but often only a couple of lines or so per file, mostly people doing return (*this);

I think this will be a great feature to have.

[HazardyKnusperkeks]

clang/unittests/Format/FormatTest.cpp

25808

Should check for __attribute((what ever))__.

[owenpan]

Don't remove the outermost double parentheses after if/while if there is an = inside. Also rename the options.

[owenpan]

In D154484#4475377, @MyDeveloperDay wrote:

I reckon you could detect the = condition between the matching paren don't you?

Absolutely.

clang/docs/ClangFormatStyleOptions.rst

4363

Renamed it to MultipleParentheses, which means to remove redundant parentheses found in multiple (double, triple, etc.) parens. (The comment on line 3372 in Format.h says Types of redundant parentheses to remove, but it's changed to Possible values by dump_format_style.py in the documentation.) This is IMO more consistent with ReturnStatement below, but I'm open to other suggestions.

clang/unittests/Format/FormatTest.cpp

25808

What's special about this? It's agnostic about this option as it should be.

[HazardyKnusperkeks]

clang/unittests/Format/FormatTest.cpp

25808

I did not see, that you already had an example with __attribute__.

[sstwcw]

Did you forget about GNU statement expressions?

if (({
      foo();
      bar();
    })) {
}

clang/docs/ReleaseNotes.rst

804

The option is misspelled.

[owenpan]

Don't treat ( as l_paren if it's followed by {.

[owenpan]

In D154484#4483659, @sstwcw wrote:

Did you forget about GNU statement expressions?

if (({
      foo();
      bar();
    })) {
}

Thanks!

[HazardyKnusperkeks]

In D154484#4483659, @sstwcw wrote:

Did you forget about GNU statement expressions?

if (({
      foo();
      bar();
    })) {
}

I wouldn't haven forgotten it, I've never heard of it.

clang/docs/ClangFormatStyleOptions.rst

.. code-block:: c++
     } else {                                   else if (c)
       if (c) {                                   d();
         d();                                   else
       } else {                                   e();
         e();
       }
     }
 
+.. _RemoveParentheses:
+
+**RemoveParentheses** (``RemoveParenthesesStyle``) :versionbadge:`clang-format 17` :ref:`¶ <RemoveParentheses>`
+  Remove redundant parentheses.
+
+  .. warning::
+
+   Setting this option to any value other than ``None`` 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_None`` (in configuration: ``None``)
+    Do not remove parentheses.
+
+    .. code-block:: c++
+
+      class __declspec((dllimport)) X {};
+      co_return (((0)));
+      return ((a + b) - ((c + d)));
+
+  * ``RPS_DoubleParentheses`` (in configuration: ``DoubleParentheses``)
+    Replace double, triple, etc. 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>`
   Remove semicolons after the closing brace of a non-empty function.
 
   .. warning::
 
    Setting this option to `true` could lead to incorrect code formatting due

clang/docs/ReleaseNotes.rst

- Add ``NextLineOnly`` style to option ``PackConstructorInitializers``.
   put the initializers on the current line first, instead, it will try to
   put the initializers on the next line only.
 - Add additional Qualifier Ordering support for special cases such
   as templates, requires clauses, long qualified names.
 - Fix all known issues associated with ``LambdaBodyIndentation: OuterScope``.
 - 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 ``RemoveParenthese`` to remove redundant parentheses.
 
 libclang
 --------
 
 - Introduced the new function ``clang_CXXMethod_isExplicit``,
   which identifies whether a constructor or conversion function cursor
   was marked with the explicit identifier.
 

clang/include/clang/Format/Format.h

   ///     } else {                                   e();
   ///       e();
   ///     }
   ///   }
   /// \endcode
   /// \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_None,
+    /// Replace double, triple, etc. parentheses with single parentheses.
+    /// \code
+    ///   class __declspec(dllimport) X {};
+    ///   co_return (0);
+    ///   return ((a + b) - (c + d));
+    /// \endcode
+    RPS_DoubleParentheses,
+    /// 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 ``None`` 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
   ///  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
   /// \code
   ///   false:                                     true:
return AccessModifierOffset == R.AccessModifierOffset &&
            PenaltyExcessCharacter == R.PenaltyExcessCharacter &&
            PenaltyReturnTypeOnItsOwnLine == R.PenaltyReturnTypeOnItsOwnLine &&
            PointerAlignment == R.PointerAlignment &&
            QualifierAlignment == R.QualifierAlignment &&
            QualifierOrder == R.QualifierOrder &&
            RawStringFormats == R.RawStringFormats &&
            ReferenceAlignment == R.ReferenceAlignment &&
            RemoveBracesLLVM == R.RemoveBracesLLVM &&
+           RemoveParentheses == R.RemoveParentheses &&
            RemoveSemicolon == R.RemoveSemicolon &&
            RequiresClausePosition == R.RequiresClausePosition &&
            RequiresExpressionIndentation == R.RequiresExpressionIndentation &&
            SeparateDefinitionBlocks == R.SeparateDefinitionBlocks &&
            ShortNamespaceLines == R.ShortNamespaceLines &&
            SortIncludes == R.SortIncludes &&
            SortJavaStaticImport == R.SortJavaStaticImport &&
            SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&

clang/lib/Format/Format.cpp

static void enumeration(IO &IO, FormatStyle::ReferenceAlignmentStyle &Value) {
     IO.enumCase(Value, "Pointer", FormatStyle::RAS_Pointer);
     IO.enumCase(Value, "Middle", FormatStyle::RAS_Middle);
     IO.enumCase(Value, "Left", FormatStyle::RAS_Left);
     IO.enumCase(Value, "Right", FormatStyle::RAS_Right);
   }
 };
 
 template <>
+struct ScalarEnumerationTraits<FormatStyle::RemoveParenthesesStyle> {
+  static void enumeration(IO &IO, FormatStyle::RemoveParenthesesStyle &Value) {
+    IO.enumCase(Value, "None", FormatStyle::RPS_None);
+    IO.enumCase(Value, "DoubleParentheses", FormatStyle::RPS_DoubleParentheses);
+    IO.enumCase(Value, "ReturnStatement", FormatStyle::RPS_ReturnStatement);
+  }
+};
+
+template <>
 struct ScalarEnumerationTraits<FormatStyle::RequiresClausePositionStyle> {
   static void enumeration(IO &IO,
                           FormatStyle::RequiresClausePositionStyle &Value) {
     IO.enumCase(Value, "OwnLine", FormatStyle::RCPS_OwnLine);
     IO.enumCase(Value, "WithPreceding", FormatStyle::RCPS_WithPreceding);
     IO.enumCase(Value, "WithFollowing", FormatStyle::RCPS_WithFollowing);
     IO.enumCase(Value, "SingleLine", FormatStyle::RCPS_SingleLine);
   }
static void mapping(IO &IO, FormatStyle &Style) {
     else if (Style.QualifierAlignment == FormatStyle::QAS_Left)
       Style.QualifierOrder = {"const", "volatile", "type"};
     else if (Style.QualifierAlignment == FormatStyle::QAS_Custom)
       IO.mapOptional("QualifierOrder", Style.QualifierOrder);
     IO.mapOptional("RawStringFormats", Style.RawStringFormats);
     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",
                    Style.RequiresExpressionIndentation);
     IO.mapOptional("SeparateDefinitionBlocks", Style.SeparateDefinitionBlocks);
     IO.mapOptional("ShortNamespaceLines", Style.ShortNamespaceLines);
     IO.mapOptional("SortIncludes", Style.SortIncludes);
     IO.mapOptional("SortJavaStaticImport", Style.SortJavaStaticImport);
FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.ObjCSpaceBeforeProtocolList = true;
   LLVMStyle.PackConstructorInitializers = FormatStyle::PCIS_BinPack;
   LLVMStyle.PointerAlignment = FormatStyle::PAS_Right;
   LLVMStyle.PPIndentWidth = -1;
   LLVMStyle.QualifierAlignment = FormatStyle::QAS_Leave;
   LLVMStyle.ReferenceAlignment = FormatStyle::RAS_Pointer;
   LLVMStyle.ReflowComments = true;
   LLVMStyle.RemoveBracesLLVM = false;
+  LLVMStyle.RemoveParentheses = FormatStyle::RPS_None;
   LLVMStyle.RemoveSemicolon = false;
   LLVMStyle.RequiresClausePosition = FormatStyle::RCPS_OwnLine;
   LLVMStyle.RequiresExpressionIndentation = FormatStyle::REI_OuterScope;
   LLVMStyle.SeparateDefinitionBlocks = FormatStyle::SDS_Leave;
   LLVMStyle.ShortNamespaceLines = 1;
   LLVMStyle.SortIncludes = FormatStyle::SI_CaseSensitive;
   LLVMStyle.SortJavaStaticImport = FormatStyle::SJSIO_Before;
   LLVMStyle.SortUsingDeclarations = FormatStyle::SUD_LexicographicNumeric;

 
 std::optional<FormatStyle>
 FormatStyle::GetLanguageStyle(FormatStyle::LanguageKind Language) const {
   return StyleSet.Get(Language);
 }
 
 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)
       : TokenAnalyzer(Env, Style) {}
 
   std::pair<tooling::Replacements, unsigned>
   analyze(TokenAnnotator &Annotator,
           SmallVectorImpl<AnnotatedLine *> &AnnotatedLines,
reformat(const FormatStyle &Style, StringRef Code,
          ArrayRef<tooling::Range> Ranges, unsigned FirstStartColumn,
          unsigned NextStartColumn, unsigned LastStartColumn, StringRef FileName,
          FormattingAttemptStatus *Status) {
   FormatStyle Expanded = Style;
   expandPresetsBraceWrapping(Expanded);
   expandPresetsSpaceBeforeParens(Expanded);
   Expanded.InsertBraces = false;
   Expanded.RemoveBracesLLVM = false;
+  Expanded.RemoveParentheses = FormatStyle::RPS_None;
   Expanded.RemoveSemicolon = false;
   switch (Expanded.RequiresClausePosition) {
   case FormatStyle::RCPS_SingleLine:
   case FormatStyle::RCPS_WithPreceding:
     Expanded.IndentRequiresClause = false;
     break;
   default:
     break;
reformat(const FormatStyle &Style, StringRef Code,
   Passes.emplace_back([&](const Environment &Env) {
     return IntegerLiteralSeparatorFixer().process(Env, Expanded);
   });
 
   if (Style.isCpp()) {
     if (Style.QualifierAlignment != FormatStyle::QAS_Leave)
       addQualifierAlignmentFixerPasses(Expanded, Passes);
 
+    if (Style.RemoveParentheses != FormatStyle::RPS_None) {
+      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;
       Passes.emplace_back([&, S = std::move(S)](const Environment &Env) {
         return BracesInserter(Env, S).process(/*SkipAnnotation=*/true);
       });
     }
 

clang/lib/Format/UnwrappedLineParser.cpp

bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
   return false;
 }
 
 /// \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) {
   assert(FormatTok->is(tok::l_paren) && "'(' expected.");
+  auto *LeftParen = FormatTok;
   nextToken();
   do {
     switch (FormatTok->Tok.getKind()) {
     case tok::l_paren:
       parseParens(AmpAmpTokenType);
       if (Style.Language == FormatStyle::LK_Java && FormatTok->is(tok::l_brace))
         parseChildBlock();
       break;
     case tok::r_paren:
+      if (Style.RemoveParentheses > FormatStyle::RPS_None) {
+        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 AttributeParens =
+            Style.isCpp() && PrevPrev && PrevPrev->is(tok::kw___attribute);
+        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 && !AttributeParens) || ReturnParens) {
+          LeftParen->Optional = true;
+          FormatTok->Optional = true;
+        }
+      }
       nextToken();
       return;
     case tok::r_brace:
       // A "}" inside parenthesis is an error if there wasn't a matching "{".
       return;
     case tok::l_square:
       tryToParseLambda();
       break;

clang/unittests/Format/ConfigParseTest.cpp

#undef CHECK_ALIGN_CONSECUTIVE
   // For backward compatibility:
   CHECK_PARSE("DeriveLineEnding: false", LineEnding, FormatStyle::LE_LF);
   Style.LineEnding = DefaultLineEnding;
   CHECK_PARSE("DeriveLineEnding: false\n"
               "UseCRLF: true",
               LineEnding, FormatStyle::LE_CRLF);
   Style.LineEnding = DefaultLineEnding;
   CHECK_PARSE("UseCRLF: true", LineEnding, FormatStyle::LE_DeriveCRLF);
+
+  CHECK_PARSE("RemoveParentheses: DoubleParentheses", RemoveParentheses,
+              FormatStyle::RPS_DoubleParentheses);
+  CHECK_PARSE("RemoveParentheses: ReturnStatement", RemoveParentheses,
+              FormatStyle::RPS_ReturnStatement);
+  CHECK_PARSE("RemoveParentheses: None", RemoveParentheses,
+              FormatStyle::RPS_None);
 }
 
 TEST(ConfigParseTest, ParsesConfigurationWithLanguages) {
   FormatStyle Style = {};
   Style.Language = FormatStyle::LK_Cpp;
   CHECK_PARSE("Language: Cpp\n"
               "IndentWidth: 12",
               IndentWidth, 12u);

clang/unittests/Format/FormatTest.cpp

verifyFormat("#ifdef\n\n"
                "#ifdef \n"
                "    \n"
                "\n"
                "#else \n"
                "#endif \n",
                getGoogleStyle());
 }
 
+TEST_F(FormatTest, RemoveParentheses) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_EQ(Style.RemoveParentheses, FormatStyle::RPS_None);
+
+  verifyFormat("class __declspec((dllimport)) X {};", Style);
+  verifyFormat("return (0);", Style);
+
+  Style.RemoveParentheses = FormatStyle::RPS_DoubleParentheses;
+  verifyFormat("int x __attribute__ /**/ ((aligned(16))) = 0;", Style);
+  verifyFormat("class __declspec(dllimport) X {};",
+               "class __declspec((dllimport)) X {};", 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);
+
+  Style.ColumnLimit = 25;
+  verifyFormat("return (a + b) - (c + d);",
+               "return (((a + b)) -\n"
+               "        ((c + d)));",
+               Style);
+}
+
 } // namespace
 } // namespace test
 } // namespace format
 } // namespace clang