[PATCH] D30345: [CodeGen][Blocks] Refactor capture handling in code that generates block copy/destroy routines

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 28 10:57:30 PST 2017


rjmccall added a comment.

In https://reviews.llvm.org/D30345#688298, @arphaman wrote:

> In https://reviews.llvm.org/D30345#688144, @rjmccall wrote:
>
> > You're doing this refactor to... support doing another refactor of the same code?  Why are these patches separate?
>
>
> Not quite, by "merging block copy/destroy routines" I meant that my next patch will try to generate the IR only for unique copy/destroy functions, so individual functions will be merged.


Ah, okay, sure.



================
Comment at: lib/CodeGen/CGBlocks.cpp:1380
+/// entity that's captured by a block.
+enum class BlockCaptureEntityType {
+  CXXRecord, // Copy or destroy
----------------
BlockCaptureEntityKind, please.


================
Comment at: lib/CodeGen/CGBlocks.cpp:1389
+/// Represents an entity captured by a block that requires custom operations
+/// to copy/release this entity.
+struct BlockCaptureManagedEntity {
----------------
Grammar, and the complementary generic operation to "copy" is "destroy", not "release".


================
Comment at: lib/CodeGen/CGBlocks.cpp:1391
+struct BlockCaptureManagedEntity {
+  BlockCaptureEntityType Type;
+  BlockFieldFlags Flags;
----------------
Similarly, please name this Kind.


================
Comment at: lib/CodeGen/CGBlocks.cpp:1608
+static std::pair<BlockCaptureEntityType, BlockFieldFlags>
+computeDestroyInfoForBlockCapture(const BlockDecl::Capture &CI, QualType Type,
+                                  const LangOptions &LangOpts) {
----------------
Please don't name local variables "Type".  "QT" or "T" would be fine.


Repository:
  rL LLVM

https://reviews.llvm.org/D30345





More information about the cfe-commits mailing list