Skip to content

Commit

Permalink
[ffigen] Return structs by value from ObjC methods (#1556)
Browse files Browse the repository at this point in the history
  • Loading branch information
liamappelbe authored Sep 30, 2024
1 parent f9a3888 commit 001bd08
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 70 deletions.
3 changes: 3 additions & 0 deletions pkgs/ffigen/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## 15.0.0-wip

- Bump minimum Dart version to 3.4.
- Dedupe `ObjCBlock` trampolines to reduce generated ObjC code.
- ObjC objects now include the methods from the protocols they implement. Both
required and optional methods are included. Optional methods will throw an
Expand All @@ -16,6 +17,8 @@
- `sort:` config option now affects ObjC interface/protocol methods.
- Fix a bug where `NSRange` was not being imported from package:objective_c:
https://github.com/dart-lang/native/issues/1180
- __Breaking change__: Return structs from ObjC methods by value instead of
taking a struct return pointer.

## 14.0.1

Expand Down
100 changes: 47 additions & 53 deletions pkgs/ffigen/lib/src/code_generator/objc_interface.dart
Original file line number Diff line number Diff line change
Expand Up @@ -121,28 +121,17 @@ class ObjCInterface extends BindingType with ObjCMethods {
for (final m in methods) {
final methodName = m.getDartMethodName(methodNamer);
final isStatic = m.isClassMethod;
final isStret = m.msgSend!.isStret;

var returnType = m.returnType;
var params = m.params;
if (isStret) {
params = [
Parameter(
name: 'stret',
type: PointerType(returnType),
objCConsumed: false),
...params
];
returnType = voidType;
}

final returnType = m.returnType;
final returnTypeStr = _getConvertedType(returnType, w, name);
final params = m.params;

// The method declaration.
s.write('\n ');
s.write(makeDartDoc(m.dartDoc ?? m.originalName));
s.write(' ');
if (isStatic) {
s.write('static ');
s.write(_getConvertedType(returnType, w, name));
s.write('static $returnTypeStr');

switch (m.kind) {
case ObjCMethodKind.method:
Expand All @@ -165,21 +154,12 @@ class ObjCInterface extends BindingType with ObjCMethods {
switch (m.kind) {
case ObjCMethodKind.method:
// returnType methodName(...)
s.write(_getConvertedType(returnType, w, name));
s.write(' $methodName');
s.write('$returnTypeStr $methodName');
s.write(paramsToString(params));
break;
case ObjCMethodKind.propertyGetter:
s.write(_getConvertedType(returnType, w, name));
if (isStret) {
// void getMethodName(Pointer<returnType> stret)
s.write(' get');
s.write(methodName[0].toUpperCase() + methodName.substring(1));
s.write(paramsToString(params));
} else {
// returnType get methodName
s.write(' get $methodName');
}
// returnType get methodName
s.write('$returnTypeStr get $methodName');
break;
case ObjCMethodKind.propertySetter:
// set methodName(...)
Expand All @@ -204,36 +184,50 @@ class ObjCInterface extends BindingType with ObjCMethods {
final convertReturn = m.kind != ObjCMethodKind.propertySetter &&
!returnType.sameDartAndFfiDartType;

if (returnType != voidType) {
s.write(' ${convertReturn ? 'final _ret = ' : 'return '}');
}
s.write(m.msgSend!.invoke(
w,
isStatic
? _classObject.name
: convertDartTypeToFfiDartType(
w,
'this',
objCRetain: m.consumesSelf,
objCAutorelease: false,
),
sel,
final target = isStatic
? _classObject.name
: convertDartTypeToFfiDartType(
w,
'this',
objCRetain: m.consumesSelf,
objCAutorelease: false,
);
final msgSendParams =
m.params.map((p) => p.type.convertDartTypeToFfiDartType(
w,
p.name,
objCRetain: p.objCConsumed,
objCAutorelease: false,
)),
structRetPtr: 'stret'));
s.write(';\n');
if (convertReturn) {
final result = returnType.convertFfiDartTypeToDartType(
w,
'_ret',
objCRetain: !m.returnsRetained,
objCEnclosingClass: name,
);
s.write(' return $result;');
));
if (m.msgSend!.isStret) {
assert(!convertReturn);
final calloc = '${w.ffiPkgLibraryPrefix}.calloc';
final sizeOf = '${w.ffiLibraryPrefix}.sizeOf';
final uint8Type = NativeType(SupportedNativeType.uint8).getCType(w);
final invoke = m.msgSend!
.invoke(w, target, sel, msgSendParams, structRetPtr: '_ptr');
s.write('''
final _ptr = $calloc<$returnTypeStr>();
$invoke;
final _finalizable = _ptr.cast<$uint8Type>().asTypedList(
$sizeOf<$returnTypeStr>(), finalizer: $calloc.nativeFree);
return ${w.ffiLibraryPrefix}.Struct.create<$returnTypeStr>(_finalizable);
''');
} else {
if (returnType != voidType) {
s.write(' ${convertReturn ? 'final _ret = ' : 'return '}');
}
s.write(m.msgSend!.invoke(w, target, sel, msgSendParams));
s.write(';\n');
if (convertReturn) {
final result = returnType.convertFfiDartTypeToDartType(
w,
'_ret',
objCRetain: !m.returnsRetained,
objCEnclosingClass: name,
);
s.write(' return $result;');
}
}

s.write('\n }\n');
Expand Down
4 changes: 2 additions & 2 deletions pkgs/ffigen/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ topics:
- codegen

environment:
sdk: '>=3.3.0 <4.0.0'
sdk: '>=3.4.0 <4.0.0'

dependencies:
args: ^2.0.0
Expand All @@ -38,7 +38,7 @@ dev_dependencies:
json_schema: ^5.1.1
leak_tracker: ^10.0.7
meta: ^1.11.0
objective_c: ^2.0.0
objective_c: ^2.1.0
test: ^1.16.2

dependency_overrides:
Expand Down
4 changes: 1 addition & 3 deletions pkgs/ffigen/test/native_objc_test/block_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,7 @@ void main() {
expect(result1.z, 7.8);
expect(result1.w, 1.2);

final result2Ptr = arena<Vec4>();
final result2 = result2Ptr.ref;
BlockTester.callVec4Block_(result2Ptr, block);
final result2 = BlockTester.callVec4Block_(block);
expect(result2.x, 3.4);
expect(result2.y, 5.6);
expect(result2.z, 7.8);
Expand Down
10 changes: 2 additions & 8 deletions pkgs/ffigen/test/native_objc_test/method_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,13 @@ void main() {
input.z = 5.6;
input.w = 7.8;

final resultPtr = calloc<Vec4>();
final result = resultPtr.ref;
testInstance.twiddleVec4Components_(resultPtr, input);
final result = testInstance.twiddleVec4Components_(input);
expect(result.x, 3.4);
expect(result.y, 5.6);
expect(result.z, 7.8);
expect(result.w, 1.2);

calloc.free(inputPtr);
calloc.free(resultPtr);
});

test('Floats', () {
Expand All @@ -95,14 +92,11 @@ void main() {

test('Method with same name as a type', () {
// Test for https://github.com/dart-lang/native/issues/1007
final resultPtr = calloc<Vec4>();
final result = resultPtr.ref;
testInstance.Vec41(resultPtr); // A slightly unfortunate rename :P
final result = testInstance.Vec41(); // A slightly unfortunate rename :P
expect(result.x, 1);
expect(result.y, 2);
expect(result.z, 3);
expect(result.w, 4);
calloc.free(resultPtr);
});
});
});
Expand Down
5 changes: 1 addition & 4 deletions pkgs/ffigen/test/native_objc_test/property_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,14 @@ void main() {
input.z = 5.6;
input.w = 7.8;

final resultPtr = calloc<Vec4>();
final result = resultPtr.ref;
testInstance.structProperty = input;
testInstance.getStructProperty(resultPtr);
final result = testInstance.structProperty;
expect(result.x, 1.2);
expect(result.y, 3.4);
expect(result.z, 5.6);
expect(result.w, 7.8);

calloc.free(inputPtr);
calloc.free(resultPtr);
});

test('Floats', () {
Expand Down

0 comments on commit 001bd08

Please sign in to comment.