NoQ accepted this revision. NoQ added a comment. Looks wonderful, thanks!
================ Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:355 + template <bool ShouldFreeOnFail> + void checkRealloc(CheckerContext &C, const CallExpr *CE, + ProgramStateRef State) const; ---------------- Charusso wrote: > balazske wrote: > > The `CHECK_FN` could be used even here? > > ``` > > template <bool ShouldFreeOnFail> > > CHECK_FN(checkRealloc) > > ``` > I think about the opposite, passing around arguments by templates is not > cool. They meant to be arguments. > > This type of callback simply could be a third `CallDescriptionMap` for future > profeness. > passing around arguments by templates is not cool. They meant to be arguments. +1! It's accidental that these values are currently known at compile time. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1184 + checkCXXNewOrCXXDelete(C, CE, State); + + checkOwnershipAttr(C, CE, State); ---------------- balazske wrote: > If these cases are exclusive the code looks better when `else` statements are > added? Yup, or even better, `return`s, because this code is very fragile because it'll do horrible things if we accidentally invoke two callbacks at once. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68165/new/ https://reviews.llvm.org/D68165 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
