Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/readability/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ add_clang_library(clangTidyReadabilityModule STATIC
RedundantDeclarationCheck.cpp
RedundantFunctionPtrDereferenceCheck.cpp
RedundantMemberInitCheck.cpp
RedundantParenthesesCheck.cpp
RedundantPreprocessorCheck.cpp
RedundantSmartptrGetCheck.cpp
RedundantStringCStrCheck.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include "RedundantFunctionPtrDereferenceCheck.h"
#include "RedundantInlineSpecifierCheck.h"
#include "RedundantMemberInitCheck.h"
#include "RedundantParenthesesCheck.h"
#include "RedundantPreprocessorCheck.h"
#include "RedundantSmartptrGetCheck.h"
#include "RedundantStringCStrCheck.h"
Expand Down Expand Up @@ -138,6 +139,8 @@ class ReadabilityModule : public ClangTidyModule {
"readability-redundant-function-ptr-dereference");
CheckFactories.registerCheck<RedundantMemberInitCheck>(
"readability-redundant-member-init");
CheckFactories.registerCheck<RedundantParenthesesCheck>(
"readability-redundant-parentheses");
CheckFactories.registerCheck<RedundantPreprocessorCheck>(
"readability-redundant-preprocessor");
CheckFactories.registerCheck<ReferenceToConstructedTemporaryCheck>(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include "RedundantParenthesesCheck.h"
#include "clang/AST/Expr.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/ASTMatchers/ASTMatchersMacros.h"
#include <cassert>

using namespace clang::ast_matchers;

namespace clang::tidy::readability {

namespace {

AST_MATCHER_P(ParenExpr, subExpr, ast_matchers::internal::Matcher<Expr>,
InnerMatcher) {
return InnerMatcher.matches(*Node.getSubExpr(), Finder, Builder);
}

AST_MATCHER(ParenExpr, isInMacro) {
const Expr *E = Node.getSubExpr();
return Node.getLParen().isMacroID() || Node.getRParen().isMacroID() ||
E->getBeginLoc().isMacroID() || E->getEndLoc().isMacroID();
}

} // namespace

void RedundantParenthesesCheck::registerMatchers(MatchFinder *Finder) {
const auto ConstantExpr =
expr(anyOf(integerLiteral(), floatLiteral(), characterLiteral(),
cxxBoolLiteral(), stringLiteral(), cxxNullPtrLiteralExpr()));
Finder->addMatcher(
parenExpr(subExpr(anyOf(parenExpr(), ConstantExpr, declRefExpr())),
unless(anyOf(isInMacro(),
// sizeof(...) is common used.
hasParent(unaryExprOrTypeTraitExpr()))))
.bind("dup"),
this);
}

void RedundantParenthesesCheck::check(const MatchFinder::MatchResult &Result) {
const auto *PE = Result.Nodes.getNodeAs<ParenExpr>("dup");
diag(PE->getBeginLoc(), "redundant parentheses around expression")
<< FixItHint::CreateRemoval(PE->getLParen())
<< FixItHint::CreateRemoval(PE->getRParen());
}

} // namespace clang::tidy::readability
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPARENTHESESCHECK_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPARENTHESESCHECK_H

#include "../ClangTidyCheck.h"
#include "clang/Basic/LangOptions.h"

namespace clang::tidy::readability {

/// Detect redundant parentheses.
///
/// For the user-facing documentation see:
/// https://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-parentheses.html
class RedundantParenthesesCheck : public ClangTidyCheck {
public:
RedundantParenthesesCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we set isLanguageVersionSupported to CPlusPlus || C99 otherwise the check may run on ObjC, which may have different semantics I guess..

I wonder, could we make the default to CPlusPlus || C99 instead of true here:

virtual bool isLanguageVersionSupported(const LangOptions &LangOpts) const {
return true;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder, could we make the default to CPlusPlus || C99 instead of true here

It needs RFC IMO. I have never written OC so I don't know the ecosystem of OC. Is clang-tidy a common used tools in OC's world?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder, could we make the default to CPlusPlus || C99 instead of true here

I have never written OC so I don't know the ecosystem of OC. Is clang-tidy a common used tools in OC's world?

I've never used OC too, so we'd definitely need an RFC for that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the goal is to disable the check for OC, shouldn't we just do return !LangOpts.ObjC? It expresses intent more clearly. (Although if ObjC isn't set for Objective-C++, that won't work.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.CPlusPlus | LangOpts.C99;
}
};

} // namespace clang::tidy::readability

#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTPARENTHESESCHECK_H
5 changes: 5 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,11 @@ New checks
Finds virtual function overrides with different visibility than the function
in the base class.

- New :doc:`readability-redundant-parentheses
<clang-tidy/checks/readability/redundant-parentheses>` check.

Detect redundant parentheses.

New check aliases
^^^^^^^^^^^^^^^^^

Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/docs/clang-tidy/checks/list.rst
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,7 @@ Clang-Tidy Checks
:doc:`readability-redundant-function-ptr-dereference <readability/redundant-function-ptr-dereference>`, "Yes"
:doc:`readability-redundant-inline-specifier <readability/redundant-inline-specifier>`, "Yes"
:doc:`readability-redundant-member-init <readability/redundant-member-init>`, "Yes"
:doc:`readability-redundant-parentheses <readability/redundant-parentheses>`, "Yes"
:doc:`readability-redundant-preprocessor <readability/redundant-preprocessor>`,
:doc:`readability-redundant-smartptr-get <readability/redundant-smartptr-get>`, "Yes"
:doc:`readability-redundant-string-cstr <readability/redundant-string-cstr>`, "Yes"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
.. title:: clang-tidy - readability-redundant-parentheses

readability-redundant-parentheses
=================================

Detect redundant parentheses.

When modifying code, one often forgets to remove the corresponding parentheses.
This results in overly lengthy code. When the expression is complex, finding
the matching parentheses becomes particularly difficult.

Example
-------

.. code-block:: c++

(1);
((a + 2)) * 3;
(a);
("aaa");

Currently this check does not take into account the precedence of operations.
Even if the expression within the parentheses has a higher priority than that
outside the parentheses. In other words, removing the parentheses will not
affect the semantics.

.. code-block:: c++

int a = (1 * 2) + 3; // no warning
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// RUN: %check_clang_tidy %s readability-redundant-parentheses %t

void parenExpr() {
1 + 1;
(1 + 1);
((1 + 1));
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant parentheses around expression [readability-redundant-parentheses]
// CHECK-FIXES: (1 + 1);
(((1 + 1)));
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant parentheses around expression [readability-redundant-parentheses]
// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: redundant parentheses around expression [readability-redundant-parentheses]
// CHECK-FIXES: (1 + 1);
((((1 + 1))));
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant parentheses around expression [readability-redundant-parentheses]
// CHECK-MESSAGES: :[[@LINE-2]]:4: warning: redundant parentheses around expression [readability-redundant-parentheses]
// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: redundant parentheses around expression [readability-redundant-parentheses]
// CHECK-FIXES: (1 + 1);
}

#define EXP (1 + 1)
#define PAREN(e) (e)
void parenExprWithMacro() {
EXP; // 1
(EXP); // 2
((EXP)); // 3
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant parentheses around expression [readability-redundant-parentheses]
// CHECK-FIXES: (EXP); // 3
PAREN((1));
}

void constant() {
(1);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant parentheses around expression [readability-redundant-parentheses]
// CHECK-FIXES: 1;
(1.0);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant parentheses around expression [readability-redundant-parentheses]
// CHECK-FIXES: 1.0;
(true);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant parentheses around expression [readability-redundant-parentheses]
// CHECK-FIXES: true;
(',');
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant parentheses around expression [readability-redundant-parentheses]
// CHECK-FIXES: ',';
("v4");
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant parentheses around expression [readability-redundant-parentheses]
// CHECK-FIXES: "v4";
(nullptr);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant parentheses around expression [readability-redundant-parentheses]
// CHECK-FIXES: nullptr;
}

void declRefExpr(int a) {
(a);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: redundant parentheses around expression [readability-redundant-parentheses]
// CHECK-FIXES: a;
}

void exceptions() {
sizeof(1);
alignof(2);
alignof((3));
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant parentheses around expression [readability-redundant-parentheses]
// CHECK-FIXES: alignof(3);
}
Loading