theraven added a comment.
Herald added a reviewer: aaron.ballman.
Herald added a reviewer: jdoerfert.
Sorry for getting to this late, I assumed it was a runtime-agnostic feature 
until someone filed a bug against the GNUstep runtime saying that we didn't 
implement it.  It would be polite to tag me in reviews for features that 
contain runtime-specific things.

The current implementation doesn't look as if it's runtime-specific.  Is there 
a reason that it's in CGObjCMacCommon rather than in the generic CGObjCRuntime 
class?

Looking at the code, it appears that it is correct only for objects created 
with `+alloc`.  Consider the following example:

  #import <Foundation/Foundation.h>
  #include <objc/runtime.h>
  
  @interface X : NSObject
  - (void)test __attribute__((objc_direct));
  @end
  
  @implementation X
  + (void)initialize
  {
        printf("+initialize called on %p\n", self);
  }
  - (void)test
  {
        printf("Test called on %p\n", self);
  }
  @end
  
  int main()
  {
        @autoreleasepool
        {
                X *x = class_createInstance(objc_getClass("X"), 0);
                [x test];
        }
  }

I don't believe this does the right thing (unfortunately, the latest XCode 
requires Catalina and it breaks too many things for me to upgrade from Mojave, 
so I can't test it and see if there are any tweaks to the runtime).  The API 
guarantee for `class_createInstance` does not require it to call `+initialize`. 
 I can see a few ways of fixing this:

Either:

- Unconditionally do the `[self self]` dance on all direct methods (not ideal, 
since it's likely to offset the performance win) for runtimes without knowledge 
of direct functions and one out of:
  - Set a metadata flag on any class with direct methods to require 
`class_createInstance` (and any other allocation paths that I haven't thought 
of) in the runtime to call `+initialize` on instantiation.
  - Modify the API contract of `class_createInstance` to send `+initialize` 
unconditionally.
  - Provide an `objc_msgSendDirect` that takes the target function in place of 
the selector and checks that the receiver is initialise, teach the optimisers 
to inline through this in cases where they can prove that the receiver is 
already initialised.
- Document that direct methods do not trigger `+initialize`, generalise that to 
class methods and remove the existing `+self` dance.

For direct class methods, avoiding the `+self` dance might best avoided by 
using a construct like the C++ static initialiser for a pointer to the class at 
the call site (rather than a direct reference to the class), which guarantees 
that it's initialised in the callee and skipping this when the receiver is 
`self` in an method (`+initialize` is guaranteed to have been called on `self` 
on entry to any method in this case).  The pseudocode is something like (where 
`+direct` is a direct method):

The commit message for this refers to `objc_opt_self`.  This is presumably a 
fast-path for a `self` message send, but the only reference to that anywhere in 
the LLVM tree is in LLDB.  We don't appear to have any tests in clang for it 
and neither upstream nor Apple clang appear to generate calls for it, so I'm 
not sure what 'an appropriate deployment target' is in the commit message.


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

https://reviews.llvm.org/D69991



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to