[PATCH] D86293: [analyzer] Add modeling of Eq operator in smart ptr

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:351
 
+bool SmartPtrModeling::handleEqOp(const CallEvent &Call,
+                                  CheckerContext &C) const {
----------------
xazax.hun wrote:
> I'd prefer to call this AssignOp to avoid confusion with `==`. While your naming is correct, I always found this nomenclature confusing.
IMO it's not a question of preference with this one, `EqOp` is misleading.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:399
+        ArgRegion->printPretty(OS);
+        OS << " is null after moved to ";
+        ThisRegion->printPretty(OS);
----------------
The grammar seems off in this one.  I think it should be smith like "after being moved to" or "after it was moved to".


================
Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:132
+  std::unique_ptr<A> PToMove; // expected-note {{Default constructed smart pointer 'PToMove' is null}}
+  P = std::move(PToMove); // expected-note {{Smart pointer 'P' is null after a null value moved from 'PToMove'}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
----------------
NoQ wrote:
> I suggest: `Null pointer value move-assigned to 'P'`.
+1


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86293/new/

https://reviews.llvm.org/D86293