ahatanak added inline comments.
================ Comment at: clang/lib/CodeGen/CGBlocks.cpp:2142 + if (capture.isConstantOrTrivial()) + continue; + ---------------- rjmccall wrote: > ahatanak wrote: > > rjmccall wrote: > > > Should this condition be specific to whether it's trivial *to destroy*? > > > C++ types could be trivial to destroy but not to copy (and, > > > theoretically, vice-versa). > > Adding the check here won't change anything since `pushCaptureCleanup` is a > > no-op if the type is trivial to destroy. But we can replace > > `NeedsCopyDispose` with `NeedsCopy` and `NeedsDispose` so that we can track > > whether copy and dispose helpers are needed separately and avoid emitting > > empty dispose helper functions. > > > > I'm not sure whether we should do that in this patch or in another patch. > Technically, it would save emitting a GEP, but I agree that that's probably > not worth thinking about, so nevermind. > > Avoiding emitting the functions is probably best to do in a separate patch. > The runtime does allow null pointers here even if the descriptor says that > the helpers exist? Yes, the runtime checks whether the copy/dispose helper function field is null before calling the function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116948/new/ https://reviews.llvm.org/D116948 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
