[PATCH] D41406: [analyzer] Add a new checker callback, check::NewAllocator.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In https://reviews.llvm.org/D41406#970985, @xazax.hun wrote:

> Also, I was wondering if it would make sense to only have the PostCall callback return the casted memory region in these specific cases instead of introducing a new callback.


Yep, i actually think it's a very good idea. Like, it was obvious to me that it's a bad idea because `PostCall` and `CallEvent` should describe the function call just like before - in particular, the moment they capture and the return value should correspond to the actual value returned from the function. And it's kinda obvious by now that there are two different values, both of which may be of interest to the user, and certain amount of modeling (namely, pointer cast) occurs between these two moments.

But i guess we could also squeeze both values into `CallEvent`. Like, make a new `CallEvent` sub-class called, say, `CXXNewAllocatorCall`, which would additionally provide a method, say, `.getAllocatedObject()`, which would re-evaluate the cast and return the casted object. The whole reusable casting procedure would in this case live in `SValBuilder` and be accessed from both `ExprEngine` and `CallEvent` (in the latter case through `.getState()->getStateManager()`).

Performance-wise, switching to the `CallEvent` approach would trade an additional exploded node or two for a potentially unlimited number of re-evaluations of the cast (in every checker) - both seem lightweight enough for now to never bother. Storing the casted value in `CallEvent` would require making the `CallEvent` object more bulky, which doesn't seem justified. Futureproofness-wise, we may potentially re-introduce the callback when we actually require two program points, or if our cast procedure becomes slow and we'd want to re-use the result, but both is unlikely.


https://reviews.llvm.org/D41406