-
Notifications
You must be signed in to change notification settings - Fork 55
Conversation
@@ -5,7 +5,7 @@ import 'dart:ffi' as ffi; | |||
import 'package:ffi/ffi.dart' as pkg_ffi; | |||
|
|||
class Foo extends ffi.Struct { | |||
@BOOL() | |||
@ffi.Uint8() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do actually have a Bool
type in dart:ffi
, is it possible to target that instead?
sel, | ||
aSelector, | ||
) != | ||
0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do have a Bool
type in dart:ffi
, can we use that instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using the existing bool infrastructure for functions and structs. The code being generated here is consistent with existing cases:
https://github.com/dart-lang/ffigen/blob/master/test/code_generator_tests/expected_bindings/_expected_boolean_dartbool_bindings.dart
If you think this should generate Bool instead, that should probably be in a separate bug/PR. It would involve changing func.dart and compound.dart, and isn't really related to this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, that should be a separate PR if we don't target bool already.
Objective C's
BOOL
type can be eitherbool
orsigned char
, depending on the platform. We want to present a consistent API to the user, and those two types are ABI compatible, so just returnbool
regardless.Fixes dart-lang/native#297
Also fix the one other known inconsistency, with
va_list
, by excluding irrelevant stuff from the native_objc_test.Fixes dart-lang/native#309