Skip to content

Commit

Permalink
Enum integer type via libclang. (#1187)
Browse files Browse the repository at this point in the history
  • Loading branch information
mannprerak2 authored Jun 14, 2024
1 parent cc29975 commit 125af4a
Show file tree
Hide file tree
Showing 23 changed files with 461 additions and 166 deletions.
4 changes: 4 additions & 0 deletions pkgs/ffigen/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
values are handled properly, and are equal to each other in Dart as well.
- Rename ObjC interface methods that clash with type names. Fixes
https://github.com/dart-lang/native/issues/1007.
- __Breaking change__: Enum integer types are implementation-defined and not
part of the ABI. Therefore FFIgen does a best-effort approach trying to mimic
the most common compilers for the various OS and architecture combinations.
To silence the warning set config `silence-enum-warning` to `true`.

## 12.0.0

Expand Down
14 changes: 14 additions & 0 deletions pkgs/ffigen/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,21 @@ and/or via the command line -
```bash
dart run ffigen --ignore-source-errors
```
</td>
</tr>
<tr>
<td>silence-enum-warning</td>
<td>Where to silence warning for enum integer type mimicking.<br>
The integer type used for enums is implementation-defined, and not part of
the ABI. FFIgen tries to mimic the integer sizes chosen by the most common
compilers for the various OS and architecture combinations.<br>
<b>Default: false</b>
</td>
<td>

```yaml
silence-enum-warning: true
```
</td>
</tr>
<tr>
Expand Down
141 changes: 81 additions & 60 deletions pkgs/ffigen/example/libclang-example/generated_bindings.dart

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions pkgs/ffigen/ffigen.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,9 @@
]
}
]
},
"silence-enum-warning": {
"type": "boolean"
}
},
"required": [
Expand Down
14 changes: 10 additions & 4 deletions pkgs/ffigen/lib/src/code_generator/enum_class.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import 'package:collection/collection.dart';

import 'binding.dart';
import 'binding_string.dart';
import 'native_type.dart';
import 'imports.dart';
import 'type.dart';
import 'utils.dart';
import 'writer.dart';
Expand Down Expand Up @@ -37,7 +37,8 @@ import 'writer.dart';
/// }
/// ```
class EnumClass extends BindingType {
static final nativeType = NativeType(SupportedNativeType.Int32);
/// Backing integer type for this enum.
Type nativeType;

/// The amount of indentation in every line.
static const depth = ' ';
Expand All @@ -53,8 +54,10 @@ class EnumClass extends BindingType {
super.originalName,
required super.name,
super.dartDoc,
Type? nativeType,
List<EnumConstant>? enumConstants,
}) : enumConstants = enumConstants ?? [],
}) : nativeType = nativeType ?? intType,
enumConstants = enumConstants ?? [],
namer = UniqueNamer({name});

/// The names of all the enum members generated by [namer].
Expand Down Expand Up @@ -225,7 +228,10 @@ class EnumClass extends BindingType {
}

@override
String getCType(Writer w) => nativeType.getCType(w);
String getCType(Writer w) {
w.usedEnumCType = true;
return nativeType.getCType(w);
}

@override
String getFfiDartType(Writer w) => nativeType.getFfiDartType(w);
Expand Down
2 changes: 2 additions & 0 deletions pkgs/ffigen/lib/src/code_generator/library.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class Library {
bool generateForPackageObjectiveC = false,
StructPackingOverride? packingOverride,
Set<LibraryImport>? libraryImports,
bool silenceEnumWarning = false,
}) {
_findBindings(bindings, sort);

Expand Down Expand Up @@ -84,6 +85,7 @@ class Library {
header: header,
additionalImports: libraryImports,
generateForPackageObjectiveC: generateForPackageObjectiveC,
silenceEnumWarning: silenceEnumWarning,
);
}

Expand Down
16 changes: 16 additions & 0 deletions pkgs/ffigen/lib/src/code_generator/writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ class Writer {

final bool generateForPackageObjectiveC;

/// Tracks where enumType.getCType is called. Reset everytime [generate] is
/// called.
bool usedEnumCType = false;

String? _ffiLibraryPrefix;
String get ffiLibraryPrefix {
if (_ffiLibraryPrefix != null) {
Expand Down Expand Up @@ -110,6 +114,8 @@ class Writer {
bool get canGenerateSymbolOutput => _canGenerateSymbolOutput;
bool _canGenerateSymbolOutput = false;

final bool silenceEnumWarning;

/// [_usedUpNames] should contain names of all the declarations which are
/// already used. This is used to avoid name collisions.
Writer({
Expand All @@ -122,6 +128,7 @@ class Writer {
this.classDocComment,
this.header,
required this.generateForPackageObjectiveC,
required this.silenceEnumWarning,
}) {
final globalLevelNameSet = noLookUpBindings.map((e) => e.name).toSet();
final wrapperLevelNameSet = lookUpBindings.map((e) => e.name).toSet();
Expand Down Expand Up @@ -230,6 +237,9 @@ class Writer {
// Reset unique namers to initial state.
_resetUniqueNamersNamers();

// Reset [usedEnumCType].
usedEnumCType = false;

// Write file header (if any).
if (header != null) {
result.writeln(header);
Expand Down Expand Up @@ -314,6 +324,12 @@ class Writer {
}
result.write(s);

// Warn about Enum usage in API surface.
if (!silenceEnumWarning && usedEnumCType) {
_logger.severe(
"The integer type used for enums is implementation-defined. FFIgen tries to mimic the integer sizes chosen by the most common compilers for the various OS and architecture combinations. To prevent any crashes, remove the enums from your API surface. To rely on the (unsafe!) mimicking, you can silence this warning by adding silence-enum-warning: true to the FFIgen config.");
}

_canGenerateSymbolOutput = true;
return result.toString();
}
Expand Down
10 changes: 10 additions & 0 deletions pkgs/ffigen/lib/src/config_provider/config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,10 @@ class Config {
bool get useDartHandle => _useDartHandle;
late bool _useDartHandle;

/// Where to silence warning for enum integer type mimicking.
bool get silenceEnumWarning => _silenceEnumWarning;
late bool _silenceEnumWarning;

Includer get exposeFunctionTypedefs => _exposeFunctionTypedefs;
late Includer _exposeFunctionTypedefs;

Expand Down Expand Up @@ -687,6 +691,12 @@ class Config {
resultOrDefault: (node) =>
_ffiNativeConfig = (node.value) as FfiNativeConfig,
),
HeterogeneousMapEntry(
key: strings.silenceEnumWarning,
valueConfigSpec: BoolConfigSpec(),
defaultValue: (node) => false,
resultOrDefault: (node) => _silenceEnumWarning = node.value as bool,
),
],
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ class Clang {
}

late final _clang_getDiagnosticSeverityPtr =
_lookup<ffi.NativeFunction<ffi.Int32 Function(CXDiagnostic)>>(
_lookup<ffi.NativeFunction<ffi.UnsignedInt Function(CXDiagnostic)>>(
'clang_getDiagnosticSeverity');
late final _clang_getDiagnosticSeverity =
_clang_getDiagnosticSeverityPtr.asFunction<int Function(CXDiagnostic)>();
Expand Down Expand Up @@ -435,7 +435,7 @@ class Clang {
}

late final _clang_getCursorKindPtr =
_lookup<ffi.NativeFunction<ffi.Int32 Function(CXCursor)>>(
_lookup<ffi.NativeFunction<ffi.UnsignedInt Function(CXCursor)>>(
'clang_getCursorKind');
late final _clang_getCursorKind =
_clang_getCursorKindPtr.asFunction<int Function(CXCursor)>();
Expand Down Expand Up @@ -529,6 +529,24 @@ class Clang {
_clang_getTypedefDeclUnderlyingTypePtr
.asFunction<CXType Function(CXCursor)>();

/// Retrieve the integer type of an enum declaration.
///
/// If the cursor does not reference an enum declaration, an invalid type is
/// returned.
CXType clang_getEnumDeclIntegerType(
CXCursor C,
) {
return _clang_getEnumDeclIntegerType(
C,
);
}

late final _clang_getEnumDeclIntegerTypePtr =
_lookup<ffi.NativeFunction<CXType Function(CXCursor)>>(
'clang_getEnumDeclIntegerType');
late final _clang_getEnumDeclIntegerType =
_clang_getEnumDeclIntegerTypePtr.asFunction<CXType Function(CXCursor)>();

/// Retrieve the integer value of an enum constant declaration as a signed
/// long long.
///
Expand Down Expand Up @@ -746,7 +764,7 @@ class Clang {
}

late final _clang_getTypeKindSpellingPtr =
_lookup<ffi.NativeFunction<CXString Function(ffi.Int32)>>(
_lookup<ffi.NativeFunction<CXString Function(ffi.UnsignedInt)>>(
'clang_getTypeKindSpelling');
late final _clang_getTypeKindSpelling =
_clang_getTypeKindSpellingPtr.asFunction<CXString Function(int)>();
Expand Down Expand Up @@ -918,7 +936,7 @@ class Clang {
}

late final _clang_Type_getNullabilityPtr =
_lookup<ffi.NativeFunction<ffi.Int32 Function(CXType)>>(
_lookup<ffi.NativeFunction<ffi.UnsignedInt Function(CXType)>>(
'clang_Type_getNullability');
late final _clang_Type_getNullability =
_clang_Type_getNullabilityPtr.asFunction<int Function(CXType)>();
Expand Down Expand Up @@ -1010,7 +1028,7 @@ class Clang {
}

late final _clang_Cursor_getStorageClassPtr =
_lookup<ffi.NativeFunction<ffi.Int32 Function(CXCursor)>>(
_lookup<ffi.NativeFunction<ffi.UnsignedInt Function(CXCursor)>>(
'clang_Cursor_getStorageClass');
late final _clang_Cursor_getStorageClass =
_clang_Cursor_getStorageClassPtr.asFunction<int Function(CXCursor)>();
Expand Down Expand Up @@ -1254,7 +1272,7 @@ class Clang {
}

late final _clang_getCursorKindSpellingPtr =
_lookup<ffi.NativeFunction<CXString Function(ffi.Int32)>>(
_lookup<ffi.NativeFunction<CXString Function(ffi.UnsignedInt)>>(
'clang_getCursorKindSpelling');
late final _clang_getCursorKindSpelling =
_clang_getCursorKindSpellingPtr.asFunction<CXString Function(int)>();
Expand Down Expand Up @@ -1297,7 +1315,7 @@ class Clang {
}

late final _clang_EvalResult_getKindPtr =
_lookup<ffi.NativeFunction<ffi.Int32 Function(CXEvalResult)>>(
_lookup<ffi.NativeFunction<ffi.UnsignedInt Function(CXEvalResult)>>(
'clang_EvalResult_getKind');
late final _clang_EvalResult_getKind =
_clang_EvalResult_getKindPtr.asFunction<int Function(CXEvalResult)>();
Expand Down Expand Up @@ -2462,7 +2480,7 @@ abstract class CXCursorKind {
/// to the entity that resides at that location, allowing one to map from the
/// source code into the AST.
final class CXCursor extends ffi.Struct {
@ffi.Int32()
@ffi.UnsignedInt()
external int kind;

@ffi.Int()
Expand Down Expand Up @@ -2604,7 +2622,7 @@ abstract class CXTypeKind {

/// The type of an element in the abstract syntax tree.
final class CXType extends ffi.Struct {
@ffi.Int32()
@ffi.UnsignedInt()
external int kind;

@ffi.Array.multi([2])
Expand Down Expand Up @@ -2697,7 +2715,7 @@ abstract class CXChildVisitResult {
/// to direct clang_visitCursorChildren().
typedef CXCursorVisitor
= ffi.Pointer<ffi.NativeFunction<CXCursorVisitorFunction>>;
typedef CXCursorVisitorFunction = ffi.Int32 Function(
typedef CXCursorVisitorFunction = ffi.UnsignedInt Function(
CXCursor cursor, CXCursor parent, CXClientData client_data);
typedef DartCXCursorVisitorFunction = int Function(
CXCursor cursor, CXCursor parent, CXClientData client_data);
Expand Down
1 change: 1 addition & 0 deletions pkgs/ffigen/lib/src/header_parser/parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Library parse(Config c) {
generateForPackageObjectiveC: c.generateForPackageObjectiveC,
packingOverride: c.structPackingOverride,
libraryImports: c.libraryImports.values.toSet(),
silenceEnumWarning: c.silenceEnumWarning,
);

return library;
Expand Down
38 changes: 30 additions & 8 deletions pkgs/ffigen/lib/src/header_parser/sub_parsers/enumdecl_parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import 'package:ffigen/src/code_generator.dart';
import 'package:ffigen/src/header_parser/data.dart';
import 'package:ffigen/src/header_parser/sub_parsers/unnamed_enumdecl_parser.dart';
import 'package:ffigen/src/header_parser/type_extractor/cxtypekindmap.dart';
import 'package:logging/logging.dart';

import '../clang_bindings/clang_bindings.dart' as clang_types;
Expand All @@ -13,13 +14,15 @@ import '../utils.dart';

final _logger = Logger('ffigen.header_parser.enumdecl_parser');

/// Parses an enum declaration.
EnumClass? parseEnumDeclaration(
/// Parses an enum declaration. Returns ([enumClass], [nativeType]). enumClass
/// is null for anonymouse enums.
(EnumClass? enumClass, Type nativeType) parseEnumDeclaration(
clang_types.CXCursor cursor, {
/// Option to ignore declaration filter (Useful in case of extracting
/// declarations when they are passed/returned by an included function.)
bool ignoreFilter = false,
}) {
EnumClass? enumClass;
// Parse the cursor definition instead, if this is a forward declaration.
cursor = cursorIndex.getDefinition(cursor);

Expand All @@ -34,24 +37,32 @@ EnumClass? parseEnumDeclaration(
} else {
enumName = '';
}
var nativeType = clang.clang_getEnumDeclIntegerType(cursor).toCodeGenType();
// Change to unsigned type by default.
nativeType = signedToUnsignedNativeIntType[nativeType] ?? nativeType;
bool hasNegativeEnumConstants = false;

if (enumName.isEmpty) {
_logger.fine('Saving anonymous enum.');
saveUnNamedEnum(cursor);
final addedConstants = saveUnNamedEnum(cursor);
hasNegativeEnumConstants =
addedConstants.where((c) => c.rawValue.startsWith("-")).isNotEmpty;
} else if (ignoreFilter || shouldIncludeEnumClass(enumUsr, enumName)) {
_logger.fine('++++ Adding Enum: ${cursor.completeStringRepr()}');
final enumClass = EnumClass(
enumClass = EnumClass(
usr: enumUsr,
dartDoc: getCursorDocComment(cursor),
originalName: enumName,
name: config.enumClassDecl.renameUsingConfig(enumName),
nativeType: nativeType,
);
cursor.visitChildren((clang_types.CXCursor child) {
try {
_logger.finest(' enumCursorVisitor: ${child.completeStringRepr()}');
switch (clang.clang_getCursorKind(child)) {
case clang_types.CXCursorKind.CXCursor_EnumConstantDecl:
enumClass.enumConstants.add(
final enumIntValue = clang.clang_getEnumConstantDeclValue(child);
enumClass!.enumConstants.add(
EnumConstant(
dartDoc: getCursorDocComment(
child,
Expand All @@ -62,8 +73,11 @@ EnumClass? parseEnumDeclaration(
enumClass.originalName,
child.spelling(),
),
value: clang.clang_getEnumConstantDeclValue(child)),
value: enumIntValue),
);
if (enumIntValue < 0) {
hasNegativeEnumConstants = true;
}
break;
case clang_types.CXCursorKind.CXCursor_UnexposedAttr:
// Ignore.
Expand All @@ -77,7 +91,15 @@ EnumClass? parseEnumDeclaration(
rethrow;
}
});
return enumClass;
}
return null;

if (hasNegativeEnumConstants) {
// Change enum native type to signed type.
_logger.fine(
'For enum $enumUsr - using signed type for $nativeType : ${unsignedToSignedNativeIntType[nativeType]}');
nativeType = unsignedToSignedNativeIntType[nativeType] ?? nativeType;
enumClass?.nativeType = nativeType;
}

return (enumClass, nativeType);
}
Loading

0 comments on commit 125af4a

Please sign in to comment.