From b2595eb793f1f8ee358b7fb2d63ce51be211c287 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Fri, 8 Nov 2024 11:46:04 +1100 Subject: [PATCH 01/11] WIP bad overrides --- pkgs/ffigen/lib/src/code_generator/type.dart | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkgs/ffigen/lib/src/code_generator/type.dart b/pkgs/ffigen/lib/src/code_generator/type.dart index ae68cc61e5..19ff9b2982 100644 --- a/pkgs/ffigen/lib/src/code_generator/type.dart +++ b/pkgs/ffigen/lib/src/code_generator/type.dart @@ -33,6 +33,9 @@ abstract class Type extends AstNode { /// Returns true if the type is a [Compound] and is incomplete. bool get isIncompleteCompound => false; + /// Returns true if [other] is a subtype of this type. + bool isSubtype(Type other) => this == other; + /// Returns the C type of the Type. This is the FFI compatible type that is /// passed to native code. String getCType(Writer w) => @@ -152,6 +155,9 @@ abstract class BindingType extends NoLookUpBinding implements Type { @override bool get isIncompleteCompound => false; + @override + bool isSubtype(Type other) => this == other; + @override String getFfiDartType(Writer w) => getCType(w); From be2d3f2b3702f0b7853e57f10b5ca081295faccb Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Fri, 8 Nov 2024 11:47:04 +1100 Subject: [PATCH 02/11] WIP bad overrides --- .../native_objc_test/bad_override_config.yaml | 18 +++++++ .../native_objc_test/bad_override_test.dart | 37 +++++++++++++ .../test/native_objc_test/bad_override_test.m | 52 +++++++++++++++++++ 3 files changed, 107 insertions(+) create mode 100644 pkgs/ffigen/test/native_objc_test/bad_override_config.yaml create mode 100644 pkgs/ffigen/test/native_objc_test/bad_override_test.dart create mode 100644 pkgs/ffigen/test/native_objc_test/bad_override_test.m diff --git a/pkgs/ffigen/test/native_objc_test/bad_override_config.yaml b/pkgs/ffigen/test/native_objc_test/bad_override_config.yaml new file mode 100644 index 0000000000..c6a47cd2f4 --- /dev/null +++ b/pkgs/ffigen/test/native_objc_test/bad_override_config.yaml @@ -0,0 +1,18 @@ +name: BadOverrideTestObjCLibrary +description: 'Tests various overrides that are valid in ObjC but invalid in Dart' +language: objc +output: 'bad_override_bindings.dart' +exclude-all-by-default: true +objc-interfaces: + include: + - Polygon + - Triangle + - Rectangle + - Square + - BadOverrideBase + - BadOverrideChild +headers: + entry-points: + - 'bad_override_test.m' +preamble: | + // ignore_for_file: camel_case_types, non_constant_identifier_names, unnecessary_non_null_assertion, unused_element, unused_field diff --git a/pkgs/ffigen/test/native_objc_test/bad_override_test.dart b/pkgs/ffigen/test/native_objc_test/bad_override_test.dart new file mode 100644 index 0000000000..8e4eae398b --- /dev/null +++ b/pkgs/ffigen/test/native_objc_test/bad_override_test.dart @@ -0,0 +1,37 @@ +// Copyright (c) 2024, 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') + +import 'dart:ffi'; +import 'dart:io'; + +import 'package:ffi/ffi.dart'; +import 'package:test/test.dart'; +import '../test_utils.dart'; +import 'bad_override_bindings.dart'; +import 'util.dart'; + +void main() { + group('bad overrides', () { + setUpAll(() { + // TODO(https://github.com/dart-lang/native/issues/1068): Remove this. + DynamicLibrary.open('../objective_c/test/objective_c.dylib'); + final dylib = File('test/native_objc_test/objc_test.dylib'); + verifySetupFile(dylib); + DynamicLibrary.open(dylib.absolute.path); + generateBindingsForCoverage('bad_override'); + }); + + test('Contravariant returns', () { + // Return types are supposed to be covariant, but ObjC allows them to be + // contravariant. + // https://github.com/dart-lang/native/issues/1220 + }); + + test('Subtyped args', () { + }); + }); +} diff --git a/pkgs/ffigen/test/native_objc_test/bad_override_test.m b/pkgs/ffigen/test/native_objc_test/bad_override_test.m new file mode 100644 index 0000000000..f44cc6ace9 --- /dev/null +++ b/pkgs/ffigen/test/native_objc_test/bad_override_test.m @@ -0,0 +1,52 @@ +// Copyright (c) 2024, 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 Polygon : NSObject {} +-(NSString*)name; +@end +@implementation Polygon +-(NSString*)name { return @"Polygon"; } +@end + +@interface Triangle : Polygon {} +-(NSString*)name; +@end +@implementation Triangle +-(NSString*)name { return @"Triangle"; } +@end + +@interface Rectangle : Polygon {} +-(NSString*)name; +@end +@implementation Rectangle +-(NSString*)name { return @"Rectangle"; } +@end + +@interface Square : Rectangle {} +-(NSString*)name; +@end +@implementation Square +-(NSString*)name { return @"Square"; } +@end + + + +@interface BadOverrideBase : NSObject {} +-(Square*)contravariantReturn; +@end + +@implementation BadOverrideBase +-(Square*)contravariantReturn { return [Square new]; } +@end + +@interface BadOverrideChild : BadOverrideBase {} +-(Rectangle*)contravariantReturn; +@end + +@implementation BadOverrideChild +-(Rectangle*)contravariantReturn { return [Rectangle new]; } +@end From e898302f55a4380b3af6d309e6c523ef02ed3032 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Mon, 11 Nov 2024 14:31:12 +1100 Subject: [PATCH 03/11] implement subtype test --- .../lib/src/code_generator/func_type.dart | 21 ++++++++++++ .../lib/src/code_generator/objc_block.dart | 14 ++++++++ .../src/code_generator/objc_interface.dart | 11 ++++++ .../lib/src/code_generator/objc_nullable.dart | 13 +++++++ .../lib/src/code_generator/pointer.dart | 9 +++++ pkgs/ffigen/lib/src/code_generator/type.dart | 34 +++++++++++++++++-- .../lib/src/code_generator/typealias.dart | 3 ++ .../test/native_objc_test/bad_override_test.m | 5 ++- 8 files changed, 106 insertions(+), 4 deletions(-) diff --git a/pkgs/ffigen/lib/src/code_generator/func_type.dart b/pkgs/ffigen/lib/src/code_generator/func_type.dart index d3a3c2bd0f..7bed4cf283 100644 --- a/pkgs/ffigen/lib/src/code_generator/func_type.dart +++ b/pkgs/ffigen/lib/src/code_generator/func_type.dart @@ -119,6 +119,20 @@ class FunctionType extends Type { visitor.visitAll(parameters); visitor.visitAll(varArgParameters); } + + @override + bool isSupertypeOf(Type other) { + other = other.typealiasType; + if (other is FunctionType) { + return isSupertypeOfVariance( + coLeft: [returnType], + coRight: [other.returnType], + contraLeft: dartTypeParameters.map((p) => p.type).toList(), + contraRight: other.dartTypeParameters.map((p) => p.type).toList(), + ); + } + return false; + } } /// Represents a NativeFunction. @@ -165,4 +179,11 @@ class NativeFunc extends Type { super.visitChildren(visitor); visitor.visit(_type); } + + @override + bool isSupertypeOf(Type other) { + other = other.typealiasType; + if (other is NativeFunc) return type.isSupertypeOf(other.type); + return false; + } } diff --git a/pkgs/ffigen/lib/src/code_generator/objc_block.dart b/pkgs/ffigen/lib/src/code_generator/objc_block.dart index 540584ebcd..8177b47112 100644 --- a/pkgs/ffigen/lib/src/code_generator/objc_block.dart +++ b/pkgs/ffigen/lib/src/code_generator/objc_block.dart @@ -389,4 +389,18 @@ $blockName $fnName($blockName block) NS_RETURNS_RETAINED { visitor.visitAll(params); visitor.visit(_wrapListenerBlock); } + + @override + bool isSupertypeOf(Type other) { + other = other.typealiasType; + if (other is ObjCBlock) { + return isSupertypeOfVariance( + coLeft: [returnType], + coRight: [other.returnType], + contraLeft: params.map((p) => p.type).toList(), + contraRight: other.params.map((p) => p.type).toList(), + ); + } + return false; + } } diff --git a/pkgs/ffigen/lib/src/code_generator/objc_interface.dart b/pkgs/ffigen/lib/src/code_generator/objc_interface.dart index ee85890a4c..6d9e03ed2f 100644 --- a/pkgs/ffigen/lib/src/code_generator/objc_interface.dart +++ b/pkgs/ffigen/lib/src/code_generator/objc_interface.dart @@ -200,4 +200,15 @@ ${generateAsStub ? '' : _generateMethods(w)} // inclusion. Including an interface shouldn't auto-include all its // subtypes, even as stubs. } + + @override + bool isSupertypeOf(Type other) { + other = other.typealiasType; + if (other is ObjCInterface) { + for (ObjCInterface? t = other; t != null; t = t.superType) { + if (t == this) return true; + } + } + return false; + } } diff --git a/pkgs/ffigen/lib/src/code_generator/objc_nullable.dart b/pkgs/ffigen/lib/src/code_generator/objc_nullable.dart index 4d508fa564..e8bbb5309b 100644 --- a/pkgs/ffigen/lib/src/code_generator/objc_nullable.dart +++ b/pkgs/ffigen/lib/src/code_generator/objc_nullable.dart @@ -98,4 +98,17 @@ class ObjCNullable extends Type { super.visitChildren(visitor); visitor.visit(child); } + + @override + bool isSupertypeOf(Type other) { + other = other.typealiasType; + + if (other is ObjCNullable) { + // T? :> S? if T :> S + return child.isSupertypeOf(other.child); + } else { + // T? :> S if T :> S + return child.isSupertypeOf(other); + } + } } diff --git a/pkgs/ffigen/lib/src/code_generator/pointer.dart b/pkgs/ffigen/lib/src/code_generator/pointer.dart index 320f313650..b4a21c01e9 100644 --- a/pkgs/ffigen/lib/src/code_generator/pointer.dart +++ b/pkgs/ffigen/lib/src/code_generator/pointer.dart @@ -51,6 +51,15 @@ class PointerType extends Type { @override void visit(Visitation visitation) => visitation.visitPointerType(this); + + @override + bool isSupertypeOf(Type other) { + other = other.typealiasType; + if (other is PointerType) { + return child.isSupertypeOf(other.child); + } + return false; + } } /// Represents a constant array, which has a fixed size. diff --git a/pkgs/ffigen/lib/src/code_generator/type.dart b/pkgs/ffigen/lib/src/code_generator/type.dart index 19ff9b2982..27552b5cfc 100644 --- a/pkgs/ffigen/lib/src/code_generator/type.dart +++ b/pkgs/ffigen/lib/src/code_generator/type.dart @@ -33,8 +33,13 @@ abstract class Type extends AstNode { /// Returns true if the type is a [Compound] and is incomplete. bool get isIncompleteCompound => false; - /// Returns true if [other] is a subtype of this type. - bool isSubtype(Type other) => this == other; + /// Returns true if [this] is a subtype of [other]. That is this <: other. + /// + /// Note: Implementers should implement [isSupertypeOf]. + bool isSubtypeOf(Type other) => other.isSupertypeOf(this); + + /// Returns true if [this] is a supertype of [other]. That is this :> other. + bool isSupertypeOf(Type other) => this == other; /// Returns the C type of the Type. This is the FFI compatible type that is /// passed to native code. @@ -127,6 +132,26 @@ abstract class Type extends AstNode { @override void visit(Visitation visitation) => visitation.visitType(this); + + // Helper for [isSupertypeOf] that applies variance rules. + static bool isSupertypeOfVariance({ + List covariantLeft = const [], + List covariantRight = const [], + List contravariantLeft = const [], + List contravariantRight = const [], + }) => isSupertypeOfCovariance(left: covariantLeft, right: covariantRight) && + isSupertypeOfCovariance(left: contravariantRight, right: contravariantLeft); + + static bool isSupertypeOfCovariance({ + required List left, + required List right, + }) { + if (left.length != right.length) return false; + for (var i = 0; i < params.length; ++i) { + if (!left[i].isSupertypeOf(right[i])) return false; + } + return true; + } } /// Base class for all Type bindings. @@ -156,7 +181,10 @@ abstract class BindingType extends NoLookUpBinding implements Type { bool get isIncompleteCompound => false; @override - bool isSubtype(Type other) => this == other; + bool isSubtypeOf(Type other) => other.isSupertypeOf(this); + + @override + bool isSupertypeOf(Type other) => this == other; @override String getFfiDartType(Writer w) => getCType(w); diff --git a/pkgs/ffigen/lib/src/code_generator/typealias.dart b/pkgs/ffigen/lib/src/code_generator/typealias.dart index 6dee9875dc..739e00d47f 100644 --- a/pkgs/ffigen/lib/src/code_generator/typealias.dart +++ b/pkgs/ffigen/lib/src/code_generator/typealias.dart @@ -209,6 +209,9 @@ class Typealias extends BindingType { @override void visit(Visitation visitation) => visitation.visitTypealias(this); + + @override + bool isSupertypeOf(Type other) => type.isSupertypeOf(other); } /// Objective C's instancetype. diff --git a/pkgs/ffigen/test/native_objc_test/bad_override_test.m b/pkgs/ffigen/test/native_objc_test/bad_override_test.m index 8d6a30d72f..5a20239303 100644 --- a/pkgs/ffigen/test/native_objc_test/bad_override_test.m +++ b/pkgs/ffigen/test/native_objc_test/bad_override_test.m @@ -40,10 +40,11 @@ @implementation BadOverrideGrandparent @end @interface BadOverrideParent : BadOverrideGrandparent {} +-(Rectangle*)contravariantReturn; -(int32_t)methodVsGetter; -@property (readonly) int32_t methodVsGetter; @end @implementation BadOverrideParent +-(Rectangle*)contravariantReturn { return [Rectangle new]; } -(int32_t)methodVsGetter { return 1; } @end @@ -60,9 +61,11 @@ @implementation BadOverrideAunt @end @interface BadOverrideChild : BadOverrideParent {} +-(Polygon*)contravariantReturn; @property (readonly) int32_t methodVsGetter; @end @implementation BadOverrideChild +-(Polygon*)contravariantReturn { return [Triangle new]; } -(int32_t)methodVsGetter { return 11; } @end From 6b8b9597a1ed40691f2f9c444a8ea68abbfea1a0 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 12 Nov 2024 11:42:17 +1100 Subject: [PATCH 04/11] Fix and test subtyping --- .../lib/src/code_generator/func_type.dart | 11 +- .../lib/src/code_generator/objc_block.dart | 10 +- pkgs/ffigen/lib/src/code_generator/type.dart | 24 +- .../test/unit_tests/subtyping_test.dart | 243 ++++++++++++++++++ 4 files changed, 269 insertions(+), 19 deletions(-) create mode 100644 pkgs/ffigen/test/unit_tests/subtyping_test.dart diff --git a/pkgs/ffigen/lib/src/code_generator/func_type.dart b/pkgs/ffigen/lib/src/code_generator/func_type.dart index 7bed4cf283..f0c6d9b9e6 100644 --- a/pkgs/ffigen/lib/src/code_generator/func_type.dart +++ b/pkgs/ffigen/lib/src/code_generator/func_type.dart @@ -124,11 +124,12 @@ class FunctionType extends Type { bool isSupertypeOf(Type other) { other = other.typealiasType; if (other is FunctionType) { - return isSupertypeOfVariance( - coLeft: [returnType], - coRight: [other.returnType], - contraLeft: dartTypeParameters.map((p) => p.type).toList(), - contraRight: other.dartTypeParameters.map((p) => p.type).toList(), + return Type.isSupertypeOfVariance( + covariantLeft: [returnType], + covariantRight: [other.returnType], + contravariantLeft: dartTypeParameters.map((p) => p.type).toList(), + contravariantRight: + other.dartTypeParameters.map((p) => p.type).toList(), ); } return false; diff --git a/pkgs/ffigen/lib/src/code_generator/objc_block.dart b/pkgs/ffigen/lib/src/code_generator/objc_block.dart index 8177b47112..f466c623d4 100644 --- a/pkgs/ffigen/lib/src/code_generator/objc_block.dart +++ b/pkgs/ffigen/lib/src/code_generator/objc_block.dart @@ -394,11 +394,11 @@ $blockName $fnName($blockName block) NS_RETURNS_RETAINED { bool isSupertypeOf(Type other) { other = other.typealiasType; if (other is ObjCBlock) { - return isSupertypeOfVariance( - coLeft: [returnType], - coRight: [other.returnType], - contraLeft: params.map((p) => p.type).toList(), - contraRight: other.params.map((p) => p.type).toList(), + return Type.isSupertypeOfVariance( + covariantLeft: [returnType], + covariantRight: [other.returnType], + contravariantLeft: params.map((p) => p.type).toList(), + contravariantRight: other.params.map((p) => p.type).toList(), ); } return false; diff --git a/pkgs/ffigen/lib/src/code_generator/type.dart b/pkgs/ffigen/lib/src/code_generator/type.dart index 27552b5cfc..b60aed0cba 100644 --- a/pkgs/ffigen/lib/src/code_generator/type.dart +++ b/pkgs/ffigen/lib/src/code_generator/type.dart @@ -35,6 +35,10 @@ abstract class Type extends AstNode { /// Returns true if [this] is a subtype of [other]. That is this <: other. /// + /// The behavior of this function should mirror Dart's subtyping logic, not + /// Objective-C's. It's used to detect and fix cases where the generated + /// bindings would fail `dart analyze` due to Dart's subtyping rules. + /// /// Note: Implementers should implement [isSupertypeOf]. bool isSubtypeOf(Type other) => other.isSupertypeOf(this); @@ -135,19 +139,21 @@ abstract class Type extends AstNode { // Helper for [isSupertypeOf] that applies variance rules. static bool isSupertypeOfVariance({ - List covariantLeft = const [], - List covariantRight = const [], - List contravariantLeft = const [], - List contravariantRight = const [], - }) => isSupertypeOfCovariance(left: covariantLeft, right: covariantRight) && - isSupertypeOfCovariance(left: contravariantRight, right: contravariantLeft); + List covariantLeft = const [], + List covariantRight = const [], + List contravariantLeft = const [], + List contravariantRight = const [], + }) => + isSupertypeOfCovariance(left: covariantLeft, right: covariantRight) && + isSupertypeOfCovariance( + left: contravariantRight, right: contravariantLeft); static bool isSupertypeOfCovariance({ - required List left, - required List right, + required List left, + required List right, }) { if (left.length != right.length) return false; - for (var i = 0; i < params.length; ++i) { + for (var i = 0; i < left.length; ++i) { if (!left[i].isSupertypeOf(right[i])) return false; } return true; diff --git a/pkgs/ffigen/test/unit_tests/subtyping_test.dart b/pkgs/ffigen/test/unit_tests/subtyping_test.dart new file mode 100644 index 0000000000..e4aa46176e --- /dev/null +++ b/pkgs/ffigen/test/unit_tests/subtyping_test.dart @@ -0,0 +1,243 @@ +// Copyright (c) 2024, 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 'dart:io'; + +import 'package:ffigen/src/code_generator.dart'; +import 'package:test/test.dart'; + +void main() { + group('subtyping', () { + final builtInFunctions = ObjCBuiltInFunctions('', false); + + ObjCInterface makeInterface(String name, ObjCInterface? superType, + [List methods = const []]) { + final itf = ObjCInterface( + usr: name, + originalName: name, + builtInFunctions: builtInFunctions, + ); + if (superType != null) { + itf.superType = superType; + superType.subtypes.add(itf); + } + for (final m in methods) { + itf.addMethod(m); + } + itf.filled = true; + return itf; + } + + final grandparent = makeInterface('Grandparent', null); + final parent = makeInterface('Parent', grandparent); + final uncle = makeInterface('Uncle', grandparent); + final child = makeInterface('Child', parent); + + group('ObjCInterface', () { + test('subtype', () { + expect(parent.isSubtypeOf(parent), isTrue); + expect(child.isSubtypeOf(child), isTrue); + + expect(parent.isSubtypeOf(grandparent), isTrue); + expect(grandparent.isSubtypeOf(parent), isFalse); + + expect(child.isSubtypeOf(grandparent), isTrue); + expect(grandparent.isSubtypeOf(child), isFalse); + + expect(child.isSubtypeOf(uncle), isFalse); + expect(uncle.isSubtypeOf(child), isFalse); + }); + + test('supertype', () { + expect(parent.isSupertypeOf(parent), isTrue); + expect(child.isSupertypeOf(child), isTrue); + + expect(parent.isSupertypeOf(grandparent), isFalse); + expect(grandparent.isSupertypeOf(parent), isTrue); + + expect(child.isSupertypeOf(grandparent), isFalse); + expect(grandparent.isSupertypeOf(child), isTrue); + + expect(child.isSupertypeOf(uncle), isFalse); + expect(uncle.isSupertypeOf(child), isFalse); + }); + }); + + group('FunctionType', () { + FunctionType makeFunc(Type returnType, List argTypes) => + FunctionType(returnType: returnType, parameters: [ + for (final t in argTypes) Parameter(type: t, objCConsumed: false), + ]); + + test('covariant returns', () { + // Return types are covariant. S Function() <: T Function() if S <: T. + final returnsParent = makeFunc(parent, []); + final returnsChild = makeFunc(child, []); + + expect(returnsParent.isSubtypeOf(returnsParent), isTrue); + expect(returnsChild.isSubtypeOf(returnsChild), isTrue); + expect(returnsChild.isSubtypeOf(returnsParent), isTrue); + expect(returnsParent.isSubtypeOf(returnsChild), isFalse); + }); + + test('contravariant args', () { + // Arg types are contravariant. Function(S) <: Function(T) if T <: S. + final acceptsParent = makeFunc(voidType, [parent]); + final acceptsChild = makeFunc(voidType, [child]); + + expect(acceptsParent.isSubtypeOf(acceptsParent), isTrue); + expect(acceptsChild.isSubtypeOf(acceptsChild), isTrue); + expect(acceptsChild.isSubtypeOf(acceptsParent), isFalse); + expect(acceptsParent.isSubtypeOf(acceptsChild), isTrue); + }); + + test('multiple args', () { + expect( + makeFunc(voidType, [parent, parent]) + .isSubtypeOf(makeFunc(voidType, [parent, parent])), + isTrue); + expect( + makeFunc(voidType, [parent, parent]) + .isSubtypeOf(makeFunc(voidType, [child, child])), + isTrue); + expect( + makeFunc(voidType, [child, child]) + .isSubtypeOf(makeFunc(voidType, [parent, parent])), + isFalse); + expect( + makeFunc(voidType, [child, parent]) + .isSubtypeOf(makeFunc(voidType, [parent, child])), + isFalse); + expect( + makeFunc(voidType, [parent, parent, parent]) + .isSubtypeOf(makeFunc(voidType, [child, child])), + isFalse); + expect( + makeFunc(voidType, [parent]) + .isSubtypeOf(makeFunc(voidType, [child, child])), + isFalse); + }); + + test('args and returns', () { + expect(makeFunc(child, [parent]).isSubtypeOf(makeFunc(parent, [child])), + isTrue); + expect(makeFunc(parent, [parent]).isSubtypeOf(makeFunc(child, [child])), + isFalse); + expect(makeFunc(child, [child]).isSubtypeOf(makeFunc(parent, [parent])), + isFalse); + expect(makeFunc(parent, [child]).isSubtypeOf(makeFunc(child, [parent])), + isFalse); + }); + + test('NativeFunc', () { + final returnsParent = NativeFunc(makeFunc(parent, [])); + final returnsChild = NativeFunc(makeFunc(child, [])); + + expect(returnsParent.isSubtypeOf(returnsParent), isTrue); + expect(returnsChild.isSubtypeOf(returnsChild), isTrue); + expect(returnsChild.isSubtypeOf(returnsParent), isTrue); + expect(returnsParent.isSubtypeOf(returnsChild), isFalse); + }); + }); + + group('ObjCBlock', () { + ObjCBlock makeBlock(Type returnType, List argTypes) => ObjCBlock( + returnType: returnType, + params: [ + for (final t in argTypes) Parameter(type: t, objCConsumed: false), + ], + returnsRetained: false, + builtInFunctions: builtInFunctions); + + test('covariant returns', () { + // Return types are covariant. S Function() <: T Function() if S <: T. + final returnsParent = makeBlock(parent, []); + final returnsChild = makeBlock(child, []); + + expect(returnsParent.isSubtypeOf(returnsParent), isTrue); + expect(returnsChild.isSubtypeOf(returnsChild), isTrue); + expect(returnsChild.isSubtypeOf(returnsParent), isTrue); + expect(returnsParent.isSubtypeOf(returnsChild), isFalse); + }); + + test('contravariant args', () { + // Arg types are contravariant. Function(S) <: Function(T) if T <: S. + final acceptsParent = makeBlock(voidType, [parent]); + final acceptsChild = makeBlock(voidType, [child]); + + expect(acceptsParent.isSubtypeOf(acceptsParent), isTrue); + expect(acceptsChild.isSubtypeOf(acceptsChild), isTrue); + expect(acceptsChild.isSubtypeOf(acceptsParent), isFalse); + expect(acceptsParent.isSubtypeOf(acceptsChild), isTrue); + }); + + test('multiple args', () { + expect( + makeBlock(voidType, [parent, parent]) + .isSubtypeOf(makeBlock(voidType, [parent, parent])), + isTrue); + expect( + makeBlock(voidType, [parent, parent]) + .isSubtypeOf(makeBlock(voidType, [child, child])), + isTrue); + expect( + makeBlock(voidType, [child, child]) + .isSubtypeOf(makeBlock(voidType, [parent, parent])), + isFalse); + expect( + makeBlock(voidType, [child, parent]) + .isSubtypeOf(makeBlock(voidType, [parent, child])), + isFalse); + expect( + makeBlock(voidType, [parent, parent, parent]) + .isSubtypeOf(makeBlock(voidType, [child, child])), + isFalse); + expect( + makeBlock(voidType, [parent]) + .isSubtypeOf(makeBlock(voidType, [child, child])), + isFalse); + }); + + test('args and returns', () { + expect( + makeBlock(child, [parent]).isSubtypeOf(makeBlock(parent, [child])), + isTrue); + expect( + makeBlock(parent, [parent]).isSubtypeOf(makeBlock(child, [child])), + isFalse); + expect( + makeBlock(child, [child]).isSubtypeOf(makeBlock(parent, [parent])), + isFalse); + expect( + makeBlock(parent, [child]).isSubtypeOf(makeBlock(child, [parent])), + isFalse); + }); + }); + + test('ObjCNullable', () { + expect(ObjCNullable(parent).isSubtypeOf(ObjCNullable(parent)), isTrue); + expect(ObjCNullable(child).isSubtypeOf(ObjCNullable(parent)), isTrue); + expect(ObjCNullable(parent).isSubtypeOf(ObjCNullable(child)), isFalse); + expect(parent.isSubtypeOf(ObjCNullable(parent)), isTrue); + expect(ObjCNullable(parent).isSubtypeOf(parent), isFalse); + expect(child.isSubtypeOf(ObjCNullable(parent)), isTrue); + expect(ObjCNullable(child).isSubtypeOf(parent), isFalse); + expect(ObjCNullable(parent).isSubtypeOf(child), isFalse); + expect(parent.isSubtypeOf(ObjCNullable(child)), isFalse); + }); + + test('Typealias', () { + Typealias makeTypealias(Type t) => Typealias(name: '', type: t); + expect(makeTypealias(parent).isSubtypeOf(makeTypealias(parent)), isTrue); + expect(makeTypealias(child).isSubtypeOf(makeTypealias(parent)), isTrue); + expect(makeTypealias(parent).isSubtypeOf(makeTypealias(child)), isFalse); + expect(parent.isSubtypeOf(makeTypealias(parent)), isTrue); + expect(makeTypealias(parent).isSubtypeOf(parent), isTrue); + expect(child.isSubtypeOf(makeTypealias(parent)), isTrue); + expect(makeTypealias(child).isSubtypeOf(parent), isTrue); + expect(makeTypealias(parent).isSubtypeOf(child), isFalse); + expect(parent.isSubtypeOf(makeTypealias(child)), isFalse); + }); + }); +} From 6d08ce2a0522d4244c15e5727235a309cbe45b01 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 12 Nov 2024 14:29:19 +1100 Subject: [PATCH 05/11] Fix and test the method variance issues --- pkgs/ffigen/lib/src/code_generator/func.dart | 1 + .../lib/src/code_generator/objc_methods.dart | 4 +- .../src/visitor/fix_overridden_methods.dart | 155 +++++++++++++++--- .../native_objc_test/bad_override_test.dart | 15 ++ .../test/native_objc_test/bad_override_test.m | 19 ++- 5 files changed, 165 insertions(+), 29 deletions(-) diff --git a/pkgs/ffigen/lib/src/code_generator/func.dart b/pkgs/ffigen/lib/src/code_generator/func.dart index 479242c2b6..eeedbd136e 100644 --- a/pkgs/ffigen/lib/src/code_generator/func.dart +++ b/pkgs/ffigen/lib/src/code_generator/func.dart @@ -234,6 +234,7 @@ class Parameter extends AstNode { String name; Type type; final bool objCConsumed; + bool isCovariant = false; Parameter({ String? originalName, diff --git a/pkgs/ffigen/lib/src/code_generator/objc_methods.dart b/pkgs/ffigen/lib/src/code_generator/objc_methods.dart index c8ad658eef..59054dce54 100644 --- a/pkgs/ffigen/lib/src/code_generator/objc_methods.dart +++ b/pkgs/ffigen/lib/src/code_generator/objc_methods.dart @@ -333,7 +333,9 @@ class ObjCMethod extends AstNode { final targetType = target.getDartType(w); final returnTypeStr = _getConvertedReturnType(w, targetType); final paramStr = [ - for (final p in params) '${p.type.getDartType(w)} ${p.name}', + for (final p in params) + '${p.isCovariant ? 'covariant ' : ''}' + '${p.type.getDartType(w)} ${p.name}', ].join(', '); // The method declaration. diff --git a/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart b/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart index aa6e52a3b5..097c8c374d 100644 --- a/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart +++ b/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart @@ -2,16 +2,27 @@ // 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 'package:logging/logging.dart'; + import '../code_generator.dart'; import 'ast.dart'; +final _logger = Logger('ffigen.visitor.FixOverriddenMethodsVisitation'); + class FixOverriddenMethodsVisitation extends Visitation { @override void visitObjCInterface(ObjCInterface node) { - node.visitChildren(visitor); - _fixNullability(node); + // Visit the supertype, then perform all AST mutations, then visit the other + // children. That way we can be sure that the supertype's AST mutations have + // been completed before this node's mutations (this is important for + // _fixContravariantReturns). + visitor.visit(node.superType); + + _fixMethodVariance(node); _fixMethodsVsProperties(node); + + node.visitChildren(visitor); } void _fixNullability(ObjCInterface node) { @@ -20,28 +31,121 @@ class FixOverriddenMethodsVisitation extends Visitation { // it's return type from non-null to nullable, or its arg type from nullable // to non-null. So in these cases we have to make the non-null type // nullable, to avoid Dart compile errors. - for (var t = node.superType; t != null; t = t.superType) { - for (final method in node.methods) { - final superMethod = t.getSimilarMethod(method); - if (superMethod != null && - !superMethod.isClassMethod && - !method.isClassMethod) { - if (superMethod.returnType.typealiasType is! ObjCNullable && - method.returnType.typealiasType is ObjCNullable) { - superMethod.returnType = ObjCNullable(superMethod.returnType); - } - final numArgs = method.params.length < superMethod.params.length - ? method.params.length - : superMethod.params.length; - for (var i = 0; i < numArgs; ++i) { - final param = method.params[i]; - final superParam = superMethod.params[i]; - if (superParam.type.typealiasType is ObjCNullable && - param.type.typealiasType is! ObjCNullable) { - param.type = ObjCNullable(param.type); - } - } - } + // for (var t = node.superType; t != null; t = t.superType) { + // for (final method in node.methods) { + // final superMethod = t.getSimilarMethod(method); + // if (superMethod != null && + // !superMethod.isClassMethod && + // !method.isClassMethod) { + // if (superMethod.returnType.typealiasType is! ObjCNullable && + // method.returnType.typealiasType is ObjCNullable) { + // superMethod.returnType = ObjCNullable(superMethod.returnType); + // } + // final numArgs = method.params.length < superMethod.params.length + // ? method.params.length + // : superMethod.params.length; + // for (var i = 0; i < numArgs; ++i) { + // final param = method.params[i]; + // final superParam = superMethod.params[i]; + // if (superParam.type.typealiasType is ObjCNullable && + // param.type.typealiasType is! ObjCNullable) { + // param.type = ObjCNullable(param.type); + // } + // } + // } + // } + // } + } + + (ObjCInterface?, ObjCMethod?) _findNearestWithMethod( + ObjCInterface node, ObjCMethod method) { + for (ObjCInterface? t = node.superType; t != null; t = t.superType) { + final tMethod = t.getSimilarMethod(method); + if (tMethod != null) { + return (t, tMethod); + } + } + return (null, null); + } + + void _fixContravariantReturns(ObjCInterface node, ObjCMethod method, + ObjCInterface superType, ObjCMethod superMethod) { + // In Dart, method return types are covariant, but ObjC allows them to be + // contravariant. So fix these cases by changing the supertype's return type + // to match the subtype's return type. + + if (method.returnType.isSubtypeOf(superMethod.returnType)) { + // Covariant return, nothing to fix. + return; + } + + if (!superMethod.returnType.isSubtypeOf(method.returnType)) { + // Types are unrelated, so this can't be sensibly fixed. + _logger.severe( + '${node.originalName} is a subtype of ${superType.originalName} but ' + 'the return types of their ${method.originalName} methods are ' + 'unrelated'); + return; + } + + superMethod.returnType = method.returnType; + _logger.info('Changed the return type of ' + '${superType.originalName}.${superMethod.originalName} to ' + '${method.returnType} to match ${node.originalName}'); + + final (superSuperType, superSuperMethod) = + _findNearestWithMethod(superType, superMethod); + if (superSuperType != null && superSuperMethod != null) { + _fixContravariantReturns(node, method, superSuperType, superSuperMethod); + } + } + + void _fixCoavariantArgs(ObjCInterface node, ObjCMethod method, + ObjCInterface superType, ObjCMethod superMethod) { + // In Dart, method arg types are contavariant, but ObjC allows them to be + // covariant. So fix these cases by adding the `covariant` keyword to the + // parameter. + final n = method.params.length; + if (n != superMethod.params.length) { + _logger.severe( + '${node.originalName} is a subtype of ${superType.originalName} but ' + 'their ${method.originalName} methods have a different number of ' + 'parameters'); + return; + } + + for (var i = 0; i < n; ++i) { + final pt = method.params[i].type; + final st = superMethod.params[i].type; + + if (st.isSubtypeOf(pt)) { + // Contravariant param, nothing to fix. + continue; + } + + if (!pt.isSubtypeOf(st)) { + // Types are unrelated, so this can't be sensibly fixed. + _logger.severe( + '${node.originalName} is a subtype of ${superType.originalName} ' + 'but their ${method.originalName} methods have a parameter at ' + 'position ${i + 1} with an unrelated type'); + return; + } + + _logger.info('Set the parameter of ' + '${node.originalName}.${method.originalName} at position ${i + 1} to ' + 'be covariant'); + method.params[i].isCovariant = true; + } + } + + void _fixMethodVariance(ObjCInterface node) { + for (final method in node.methods) { + if (method.isClassMethod) continue; + final (superType, superMethod) = _findNearestWithMethod(node, method); + if (superType != null && superMethod != null) { + _fixContravariantReturns(node, method, superType, superMethod); + _fixCoavariantArgs(node, method, superType, superMethod); } } } @@ -54,6 +158,7 @@ class FixOverriddenMethodsVisitation extends Visitation { // up to find the root of the subtree that has this method, then we walk // down the subtree to change all conflicting methods to properties. for (final method in node.methods) { + if (method.isClassMethod) continue; final (root, rootMethod) = _findRootWithMethod(node, method); if (method.isProperty == rootMethod.isProperty) continue; _convertAllSubtreeMethodsToProperties(root, rootMethod); @@ -79,6 +184,8 @@ class FixOverriddenMethodsVisitation extends Visitation { final method = node.getSimilarMethod(rootMethod); if (method != null && method.kind == ObjCMethodKind.method) { method.kind = ObjCMethodKind.propertyGetter; + _logger.info( + 'Converted ${node.originalName}.${method.originalName} to a getter'); } for (final t in node.subtypes) { _convertAllSubtreeMethodsToProperties(t, rootMethod); diff --git a/pkgs/ffigen/test/native_objc_test/bad_override_test.dart b/pkgs/ffigen/test/native_objc_test/bad_override_test.dart index 3e1693198b..9be1a9d4a8 100644 --- a/pkgs/ffigen/test/native_objc_test/bad_override_test.dart +++ b/pkgs/ffigen/test/native_objc_test/bad_override_test.dart @@ -52,12 +52,27 @@ void main() { // Return types are supposed to be covariant, but ObjC allows them to be // contravariant. // https://github.com/dart-lang/native/issues/1220 + Polygon parentResult = BadOverrideParent.new1().contravariantReturn(); + expect(parentResult.name().toString(), 'Rectangle'); + + Polygon childResult = BadOverrideChild.new1().contravariantReturn(); + expect(childResult.name().toString(), 'Triangle'); }); test('Covariant args', () { // Arg types are supposed to be contravariant, but ObjC allows them to be // covariant. // https://github.com/dart-lang/native/issues/1220 + final square = Square.new1(); + final triangle = Triangle.new1(); + + var parent = BadOverrideParent.new1(); + expect(parent.covariantArg_(square).toString(), 'Polygon: Square'); + expect(parent.covariantArg_(triangle).toString(), 'Polygon: Triangle'); + + parent = BadOverrideChild.new1(); + expect(parent.covariantArg_(square).toString(), 'Rectangle: Square'); + expect(() => parent.covariantArg_(triangle), throwsA(isA())); }); }); } diff --git a/pkgs/ffigen/test/native_objc_test/bad_override_test.m b/pkgs/ffigen/test/native_objc_test/bad_override_test.m index 5a20239303..12376721f7 100644 --- a/pkgs/ffigen/test/native_objc_test/bad_override_test.m +++ b/pkgs/ffigen/test/native_objc_test/bad_override_test.m @@ -3,6 +3,7 @@ // BSD-style license that can be found in the LICENSE file. #import +#import @interface Polygon : NSObject {} -(NSString*)name; @@ -40,12 +41,17 @@ @implementation BadOverrideGrandparent @end @interface BadOverrideParent : BadOverrideGrandparent {} --(Rectangle*)contravariantReturn; -(int32_t)methodVsGetter; +-(Rectangle*)contravariantReturn; +-(NSString*)covariantArg:(Polygon*)poly; @end @implementation BadOverrideParent --(Rectangle*)contravariantReturn { return [Rectangle new]; } -(int32_t)methodVsGetter { return 1; } +-(Rectangle*)contravariantReturn { return [Rectangle new]; } + +-(NSString*)covariantArg:(Polygon*)poly { + return [@"Polygon: " stringByAppendingString: [poly name]]; +} @end @interface BadOverrideUncle : BadOverrideGrandparent {} @@ -61,12 +67,17 @@ @implementation BadOverrideAunt @end @interface BadOverrideChild : BadOverrideParent {} --(Polygon*)contravariantReturn; @property (readonly) int32_t methodVsGetter; +-(Polygon*)contravariantReturn; +-(NSString*)covariantArg:(Rectangle*)rect; @end @implementation BadOverrideChild --(Polygon*)contravariantReturn { return [Triangle new]; } -(int32_t)methodVsGetter { return 11; } +-(Polygon*)contravariantReturn { return [Triangle new]; } + +-(NSString*)covariantArg:(Rectangle*)rect { + return [@"Rectangle: " stringByAppendingString: [rect name]]; +} @end @interface BadOverrideSibbling : BadOverrideParent {} From bccd0164294c9f4d52693942f5d1ef308172ac4c Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 12 Nov 2024 14:43:48 +1100 Subject: [PATCH 06/11] clean up --- .../src/visitor/fix_overridden_methods.dart | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart b/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart index 097c8c374d..5eaad4c5bc 100644 --- a/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart +++ b/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart @@ -25,38 +25,6 @@ class FixOverriddenMethodsVisitation extends Visitation { node.visitChildren(visitor); } - void _fixNullability(ObjCInterface node) { - // ObjC ignores nullability when deciding if an override for an inherited - // method is valid. But in Dart it's invalid to override a method and change - // it's return type from non-null to nullable, or its arg type from nullable - // to non-null. So in these cases we have to make the non-null type - // nullable, to avoid Dart compile errors. - // for (var t = node.superType; t != null; t = t.superType) { - // for (final method in node.methods) { - // final superMethod = t.getSimilarMethod(method); - // if (superMethod != null && - // !superMethod.isClassMethod && - // !method.isClassMethod) { - // if (superMethod.returnType.typealiasType is! ObjCNullable && - // method.returnType.typealiasType is ObjCNullable) { - // superMethod.returnType = ObjCNullable(superMethod.returnType); - // } - // final numArgs = method.params.length < superMethod.params.length - // ? method.params.length - // : superMethod.params.length; - // for (var i = 0; i < numArgs; ++i) { - // final param = method.params[i]; - // final superParam = superMethod.params[i]; - // if (superParam.type.typealiasType is ObjCNullable && - // param.type.typealiasType is! ObjCNullable) { - // param.type = ObjCNullable(param.type); - // } - // } - // } - // } - // } - } - (ObjCInterface?, ObjCMethod?) _findNearestWithMethod( ObjCInterface node, ObjCMethod method) { for (ObjCInterface? t = node.superType; t != null; t = t.superType) { From b77e3ff62c71d800cfd532014da765e125d6cfec Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Tue, 12 Nov 2024 16:20:35 +1100 Subject: [PATCH 07/11] Fix large test --- pkgs/ffigen/lib/src/code_generator/type.dart | 4 +- .../sub_parsers/objcinterfacedecl_parser.dart | 12 ++++ .../sub_parsers/objcprotocoldecl_parser.dart | 6 ++ .../visitor/copy_methods_from_super_type.dart | 8 +-- .../large_objc_test.dart | 24 +------- .../src/objective_c_bindings_generated.dart | 55 +++++++++++-------- 6 files changed, 55 insertions(+), 54 deletions(-) diff --git a/pkgs/ffigen/lib/src/code_generator/type.dart b/pkgs/ffigen/lib/src/code_generator/type.dart index b60aed0cba..fe27ea0e1a 100644 --- a/pkgs/ffigen/lib/src/code_generator/type.dart +++ b/pkgs/ffigen/lib/src/code_generator/type.dart @@ -43,7 +43,7 @@ abstract class Type extends AstNode { bool isSubtypeOf(Type other) => other.isSupertypeOf(this); /// Returns true if [this] is a supertype of [other]. That is this :> other. - bool isSupertypeOf(Type other) => this == other; + bool isSupertypeOf(Type other) => this.typealiasType == other.typealiasType; /// Returns the C type of the Type. This is the FFI compatible type that is /// passed to native code. @@ -190,7 +190,7 @@ abstract class BindingType extends NoLookUpBinding implements Type { bool isSubtypeOf(Type other) => other.isSupertypeOf(this); @override - bool isSupertypeOf(Type other) => this == other; + bool isSupertypeOf(Type other) => this.typealiasType == other.typealiasType; @override String getFfiDartType(Writer w) => getCType(w); diff --git a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart index db4e70f0f9..8e9720ce02 100644 --- a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart +++ b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart @@ -2,6 +2,8 @@ // 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 'dart:io'; + import 'package:logging/logging.dart'; import '../../code_generator.dart'; @@ -180,6 +182,11 @@ void _parseSuperType(clang_types.CXCursor cursor, ObjCInterface itf) { setter.params .add(Parameter(name: 'value', type: fieldType, objCConsumed: false)); } + + if (fieldName == 'identifier') { + stderr.writeln("ZZZZZZZZZZZ: PROP ${decl.originalName}\t\t$getter\t\t$setter"); + } + return (getter, setter); } @@ -216,6 +223,11 @@ ObjCMethod? parseObjCMethod(clang_types.CXCursor cursor, Declaration itfDecl, ); _logger.fine(' > ${isClassMethod ? 'Class' : 'Instance'} method: ' '${method.originalName} ${cursor.completeStringRepr()}'); + + if (methodName == 'identifier') { + stderr.writeln("ZZZZZZZZZZZ: MTHD ${itfDecl.originalName}\t\t$method"); + } + var hasError = false; cursor.visitChildren((child) { switch (child.kind) { diff --git a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcprotocoldecl_parser.dart b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcprotocoldecl_parser.dart index d9ab7df2fb..dde0a22c88 100644 --- a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcprotocoldecl_parser.dart +++ b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcprotocoldecl_parser.dart @@ -61,6 +61,12 @@ ObjCProtocol? parseObjCProtocolDeclaration(clang_types.CXCursor cursor) { protocol.superProtocols.add(superProtocol); } break; + case clang_types.CXCursorKind.CXCursor_ObjCPropertyDecl: + final (getter, setter) = + parseObjCProperty(child, decl, config.objcProtocols); + protocol.addMethod(getter); + protocol.addMethod(setter); + break; case clang_types.CXCursorKind.CXCursor_ObjCInstanceMethodDecl: case clang_types.CXCursorKind.CXCursor_ObjCClassMethodDecl: protocol.addMethod(parseObjCMethod(child, decl, config.objcProtocols)); diff --git a/pkgs/ffigen/lib/src/visitor/copy_methods_from_super_type.dart b/pkgs/ffigen/lib/src/visitor/copy_methods_from_super_type.dart index 65431f6f55..91a2ce11e2 100644 --- a/pkgs/ffigen/lib/src/visitor/copy_methods_from_super_type.dart +++ b/pkgs/ffigen/lib/src/visitor/copy_methods_from_super_type.dart @@ -87,13 +87,7 @@ class CopyMethodsFromSuperTypesVisitation extends Visitation { for (final proto in protocols) { for (final m in proto.methods) { if (isNSObject) { - if (m.originalName == 'description' || m.originalName == 'hash') { - // TODO(https://github.com/dart-lang/native/issues/1220): Remove - // this special case. These methods only clash because they're - // sometimes declared as getters and sometimes as normal methods. - } else { - addMethod(m); - } + addMethod(m); } else if (!_excludedNSObjectMethods.contains(m.originalName)) { addMethod(m); } diff --git a/pkgs/ffigen/test/large_integration_tests/large_objc_test.dart b/pkgs/ffigen/test/large_integration_tests/large_objc_test.dart index 06ba756458..5d003f351d 100644 --- a/pkgs/ffigen/test/large_integration_tests/large_objc_test.dart +++ b/pkgs/ffigen/test/large_integration_tests/large_objc_test.dart @@ -39,28 +39,6 @@ void main() { shouldIncludeMember: randInclude, ); - // TODO(https://github.com/dart-lang/native/issues/1220): Allow these. - const disallowedMethods = { - 'accessKey', - 'allowsCellularAccess', - 'allowsConstrainedNetworkAccess', - 'attributedString', - 'cachePolicy', - 'candidateListTouchBarItem', - 'delegate', - 'hyphenationFactor', - 'image', - 'isProxy', - 'objCType', - 'tag', - 'title', - }; - final interfaceFilter = DeclarationFilters( - shouldInclude: randInclude, - shouldIncludeMember: (_, method) => - randInclude() && !disallowedMethods.contains(method), - ); - const outFile = 'test/large_integration_tests/large_objc_bindings.dart'; const outObjCFile = 'test/large_integration_tests/large_objc_bindings.m'; final config = Config( @@ -80,7 +58,7 @@ void main() { unnamedEnumConstants: randomFilter, globals: randomFilter, typedefs: randomFilter, - objcInterfaces: interfaceFilter, + objcInterfaces: randomFilter, objcProtocols: randomFilter, objcCategories: randomFilter, externalVersions: ExternalVersions( diff --git a/pkgs/objective_c/lib/src/objective_c_bindings_generated.dart b/pkgs/objective_c/lib/src/objective_c_bindings_generated.dart index a8117be61e..67b86779e3 100644 --- a/pkgs/objective_c/lib/src/objective_c_bindings_generated.dart +++ b/pkgs/objective_c/lib/src/objective_c_bindings_generated.dart @@ -412,7 +412,7 @@ class NSArray extends NSObject { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635(_class_NSArray, _sel_supportsSecureCoding); } @@ -654,7 +654,7 @@ class NSCharacterSet extends NSObject { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635( _class_NSCharacterSet, _sel_supportsSecureCoding); } @@ -930,7 +930,7 @@ class NSData extends NSObject { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635(_class_NSData, _sel_supportsSecureCoding); } @@ -1305,7 +1305,7 @@ class NSDate extends NSObject { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635(_class_NSDate, _sel_supportsSecureCoding); } @@ -1485,7 +1485,7 @@ class NSDictionary extends NSObject { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635(_class_NSDictionary, _sel_supportsSecureCoding); } @@ -1709,7 +1709,7 @@ class NSError extends NSObject { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635(_class_NSError, _sel_supportsSecureCoding); } @@ -1944,7 +1944,7 @@ class NSIndexSet extends NSObject { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635(_class_NSIndexSet, _sel_supportsSecureCoding); } @@ -2627,7 +2627,7 @@ class NSMutableArray extends NSArray { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635( _class_NSMutableArray, _sel_supportsSecureCoding); } @@ -2860,7 +2860,7 @@ class NSMutableData extends NSData { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635( _class_NSMutableData, _sel_supportsSecureCoding); } @@ -3152,7 +3152,7 @@ class NSMutableDictionary extends NSDictionary { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635( _class_NSMutableDictionary, _sel_supportsSecureCoding); } @@ -3305,7 +3305,7 @@ class NSMutableIndexSet extends NSIndexSet { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635( _class_NSMutableIndexSet, _sel_supportsSecureCoding); } @@ -3540,7 +3540,7 @@ class NSMutableOrderedSet extends NSOrderedSet { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635( _class_NSMutableOrderedSet, _sel_supportsSecureCoding); } @@ -3793,7 +3793,7 @@ class NSMutableSet extends NSSet { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635(_class_NSMutableSet, _sel_supportsSecureCoding); } @@ -4075,7 +4075,7 @@ class NSMutableString extends NSString { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635( _class_NSMutableString, _sel_supportsSecureCoding); } @@ -4493,7 +4493,7 @@ class NSNumber extends NSValue { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635(_class_NSNumber, _sel_supportsSecureCoding); } @@ -4901,7 +4901,7 @@ class NSObject extends objc.ObjCObjectBase { } /// debugDescription - NSString debugDescription1() { + NSString get debugDescription1 { if (!objc.respondsToSelector(this.ref.pointer, _sel_debugDescription)) { throw objc.UnimplementedOptionalMethodException( 'NSObject', 'debugDescription'); @@ -4910,6 +4910,12 @@ class NSObject extends objc.ObjCObjectBase { return NSString.castFromPointer(_ret, retain: true, release: true); } + /// description + NSString get description1 { + final _ret = _objc_msgSend_1x359cv(this.ref.pointer, _sel_description); + return NSString.castFromPointer(_ret, retain: true, release: true); + } + /// doesNotRecognizeSelector: void doesNotRecognizeSelector_(ffi.Pointer aSelector) { _objc_msgSend_1d9e4oe( @@ -4930,6 +4936,11 @@ class NSObject extends objc.ObjCObjectBase { return objc.ObjCObjectBase(_ret, retain: true, release: true); } + /// hash + int get hash1 { + return _objc_msgSend_xw2lbc(this.ref.pointer, _sel_hash); + } + /// init NSObject init() { final _ret = @@ -5040,7 +5051,7 @@ class NSObject extends objc.ObjCObjectBase { } /// superclass - objc.ObjCObjectBase superclass1() { + objc.ObjCObjectBase get superclass1 { final _ret = _objc_msgSend_1x359cv(this.ref.pointer, _sel_superclass); return objc.ObjCObjectBase(_ret, retain: true, release: true); } @@ -5328,7 +5339,7 @@ class NSOrderedSet extends NSObject { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635(_class_NSOrderedSet, _sel_supportsSecureCoding); } @@ -5862,7 +5873,7 @@ class NSSet extends NSObject { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635(_class_NSSet, _sel_supportsSecureCoding); } @@ -6426,7 +6437,7 @@ class NSString extends NSObject { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635(_class_NSString, _sel_supportsSecureCoding); } @@ -7735,7 +7746,7 @@ class NSURL extends NSObject { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635(_class_NSURL, _sel_supportsSecureCoding); } @@ -8335,7 +8346,7 @@ class NSValue extends NSObject { } /// supportsSecureCoding - static bool supportsSecureCoding() { + static bool getSupportsSecureCoding() { return _objc_msgSend_91o635(_class_NSValue, _sel_supportsSecureCoding); } From f69e35937048bec15494b2dd4cba4a82f2925de3 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Wed, 13 Nov 2024 10:05:15 +1100 Subject: [PATCH 08/11] fmt --- .../header_parser/sub_parsers/objcinterfacedecl_parser.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart index 8e9720ce02..bef1644453 100644 --- a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart +++ b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart @@ -184,7 +184,8 @@ void _parseSuperType(clang_types.CXCursor cursor, ObjCInterface itf) { } if (fieldName == 'identifier') { - stderr.writeln("ZZZZZZZZZZZ: PROP ${decl.originalName}\t\t$getter\t\t$setter"); + stderr.writeln( + "ZZZZZZZZZZZ: PROP ${decl.originalName}\t\t$getter\t\t$setter"); } return (getter, setter); From 48d897b6e3bf10741c218ed9a838b98c058caabe Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Wed, 13 Nov 2024 10:08:59 +1100 Subject: [PATCH 09/11] changelog --- pkgs/ffigen/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkgs/ffigen/CHANGELOG.md b/pkgs/ffigen/CHANGELOG.md index 77aa320421..4703c7ee77 100644 --- a/pkgs/ffigen/CHANGELOG.md +++ b/pkgs/ffigen/CHANGELOG.md @@ -25,6 +25,8 @@ generating them as stubs. `include-transitive-objc-protocols` defaults to false, and `include-transitive-objc-categories` defaults to true, but these both replicate the existing behavior. +- Fix [bugs](https://github.com/dart-lang/native/issues/1220) caused by + mismatches between ObjC and Dart's inheritance rules. ## 15.0.0 From 9a8d828b6ab62c5607bfb6bd6827afc01e504b80 Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Wed, 13 Nov 2024 10:19:22 +1100 Subject: [PATCH 10/11] fix analysis --- pkgs/ffigen/lib/src/code_generator/type.dart | 8 ++++---- .../sub_parsers/objcinterfacedecl_parser.dart | 11 ----------- .../lib/src/visitor/fix_overridden_methods.dart | 2 +- pkgs/ffigen/test/unit_tests/subtyping_test.dart | 2 -- 4 files changed, 5 insertions(+), 18 deletions(-) diff --git a/pkgs/ffigen/lib/src/code_generator/type.dart b/pkgs/ffigen/lib/src/code_generator/type.dart index fe27ea0e1a..552945c942 100644 --- a/pkgs/ffigen/lib/src/code_generator/type.dart +++ b/pkgs/ffigen/lib/src/code_generator/type.dart @@ -33,7 +33,7 @@ abstract class Type extends AstNode { /// Returns true if the type is a [Compound] and is incomplete. bool get isIncompleteCompound => false; - /// Returns true if [this] is a subtype of [other]. That is this <: other. + /// Returns true if this is a subtype of [other]. That is this <: other. /// /// The behavior of this function should mirror Dart's subtyping logic, not /// Objective-C's. It's used to detect and fix cases where the generated @@ -42,8 +42,8 @@ abstract class Type extends AstNode { /// Note: Implementers should implement [isSupertypeOf]. bool isSubtypeOf(Type other) => other.isSupertypeOf(this); - /// Returns true if [this] is a supertype of [other]. That is this :> other. - bool isSupertypeOf(Type other) => this.typealiasType == other.typealiasType; + /// Returns true if this is a supertype of [other]. That is this :> other. + bool isSupertypeOf(Type other) => typealiasType == other.typealiasType; /// Returns the C type of the Type. This is the FFI compatible type that is /// passed to native code. @@ -190,7 +190,7 @@ abstract class BindingType extends NoLookUpBinding implements Type { bool isSubtypeOf(Type other) => other.isSupertypeOf(this); @override - bool isSupertypeOf(Type other) => this.typealiasType == other.typealiasType; + bool isSupertypeOf(Type other) => typealiasType == other.typealiasType; @override String getFfiDartType(Writer w) => getCType(w); diff --git a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart index bef1644453..0d7564bc2d 100644 --- a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart +++ b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart @@ -2,8 +2,6 @@ // 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 'dart:io'; - import 'package:logging/logging.dart'; import '../../code_generator.dart'; @@ -183,11 +181,6 @@ void _parseSuperType(clang_types.CXCursor cursor, ObjCInterface itf) { .add(Parameter(name: 'value', type: fieldType, objCConsumed: false)); } - if (fieldName == 'identifier') { - stderr.writeln( - "ZZZZZZZZZZZ: PROP ${decl.originalName}\t\t$getter\t\t$setter"); - } - return (getter, setter); } @@ -225,10 +218,6 @@ ObjCMethod? parseObjCMethod(clang_types.CXCursor cursor, Declaration itfDecl, _logger.fine(' > ${isClassMethod ? 'Class' : 'Instance'} method: ' '${method.originalName} ${cursor.completeStringRepr()}'); - if (methodName == 'identifier') { - stderr.writeln("ZZZZZZZZZZZ: MTHD ${itfDecl.originalName}\t\t$method"); - } - var hasError = false; cursor.visitChildren((child) { switch (child.kind) { diff --git a/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart b/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart index 5eaad4c5bc..6360b8bcae 100644 --- a/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart +++ b/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart @@ -27,7 +27,7 @@ class FixOverriddenMethodsVisitation extends Visitation { (ObjCInterface?, ObjCMethod?) _findNearestWithMethod( ObjCInterface node, ObjCMethod method) { - for (ObjCInterface? t = node.superType; t != null; t = t.superType) { + for (var t = node.superType; t != null; t = t.superType) { final tMethod = t.getSimilarMethod(method); if (tMethod != null) { return (t, tMethod); diff --git a/pkgs/ffigen/test/unit_tests/subtyping_test.dart b/pkgs/ffigen/test/unit_tests/subtyping_test.dart index e4aa46176e..5ac548ca83 100644 --- a/pkgs/ffigen/test/unit_tests/subtyping_test.dart +++ b/pkgs/ffigen/test/unit_tests/subtyping_test.dart @@ -2,8 +2,6 @@ // 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 'dart:io'; - import 'package:ffigen/src/code_generator.dart'; import 'package:test/test.dart'; From 3328db7b3ee8b71d1b7026b504f5c9600d722a1f Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Wed, 13 Nov 2024 10:37:37 +1100 Subject: [PATCH 11/11] clean up --- .../src/header_parser/sub_parsers/objcinterfacedecl_parser.dart | 2 -- pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart index 0d7564bc2d..db4e70f0f9 100644 --- a/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart +++ b/pkgs/ffigen/lib/src/header_parser/sub_parsers/objcinterfacedecl_parser.dart @@ -180,7 +180,6 @@ void _parseSuperType(clang_types.CXCursor cursor, ObjCInterface itf) { setter.params .add(Parameter(name: 'value', type: fieldType, objCConsumed: false)); } - return (getter, setter); } @@ -217,7 +216,6 @@ ObjCMethod? parseObjCMethod(clang_types.CXCursor cursor, Declaration itfDecl, ); _logger.fine(' > ${isClassMethod ? 'Class' : 'Instance'} method: ' '${method.originalName} ${cursor.completeStringRepr()}'); - var hasError = false; cursor.visitChildren((child) { switch (child.kind) { diff --git a/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart b/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart index 6360b8bcae..8e989559da 100644 --- a/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart +++ b/pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart @@ -70,7 +70,7 @@ class FixOverriddenMethodsVisitation extends Visitation { void _fixCoavariantArgs(ObjCInterface node, ObjCMethod method, ObjCInterface superType, ObjCMethod superMethod) { - // In Dart, method arg types are contavariant, but ObjC allows them to be + // In Dart, method arg types are contravariant, but ObjC allows them to be // covariant. So fix these cases by adding the `covariant` keyword to the // parameter. final n = method.params.length;