Skip to content

Commit

Permalink
ffigen: Don't generate setters for constant globals (#828)
Browse files Browse the repository at this point in the history
  • Loading branch information
simolus3 authored Nov 24, 2023
1 parent 13b6b7e commit c371539
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 9 deletions.
1 change: 1 addition & 0 deletions pkgs/ffigen/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
bindings to **not** be generated by default, since it may result in invalid
bindings if the compiler makes a wrong guess. A flag `--ignore-source-errors` (or yaml config `ignore-source-errors: true`)
must be passed to change this behaviour.
- __Breaking change__: Stop generating setters for global variables marked `const` in C.

## 10.0.0

Expand Down
8 changes: 6 additions & 2 deletions pkgs/ffigen/lib/src/code_generator/global.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import 'writer.dart';
class Global extends LookUpBinding {
final Type type;
final bool exposeSymbolAddress;
final bool constant;

Global({
super.usr,
Expand All @@ -30,6 +31,7 @@ class Global extends LookUpBinding {
required this.type,
super.dartDoc,
this.exposeSymbolAddress = false,
this.constant = false,
});

@override
Expand All @@ -55,8 +57,10 @@ class Global extends LookUpBinding {
}
} else {
s.write('$dartType get $globalVarName => $pointerName.value;\n\n');
s.write(
'set $globalVarName($dartType value) => $pointerName.value = value;\n\n');
if (!constant) {
s.write(
'set $globalVarName($dartType value) => $pointerName.value = value;\n\n');
}
}

if (exposeSymbolAddress) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,23 @@ class Clang {
late final _clang_getCanonicalType =
_clang_getCanonicalTypePtr.asFunction<CXType Function(CXType)>();

/// Determine whether a CXType has the "const" qualifier set,
/// without looking through typedefs that may have added "const" at a
/// different level.
int clang_isConstQualifiedType(
CXType T,
) {
return _clang_isConstQualifiedType(
T,
);
}

late final _clang_isConstQualifiedTypePtr =
_lookup<ffi.NativeFunction<ffi.UnsignedInt Function(CXType)>>(
'clang_isConstQualifiedType');
late final _clang_isConstQualifiedType =
_clang_isConstQualifiedTypePtr.asFunction<int Function(CXType)>();

/// Determine whether a CXCursor that is a macro, is
/// function like.
int clang_Cursor_isMacroFunctionLike(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ Global? parseVarDeclaration(clang_types.CXCursor cursor) {

_logger.fine('++++ Adding Global: ${cursor.completeStringRepr()}');

final type = cursor.type().toCodeGenType();
final cType = cursor.type();
final type = cType.toCodeGenType();
if (type.baseType is UnimplementedType) {
_logger.fine('---- Removed Global, reason: unsupported type: '
'${cursor.completeStringRepr()}');
Expand All @@ -46,6 +47,7 @@ Global? parseVarDeclaration(clang_types.CXCursor cursor) {
type: type,
dartDoc: getCursorDocComment(cursor),
exposeSymbolAddress: config.functionDecl.shouldIncludeSymbolAddress(name),
constant: cType.isConstQualified,
);
bindingsIndex.addGlobalVarToSeen(usr, global);
return global;
Expand Down
4 changes: 4 additions & 0 deletions pkgs/ffigen/lib/src/header_parser/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,10 @@ extension CXTypeExt on clang_types.CXType {
'(Type) spelling: ${spelling()}, kind: ${kind()}, kindSpelling: ${kindSpelling()}';
return s;
}

bool get isConstQualified {
return clang.clang_isConstQualifiedType(this) != 0;
}
}

extension CXStringExt on clang_types.CXString {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ void main() {
SupportedNativeType.Float,
),
),
constant: true,
),
structSome,
Global(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ class Bindings {

ffi.Pointer<ffi.Float> get test2 => _test2.value;

set test2(ffi.Pointer<ffi.Float> value) => _test2.value = value;

late final ffi.Pointer<ffi.Pointer<Some>> _test5 =
_lookup<ffi.Pointer<Some>>('test5');

Expand Down
5 changes: 4 additions & 1 deletion pkgs/ffigen/test/header_parser_tests/globals.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@

bool coolGlobal;
int32_t myInt;
int32_t *aGlobalPointer;
int32_t *aGlobalPointer0;
int32_t *const aGlobalPointer1;
const int32_t *aGlobalPointer2;
const int32_t *const aGlobalPointer3;
long double longDouble;
long double *pointerToLongDouble;

Expand Down
42 changes: 39 additions & 3 deletions pkgs/ffigen/test/header_parser_tests/globals_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ ${strings.globals}:
- pointerToLongDouble
- globalStruct
${strings.compilerOpts}: '-Wno-nullability-completeness'
${strings.ignoreSourceErrors}: true
'''),
);
});
Expand All @@ -48,8 +49,14 @@ ${strings.compilerOpts}: '-Wno-nullability-completeness'
expected.getBindingAsString('coolGlobal'));
expect(actual.getBindingAsString('myInt'),
expected.getBindingAsString('myInt'));
expect(actual.getBindingAsString('aGlobalPointer'),
expected.getBindingAsString('aGlobalPointer'));
expect(actual.getBindingAsString('aGlobalPointer0'),
expected.getBindingAsString('aGlobalPointer0'));
expect(actual.getBindingAsString('aGlobalPointer1'),
expected.getBindingAsString('aGlobalPointer1'));
expect(actual.getBindingAsString('aGlobalPointer2'),
expected.getBindingAsString('aGlobalPointer2'));
expect(actual.getBindingAsString('aGlobalPointer3'),
expected.getBindingAsString('aGlobalPointer3'));
});

test('Ignore global values', () {
Expand All @@ -60,6 +67,18 @@ ${strings.compilerOpts}: '-Wno-nullability-completeness'
expect(() => actual.getBindingAsString('pointerToLongDouble'),
throwsA(TypeMatcher<NotFoundException>()));
});

test('identifies constant globals', () {
final globalPointers = [
for (var i = 0; i < 4; i++)
actual.getBinding('aGlobalPointer$i') as Global,
];

expect(globalPointers[0].constant, isFalse); // int32_t*
expect(globalPointers[1].constant, isTrue); // int32_t* const
expect(globalPointers[2].constant, isFalse); // const int32_t*
expect(globalPointers[3].constant, isTrue); // const int32_t* const
});
});
}

Expand All @@ -76,8 +95,25 @@ Library expectedLibrary() {
),
Global(
type: PointerType(NativeType(SupportedNativeType.Int32)),
name: 'aGlobalPointer',
name: 'aGlobalPointer0',
exposeSymbolAddress: true,
),
Global(
type: PointerType(NativeType(SupportedNativeType.Int32)),
name: 'aGlobalPointer1',
exposeSymbolAddress: true,
constant: true,
),
Global(
type: PointerType(NativeType(SupportedNativeType.Int32)),
name: 'aGlobalPointer2',
exposeSymbolAddress: true,
),
Global(
type: PointerType(NativeType(SupportedNativeType.Int32)),
name: 'aGlobalPointer3',
exposeSymbolAddress: true,
constant: true,
),
globalStruct,
Global(
Expand Down
1 change: 1 addition & 0 deletions pkgs/ffigen/tool/libclang_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ functions:
- clang_Cursor_getArgument
- clang_getNumArgTypes
- clang_getArgType
- clang_isConstQualifiedType
- clang_isFunctionTypeVariadic
- clang_Cursor_getStorageClass
- clang_getCursorResultType
Expand Down

0 comments on commit c371539

Please sign in to comment.