danielmarjamaki marked an inline comment as done.
danielmarjamaki added inline comments.


================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:107
+/** Recursively check if variable is changed in code. */
+static bool isChanged(const Stmt *S, const VarDecl *VD, bool Write) {
+  if (!S)
----------------
xazax.hun wrote:
> Usually, we do not like bug recursions since it might eat up the stack. 
> Also, do you consider the case when the variable is passed by reference to a 
> function in another translation unit? 
I rewrote the code to reuse the ast matchers in LoopUnroll.. and that is not 
recursive. I rewrote the getGlobalStaticVars (this code is longer and in my 
opinion less readable but it's not recursive).


================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:169
+  SVal Constraint_untested =
+      evalBinOp(State, BO_EQ, V, svalBuilder.makeIntVal(InitVal),
+                svalBuilder.getConditionType());
----------------
xazax.hun wrote:
> Does this work for non-integer typed e.g. structs? 
Currently static struct objects are unaffected by these changes. There is no 
regression.

Reviews are taking too long time. I am not against getting reviews but I am 
against waiting for reviews for months, ideally people would only need to wait 
for at most a week to get a review. This is why I don't want to handle structs 
now -- I am not eager to prolong the review process for this patch.



Repository:
  rL LLVM

https://reviews.llvm.org/D37897



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

Reply via email to