zaks.anna added a comment.
Why is there such a large jump in the number of warnings reported in the last
patch iteration?
It went from "1678 projects where scanned. In total I got 124 warnings" to "In
2215 projects it found 875 warnings." Did the number of warnings in the initial
1678 projects stay the same?
Is it possible to take a look at the nature of the false positives, as per
NoQ's request above?
This checker would benefit greatly from explaining why the errors occur. For
example, where the values of variables are being constrained. Other checkers
use BugReporterVisitor to generate the rich diagnostic information. Dow you
have plans on how to accomplish that for this checker?
================
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:11
@@ +10,3 @@
+// This defines ConversionChecker that warns about dangerous conversions where
+// there is possible loss of precision.
+//
----------------
Please, make this more specific.
Would be useful to summarize which heuristics you used to bring the number of
false positives down.
================
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:29
@@ +28,3 @@
+public:
+ void checkPreStmt(const ImplicitCastExpr *Cast, CheckerContext &C) const {
+ // TODO: For now we only warn about DeclRefExpr, to avoid noise. Warn for
----------------
nit: Please move the body out of the declaration.
================
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:74
@@ +73,3 @@
+ if (!BT)
+ BT.reset(new BuiltinBug(this, "Conversion", "loss of sign/precision."));
+
----------------
Please, capitalize the sentence.
================
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:84
@@ +83,3 @@
+// Can E value be greater or equal than Val?
+static bool canBeGreaterEqual(CheckerContext &C, const Expr *E,
+ unsigned long long Val) {
----------------
This function returns true if the value "is" greater or equal, not "can be"
greater or equal. The latter would be "return StGE".
Also, it's slightly better to return the StGE state and use it to report the
bug. This way, our assumption is explicitly recorded in the error state.
================
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:147
@@ +146,3 @@
+ if (canBeGreaterEqual(C, Cast->getSubExpr(), MaxVal))
+ reportBug(C, "loss of precision in implicit conversion");
+}
----------------
Please, use capitalization and punctuation.
http://reviews.llvm.org/D13126
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
