xazax.hun added inline comments.
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:483
// Did not track -> released. Other state (allocated) -> released.
- return (Stmt && (isa<CallExpr>(Stmt) || isa<CXXDeleteExpr>(Stmt)) &&
- (S && S->isReleased()) && (!SPrev || !SPrev->isReleased()));
+ // The statement associated with the release might be missing.
+ return (S && S->isReleased()) && (!SPrev || !SPrev->isReleased());
----------------
To be honest, I am not sure what was the purpose of the AST based checks in the
original version. IMHO, looking at the states only should be sufficient without
examining the AST.
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2914
+ default:
+ llvm_unreachable("unhandled allocation family");
+ }
----------------
I think these messages should be full sentences starting with a capital letter
and ending with a period.
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2988
+ if (!S) {
+ auto PostImplCall = N->getLocation().getAs<PostImplicitCall>();
+ if (!PostImplCall.hasValue())
----------------
This new code should only be used for `AF_InternalBuffer` allocation family? If
that is the case, maybe adding an assert here and running it on some large
codebases to se if this is triggered would be great.
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2989
+ auto PostImplCall = N->getLocation().getAs<PostImplicitCall>();
+ if (!PostImplCall.hasValue())
+ return nullptr;
----------------
LLVM's optional can implicitly convert to bool. I think it is perfectly fine to
rely on the implicit conversion here.
Repository:
rC Clang
https://reviews.llvm.org/D48521
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
