[PATCH] D16286: [clang-tidy] Readability check for redundant parenthesis in return expression.

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

A high-level comment: why do we want to limit this check to only remove parentheses from `return ()`? Anything wrong with removing unnecessary parentheses everywhere (except for macro bodies)?


================
Comment at: clang-tidy/readability/ReturnWithRedundantParensCheck.cpp:28
@@ +27,3 @@
+
+  const ParenExpr *ParenExpression =
+      Result.Nodes.getNodeAs<ParenExpr>("paren_expr");
----------------
Please use `const auto *`: repeating the type name doesn't make anything clearer.

================
Comment at: clang-tidy/readability/ReturnWithRedundantParensCheck.cpp:31
@@ +30,3 @@
+
+  const SourceLocation &LParenLoc = ParenExpression->getLParen();
+  const SourceLocation &RParenLoc = ParenExpression->getRParen();
----------------
SourceLocations are small and trivially copyable, so they should be passed by value.

================
Comment at: test/clang-tidy/readability-return-with-redundant-parens.cpp:1
@@ +1,2 @@
+// RUN: %check_clang_tidy %s readability-return-with-redundant-parens %t
+
----------------
Please add tests with macros:
  1. where superfluous parentheses are in the macro body (and they usually shouldn't be removed);
  2. where superfluous parentheses are in a macro argument (which is not itself located in a macro body (e.g. `EXPECT_EQ(x, (y + z));`) - I'd say that in this case we usually can remove parentheses;

Please add a test to ensure replacements are only applied once in templates with multiple instantiations.


http://reviews.llvm.org/D16286