[PATCH] D103750: [analyzer] Handle std::make_unique for SmartPtrModeling

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:201-202
+    const TypedValueRegion *TVR = llvm::dyn_cast<TypedValueRegion>(ThisRegion);
+    assert(TVR && "expected std::make_unique to return a std::unique_ptr "
+                  "object (which is typed)");
+    const QualType InnerPtrType =
----------------
Untyped region isn't a region without a type; everything has a type. Untyped region is when we //don't know// the type. A typical situation that produces untyped region is when the region comes in through a void pointer.

I vaguely remember that one way to trick your specific code may be to do
```lang=c++
std::unique_ptr<int> foo() {
  return make_unique<int>(123);
}
```
which will RVO into an unknown region. I also wouldn't rely on it being typed in all other cases.

A much safer way to access the inner pointer type would be to query the function's template parameter.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:214-217
+    auto &Engine = State->getStateManager().getOwningEngine();
+    State = Engine.updateObjectsUnderConstruction(
+        *ThisRegionOpt, nullptr, State, C.getLocationContext(),
+        Call.getConstructionContext(), {});
----------------
I suggest a `TODO: ExprEngine should do this for us.`.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:219
+
+    C.addTransition(State);
+    return true;
----------------
Do we need a note here as well? I guess we don't because we'll never emit null dereference reports against a non-null pointer. But if we later emit more sophisticated bug reports, we might need one. Maybe leave a comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103750