From a9179ad6fbb1e318e606c4b84e256029c00e79e9 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Fri, 8 Sep 2023 14:20:45 +1200 Subject: [PATCH 1/3] Fix #486 --- lib/src/code_generator/objc_interface.dart | 13 +++-- .../inherited_instancetype_config.yaml | 13 +++++ .../inherited_instancetype_test.dart | 57 +++++++++++++++++++ .../inherited_instancetype_test.m | 33 +++++++++++ 4 files changed, 112 insertions(+), 4 deletions(-) create mode 100644 test/native_objc_test/inherited_instancetype_config.yaml create mode 100644 test/native_objc_test/inherited_instancetype_test.dart create mode 100644 test/native_objc_test/inherited_instancetype_test.m diff --git a/lib/src/code_generator/objc_interface.dart b/lib/src/code_generator/objc_interface.dart index bf4d8872..782d75c9 100644 --- a/lib/src/code_generator/objc_interface.dart +++ b/lib/src/code_generator/objc_interface.dart @@ -263,7 +263,7 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} { if (superType != null) { superType!.addDependencies(dependencies); - _copyClassMethodsFromSuperType(); + _copyMethodsFromSuperType(); } for (final m in methods.values) { @@ -271,13 +271,18 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} { } } - void _copyClassMethodsFromSuperType() { - // Copy class methods from the super type, because Dart classes don't - // inherit static methods. + void _copyMethodsFromSuperType() { + // We need to copy certain methods from the super type: + // - Class methods, because Dart classes don't inherit static methods. + // - Methods that return instancetype, because the subclass's copy of the + // method needs to return the subclass, not the super class. + // Note: instancetype is only allowed as a return type, not an arg type. for (final m in superType!.methods.values) { if (m.isClass && !_excludedNSObjectClassMethods.contains(m.originalName)) { addMethod(m); + } else if (_isInstanceType(m.returnType)) { + addMethod(m); } } } diff --git a/test/native_objc_test/inherited_instancetype_config.yaml b/test/native_objc_test/inherited_instancetype_config.yaml new file mode 100644 index 00000000..92619792 --- /dev/null +++ b/test/native_objc_test/inherited_instancetype_config.yaml @@ -0,0 +1,13 @@ +name: InheritedInstancetypeTestObjCLibrary +description: 'Regression tests for https://github.com/dart-lang/ffigen/issues/486' +language: objc +output: 'inherited_instancetype_bindings.dart' +exclude-all-by-default: true +objc-interfaces: + include: + - ChildClass +headers: + entry-points: + - 'inherited_instancetype_test.m' +preamble: | + // ignore_for_file: camel_case_types, non_constant_identifier_names, unused_element, unused_field diff --git a/test/native_objc_test/inherited_instancetype_test.dart b/test/native_objc_test/inherited_instancetype_test.dart new file mode 100644 index 00000000..634b410c --- /dev/null +++ b/test/native_objc_test/inherited_instancetype_test.dart @@ -0,0 +1,57 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// Objective C support is only available on mac. +@TestOn('mac-os') + +// Regression tests for https://github.com/dart-lang/ffigen/issues/486 + +import 'dart:ffi'; +import 'dart:io'; + +import 'package:test/test.dart'; +import '../test_utils.dart'; +import 'inherited_instancetype_bindings.dart'; +import 'util.dart'; + +void main() { + late InheritedInstancetypeTestObjCLibrary lib; + + group('inheritedInstancetype', () { + setUpAll(() { + logWarnings(); + final dylib = File('test/native_objc_test/inherited_instancetype_test.dylib'); + verifySetupFile(dylib); + lib = InheritedInstancetypeTestObjCLibrary(DynamicLibrary.open(dylib.absolute.path)); + generateBindingsForCoverage('inherited_instancetype'); + }); + + test('Ordinary init method', () { + final ChildClass child = ChildClass.alloc(lib).init(); + expect(child.field, 123); + final ChildClass sameChild = child.getSelf(); + sameChild.field = 456; + expect(child.field, 456); + }); + + test('Custom create method', () { + final ChildClass child = ChildClass.create(lib); + expect(child.field, 123); + final ChildClass sameChild = child.getSelf(); + sameChild.field = 456; + expect(child.field, 456); + }); + + test('Polymorphism', () { + final ChildClass child = ChildClass.alloc(lib).init(); + final BaseClass base = child; + + // Calling base.getSelf() should still go through ChildClass.getSelf, so + // the result will have a compile time type of BaseClass, but a runtime + // type of ChildClass. + final BaseClass sameChild = base.getSelf(); + expect(sameChild, isA()); + }); + }); +} diff --git a/test/native_objc_test/inherited_instancetype_test.m b/test/native_objc_test/inherited_instancetype_test.m new file mode 100644 index 00000000..8f1b20b8 --- /dev/null +++ b/test/native_objc_test/inherited_instancetype_test.m @@ -0,0 +1,33 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +#import + +@interface BaseClass : NSObject {} ++ (instancetype)create; +- (instancetype)getSelf; +@end + +@interface ChildClass : BaseClass {} +@property int32_t field; +@end + +@implementation BaseClass ++ (instancetype)create { + return [[[self class] alloc] init]; +} + +- (instancetype)getSelf { + return self; +} +@end + +@implementation ChildClass +- (instancetype)init { + if (self = [super init]) { + self.field = 123; + } + return self; +} +@end From 8422f19141ece5111e9994cae6074f2e5ce57144 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Fri, 8 Sep 2023 14:35:57 +1200 Subject: [PATCH 2/3] changelog and format --- CHANGELOG.md | 1 + test/native_objc_test/inherited_instancetype_test.dart | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b90e6198..efc5af31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Fix invalid exceptional return value ObjCBlocks that return floats. - Fix return_of_invalid_type analysis error for ObjCBlocks. - Fix crash in ObjC methods and blocks that return structs by value. +- Fix ObjC methods returning instancetype having the wrong type in sublasses. - Bump min SDK version to 3.2.0-114.0.dev. # 9.0.1 diff --git a/test/native_objc_test/inherited_instancetype_test.dart b/test/native_objc_test/inherited_instancetype_test.dart index 634b410c..8b012acb 100644 --- a/test/native_objc_test/inherited_instancetype_test.dart +++ b/test/native_objc_test/inherited_instancetype_test.dart @@ -21,9 +21,11 @@ void main() { group('inheritedInstancetype', () { setUpAll(() { logWarnings(); - final dylib = File('test/native_objc_test/inherited_instancetype_test.dylib'); + final dylib = + File('test/native_objc_test/inherited_instancetype_test.dylib'); verifySetupFile(dylib); - lib = InheritedInstancetypeTestObjCLibrary(DynamicLibrary.open(dylib.absolute.path)); + lib = InheritedInstancetypeTestObjCLibrary( + DynamicLibrary.open(dylib.absolute.path)); generateBindingsForCoverage('inherited_instancetype'); }); From a486b04a4c9352e4a9d3898ff1f93c226be33b7b Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Mon, 11 Sep 2023 11:59:37 +1200 Subject: [PATCH 3/3] Fix nit --- test/native_objc_test/inherited_instancetype_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/native_objc_test/inherited_instancetype_test.dart b/test/native_objc_test/inherited_instancetype_test.dart index 8b012acb..93f93556 100644 --- a/test/native_objc_test/inherited_instancetype_test.dart +++ b/test/native_objc_test/inherited_instancetype_test.dart @@ -5,7 +5,7 @@ // Objective C support is only available on mac. @TestOn('mac-os') -// Regression tests for https://github.com/dart-lang/ffigen/issues/486 +// Regression tests for https://github.com/dart-lang/ffigen/issues/486. import 'dart:ffi'; import 'dart:io';