NoQ added inline comments.
================ Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2847 + + AtomicExpr::AtomicOp Op = AE->getOp(); + if (Op != AtomicExpr::AO__c11_atomic_fetch_add && ---------------- george.karpenkov wrote: > IMO would be slightly easier to read with logic reversed, e.g. > > ``` > if (AE == dyn_cast<>(...)) > if (Op == add || Op == sub) > for (...) > if (...) > return true > return false > ``` > > But this is a preference and can be ignored. Ok. Just in case, this is usually regulated via https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code but here it seems fine. ================ Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2899 + // to find out if a likely-false-positive suppression should kick in. + for (const LocationContext *LC = CurrentLC; LC; LC = LC->getParent()) { + if (isa<CXXDestructorDecl>(LC->getDecl())) { ---------------- george.karpenkov wrote: > I'm not sure what is going on here. > We are just traversing the stack frame, finding the first destructor in the > `isReleased` branch? Do the updated comments make it more clear? https://reviews.llvm.org/D43791 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
