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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to