slavapestov marked 11 inline comments as done.
slavapestov added a comment.
Thanks for the comments everyone. In the next version of the patch, I fixed
most of the review feedback and also addressed an issue where super message
sends were not calling `objc_loadClassRef()`.
================
Comment at: include/clang/Basic/Attr.td:1802
+ let Documentation = [ObjCClassStubDocs];
+}
+
----------------
aaron.ballman wrote:
> Does this attribute make sense outside of ObjC? e.g., should it require an
> ObjC compiland? If it should only be used in ObjC, then it should have a
> `LangOpts` field.
The other ObjC attributes, such as ObjCSubclassingRestricted, do not have a
LangOpts. Do you want me to add a LangOpts field to those too? Or is it
unnecessary since they're already restricted to InterfaceDecls?
================
Comment at: include/clang/Basic/AttrDocs.td:1116
+def ObjCClassStubDocs : Documentation {
+ let Category = DocCatFunction;
+ let Content = [{
----------------
aaron.ballman wrote:
> This seems like the wrong category -- the attribute doesn't apply to
> functions.
Would DocCatType make more sense? Would you like me to change
ObjCRuntimeVisible and a handful of other miscategorized attributes too?
================
Comment at: lib/CodeGen/CGObjCMac.cpp:7274
+ // Classrefs pointing at Objective-C stub classes have the least
+ // significant bit set to 1.
+ auto *Tag = llvm::ConstantInt::get(CGM.IntPtrTy, 1);
----------------
rjmccall wrote:
> This isn't for an arbitrary class ref, it's for the global class list. I'd
> say something like "the global class list sets the LSB to 1 on any class
> stubs".
Can you explain what the difference is? My understanding is that the thing you
pass to objc_loadClassref() (or load from directly) is a "classref". This is
for classes you reference, not classes you define.
================
Comment at: lib/CodeGen/CGObjCMac.cpp:7285
- Entry = new llvm::GlobalVariable(CGM.getModule(),
ObjCTypes.ClassnfABIPtrTy,
+ Entry = new llvm::GlobalVariable(CGM.getModule(), ClassGV->getType(),
false, llvm::GlobalValue::PrivateLinkage,
----------------
jordan_rose wrote:
> Is there a concern here in the non-stub case if GetClassGlobal no longer
> produces a ObjCTypes.ClassnfABIPtrTy? (Probably not, but thought I'd check
> [again].)
You raise a good point. I brought back the assert in the next iteration of the
patch.
Repository:
rC Clang
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
