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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Tests pls!

Yes I think you should totally do an `evalCall()` here. The function has no other side effects apart from making a pointer so it's valuable to fully model it so that to avoid unnecessary store invalidations.

In D103750#2801320 <https://reviews.llvm.org/D103750#2801320>, @xazax.hun wrote:

>> Since we have CallExpr, we can easily conjure up an SVal. But I don't see how I can do it similarly in this patch.
>
> You should have a `CallExpr` for `std::make_unique` too. I believe that expression is used to determine how the conjured symbol was created (to give it an identity). So probably it should be ok to use the `make_unique` call to create this symbol (but be careful to create the symbol with the right type).

That's right, a conjured symbol isn't necessarily the value of its respective expression. So you can conjure it and assume that it's non-null. Also consider using a //heap// conjured symbol (a-la `getConjuredHeapSymbolVal()`). That said, `getConjuredHeapSymbolVal()` doesn't provide an overload for overriding the type; there's no reason it shouldn't though, please feel free to extend it.

There's a separate story about `SymbolConjured` being the right class for the problem. I'm still in favor of having `SymbolMetadata` instead, so that to properly deduplicate symbols across different branches and merge more paths.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:204
+MakeUniqueKind
+SmartPtrModeling::isStdMakeUniqueCall(const CallEvent &Call) const {
+  if (Call.getKind() != CallEventKind::CE_Function)
----------------
Please use `CallDescription` for most of this stuff.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:337
+
+bool isUniquePtrType(QualType QT) {
+  const auto *T = QT.getTypePtr();
----------------
Please merge with `isStdSmartPtrCall`.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:338-341
+  const auto *T = QT.getTypePtr();
+  if (!T)
+    return false;
+  const auto *Decl = T->getAsCXXRecordDecl();
----------------
`getTypePtr()` is almost never necessary because `QualType` has an overloaded `operator->()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103750