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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits