Skip to content
This repository has been archived by the owner on Jan 28, 2024. It is now read-only.

Separate getDartType and getUserType #623

Merged
merged 5 commits into from
Sep 24, 2023
Merged

Separate getDartType and getUserType #623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
51e4bc7
Separate getDartType and getUserType
liamappelbe Sep 21, 2023
876fa36
Fix analysis
liamappelbe Sep 21, 2023
0dc2797
Document instancetype special casing
liamappelbe Sep 22, 2023
724085d
s/getDartType/getFfiDartType
liamappelbe Sep 22, 2023
276f30c
s/getUserType/getDartType
liamappelbe Sep 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/src/code_generator/compound.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ abstract class Compound extends BindingType {
/// Marking type names because dart doesn't allow class member to have the
/// same name as a type name used internally.
for (final m in members) {
localUniqueNamer.markUsed(m.type.getDartType(w));
localUniqueNamer.markUsed(m.type.getFfiDartType(w));
}

/// Write @Packed(X) annotation if struct is packed.
Expand Down Expand Up @@ -148,7 +148,7 @@ abstract class Compound extends BindingType {
if (!sameDartAndCType(m.type, w)) {
s.write('$depth@${m.type.getCType(w)}()\n');
}
s.write('${depth}external ${m.type.getDartType(w)} ${m.name};\n\n');
s.write('${depth}external ${m.type.getFfiDartType(w)} ${m.name};\n\n');
}
}
s.write('}\n\n');
Expand Down
2 changes: 1 addition & 1 deletion lib/src/code_generator/enum_class.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class EnumClass extends BindingType {
String getCType(Writer w) => nativeType.getCType(w);

@override
String getDartType(Writer w) => nativeType.getDartType(w);
String getFfiDartType(Writer w) => nativeType.getFfiDartType(w);

@override
String? getDefaultValue(Writer w, String nativeLib) => '0';
Expand Down
10 changes: 5 additions & 5 deletions lib/src/code_generator/func.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ class Func extends LookUpBinding {
: functionType.getCType(w, writeArgumentNames: false);
final dartType = exposeFunctionTypedefs
? _exposedDartFunctionTypealias!.name
: functionType.getDartType(w, writeArgumentNames: false);
: functionType.getFfiDartType(w, writeArgumentNames: false);

if (ffiNativeConfig.enabled) {
final assetString = ffiNativeConfig.asset != null
Expand All @@ -131,17 +131,17 @@ class Func extends LookUpBinding {
"@${w.ffiLibraryPrefix}.Native<$cType>(symbol: '$originalName'$assetString$isLeafString)\n");

s.write(
'external ${functionType.returnType.getDartType(w)} $enclosingFuncName(\n');
'external ${functionType.returnType.getFfiDartType(w)} $enclosingFuncName(\n');
for (final p in functionType.dartTypeParameters) {
s.write(' ${p.type.getDartType(w)} ${p.name},\n');
s.write(' ${p.type.getFfiDartType(w)} ${p.name},\n');
}
s.write(');\n\n');
} else {
// Write enclosing function.
s.write(
'${functionType.returnType.getDartType(w)} $enclosingFuncName(\n');
'${functionType.returnType.getFfiDartType(w)} $enclosingFuncName(\n');
for (final p in functionType.dartTypeParameters) {
s.write(' ${p.type.getDartType(w)} ${p.name},\n');
s.write(' ${p.type.getFfiDartType(w)} ${p.name},\n');
}
s.write(') {\n');
s.write('return $funcVarName');
Expand Down
8 changes: 4 additions & 4 deletions lib/src/code_generator/func_type.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,16 @@ class FunctionType extends Type {
}

@override
String getDartType(Writer w, {bool writeArgumentNames = true}) {
String getFfiDartType(Writer w, {bool writeArgumentNames = true}) {
final sb = StringBuffer();

// Write return Type.
sb.write(returnType.getDartType(w));
sb.write(returnType.getFfiDartType(w));

// Write Function.
sb.write(' Function(');
sb.write(dartTypeParameters.map<String>((p) {
return '${p.type.getDartType(w)} ${writeArgumentNames ? p.name : ""}';
return '${p.type.getFfiDartType(w)} ${writeArgumentNames ? p.name : ""}';
}).join(', '));
sb.write(')');

Expand Down Expand Up @@ -137,7 +137,7 @@ class NativeFunc extends Type {
'${w.ffiLibraryPrefix}.NativeFunction<${_type.getCType(w)}>';

@override
String getDartType(Writer w) => getCType(w);
String getFfiDartType(Writer w) => getCType(w);

@override
String toString() => 'NativeFunction<${_type.toString()}>';
Expand Down
2 changes: 1 addition & 1 deletion lib/src/code_generator/global.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class Global extends LookUpBinding {
s.write(makeDartDoc(dartDoc!));
}
final pointerName = w.wrapperLevelUniqueNamer.makeUnique('_$globalVarName');
final dartType = type.getDartType(w);
final dartType = type.getFfiDartType(w);
final cType = type.getCType(w);

s.write(
Expand Down
2 changes: 1 addition & 1 deletion lib/src/code_generator/handle.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class HandleType extends Type {
String getCType(Writer w) => '${w.ffiLibraryPrefix}.Handle';

@override
String getDartType(Writer w) => 'Object';
String getFfiDartType(Writer w) => 'Object';

@override
String toString() => 'Handle';
Expand Down
4 changes: 2 additions & 2 deletions lib/src/code_generator/imports.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class ImportedType extends Type {
}

@override
String getDartType(Writer w) => cType == dartType ? getCType(w) : dartType;
String getFfiDartType(Writer w) => cType == dartType ? getCType(w) : dartType;

@override
String toString() => '${libraryImport.name}.$cType';
Expand All @@ -62,7 +62,7 @@ class SelfImportedType extends Type {
String getCType(Writer w) => cType;

@override
String getDartType(Writer w) => dartType;
String getFfiDartType(Writer w) => dartType;

@override
String toString() => cType;
Expand Down
3 changes: 1 addition & 2 deletions lib/src/code_generator/native_type.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class NativeType extends Type {
String getCType(Writer w) => '${w.ffiLibraryPrefix}.$_cType';

@override
String getDartType(Writer w) => _dartType;
String getFfiDartType(Writer w) => _dartType;

@override
String toString() => _cType;
Expand All @@ -67,7 +67,6 @@ class NativeType extends Type {
}

class BooleanType extends NativeType {
// Booleans are treated as uint8.
const BooleanType._() : super._('Bool', 'bool', 'false');
static const _boolean = BooleanType._();
factory BooleanType() => _boolean;
Expand Down
29 changes: 16 additions & 13 deletions lib/src/code_generator/objc_block.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,15 @@ class ObjCBlock extends BindingType {
'${w.ffiLibraryPrefix}.NativeCallable<${trampFuncType.getCType(w)}>';

// Write the function pointer based trampoline function.
s.write(returnType.getDartType(w));
s.write(returnType.getFfiDartType(w));
s.write(' $funcPtrTrampoline(${blockPtr.getCType(w)} block');
for (int i = 0; i < params.length; ++i) {
s.write(', ${params[i].type.getDartType(w)} ${params[i].name}');
s.write(', ${params[i].type.getFfiDartType(w)} ${params[i].name}');
}
s.write(') {\n');
s.write(' ${isVoid ? '' : 'return '}block.ref.target.cast<'
'${natFnType.getDartType(w)}>().asFunction<'
'${funcType.getDartType(w)}>()(');
'${natFnType.getFfiDartType(w)}>().asFunction<'
'${funcType.getFfiDartType(w)}>()(');
for (int i = 0; i < params.length; ++i) {
s.write('${i == 0 ? '' : ', '}${params[i].name}');
}
Expand All @@ -109,17 +109,17 @@ $voidPtr $registerClosure(Function fn) {
''');

// Write the closure based trampoline function.
s.write(returnType.getDartType(w));
s.write(returnType.getFfiDartType(w));
s.write(' $closureTrampoline(${blockPtr.getCType(w)} block');
for (int i = 0; i < params.length; ++i) {
s.write(', ${params[i].type.getDartType(w)} ${params[i].name}');
s.write(', ${params[i].type.getFfiDartType(w)} ${params[i].name}');
}
s.write(') {\n');
s.write(' ${isVoid ? '' : 'return '}');
s.write('($closureRegistry[block.ref.target.address]');
s.write(' as ${returnType.getDartType(w)} Function(');
s.write(' as ${returnType.getFfiDartType(w)} Function(');
for (int i = 0; i < params.length; ++i) {
s.write('${i == 0 ? '' : ', '}${params[i].type.getDartType(w)}');
s.write('${i == 0 ? '' : ', '}${params[i].type.getFfiDartType(w)}');
}
s.write('))');
s.write('(');
Expand Down Expand Up @@ -154,7 +154,7 @@ class $name extends _ObjCBlockBase {
/// This block must be invoked by native code running on the same thread as
/// the isolate that registered it. Invoking the block on the wrong thread
/// will result in a crash.
$name.fromFunction(${w.className} lib, ${funcType.getDartType(w)} fn) :
$name.fromFunction(${w.className} lib, ${funcType.getFfiDartType(w)} fn) :
this._(lib.${builtInFunctions.newBlock.name}(
_dartFuncTrampoline ??= ${w.ffiLibraryPrefix}.Pointer.fromFunction<
${trampFuncType.getCType(w)}>($closureTrampoline
Expand All @@ -175,7 +175,7 @@ class $name extends _ObjCBlockBase {
///
/// Note that unlike the default behavior of NativeCallable.listener, listener
/// blocks do not keep the isolate alive.
$name.listener(${w.className} lib, ${funcType.getDartType(w)} fn) :
$name.listener(${w.className} lib, ${funcType.getFfiDartType(w)} fn) :
this._(lib.${builtInFunctions.newBlock.name}(
(_dartFuncListenerTrampoline ??= $nativeCallableType.listener($closureTrampoline
$exceptionalReturn)..keepIsolateAlive = false).nativeFunction.cast(),
Expand All @@ -186,15 +186,15 @@ class $name extends _ObjCBlockBase {
}

// Call method.
s.write(' ${returnType.getDartType(w)} call(');
s.write(' ${returnType.getFfiDartType(w)} call(');
for (int i = 0; i < params.length; ++i) {
s.write('${i == 0 ? '' : ', '}${params[i].type.getDartType(w)}');
s.write('${i == 0 ? '' : ', '}${params[i].type.getFfiDartType(w)}');
s.write(' ${params[i].name}');
}
s.write(''') {
${isVoid ? '' : 'return '}_id.ref.invoke.cast<
${natTrampFnType.getCType(w)}>().asFunction<
${trampFuncType.getDartType(w)}>()(_id''');
${trampFuncType.getFfiDartType(w)}>()(_id''');
for (int i = 0; i < params.length; ++i) {
s.write(', ${params[i].name}');
}
Expand Down Expand Up @@ -222,6 +222,9 @@ class $name extends _ObjCBlockBase {
String getCType(Writer w) =>
PointerType(builtInFunctions.blockStruct).getCType(w);

@override
String getDartType(Writer w) => name;

@override
String toString() => '($returnType (^)(${argTypes.join(', ')}))';
}
30 changes: 10 additions & 20 deletions lib/src/code_generator/objc_interface.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import 'package:ffigen/src/code_generator.dart';
import 'package:logging/logging.dart';

import '../strings.dart' as strings;
import 'binding_string.dart';
import 'utils.dart';
import 'writer.dart';
Expand Down Expand Up @@ -281,7 +280,7 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} {
if (m.isClass &&
!_excludedNSObjectClassMethods.contains(m.originalName)) {
addMethod(m);
} else if (_isInstanceType(m.returnType)) {
} else if (m.returnType is ObjCInstanceType) {
addMethod(m);
}
}
Expand Down Expand Up @@ -355,13 +354,8 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} {
@override
String getCType(Writer w) => PointerType(objCObjectType).getCType(w);

bool _isObject(Type type) =>
type is PointerType && type.child == objCObjectType;

bool _isInstanceType(Type type) =>
type is Typealias &&
type.originalName == strings.objcInstanceType &&
_isObject(type.type);
@override
String getDartType(Writer w) => name;

// Utils for converting between the internal types passed to native code, and
// the external types visible to the user. For example, ObjCInterfaces are
Expand All @@ -370,15 +364,11 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} {
bool _needsConverting(Type type) =>
type is ObjCInterface ||
type is ObjCBlock ||
_isObject(type) ||
_isInstanceType(type);
type is ObjCObjectPointer ||
type is ObjCInstanceType;

String _getConvertedType(Type type, Writer w, String enclosingClass) {
if (type is BooleanType) return 'bool';
if (type is ObjCInterface) return type.name;
if (type is ObjCBlock) return type.name;
if (_isObject(type)) return 'NSObject';
if (_isInstanceType(type)) return enclosingClass;
if (type is ObjCInstanceType) return enclosingClass;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup!

This special case is the instancetype right? The one that returns the type of the object itself even in subclasses.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep

return type.getDartType(w);
}

Expand All @@ -393,8 +383,8 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} {

String _doArgConversion(ObjCMethodParam arg) {
if (arg.type is ObjCInterface ||
_isObject(arg.type) ||
_isInstanceType(arg.type) ||
arg.type is ObjCObjectPointer ||
arg.type is ObjCInstanceType ||
arg.type is ObjCBlock) {
if (arg.isNullable) {
return '${arg.name}?._id ?? ffi.nullptr';
Expand All @@ -415,10 +405,10 @@ class $name extends ${superType?.name ?? '_ObjCWrapper'} {
if (type is ObjCBlock) {
return '$prefix${type.name}._($value, $library)';
}
if (_isObject(type)) {
if (type is ObjCObjectPointer) {
return '${prefix}NSObject._($value, $library, $ownerFlags)';
}
if (_isInstanceType(type)) {
if (type is ObjCInstanceType) {
return '$prefix$enclosingClass._($value, $library, $ownerFlags)';
}
return prefix + value;
Expand Down
25 changes: 22 additions & 3 deletions lib/src/code_generator/pointer.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,15 @@ import 'writer.dart';
/// Represents a pointer.
class PointerType extends Type {
final Type child;
PointerType(this.child);

PointerType._(this.child);

factory PointerType(Type child) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we maybe have something more general to recognize "internal-dart-types" and return the "user-visible-dart-types"?

For example, if we have something more general we could also recognize Pointer<Char> and make it available as String in signatures?

It should probably be a separate pass. Rather than making a choice point in factory constructors of types.

  // Parse the bindings according to config object provided.
  final library = parse(config);

  final libraryTransformed = transform(library);

  // Generate file for the parsed bindings.
  final gen = File(config.output);
  libraryTransformed.generateFile(gen);

Or maybe because we already make choices in parse based on the config, it should a Type wrapDartType(Type type) function being invoked on any type. And for now it only replaces Pointer with ObjCObjectPointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment we can handle all these cases, including your Pointer<Char> idea, using this factory pattern. The type I'm returning is still a Pointer, it's just a more specialized version of it. That's one of the main use cases of a factory constructor, so I think it's appropriate to make the decision here. I don't see the point of adding a entire transformer infrastructure if we don't actually need it. We can add something like that later if we find a case that isn't cleanly handled by this pattern.

if (child == objCObjectType) {
return ObjCObjectPointer();
}
return PointerType._(child);
}

@override
void addDependencies(Set<Binding> dependencies) {
Expand All @@ -33,7 +41,7 @@ class PointerType extends Type {
/// Represents a constant array, which has a fixed size.
class ConstantArray extends PointerType {
final int length;
ConstantArray(this.length, Type child) : super(child);
ConstantArray(this.length, Type child) : super._(child);

@override
Type get baseArrayType => child.baseArrayType;
Expand All @@ -50,7 +58,7 @@ class ConstantArray extends PointerType {

/// Represents an incomplete array, which has an unknown size.
class IncompleteArray extends PointerType {
IncompleteArray(Type child) : super(child);
IncompleteArray(Type child) : super._(child);

@override
Type get baseArrayType => child.baseArrayType;
Expand All @@ -61,3 +69,14 @@ class IncompleteArray extends PointerType {
@override
String cacheKey() => '${child.cacheKey()}[]';
}

/// A pointer to an NSObject.
class ObjCObjectPointer extends PointerType {
factory ObjCObjectPointer() => _inst;

static final _inst = ObjCObjectPointer._();
ObjCObjectPointer._() : super._(objCObjectType);

@override
String getDartType(Writer w) => 'NSObject';
}
18 changes: 13 additions & 5 deletions lib/src/code_generator/type.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,14 @@ abstract class Type {
/// passed to native code.
String getCType(Writer w) => throw 'No mapping for type: $this';

/// Returns the Dart type of the Type. This is the user visible type that is
/// passed to Dart code.
String getDartType(Writer w) => getCType(w);
/// Returns the Dart type of the Type. This is the type that is passed from
/// FFI to Dart code.
String getFfiDartType(Writer w) => getCType(w);

/// Returns the user type of the Type. This is the type that is presented to
/// users by the ffigened API to users. For C bindings this is always the same
/// as getFfiDartType. For ObjC bindings this refers to the wrapper object.
String getDartType(Writer w) => getFfiDartType(w);

/// Returns the string representation of the Type, for debugging purposes
/// only. This string should not be printed as generated code.
Expand All @@ -62,7 +67,7 @@ abstract class Type {
}

/// Function to check if the dart and C type string are same.
bool sameDartAndCType(Type t, Writer w) => t.getCType(w) == t.getDartType(w);
bool sameDartAndCType(Type t, Writer w) => t.getCType(w) == t.getFfiDartType(w);

/// Base class for all Type bindings.
///
Expand Down Expand Up @@ -97,7 +102,10 @@ abstract class BindingType extends NoLookUpBinding implements Type {
bool get isIncompleteCompound => false;

@override
String getDartType(Writer w) => getCType(w);
String getFfiDartType(Writer w) => getCType(w);

@override
String getDartType(Writer w) => getFfiDartType(w);

@override
String toString() => originalName;
Expand Down
Loading