george.karpenkov requested changes to this revision.
george.karpenkov added inline comments.
This revision now requires changes to proceed.
================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:80
typedef llvm::ImmutableMap<void*, void*> GenericDataMap;
+ typedef llvm::DenseMap<const Stmt *, StringRef> ConstraintMap;
----------------
Generally, `StringRef`'s shouldn't be stored in containers, as containers might
outlive them.
It might be fine in this case, but I would prefer to be on the safe side and
just use `std::string`
================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:238
+ /// message on that constraint being changed.
+ bool isChangedOrInsertConstraint(ConstraintMap &Constraints, const Stmt
*Cond,
+ StringRef Message) const {
----------------
whisperity wrote:
> `insertOrUpdateContraintMessage`
>
> and mention in the documentation that it returns whether or not the actual
> insertion or update change took place
If constraints is now a property of the visitor, shouldn't this method with the
typedef above be moved to the visitor as well?
================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:244
+ if (I == Constraints.end() || !Message.equals(I->second)) {
+ Constraints[Cond] = Message;
+ return true;
----------------
Isn't that equivalent to `Constraints.insert(make_pair(Cond, Message)).second` ?
I think I have written that before.
================
Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2232
+ const ExplodedNode *N, BugReport &BR)
{
+ Constraints.clear();
+}
----------------
It does not seem necessary, because a new copy of the visitor is created for
each new bug report.
https://reviews.llvm.org/D53076
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
