From c371539db2d4895230aa1d4d3c9f5f94c827ebff Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Fri, 24 Nov 2023 18:42:44 +0100 Subject: [PATCH] ffigen: Don't generate setters for constant globals (#828) --- pkgs/ffigen/CHANGELOG.md | 1 + .../ffigen/lib/src/code_generator/global.dart | 8 +++- .../clang_bindings/clang_bindings.dart | 17 ++++++++ .../header_parser/sub_parsers/var_parser.dart | 4 +- pkgs/ffigen/lib/src/header_parser/utils.dart | 4 ++ .../code_generator_test.dart | 1 + .../_expected_global_bindings.dart | 2 - .../ffigen/test/header_parser_tests/globals.h | 5 ++- .../header_parser_tests/globals_test.dart | 42 +++++++++++++++++-- pkgs/ffigen/tool/libclang_config.yaml | 1 + 10 files changed, 76 insertions(+), 9 deletions(-) diff --git a/pkgs/ffigen/CHANGELOG.md b/pkgs/ffigen/CHANGELOG.md index 08ba80a32..178656d27 100644 --- a/pkgs/ffigen/CHANGELOG.md +++ b/pkgs/ffigen/CHANGELOG.md @@ -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 diff --git a/pkgs/ffigen/lib/src/code_generator/global.dart b/pkgs/ffigen/lib/src/code_generator/global.dart index 81f61a8e6..47aee0de1 100644 --- a/pkgs/ffigen/lib/src/code_generator/global.dart +++ b/pkgs/ffigen/lib/src/code_generator/global.dart @@ -22,6 +22,7 @@ import 'writer.dart'; class Global extends LookUpBinding { final Type type; final bool exposeSymbolAddress; + final bool constant; Global({ super.usr, @@ -30,6 +31,7 @@ class Global extends LookUpBinding { required this.type, super.dartDoc, this.exposeSymbolAddress = false, + this.constant = false, }); @override @@ -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) { diff --git a/pkgs/ffigen/lib/src/header_parser/clang_bindings/clang_bindings.dart b/pkgs/ffigen/lib/src/header_parser/clang_bindings/clang_bindings.dart index 1de29c93b..bbe226e5c 100644 --- a/pkgs/ffigen/lib/src/header_parser/clang_bindings/clang_bindings.dart +++ b/pkgs/ffigen/lib/src/header_parser/clang_bindings/clang_bindings.dart @@ -626,6 +626,23 @@ class Clang { late final _clang_getCanonicalType = _clang_getCanonicalTypePtr.asFunction(); + /// 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>( + 'clang_isConstQualifiedType'); + late final _clang_isConstQualifiedType = + _clang_isConstQualifiedTypePtr.asFunction(); + /// Determine whether a CXCursor that is a macro, is /// function like. int clang_Cursor_isMacroFunctionLike( diff --git a/pkgs/ffigen/lib/src/header_parser/sub_parsers/var_parser.dart b/pkgs/ffigen/lib/src/header_parser/sub_parsers/var_parser.dart index 90b38533b..bea3e8091 100644 --- a/pkgs/ffigen/lib/src/header_parser/sub_parsers/var_parser.dart +++ b/pkgs/ffigen/lib/src/header_parser/sub_parsers/var_parser.dart @@ -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()}'); @@ -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; diff --git a/pkgs/ffigen/lib/src/header_parser/utils.dart b/pkgs/ffigen/lib/src/header_parser/utils.dart index 423d7cdba..d142370f2 100644 --- a/pkgs/ffigen/lib/src/header_parser/utils.dart +++ b/pkgs/ffigen/lib/src/header_parser/utils.dart @@ -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 { diff --git a/pkgs/ffigen/test/code_generator_tests/code_generator_test.dart b/pkgs/ffigen/test/code_generator_tests/code_generator_test.dart index e16d3d9de..f81460e53 100644 --- a/pkgs/ffigen/test/code_generator_tests/code_generator_test.dart +++ b/pkgs/ffigen/test/code_generator_tests/code_generator_test.dart @@ -250,6 +250,7 @@ void main() { SupportedNativeType.Float, ), ), + constant: true, ), structSome, Global( diff --git a/pkgs/ffigen/test/code_generator_tests/expected_bindings/_expected_global_bindings.dart b/pkgs/ffigen/test/code_generator_tests/expected_bindings/_expected_global_bindings.dart index 37081b863..ab9c3485a 100644 --- a/pkgs/ffigen/test/code_generator_tests/expected_bindings/_expected_global_bindings.dart +++ b/pkgs/ffigen/test/code_generator_tests/expected_bindings/_expected_global_bindings.dart @@ -29,8 +29,6 @@ class Bindings { ffi.Pointer get test2 => _test2.value; - set test2(ffi.Pointer value) => _test2.value = value; - late final ffi.Pointer> _test5 = _lookup>('test5'); diff --git a/pkgs/ffigen/test/header_parser_tests/globals.h b/pkgs/ffigen/test/header_parser_tests/globals.h index 5ce3fc524..4d4b77027 100644 --- a/pkgs/ffigen/test/header_parser_tests/globals.h +++ b/pkgs/ffigen/test/header_parser_tests/globals.h @@ -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; diff --git a/pkgs/ffigen/test/header_parser_tests/globals_test.dart b/pkgs/ffigen/test/header_parser_tests/globals_test.dart index 55184a860..1c2964a42 100644 --- a/pkgs/ffigen/test/header_parser_tests/globals_test.dart +++ b/pkgs/ffigen/test/header_parser_tests/globals_test.dart @@ -35,6 +35,7 @@ ${strings.globals}: - pointerToLongDouble - globalStruct ${strings.compilerOpts}: '-Wno-nullability-completeness' +${strings.ignoreSourceErrors}: true '''), ); }); @@ -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', () { @@ -60,6 +67,18 @@ ${strings.compilerOpts}: '-Wno-nullability-completeness' expect(() => actual.getBindingAsString('pointerToLongDouble'), throwsA(TypeMatcher())); }); + + 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 + }); }); } @@ -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( diff --git a/pkgs/ffigen/tool/libclang_config.yaml b/pkgs/ffigen/tool/libclang_config.yaml index 90a3206b6..7343442a5 100644 --- a/pkgs/ffigen/tool/libclang_config.yaml +++ b/pkgs/ffigen/tool/libclang_config.yaml @@ -94,6 +94,7 @@ functions: - clang_Cursor_getArgument - clang_getNumArgTypes - clang_getArgType + - clang_isConstQualifiedType - clang_isFunctionTypeVariadic - clang_Cursor_getStorageClass - clang_getCursorResultType