[PATCH] D116948: [CodeGen] Treat ObjC `__unsafe_unretained` and class types as trivial when generating copy/dispose helper functions

[Akira Hatanaka via Phabricator via cfe-commits]

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