[ffigen] Protocol polish #2060
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,22 +7,42 @@ | |
| #import <Foundation/NSDictionary.h> | ||
| #import <Foundation/NSInvocation.h> | ||
| #import <Foundation/NSValue.h> | ||
| #import <objc/runtime.h> | ||
|
|
||
| #if !__has_feature(objc_arc) | ||
| #error "This file must be compiled with ARC enabled" | ||
| #endif | ||
|
|
||
| @interface DOBJCDartProtocolClass : NSObject | ||
|
There was a problem hiding this comment. Do you have a specific reason to separate this from the builder itself? Consider a simpler setup where I also tried to follow Google Objective-C Style Guide which we otherwise seemed to have ignored. It is good to have more comments and somewhat cleaner code. @class DOBJCDartProtocol;
/**
* Class responsible for creating Objective-C classes in runtime.
* See also `...` in Dart.
*/
@interface DOBJCDartProtocolBuilder : NSObject
/**
* Initialize the builder using the given @c debugName.
*
* @param debugName prefix to use when creating an Objective-C class,
* "DOBJCDartProtocol" will be used if @c debugName is @c nil
*/
- (instancetype)initWithDebugName: (NSString*)debugName;
/**
* Use @c block to implement selector @c sel.
*
* This method can only be called before the first instance is created
* via @c newInstanceWithDisposePort.
*
* @param sel Selector which will be implemented.
* @param block Block to redirect method to.
*/
- (void)implementMethod:(SEL) sel withBlock:(void*)block;
/**
* Create an instance of the class built by this builder.
*
* Finalizes and registers Objective-C class. No new methods can be implemented
* after the first instance is created.
*
* @param port Optional port to notify when newly created instance is
* deallocated. Can be @c ILLEGAL_PORT.
*/
- (DOBJCDartProtocol*)newInstanceWithDisposePort:(Dart_Port)port;
- (void) dealloc;
@end
@interface DOBJCDartProtocol : NSObject
- (instancetype)initFromBuilder: (DOBJCDartProtocolBuilder*)builder withDisposePort:(Dart_Port)port;
- (id)getBlockImplementingSelector:(SEL)sel;
- (void)dealloc;
@end
@implementation DOBJCDartProtocolBuilder {
BOOL _finalized;
Class _cls;
@public NSMutableDictionary *_methods;
}
static _Atomic int64_t gNextProtocolId = 0;
- (instancetype)initWithDebugName: (NSString *)debugName {
// Note: no need for if (self) if you don't do `self = [super init]`.
if (!debugName) {
debugName = @"DOBJCDartProtocol";
}
int64_t id = atomic_fetch_add(&gNextProtocolId, 1);
debugName = [NSString stringWithFormat:@"%@_%lld", debugName, id];
_cls = objc_allocateClassPair(objc_lookUpClass("DOBJCDartProtocol"), [debugName UTF8String], /*extra_bytes=*/0);
_methods = [NSMutableDictionary new];
_finalized = NO;
return self;
}
- (DOBJCDartProtocol*)newInstanceWithDisposePort:(Dart_Port)port {
if (!_finalized) {
objc_registerClassPair(_cls);
_finalized = YES;
}
DOBJCDartProtocol* obj = [_cls alloc];
return [obj initFromBuilder:self withDisposePort:port];
}
- (void) dealloc {
objc_disposeClassPair(_cls);
}
@end
@implementation DOBJCDartProtocol {
DOBJCDartProtocolBuilder* _builder;
Dart_Port _dispose_port;
}
- (instancetype)initFromBuilder: (DOBJCDartProtocolBuilder*)builder withDisposePort:(Dart_Port)port {
_builder = builder;
_dispose_port = port;
return self;
}
- (void)dealloc {
if (_dispose_port != ILLEGAL_PORT) {
Dart_PostInteger_DL(_dispose_port, 0);
}
}
- (id)getBlockImplementingSelector:(SEL)sel {
return [_builder->_methods objectForKey:[NSValue valueWithPointer:sel]];
}
@endThere was a problem hiding this comment. Done, more or less. In general, all else being equal, I prefer to orchestrate in Dart (because it's a better language ;) ). But yeah, creating the class and instantiating the implementation objects on the ObjC side simplifies things a little, and the My formatting is always a little weird because I haven't managed to get clang format to work on these files (or rather, it makes nonsensical formatting decisions, like ignoring like length).
There was a problem hiding this comment. Thanks! Choices look reasonable. I do also prefer orchestrating in Dart - however I usually apply the rule of "keeping the logic concentrated in one place". So if I am already writing native code and native code will look more natural - then I will shift most of the logic into native code and make Dart code a thin wrapper. This makes the code easier to understand - otherwise the logic gets split and you need to jump back and forth to piece it together.
Makes sense. |
||
| - (instancetype)initWithClass: (Class)cls; | ||
| @end | ||
|
|
||
| @implementation DOBJCDartProtocolClass { | ||
| Class clazz; | ||
| } | ||
|
|
||
| - (instancetype)initWithClass: (Class)cls { | ||
| if (self) { | ||
| clazz = cls; | ||
| } | ||
| return self; | ||
| } | ||
|
|
||
| - (void)dealloc { | ||
| objc_disposeClassPair(clazz); | ||
| } | ||
|
|
||
| @end | ||
|
|
||
| @implementation DOBJCDartProtocolBuilder { | ||
| NSMutableDictionary *methods; | ||
| DOBJCDartProtocolClass* clazz; | ||
| } | ||
|
|
||
| - (instancetype)initWithClass: (void*)cls { | ||
| if (self) { | ||
| methods = [NSMutableDictionary new]; | ||
| clazz = [[DOBJCDartProtocolClass alloc] initWithClass: (__bridge Class)cls]; | ||
| } | ||
| return self; | ||
| } | ||
|
|
@@ -37,13 +57,19 @@ - (void)implementMethod:(SEL)sel withBlock:(void*)block { | |
| [self implement:sel withBlock:(__bridge id)block]; | ||
|
There was a problem hiding this comment. Should |
||
| } | ||
|
|
||
| - (NSDictionary*)getMethods { | ||
| return methods; | ||
| } | ||
|
|
||
| - (DOBJCDartProtocolClass*)getClass { | ||
| return clazz; | ||
| } | ||
|
|
||
| @end | ||
|
|
||
| @implementation DOBJCDartProtocol { | ||
| NSDictionary *methods; | ||
| DOBJCDartProtocolClass* clazz; | ||
| Dart_Port dispose_port; | ||
| } | ||
|
|
||
|
|
@@ -55,7 +81,8 @@ - (instancetype)initDOBJCDartProtocolFromDartProtocolBuilder: | |
| (DOBJCDartProtocolBuilder*)builder | ||
| withDisposePort:(Dart_Port)port { | ||
| if (self) { | ||
| methods = [builder getMethods]; | ||
| clazz = [builder getClass]; | ||
| dispose_port = port; | ||
| } | ||
| return self; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: a more accurate name for
bypassCacheis probablyforceClassListReloador something like this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done