[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: lib/CodeGen/CGBlocks.cpp:1638
+    switch (E.Kind) {
+    case BlockCaptureEntityKind::CXXRecord: {
+      Name += "c";
----------------
I forget whether this case has already filtered out trivial copy-constructors, but if not, you should consider doing that here.


================
Comment at: lib/CodeGen/CGBlocks.cpp:1647
+        unsigned Qs;
+        cast<CXXConstructExpr>(CE)->getConstructor()->isCopyConstructor(Qs);
+        if (Qs & Qualifiers::Volatile)
----------------
This doesn't always have to be a copy constructor.  I mean, maybe this procedure works anyway, but the constructor used to copy an object isn't always a copy constructor.  Blame constructor templates.


================
Comment at: lib/CodeGen/CGBlocks.cpp:1651
+      }
+      std::string Str = CaptureTy->getAsCXXRecordDecl()->getName();
+      Name += llvm::to_string(Str.size()) + Str;
----------------
The name of a C++ class is not unique; you need to use the mangler.   Ideally, you would find a way to mangle all the C++ types in the context of the same mangler instance so that substitutions could be reused across it.

You also need to check for a type with non-external linkage and make this helper private if you find one.  (You can still unique within the translation unit, for what it's worth.)


Repository:
  rC Clang

https://reviews.llvm.org/D50152