From edd99ed9f4b486048dba02d8cad27439ddce11f2 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Fri, 24 Nov 2023 15:01:29 +0100 Subject: [PATCH 1/2] Don't generate setters for const globals --- pkgs/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 ++++ pkgs/ffigen/test/header_parser_tests/globals.h | 5 ++++- .../test/header_parser_tests/globals_test.dart | 1 + pkgs/ffigen/tool/libclang_config.yaml | 1 + 7 files changed, 36 insertions(+), 4 deletions(-) diff --git a/pkgs/ffigen/lib/src/code_generator/global.dart b/pkgs/ffigen/lib/src/code_generator/global.dart index 81f61a8e6b..47aee0de1e 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 1de29c93ba..bbe226e5ce 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 90b38533bb..bea3e8091a 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 423d7cdba0..d142370f23 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/header_parser_tests/globals.h b/pkgs/ffigen/test/header_parser_tests/globals.h index 5ce3fc524f..664c9cdc02 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 55184a8608..b2f710acb0 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 '''), ); }); diff --git a/pkgs/ffigen/tool/libclang_config.yaml b/pkgs/ffigen/tool/libclang_config.yaml index 90a3206b6d..7343442a57 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 From 74a5df747860ca7bd74784a728ada1eaa5cc8431 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Fri, 24 Nov 2023 17:38:14 +0100 Subject: [PATCH 2/2] Add tests --- pkgs/ffigen/CHANGELOG.md | 1 + .../code_generator_test.dart | 1 + .../_expected_global_bindings.dart | 2 - .../ffigen/test/header_parser_tests/globals.h | 4 +- .../header_parser_tests/globals_test.dart | 41 +++++++++++++++++-- 5 files changed, 42 insertions(+), 7 deletions(-) diff --git a/pkgs/ffigen/CHANGELOG.md b/pkgs/ffigen/CHANGELOG.md index 08ba80a32d..178656d27f 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/test/code_generator_tests/code_generator_test.dart b/pkgs/ffigen/test/code_generator_tests/code_generator_test.dart index e16d3d9dea..f81460e539 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 37081b8634..ab9c3485a9 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 664c9cdc02..4d4b770276 100644 --- a/pkgs/ffigen/test/header_parser_tests/globals.h +++ b/pkgs/ffigen/test/header_parser_tests/globals.h @@ -8,9 +8,9 @@ bool coolGlobal; int32_t myInt; int32_t *aGlobalPointer0; -int32_t * const aGlobalPointer1; +int32_t *const aGlobalPointer1; const int32_t *aGlobalPointer2; -const int32_t * const aGlobalPointer3; +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 b2f710acb0..1c2964a42c 100644 --- a/pkgs/ffigen/test/header_parser_tests/globals_test.dart +++ b/pkgs/ffigen/test/header_parser_tests/globals_test.dart @@ -49,8 +49,14 @@ ${strings.ignoreSourceErrors}: true 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', () { @@ -61,6 +67,18 @@ ${strings.ignoreSourceErrors}: true 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 + }); }); } @@ -77,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(