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

[John McCall via Phabricator via cfe-commits]

rjmccall requested changes to this revision.
rjmccall added inline comments.
This revision now requires changes to proceed.


================
Comment at: include/clang/Basic/AttrDocs.td:1118
+    let Content = [{
+This attribute specifies that the Objective-C class to which it applies has dynamically-allocated metadata. Classes annotated with this attribute cannot be subclassed.
+    }];
----------------
You should probably check that the user doesn't try to subclass classes annotated with this attribute, then. :)


================
Comment at: lib/CodeGen/CGObjCMac.cpp:735
+  llvm::Constant *getLoadClassrefFn() const {
+    // FIXME: Other attributes?
+
----------------
It should be safe to make it `readnone`, which could theoretically optimize something like doing a class message-send in a loop.


================
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);
----------------
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".


================
Comment at: lib/CodeGen/CGObjCMac.cpp:7278
+        ClassGV = llvm::ConstantExpr::getIntToPtr(ClassGV,
+                                                  ObjCTypes.Int8PtrTy);
+      }
----------------
It's more typical to do this with a GEP, although the add should still work.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:7126
     break;
+  case ParsedAttr::AT_ObjCClassStub:
+    handleSimpleAttribute<ObjCClassStubAttr>(S, D, AL);
----------------
You should emit an error if `!getLangOpts().ObjCRuntime.allowsClassStubs()`, which should be straightforward to implement.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59628