aaron.ballman added a comment.

This is looking much closer to what I think we had in mind, so I mostly have 
some cleanup suggestions.



================
Comment at: clang/lib/Analysis/CFG.cpp:979
 
-    const BinaryOperator *BitOp = dyn_cast<BinaryOperator>(BoolExpr);
+    const BinaryOperator *BitOp = 
dyn_cast<BinaryOperator>(BoolExpr->IgnoreParens());
     if (BitOp && (BitOp->getOpcode() == BO_And ||
----------------
You can drop the `IgnoreParens()` here -- `BoolExpr` is either `LHSExpr` or 
`RHSExpr`, and both of those were initialized from a call to `IgnoreParens()`.


================
Comment at: clang/lib/Analysis/CFG.cpp:1013-1015
+  // Helper function to get the sub expressions also for the unary operators. 
If
+  // unary are encountered will solve manually. This function can return value
+  // as 12 and also -12.
----------------



================
Comment at: clang/lib/Analysis/CFG.cpp:1017-1019
+    // The value to return;
+    Optional<llvm::APInt> Value = llvm::None;
+
----------------
Removing an unnecessary variable.


================
Comment at: clang/lib/Analysis/CFG.cpp:1020-1021
+
+    const IntegerLiteral *IntLiteral = nullptr;
+    const UnaryOperator *UnOp = dyn_cast<UnaryOperator>(E->IgnoreParens());
+
----------------
Coding style nit -- if the type is spelled out explicitly in the 
initialization, we tend to prefer using `auto` for the type so we don't have to 
repeat the type information twice.

Also removes a declaration that's not needed.


================
Comment at: clang/lib/Analysis/CFG.cpp:1027
+      // Literal.
+      const Expr *SubExpr = UnOp->getSubExpr();
+      IntLiteral = dyn_cast<IntegerLiteral>(SubExpr->IgnoreParens());
----------------
This ensures everything past here doesn't have to care about parens.


================
Comment at: clang/lib/Analysis/CFG.cpp:1028-1031
+      IntLiteral = dyn_cast<IntegerLiteral>(SubExpr->IgnoreParens());
+
+      if (IntLiteral) {
+
----------------



================
Comment at: clang/lib/Analysis/CFG.cpp:1032
+
+        Value = IntLiteral->getValue();
+        // Getting the Operator Code to perform them manually.
----------------



================
Comment at: clang/lib/Analysis/CFG.cpp:1033-1034
+        Value = IntLiteral->getValue();
+        // Getting the Operator Code to perform them manually.
+        const UnaryOperatorKind OpCode = UnOp->getOpcode();
+
----------------



================
Comment at: clang/lib/Analysis/CFG.cpp:1041-1045
+          return -*Value;
+        case UO_Not:
+          return ~*Value;
+        case UO_LNot:
+          return *Value = !*Value;
----------------



================
Comment at: clang/lib/Analysis/CFG.cpp:1047
+        default:
+          return Value;
+        }
----------------
If there's a unary expression operating on an integer literal... it has to be 
one of the listed operations above, otherwise I don't think you can get here. 
So might as well assert and bail out rather than return a value that we didn't 
properly perform the unary operation on.


================
Comment at: clang/lib/Analysis/CFG.cpp:1050-1058
+
+    } else {
+
+      IntLiteral = dyn_cast<IntegerLiteral>(E->IgnoreParens());
+      if (IntLiteral)
+        return Value = IntLiteral->getValue();
+    }
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130510

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to