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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/CodeGen/CGBlocks.cpp:361
 
   /// Order by 1) all __strong together 2) next, all byfref together 3) next,
   /// all __weak together. Preserve descending alignment in all situations.
----------------
"byref"


================
Comment at: clang/lib/CodeGen/CGBlocks.cpp:1953
+  for (auto &capture : blockInfo.SortedCaptures) {
+    if (capture.isConstantOrTrivial())
+      continue;
----------------
Should this be specific to whether this is trivial to copy?


================
Comment at: clang/lib/CodeGen/CGBlocks.cpp:2142
+    if (capture.isConstantOrTrivial())
+      continue;
+
----------------
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).


================
Comment at: clang/lib/CodeGen/CGBlocks.h:268
 
   /// The mapping of allocated indexes within the block.
+  llvm::DenseMap<const VarDecl *, Capture *> Captures;
----------------
This comment is out-of-date, and it should also be updated to say that the values are pointers into `SortedCaptures`.


================
Comment at: clang/lib/CodeGen/CGBlocks.h:306
   Capture &getCapture(const VarDecl *var) {
-    llvm::DenseMap<const VarDecl*, Capture>::iterator
-      it = Captures.find(var);
+    llvm::DenseMap<const VarDecl *, Capture *>::iterator it =
+        Captures.find(var);
----------------
We can use `auto` now, and this seems like a good place for it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116948/new/

https://reviews.llvm.org/D116948