xazax.hun added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:351
+bool SmartPtrModeling::handleEqOp(const CallEvent &Call,
+ CheckerContext &C) const {
----------------
I'd prefer to call this AssignOp to avoid confusion with `==`. While your
naming is correct, I always found this nomenclature confusing.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:384
+
+ const auto *ArgRegionVal = State->get<TrackedRegionMap>(ArgRegion);
+ if (ArgRegionVal) {
----------------
I also find the names of the variables confusing.
Instead of `ArgRegion` what about `OtherSmartPtrRegion`?
Instead of `ArgRegionVal` what about `OtherInnerPtr`?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:391
+
+ C.addTransition(State, C.getNoteTag([ThisRegion, ArgRegion, IsArgValNull](
+ PathSensitiveBugReport &BR,
----------------
Adding return after every `addTransition` is a good practive that we should
follow.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:419
+ llvm::raw_ostream &OS) {
+ if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
+ !BR.isInteresting(ArgRegion))
----------------
Isn't this the same as the beginning of the note tag above?
I wonder if there is a way to deduplicate this code. Not a huge priority though
as I do not have an immediate idea for doing this in a clean way.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86293/new/
https://reviews.llvm.org/D86293
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
