NoQ added inline comments.
================
Comment at: include/clang/StaticAnalyzer/Core/CheckerManager.h:311
+ ExprEngine &Eng,
+ bool wasInlined = false);
+
----------------
dcoughlin wrote:
> Does 'wasInlined' really need to have a default argument? It looks like there
> are only two callers. (Also the capitalization is inconsistent).
That's copied from other methods (eg. `runCheckersForPostCall`), should i clean
them up as well some day?
================
Comment at: lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp:143
+ /// \param Target The allocated region, casted to the class type.
+ void checkNewAllocator(const CXXNewExpr *NE, SVal Target,
+ CheckerContext &) const {}
----------------
xazax.hun wrote:
> Is it possible to have a `NonLoc` Target? If no, would it be better to make
> it `Loc` type?
I guess it can be `UnknownVal` as well, or even `UndefinedVal`.
================
Comment at: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:343
+ if (const CXXNewExpr *CNE = dyn_cast_or_null<CXXNewExpr>(CE)) {
+ ExplodedNodeSet DstPostPostCallCallback;
+ getCheckerManager().runCheckersForPostCall(DstPostPostCallCallback,
----------------
dcoughlin wrote:
> Is it possible/desirable to factor out this logic (running the PostCall
> callbacks and then the NewAllocator callbacks) into a helper method on
> ExprEngine then call the helper from both VisitCXXNewAllocatorCall() and
> processCallExit()?
I think it's more readable inline. It's a small piece of code with subtle
differences between copies (eg. here we have one predecessor node, there we
have a set of nodes, then we need to pass the `WasInlined` flag), and also in
`VisitCXXNewAllocatorCall()` all other checker callbacks are written out
directly, so it doesn't make much sense to put these two into a sub-function
while leaving three other callbacks in place - rather hurts readability. This
code is kind of idiomatic and repeated many times in `ExprEngine` - maybe we
should clean up the whole stuff, but i don't think cleaning up just this place
is the right thing to do.
https://reviews.llvm.org/D41406
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
