[PATCH] D73775: [clang-tidy] Cover cases like (b && c && b) in the redundant expression check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:335
+    return AsTExpr;
+  else
+    return nullptr;
----------------
Please don;e use else after return.


================
Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:345
+                            TOp OpKind) {
+  auto *PartAsBinOp = checkOpKind<TExpr>(Part, OpKind);
+  if (PartAsBinOp) {
----------------
Could it be const auto *?


================
Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:347
+  if (PartAsBinOp) {
+    auto Operands = getOperands(PartAsBinOp);
+    if (areEquivalentExpr(Operands.first, Operands.second))
----------------
Please don't use auto when type is not spelled explicitly or iterator.


================
Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:352
+           collectOperands<TExpr>(Operands.second, AllOperands, OpKind);
+  } else {
+    AllOperands.push_back(Part);
----------------
Please elide braces.


================
Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:363
+                      ASTContext &Context) {
+  const auto OpKind = getOp(TheExpr);
+  // if there are no nested operators of the same kind, it's handled by
----------------
Please don't use auto when type is not spelled explicitly or iterator.


================
Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:366
+  // operands/parametersAreEquivalent
+  const auto Operands = getOperands(TheExpr);
+  if (!(checkOpKind<TExpr>(Operands.first, OpKind) ||
----------------
Please don't use auto when type is not spelled explicitly or iterator.


================
Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:375
+  for (ast_type_traits::DynTypedNode Parent : Parents) {
+    if (checkOpKind<TExpr>(Parent.get<TExpr>(), OpKind)) {
+      return false;
----------------
Please elide braces.


================
Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:402
+
+    if (FoundDuplicates) {
+      Builder->setBinding(
----------------
Please elide braces.


================
Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:1223
+    for (const auto &KeyValue : Result.Nodes.getMap()) {
+      if (StringRef(KeyValue.first).startswith("duplicate")) {
+        Diag << KeyValue.second.getSourceRange();
----------------
Please elide braces.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73775/new/

https://reviews.llvm.org/D73775