baloghadamsoftware accepted this revision.
baloghadamsoftware added a comment.
This revision is now accepted and ready to land.
Looks good, aside from the few naming issues I mentioned. Please try it on
//LLVM/Clang// before committing it to avoid unexpected crashes.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:112
OverloadedOperatorKind Op, const SVal &RetVal,
- const SVal &LHS, const SVal &RHS) const;
+ const SVal &Iterator, const SVal &Offset) const;
void handlePtrIncrOrDecr(CheckerContext &C, const Expr *Iterator,
----------------
In my subsequent patches I began to use the name `Amount` instead of `Offset`
to not confuse with `IteratorPosition::Offset`.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:281
+ const Expr *const &IterExpr = IsIterOnLHS ? LHS : RHS;
+ const Expr *const &OtherExpr = IsIterOnLHS ? RHS : LHS;
+
----------------
`AmountExpr`.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:385
+ const QualType FstType = FstExpr->getType();
+ const QualType SndType = SndExpr->getType();
+
----------------
`Expr1`, `Expr2`, `Type1`, `Type2` or something similar. `Fst` is to be
confused with //Fast// and `Snd` with //Sound//. Or spell out `First` and
`Second`.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:400
+ "Either first or second argument should have structure "
+ "or class type!");
handleRandomIncrOrDecr(C, OrigExpr, Op, Call.getReturnValue(),
----------------
This is generally true in //C++// that overloaded operators must either be
class member or must have at least one class argument. Do we have to assert it
in this particular checker?
================
Comment at: clang/test/Analysis/iterator-modeling.cpp:152
-void plus(const std::vector<int> &v) {
+void lhs_plus(const std::vector<int> &v) {
auto i1 = v.begin();
----------------
`plus_lhs`, `plus_rhs` to begin with the name of the operation.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83190/new/
https://reviews.llvm.org/D83190
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
