[clang-tools-extra] r363133 - Fixed a crash in misc-redundant-expression ClangTidy checker

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Wed Jun 12 01:40:53 2019
New Revision: 363133

URL: http://llvm.org/viewvc/llvm-project?rev=363133&view=rev
Log:
Fixed a crash in misc-redundant-expression ClangTidy checker

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

Reviewers: ilya-biryukov

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D63188

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

Modified: clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp?rev=363133&r1=363132&r2=363133&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/RedundantExpressionCheck.cpp Wed Jun 12 01:40:53 2019
@@ -555,10 +555,14 @@ static bool areSidesBinaryConstExpressio
   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 @@ static bool retrieveConstExprFromBothSid
   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;

Modified: clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp?rev=363133&r1=363132&r2=363133&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-redundant-expression.cpp Wed Jun 12 01:40:53 2019
@@ -84,6 +84,14 @@ int TestSimpleEquivalent(int X, int Y) {
   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 @@ int TestWithMinMaxInt(int X) {
 #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 @@ int operatorConfusion(int X, int Y, long
   // 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