NoQ added a comment.

Wow, i've completely forgot about this one. Sorry about that... I think this 
checker is good to have in the current shape, and probably incrementally 
developed. It'd probably move out of alpha whenever it's tweaked to somehow 
make sure it finds enough real bugs among intentional integer truncations, 
which would most likely require some nontrivial heuristics.

Added a few very minor comments, once they're fixed i'd consider committing(?)


================
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:10
@@ +9,3 @@
+//
+// Check that there is not loss of sign/precision in assignments, comparisons
+// and multiplications.
----------------
~~not~~ => no

================
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:21
@@ +20,3 @@
+// Many compilers and tools have similar checks that are based on semantic
+// analysis. Those checks are sound but has poor precision. ConversionChecker
+// is an alternative to those checks.
----------------
~~has~~ => have

================
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:66
@@ +65,3 @@
+  const Stmt *Parent = PM.getParent(Cast);
+  if (!Parent)
+    return;
----------------
I think you can assert that the parent exists here.

================
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:87
@@ +86,3 @@
+  // Generate an error node.
+  ExplodedNode *N = C.generateErrorNode(C.getState());
+  if (!N)
----------------
You are generating a fatal error node, which stops the analysis on the path on 
which the bug is found. I don't think it's desired, because the program isn't 
known to terminate upon loss of sign/precision, and it suppresses other 
warnings by other checkers. I think you need to use 
`generateNonFatalErrorNode()` here. But then you need to make sure you generate 
this node exactly once per callback and re-use for all reports in case there 
are multiple reports, otherwise the execution path behind this node may be 
duplicated.


https://reviews.llvm.org/D13126



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

Reply via email to