MTC added a comment.
Thank you for doing the cleaning that no one else is interested in! Comments is
inline below.
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:342
+ DefaultBool IsOptimistic;
+ MemFunctionInfoTy MemFunctionInfo;
public:
----------------
I can't say that abstracting `MemFunctionInfo` is a perfect solution, for
example `IsOptimistic` and `ShouldIncludeOwnershipAnnotatedFunctions ` are
close to each other in the object layout, but they do the same thing, which
makes me feel weird. And there are so many `MemFunctionInfo.` :)
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:422
+ ///
+ /// \param [in] CE The expression that allocates memory.
+ /// \param [in] IndexOfSizeArg Index of the argument that specifies the size
----------------
`CE` typo?
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:428
+ /// if unspecified, the value of expression \p E is used.
+ static ProgramStateRef ProcessZeroAllocation(CheckerContext &C, const Expr
*E,
+ const unsigned IndexOfSizeArg,
----------------
Personally, `ProcessZeroAllocation` is like half the story, maybe
`ProcessZeroAllocCheck` or `ProcessZeroAllocationCheck` is better.
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:448
+ /// \param [in] State The \c ProgramState right before allocation.
+ /// \returns The programstate right after allocation.
ProgramStateRef MallocMemReturnsAttr(CheckerContext &C,
----------------
Maybe `program state` or just `ProgramState` is better?
Same as 462, 476, 507, 575, 593.
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:592
+ /// \param [in] CE The expression that reallocated memory
+ /// \param [in] State The \c ProgramState right before reallocation.
+ /// \returns The programstate right after allocation.
----------------
`before realloction` typo?
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1087
if (FD->getKind() == Decl::Function) {
- initIdentifierInfo(C.getASTContext());
+ MemFunctionInfo.initIdentifierInfo(C.getASTContext());
IdentifierInfo *FunI = FD->getIdentifier();
----------------
If I not wrong, `MemFunctionInfo` is mainly a class member of `MallocChecker`
that hold a bunch of memory function identifiers and provide corresponding
helper APIs. And we should pick a proper time to initialize it.
I want to known why you call `initIdentifierInfo()` in `checkPostStmt(const
CallExpr *E,)`, this callback is called after `checkPreCall(const CallEvent
&Call, )` in which we need `MemFunctionInfo`.
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1247
+ CheckerContext &C, const Expr *E, const unsigned IndexOfSizeArg,
+ ProgramStateRef State, Optional<SVal> RetVal) {
if (!State)
----------------
`const` qualifier missing?
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1323
+/// pointer to the type of the constructed record or one of its bases.
+static bool hasNonTrivialConstructorCall(const CXXNewExpr *NE) {
----------------
I'm not sure `hasNonTrivialConstructorCall()` is a good name although you
explained it in details in the comments below. And the comments for this
function is difficult for me to understand, which is maybe my problem.
And I also think this function doesn't do as much as described in the comments,
it is just `true if the invoked constructor in \p NE has a parameter - pointer
or reference to a record`, right?
================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2407
const CallExpr *CE,
- bool FreesOnFail,
+ bool ShouldFreeOnFail,
ProgramStateRef State,
----------------
The parameter name `ShouldFreeOnFail` is not consistent with the declaration,
see line 577.
Repository:
rC Clang
https://reviews.llvm.org/D54823
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
