[PATCH] D105821: [analyzer] [WIP] Model destructor for std::unique_ptr

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:143
+    llvm::ArrayRef<llvm::StringLiteral> ValidNames(STD_PTR_NAMES);
+    return llvm::is_contained(ValidNames, Name);
   }
----------------
And why can't we pass `STD_PTR_NAMES` directly to `llvm::is_contained`?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:378-387
+    llvm::errs() << "Found destructor call\n";
+    State = DC->invalidateRegions(C.blockCount(), State);
+    const MemRegion *ThisRegion = DC->getCXXThisVal().getAsRegion();
+    assert(ThisRegion && "We do not support explicit calls to destructor");
+    const auto *InnerPtrVal = State->get<TrackedRegionMap>(ThisRegion);
+    State = State->remove<TrackedRegionMap>(ThisRegion);
+    if (InnerPtrVal)
----------------
I suggest to add a ton of comments with the reasoning behind these actions.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:381
+    const MemRegion *ThisRegion = DC->getCXXThisVal().getAsRegion();
+    assert(ThisRegion && "We do not support explicit calls to destructor");
+    const auto *InnerPtrVal = State->get<TrackedRegionMap>(ThisRegion);
----------------
And if it happens we are going to crash with assertion failure?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:421-422
       const auto *TrackingExpr = Call.getArgExpr(0);
-      assert(TrackingExpr->getType()->isPointerType() &&
-             "Adding a non pointer value to TrackedRegionMap");
+      if (TrackingExpr->getType()->isPointerType())
+        return false;
       auto ArgVal = Call.getArgSVal(0);
----------------
Okay, I'm either missing something or this condition is missing `!` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105821