cjdb marked an inline comment as done. cjdb added a comment. In D94640#2579447 <https://reviews.llvm.org/D94640#2579447>, @aaron.ballman wrote:
> In D94640#2560647 <https://reviews.llvm.org/D94640#2560647>, @cjdb wrote: > >> fixes block expressions >> >> @aaron.ballman I'm not sure I follow what you're after re lambdas. Could you >> please provide a test case that I can work with? > > Sorry about missing your question -- this fell off my radar. I was thinking > it'd be roughly the same as blocks, but I now think this'll create compile > errors anyway: `free([]{ return nullptr; });` (where the lambda is > accidentally not being called). So not nearly as important as I was thinking > (sorry for that, but thank you for adding the support and test cases just the > same!). > > One worry I have is the number of effectively duplicated diagnostics we're > now getting in conjunction with the static analyzer. It's not quite perfect > overlap and so we can't get rid of the static analyzer check, but is there a > long-term plan for how to resolve this? I filed issue 48767 <https://bugs.llvm.org/show_bug.cgi?id=48767>, but I don't think the analyser folks are motivated to address it any time soon. ================ Comment at: clang/lib/Sema/SemaChecking.cpp:10252 namespace { -void CheckFreeArgumentsOnLvalue(Sema &S, const std::string &CalleeName, - const UnaryOperator *UnaryExpr, - const VarDecl *Var) { - StorageClass Class = Var->getStorageClass(); - if (Class == StorageClass::SC_Extern || - Class == StorageClass::SC_PrivateExtern || - Var->getType()->isReferenceType()) - return; - - S.Diag(UnaryExpr->getBeginLoc(), diag::warn_free_nonheap_object) - << CalleeName << Var; -} +constexpr int Placeholder = 0; ---------------- aaron.ballman wrote: > After seeing my suggested workaround in practice, I'm hopeful we can figure > out how to get the diagnostics engine to not require us to jump through these > hoops. I wouldn't block the patch over this approach, but it's not very > obvious what's happening from reading the code either. Sorry, I'm not entirely sure what you're getting at here? :/ ================ Comment at: clang/lib/Sema/SemaChecking.cpp:10342 + static constexpr int NoPlaceholder = 1; + Diag(Arg->getBeginLoc(), diag::warn_free_nonheap_object) + << CalleeName << NoPlaceholder; ---------------- aaron.ballman wrote: > FWIW, this diagnostic will be awkward for lambdas because it mentions blocks > explicitly. It's not critical thing given how hard it would be to hit the > lambda case, but if you think of an easy way to solve it, I won't complain. Nothing "easy" comes to mind. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94640/new/ https://reviews.llvm.org/D94640 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
