NoQ added a comment.
Hello, sorry for your having to wait on me. The implementation looks good, i'm
only having a couple of public API concerns.
================
Comment at:
include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h:46-49
+bool isExprResultConformsComparisonRule(CheckerContext &C,
+ BinaryOperatorKind CompRule,
+ const Expr *LHSExpr, const SVal
RHSVal);
+bool isGreaterEqual(CheckerContext &C, const Expr *E, unsigned long long Val);
----------------
Because you are making these functions public, i think it's worth it to make it
obvious what they do without looking at the particular checker code. Comments
are definitely worth it. I think function names could be worded better; i
suggest `exprComparesTo(const Expr *LHSExpr, BinaryOperatorKind ComparisonOp,
SVal RHSVal, CheckerContext &C);` and `isGreaterOrEqual(...)`; i'm personally
fond of having CheckerContext at the back because that's where we have it in
checker callbacks, but that's not important.
================
Comment at: lib/StaticAnalyzer/Core/CheckerHelpers.cpp:101
+ const SVal RHSVal) {
+ ProgramStateRef State = C.getState();
+
----------------
I believe it is enough to pass only `State` to this function. `SValBuilder` and
`ConstraintManager` objects can be obtained from the state's
`ProgramStateManager`. The good thing about requiring only `State` is that we'd
be able to use these functions in checker callbacks that don't provide the
`CheckerContext` object, eg. `checkEndAnalysis` or `checkRegionChanges`.
Repository:
rL LLVM
https://reviews.llvm.org/D30295
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
