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

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 1 20:24:14 PDT 2018

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.)

  rC Clang


More information about the cfe-commits mailing list