Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support @Native fields and addressOf #860

Merged
merged 23 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
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
9 changes: 5 additions & 4 deletions .github/workflows/ffigen.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ jobs:
strategy:
fail-fast: false
matrix:
sdk: [3.2.0]
sdk: [dev]
# sdk: [3.2.0]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe update the commented stuff to 3.3.0, and file an issue to update this when 3.3.0 beta is released.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done - #876.

steps:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
- uses: dart-lang/setup-dart@b64355ae6ca0b5d484f0106a033dd1388965d06d
Expand Down Expand Up @@ -56,7 +57,7 @@ jobs:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
- uses: dart-lang/setup-dart@b64355ae6ca0b5d484f0106a033dd1388965d06d
with:
sdk: 3.2.0
sdk: dev #3.2.0
simolus3 marked this conversation as resolved.
Show resolved Hide resolved
- name: Install dependencies
run: dart pub get
- name: Install libclang-14-dev
Expand All @@ -77,7 +78,7 @@ jobs:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
- uses: dart-lang/setup-dart@b64355ae6ca0b5d484f0106a033dd1388965d06d
with:
sdk: 3.2.0
sdk: dev #3.2.0
- name: Install dependencies
run: dart pub get
- name: Build test dylib and bindings
Expand Down Expand Up @@ -110,7 +111,7 @@ jobs:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
- uses: dart-lang/setup-dart@b64355ae6ca0b5d484f0106a033dd1388965d06d
with:
sdk: 3.2.0
sdk: dev #3.2.0
- name: Install dependencies
run: dart pub get
- name: Build test dylib and bindings
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ffigen_weekly.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
- uses: dart-lang/setup-dart@b64355ae6ca0b5d484f0106a033dd1388965d06d
with:
sdk: 3.2.0
sdk: dev #3.2.0
- name: Install dependencies
run: dart pub get
- name: Build test dylib and bindings
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/health.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@ jobs:
coverage_web: false
checks: "version,changelog,license,do-not-submit,breaking"
use-flutter: true
sdk: dev
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it work if you put an actual version number here instead of "dev"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the health check is running as intended, only the publish validation workflow needed the dev version. The health check fails due to a missing license on a golden file which never had a license header (and on the libclang bindings file, which probably shouldn't have a Dart license header).

Copy link
Collaborator

@dcharkes dcharkes Jan 4, 2024

Choose a reason for hiding this comment

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

Right, that should not have a header file.

Though I'm still seeing Dart versio issues on the CI?

Done, found 52 files without license headers
Collecting packages without changed changelogs:
Done, found 5 packages.
Collecting files without license headers in those packages:
Done, found 0 packages with a need for a changelog.
Look for changes in pkgs/ffigen with base ../../../base_repo/pkgs/ffigen
Error: Exception: The current Dart SDK version is 3.2.3.

Because ffigen requires SDK version >=3.3.0-252.0.dev <4.0.0, version solving failed.


You can try the following suggestion to make the pubspec resolve:
* Try using the Dart SDK version: 3.3.0. See https://dart.dev/get-dart.

[...]

Changed 55 dependencies!
1 package has newer versions incompatible with dependency constraints.
Try `dart pub outdated` for more information.
Preparing .
Copying sources from .
Preparing package dependencies for local package .
Resolving dependencies...
The current Dart SDK version is 3.2.3.

Because ffigen requires SDK version >=3.3.0-252.0.dev <4.0.0, version solving failed.


You can try the following suggestion to make the pubspec resolve:
* Try using the Dart SDK version: 3.3.0. See https://dart.dev/get-dart.

Unhandled exception:
PathNotFoundException: Cannot open file, path = 'pkgs/ffigen/report.json' (OS Error: No such file or directory, errno = 2)
#0      _File.throwIfError (dart:io/file_impl.dart:675:7)
#1      _File.openSync (dart:io/file_impl.dart:490:5)
#2      _File.readAsBytesSync (dart:io/file_impl.dart:574:18)
#3      _File.readAsStringSync (dart:io/file_impl.dart:624:18)
#4      Health.breakingCheck (package:firehose/src/health/health.dart:137:41)
#5      Health.healthCheck (package:firehose/src/health/health.dart:82:30)
<asynchronous suspension>
#6      main (file:///opt/hostedtoolcache/flutter/stable-3.16.5-x64/.pub-cache/hosted/pub.dev/firehose-0.4.2/bin/health.dart:32:3)
<asynchronous suspension>
Error: Process completed with exit code 255.

https://github.com/dart-lang/native/actions/runs/7402310627/job/20139919918?pr=860

@mosuem Any guidance on how to make the health check happy?

  • We have files in third_party/ folders which require a different license than the Dart one
  • In various places the health check fails to resolve the Dart SDK version.

(I'm almost inclined to ignore or disable the health check, as it seems too strict for this repo.)

Copy link
Member

Choose a reason for hiding this comment

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

I will check it out. Thanks for dogfooding the health workflow :)

permissions:
pull-requests: write
2 changes: 1 addition & 1 deletion .github/workflows/publish.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ jobs:
pull-requests: write # Required for writing the pull request note
with:
write-comments: false
sdk: beta
sdk: dev
6 changes: 6 additions & 0 deletions pkgs/ffigen/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 12.0.0-dev

- Global variables are now compatible with the `ffi-native` option.
- Exposing symbol addresses of functions and globals is now compatible with the
`ffi-native` option.

## 11.0.0

- Any compiler errors/warnings in source header files will now result in
Expand Down
12 changes: 12 additions & 0 deletions pkgs/ffigen/example/ffinative/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,16 @@ headers:
entry-points:
- 'headers/example.h'
preamble: |
// Copyright (c) 2020, 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.

// ignore_for_file: deprecated_member_use
functions:
symbol-address:
include:
- sum
globals:
symbol-address:
include:
- library_version
7 changes: 7 additions & 0 deletions pkgs/ffigen/example/ffinative/headers/example.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,10 @@ float *divide(int a, int b);

/** Divides 2 floats, returns a pointer to double. */
double *dividePrecision(float a, float b);

int log_level = -1;

const int array[5] = {0, 1, 2, 3, 4};

/** Version of the native C library */
const char* const library_version = "1.0.0-native";
35 changes: 30 additions & 5 deletions pkgs/ffigen/example/ffinative/lib/generated_bindings.dart
Original file line number Diff line number Diff line change
@@ -1,22 +1,26 @@
// Copyright (c) 2020, 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.

// ignore_for_file: deprecated_member_use

// AUTO GENERATED FILE, DO NOT EDIT.
//
// Generated by `package:ffigen`.
// ignore_for_file: type=lint
import 'dart:ffi' as ffi;
import '' as self;

/// Adds 2 integers.
@ffi.Native<ffi.Int Function(ffi.Int, ffi.Int)>(
symbol: 'sum', assetId: 'package:ffinative_example/generated_bindings.dart')
assetId: 'package:ffinative_example/generated_bindings.dart')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice, reusing the symbol.

Side note which does not have to be addressed in this CL: Should we consider generating

@DefaultAsset('package:ffinative_example/generated_bindings.dart')
library;

instead of an assetId in every @Native?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should do that, especially considering that there can only be a single assetId in the config file at the moment. But even if we want different asset ids based on symbol patterns, we can always specify the id for those symbols only.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please file an issue for us to do this later, or add it in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the assetId to generate a @DefaultAsset annotation instead.

external int sum(
int a,
int b,
);

/// Subtracts 2 integers.
@ffi.Native<ffi.Int Function(ffi.Int, ffi.Int)>(
symbol: 'subtract',
assetId: 'package:ffinative_example/generated_bindings.dart')
external int subtract(
int a,
Expand All @@ -25,7 +29,6 @@ external int subtract(

/// Multiplies 2 integers, returns pointer to an integer,.
@ffi.Native<ffi.Pointer<ffi.Int> Function(ffi.Int, ffi.Int)>(
symbol: 'multiply',
assetId: 'package:ffinative_example/generated_bindings.dart')
external ffi.Pointer<ffi.Int> multiply(
int a,
Expand All @@ -34,7 +37,6 @@ external ffi.Pointer<ffi.Int> multiply(

/// Divides 2 integers, returns pointer to a float.
@ffi.Native<ffi.Pointer<ffi.Float> Function(ffi.Int, ffi.Int)>(
symbol: 'divide',
assetId: 'package:ffinative_example/generated_bindings.dart')
external ffi.Pointer<ffi.Float> divide(
int a,
Expand All @@ -43,9 +45,32 @@ external ffi.Pointer<ffi.Float> divide(

/// Divides 2 floats, returns a pointer to double.
@ffi.Native<ffi.Pointer<ffi.Double> Function(ffi.Float, ffi.Float)>(
symbol: 'dividePrecision',
assetId: 'package:ffinative_example/generated_bindings.dart')
external ffi.Pointer<ffi.Double> dividePrecision(
double a,
double b,
);

@ffi.Native<ffi.Int>(
assetId: 'package:ffinative_example/generated_bindings.dart')
external int log_level;

@ffi.Array.multi([5])
@ffi.Native<ffi.Array<ffi.Int>>(
assetId: 'package:ffinative_example/generated_bindings.dart')
external ffi.Array<ffi.Int> array;

/// Version of the native C library
@ffi.Native<ffi.Pointer<ffi.Char>>(
assetId: 'package:ffinative_example/generated_bindings.dart')
external final ffi.Pointer<ffi.Char> library_version;

const addresses = _SymbolAddresses();

class _SymbolAddresses {
const _SymbolAddresses();
ffi.Pointer<ffi.NativeFunction<ffi.Int Function(ffi.Int, ffi.Int)>> get sum =>
ffi.Native.addressOf(self.sum);
simolus3 marked this conversation as resolved.
Show resolved Hide resolved
ffi.Pointer<ffi.Pointer<ffi.Char>> get library_version =>
ffi.Native.addressOf(self.library_version);
}
2 changes: 1 addition & 1 deletion pkgs/ffigen/example/ffinative/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
name: ffinative_example

environment:
sdk: '>=3.2.0 <4.0.0'
sdk: '>=3.3.0-252.0.dev <4.0.0'

dependencies:
ffi: ^2.0.1
Expand Down
15 changes: 2 additions & 13 deletions pkgs/ffigen/lib/src/code_generator/compound.dart
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,6 @@ abstract class Compound extends BindingType {
}
}

List<int> _getArrayDimensionLengths(Type type) {
final array = <int>[];
var startType = type;
while (startType is ConstantArray) {
array.add(startType.length);
startType = startType.child;
}
return array;
}

String _getInlineArrayTypeString(Type type, Writer w) {
if (type is ConstantArray) {
return '${w.ffiLibraryPrefix}.Array<'
Expand Down Expand Up @@ -132,9 +122,8 @@ abstract class Compound extends BindingType {
s.writeAll(m.dartDoc!.split('\n'), '\n$depth/// ');
s.write('\n');
}
if (m.type is ConstantArray) {
s.write('$depth@${w.ffiLibraryPrefix}.Array.multi(');
s.write('${_getArrayDimensionLengths(m.type)})\n');
if (m.type case final ConstantArray arrayType) {
s.writeln(makeArrayAnnotation(w, arrayType));
s.write('${depth}external ${_getInlineArrayTypeString(m.type, w)} ');
s.write('${m.name};\n\n');
} else {
Expand Down
22 changes: 17 additions & 5 deletions pkgs/ffigen/lib/src/code_generator/func.dart
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,16 @@ class Func extends LookUpBinding {
}

if (ffiNativeConfig.enabled) {
final assetString = ffiNativeConfig.assetId != null
? ", assetId: '${ffiNativeConfig.assetId}'"
: '';
final isLeafString = isLeaf ? ', isLeaf:true' : '';
final nativeFuncName = needsWrapper ? funcVarName : enclosingFuncName;
s.write('''
@${w.ffiLibraryPrefix}.Native<$cType>(symbol: '$originalName'$assetString$isLeafString)
${makeNativeAnnotation(
w,
nativeType: cType,
dartName: nativeFuncName,
nativeSymbolName: originalName,
assetId: ffiNativeConfig.assetId,
isLeaf: isLeaf,
)}
external $ffiReturnType $nativeFuncName($ffiArgDeclString);

''');
Expand All @@ -164,6 +167,15 @@ $dartReturnType $enclosingFuncName($libArg$dartArgDeclString) => $funcImplCall;

''');
}

if (exposeSymbolAddress) {
// Add to SymbolAddress in writer.
w.symbolAddressWriter.addNativeSymbol(
type:
'${w.ffiLibraryPrefix}.Pointer<${w.ffiLibraryPrefix}.NativeFunction<$cType>>',
name: name,
);
}
} else {
funcPointerName = w.wrapperLevelUniqueNamer.makeUnique('_${name}Ptr');
final isLeafString = isLeaf ? 'isLeaf:true' : '';
Expand Down
77 changes: 55 additions & 22 deletions pkgs/ffigen/lib/src/code_generator/global.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
// 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 '../config_provider/config_types.dart';
import 'binding.dart';
import 'binding_string.dart';
import 'compound.dart';
import 'pointer.dart';
import 'type.dart';
import 'utils.dart';
import 'writer.dart';
Expand All @@ -22,6 +24,7 @@ import 'writer.dart';
class Global extends LookUpBinding {
final Type type;
final bool exposeSymbolAddress;
final FfiNativeConfig nativeConfig;
final bool constant;

Global({
Expand All @@ -32,6 +35,7 @@ class Global extends LookUpBinding {
super.dartDoc,
this.exposeSymbolAddress = false,
this.constant = false,
this.nativeConfig = const FfiNativeConfig(enabled: false),
});

@override
Expand All @@ -41,35 +45,64 @@ class Global extends LookUpBinding {
if (dartDoc != null) {
s.write(makeDartDoc(dartDoc!));
}
final pointerName = w.wrapperLevelUniqueNamer.makeUnique('_$globalVarName');
final dartType = type.getFfiDartType(w);
final cType = type.getCType(w);

s.write(
"late final ${w.ffiLibraryPrefix}.Pointer<$cType> $pointerName = ${w.lookupFuncIdentifier}<$cType>('$originalName');\n\n");
final baseTypealiasType = type.typealiasType;
if (baseTypealiasType is Compound) {
if (baseTypealiasType.isOpaque) {
s.write(
'${w.ffiLibraryPrefix}.Pointer<$cType> get $globalVarName => $pointerName;\n\n');
} else {
s.write('$dartType get $globalVarName => $pointerName.ref;\n\n');
if (nativeConfig.enabled) {
if (type case final ConstantArray arr) {
s.writeln(makeArrayAnnotation(w, arr));
}

s
..writeln(makeNativeAnnotation(
w,
nativeType: cType,
assetId: nativeConfig.assetId,
dartName: globalVarName,
nativeSymbolName: originalName,
isLeaf: false,
))
..write('external ');
if (constant) {
s.write('final ');
simolus3 marked this conversation as resolved.
Show resolved Hide resolved
}

s.writeln('$dartType $globalVarName;\n');

if (exposeSymbolAddress) {
w.symbolAddressWriter.addNativeSymbol(
type: '${w.ffiLibraryPrefix}.Pointer<$cType>', name: name);
}
} else {
s.write('$dartType get $globalVarName => $pointerName.value;\n\n');
if (!constant) {
s.write(
'set $globalVarName($dartType value) => $pointerName.value = value;\n\n');
final pointerName =
w.wrapperLevelUniqueNamer.makeUnique('_$globalVarName');

s.write(
"late final ${w.ffiLibraryPrefix}.Pointer<$cType> $pointerName = ${w.lookupFuncIdentifier}<$cType>('$originalName');\n\n");
final baseTypealiasType = type.typealiasType;
if (baseTypealiasType is Compound) {
if (baseTypealiasType.isOpaque) {
s.write(
'${w.ffiLibraryPrefix}.Pointer<$cType> get $globalVarName => $pointerName;\n\n');
} else {
s.write('$dartType get $globalVarName => $pointerName.ref;\n\n');
}
} else {
s.write('$dartType get $globalVarName => $pointerName.value;\n\n');
if (!constant) {
s.write(
'set $globalVarName($dartType value) => $pointerName.value = value;\n\n');
}
}
}

if (exposeSymbolAddress) {
// Add to SymbolAddress in writer.
w.symbolAddressWriter.addSymbol(
type: '${w.ffiLibraryPrefix}.Pointer<$cType>',
name: name,
ptrName: pointerName,
);
if (exposeSymbolAddress) {
// Add to SymbolAddress in writer.
w.symbolAddressWriter.addSymbol(
type: '${w.ffiLibraryPrefix}.Pointer<$cType>',
name: name,
ptrName: pointerName,
);
}
}

return BindingString(type: BindingStringType.global, string: s.toString());
Expand Down
1 change: 1 addition & 0 deletions pkgs/ffigen/lib/src/code_generator/imports.dart
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class SelfImportedType extends Type {

final ffiImport = LibraryImport('ffi', 'dart:ffi');
final ffiPkgImport = LibraryImport('pkg_ffi', 'package:ffi/ffi.dart');
final self = LibraryImport('self', '');

final voidType = ImportedType(ffiImport, 'Void', 'void');

Expand Down
Loading
Loading