[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder marked an inline comment as not done.
MadCoder added inline comments.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:3909
+called directly. It can also be used on Objective-C Categories and Extensions,
+in which case it applies to all methods declared in such an ``@interface``.
+
----------------
rjmccall wrote:
> If this is still true (rather than being done with `objc_direct_members`),
> I feel it should be documented next to the statement at the end about
> `objc_direct_members` in order to make the contrasting use cases clear.
> But we should discuss whether this should be using `objc_direct_members`
> for that use case.
`objc_direct_members` is for the @implementation only right now. `objc_direct_members` but it could make sense to be used on `@interface` instead I agree. It would make sense.

EDIT: actually I quite like this, it is much cleaner. I'll work on updating the patch.


================
Comment at: clang/include/clang/Basic/AttrDocs.td:3912
+Properties can also be declared with the ``direct`` property attribute
+which causes the property getter and setter to be direct methods as well.
+
----------------
rjmccall wrote:
> "If an Objective-C property is declared with the ``direct`` property attribute, then its getter and setter are declared to be direct unless they are explicitly declared."
> 
> I assume that's correct w.r.t the treatment of explicit declarations of getters and setters?
I would expect so, I shall have a test for it


================
Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:3885
+{
+  // GNU runtime doesn't support direct calls at this time
+}
----------------
rjmccall wrote:
> This doesn't seem to be diagnosed in Sema.
how should I do it, is there an example I can follow?


================
Comment at: clang/lib/CodeGen/CGObjCMac.cpp:1588
     // interface, we cannot perform this check.
+	//
+	// Note that for direct methods, because objc_msgSend is skipped,
----------------
rjmccall wrote:
> MadCoder wrote:
> > doh I'll fix the tabs
> The indentation still seems wrong.
yeah I fixed it in my checkout but didn't update the diff here yet


================
Comment at: clang/lib/CodeGen/CGObjCMac.cpp:4095
+    Builder.CreateStore(result.getScalarVal(),
+                        CGF.GetAddrOfLocalVar(OMD->getSelfDecl()));
+
----------------
rjmccall wrote:
> We should also be changing `selfValue` so that the null check below will do the right thing with e.g. subclasses of weak-linked classes, where IIUC the input value might be non-null but the initialized class pointer might be null.
I'm not sure what you mean, wouldn't storing to `CGF.GetAddrOfLocalVar(OMD->getSelfDecl())` actually affect selfValue itself? I'm not 100% sure I understand what you mean here.


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

https://reviews.llvm.org/D69991