[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/Attr.td:292
+  // "LangOpts" bound.
+  string CustomCode = customCode;
 }
----------------
If this is code, should it be using a `code` type rather than a `string` type?


================
Comment at: clang/include/clang/Basic/AttrDocs.td:1101
+implementations defined for them. This attribute is intended for use in
+Swift generated headers for classes defined in Swift.
+
----------------
Swift generated -> Swift-generated


================
Comment at: clang/lib/CodeGen/CGObjCMac.cpp:736
+    llvm::Type *params[] = { Int8PtrPtrTy };
+    auto F = CGM.CreateRuntimeFunction(
+        llvm::FunctionType::get(ClassnfABIPtrTy, params, false),
----------------
Please spell the type out explicitly rather than use `auto` here.


================
Comment at: clang/lib/CodeGen/CGObjCMac.cpp:7325
                                NotForDefinition);
+      assert(ClassGV->getType() == ObjCTypes.ClassnfABIPtrTy);
     }
----------------
Can you add a message to this assertion so that when it triggers, users get something more helpful to report?


================
Comment at: clang/lib/CodeGen/CGObjCMac.cpp:7366
   if (!Entry) {
-    auto ClassGV = GetClassGlobal(ID, /*metaclass*/ false, NotForDefinition);
+    auto ClassGV = GetClassGlobalForClassRef(ID);
     std::string SectionName =
----------------
Can you spell out the type here, since you're changing the initialization already?


================
Comment at: clang/lib/CodeGen/CGObjCMac.cpp:7410-7411
   if (ID->isWeakImported()) {
-    auto ClassGV = GetClassGlobal(ID, /*metaclass*/ false, NotForDefinition);
+    llvm::Constant *ClassGV = GetClassGlobal(ID, /*metaclass*/ false,
+                                             NotForDefinition);
     (void)ClassGV;
----------------
I'm not opposed to this change, but it seems unrelated to the patch. Feel free to commit separately as an NFC.


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

https://reviews.llvm.org/D59628