[PATCH] D63188: Fixed a crash in misc-redundant-expression ClangTidy checker

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
gribozavr added a reviewer: ilya-biryukov.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

It was trying to pass a dependent expression into constant evaluator.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63188

Files:
  clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
  clang-tools-extra/test/clang-tidy/misc-redundant-expression.cpp


Index: clang-tools-extra/test/clang-tidy/misc-redundant-expression.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/misc-redundant-expression.cpp
+++ clang-tools-extra/test/clang-tidy/misc-redundant-expression.cpp
@@ -84,6 +84,14 @@
   return 0;
 }
 
+template <int DX>
+int TestSimpleEquivalentDependent() {
+  if (DX > 0 && DX > 0) return 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: both sides of operator are equivalent
+
+  return 0;
+}
+
 int Valid(int X, int Y) {
   if (X != Y) return 1;
   if (X == Y + 0) return 1;
@@ -670,7 +678,7 @@
 #define FLAG3 4
 #define FLAGS (FLAG1 | FLAG2 | FLAG3)
 #define NOTFLAGS !(FLAG1 | FLAG2 | FLAG3)
-int operatorConfusion(int X, int Y, long Z)
+int TestOperatorConfusion(int X, int Y, long Z)
 {
   // Ineffective & expressions.
   Y = (Y << 8) & 0xff;
@@ -722,6 +730,12 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: ineffective logical negation operator
   // CHECK-FIXES: {{^}}  return ~(1 | 2 | 4);{{$}}
 }
+
+template <int Shift, int Mask>
+int TestOperatorConfusionDependent(int Y) {
+  int r1 = (Y << Shift) & 0xff;
+  int r2 = (Y << 8) & Mask;
+}
 #undef FLAG1
 #undef FLAG2
 #undef FLAG3
Index: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
@@ -555,10 +555,14 @@
   if (!LhsBinOp || !RhsBinOp)
     return false;
 
-  if ((LhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
-       LhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)) &&
-      (RhsBinOp->getLHS()->isIntegerConstantExpr(*AstCtx) ||
-       RhsBinOp->getRHS()->isIntegerConstantExpr(*AstCtx)))
+  auto IsIntegerConstantExpr = [AstCtx](const Expr *E) {
+    return !E->isValueDependent() && E->isIntegerConstantExpr(*AstCtx);
+  };
+
+  if ((IsIntegerConstantExpr(LhsBinOp->getLHS()) ||
+       IsIntegerConstantExpr(LhsBinOp->getRHS())) &&
+      (IsIntegerConstantExpr(RhsBinOp->getLHS()) ||
+       IsIntegerConstantExpr(RhsBinOp->getRHS())))
     return true;
   return false;
 }
@@ -580,12 +584,14 @@
   const auto *BinOpLhs = cast<BinaryOperator>(BinOp->getLHS());
   const auto *BinOpRhs = cast<BinaryOperator>(BinOp->getRHS());
 
-  LhsConst = BinOpLhs->getLHS()->isIntegerConstantExpr(*AstCtx)
-                 ? BinOpLhs->getLHS()
-                 : BinOpLhs->getRHS();
-  RhsConst = BinOpRhs->getLHS()->isIntegerConstantExpr(*AstCtx)
-                 ? BinOpRhs->getLHS()
-                 : BinOpRhs->getRHS();
+  auto IsIntegerConstantExpr = [AstCtx](const Expr *E) {
+    return !E->isValueDependent() && E->isIntegerConstantExpr(*AstCtx);
+  };
+
+  LhsConst = IsIntegerConstantExpr(BinOpLhs->getLHS()) ? BinOpLhs->getLHS()
+                                                       : BinOpLhs->getRHS();
+  RhsConst = IsIntegerConstantExpr(BinOpRhs->getLHS()) ? BinOpRhs->getLHS()
+                                                       : BinOpRhs->getRHS();
 
   if (!LhsConst || !RhsConst)
     return false;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D63188.204233.patch
Type: text/x-patch
Size: 3139 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190612/ca51c51b/attachment-0001.bin>