[PATCH] D81315: [analyzer][Draft] [Prototype] warning for default constructed unique pointer dereferences

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Best of luck on your GSoC! I don't have much else to add to your patch, but you seem to have made good progress already!

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

> some folks have automated scripts based on that tag to add themselves as subscriber/reviewer.


Hope you don't mind my intrusion :)

> I am not sure about whether I should use eval::Call or both check::PreCall and check::PostCall. In the eval::Call documentation I found this "Note, that only one checker can evaluate a call.". So I am little bit confused about using it.

Inlining (when we model a function call, https://youtu.be/yh2qdnJjizE?t=238) is rather expensive. Creating a new stack frame, parameters, new `ExplodedNode`s, running checkers, etc., eats memory for breakfast, is slow and limits how deep the analysis can go. Worse still, the analysis could lose precision if the called function's definition isn't available. `eval::Call` serves to circumvent this by allowing the analyzer to give a precise summary of the function. `StreamChecker`, for instance, uses this for functions such as `clearerr()` -- the C standard defines how this function should behave, so upon encountering a call to it, we don't need all the extra work regular inlining demands, just ask `StreamChecker` to model it for us.

Use `eval::Call` if you can provide a precise model for a function. Only a single checker is allowed to do that -- you can see that it returns with a boolean value to sign whether the checker could provide an evaluation, and as far as I know, the first checker that returns true will be doing it.

I think it would be appropriate in this instance, because we're modeling a well established API. In general, I think we should use it when appropriate more often, like in MallocChecker.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:46
+};
+} // end of anonymous namespace
+
----------------
xazax.hun wrote:
> You can merge the two anonymous namespaces, this and the one below.
[[https://llvm.org/docs/CodingStandards.html#anonymous-namespaces | I prefer them like this. ]]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81315