Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ffigen] Add config ignore-source-errors #810

Merged
merged 19 commits into from
Nov 21, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions pkgs/ffigen/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## 10.1.0
mannprerak2 marked this conversation as resolved.
Show resolved Hide resolved

- Any compiler errors/warnings in source header files will now result in
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What kind of warnings result in wrong generated code? Should we refuse to generate on all warnings?
If we refuse to generate for too many warnings, people might just start using ignore-source-errors instead of fixing the issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, for now I've only handled warnings/errors from libclang. (Excluding ObjC for now)

But I think anything which can potentially generate invalid bindings, that might silently break at runtime should be covered.

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.

## 10.0.0

- Stable release targeting Dart 3.2 using new `dart:ffi` features available
Expand Down
18 changes: 18 additions & 0 deletions pkgs/ffigen/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,24 @@ use-supported-typedefs: true
```yaml
use-dart-handle: true
```

</td>
</tr>
<tr>
<td>ignore-source-errors</td>
<td>Where to ignore compiler warnings/errors in source header files.<br>
<b>Default: false</b>
</td>
<td>

```yaml
ignore-source-errors: true
```
and/or via the command line -
```bash
dart run ffigen --ignore-source-errors
```

</td>
</tr>
<tr>
Expand Down
5 changes: 4 additions & 1 deletion pkgs/ffigen/ffigen.schema.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"$id": "https://json.schemastore.org/ffigen",
"$comment": "This file is generated. To regenerate run: dart tool/generate_json_schema.dart in github.com/dart-lang/ffigen",
"$comment": "This file is generated. To regenerate run: dart tool/generate_json_schema.dart in github.com/dart-lang/native/tree/main/pkgs/ffigen",
"$schema": "https://json-schema.org/draft/2020-12/schema",
"type": "object",
"additionalProperties": false,
Expand Down Expand Up @@ -73,6 +73,9 @@
"entry-points"
]
},
"ignore-source-errors": {
"type": "boolean"
},
"compiler-opts": {
"$oneOf": [
{
Expand Down
12 changes: 12 additions & 0 deletions pkgs/ffigen/lib/src/config_provider/config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,9 @@ class Config {
FfiNativeConfig get ffiNativeConfig => _ffiNativeConfig;
late FfiNativeConfig _ffiNativeConfig;

/// Where to ignore compiler warnings/errors in source header files.
bool ignoreSourceErrors = false;

Config._({required this.filename, required this.packageConfig});

/// Create config from Yaml map.
Expand Down Expand Up @@ -285,6 +288,15 @@ class Config {
transform: (node) => headersExtractor(node.value, filename),
result: (node) => _headers = node.value,
)),
HeterogeneousMapEntry(
key: strings.ignoreSourceErrors,
valueConfigSpec: BoolConfigSpec(),
defaultValue: (node) => false,
resultOrDefault: (node) {
// Set value to true if not already.
ignoreSourceErrors = ignoreSourceErrors || node.value as bool;
},
),
HeterogeneousMapEntry(
key: strings.compilerOpts,
valueConfigSpec: OneOfConfigSpec<List<String>, List<String>>(
Expand Down
2 changes: 1 addition & 1 deletion pkgs/ffigen/lib/src/config_provider/config_spec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ abstract class ConfigSpec<TE extends Object?, RE extends Object?> {
return {
r"$id": schemaId,
r"$comment":
"This file is generated. To regenerate run: dart tool/generate_json_schema.dart in github.com/dart-lang/ffigen",
"This file is generated. To regenerate run: dart tool/generate_json_schema.dart in github.com/dart-lang/native/tree/main/pkgs/ffigen",
r"$schema": "https://json-schema.org/draft/2020-12/schema",
...schemaMap,
r"$defs": defs,
Expand Down
10 changes: 10 additions & 0 deletions pkgs/ffigen/lib/src/executables/ffigen.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ final _logger = Logger('ffigen.ffigen');
final _ansi = Ansi(Ansi.terminalSupportsAnsi);

const compilerOpts = 'compiler-opts';
const ignoreSourceErrors = 'ignore-source-errors';
const conf = 'config';
const help = 'help';
const verbose = 'verbose';
Expand Down Expand Up @@ -87,6 +88,10 @@ Config getConfig(ArgResults result, PackageConfig? packageConfig) {
highPriority: true);
}

if (result.wasParsed(ignoreSourceErrors)) {
config.ignoreSourceErrors = true;
}

return config;
}

Expand Down Expand Up @@ -158,6 +163,11 @@ ArgResults getArgResults(List<String> args) {
compilerOpts,
help: 'Compiler options for clang. (E.g --$compilerOpts "-I/headers -W")',
);
parser.addFlag(
ignoreSourceErrors,
help: 'Ignore any compiler warnings/errors in source header files',
negatable: false,
);

ArgResults results;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,21 @@ class Clang {
late final _clang_formatDiagnostic = _clang_formatDiagnosticPtr
.asFunction<CXString Function(CXDiagnostic, int)>();

/// Determine the severity of the given diagnostic.
int clang_getDiagnosticSeverity(
CXDiagnostic arg0,
) {
return _clang_getDiagnosticSeverity(
arg0,
);
}

late final _clang_getDiagnosticSeverityPtr =
_lookup<ffi.NativeFunction<ffi.Int32 Function(CXDiagnostic)>>(
'clang_getDiagnosticSeverity');
late final _clang_getDiagnosticSeverity =
_clang_getDiagnosticSeverityPtr.asFunction<int Function(CXDiagnostic)>();

/// Same as \c clang_parseTranslationUnit2, but returns
/// the \c CXTranslationUnit instead of an error code. In case of an error this
/// routine returns a \c NULL \c CXTranslationUnit, without further detailed
Expand Down Expand Up @@ -1484,6 +1499,29 @@ abstract class CXDiagnosticDisplayOptions {
static const int CXDiagnostic_DisplayCategoryName = 32;
}

/// Describes the severity of a particular diagnostic.
abstract class CXDiagnosticSeverity {
/// A diagnostic that has been suppressed, e.g., by a command-line
/// option.
static const int CXDiagnostic_Ignored = 0;

/// This diagnostic is a note that should be attached to the
/// previous (non-note) diagnostic.
static const int CXDiagnostic_Note = 1;

/// This diagnostic indicates suspicious code that may not be
/// wrong.
static const int CXDiagnostic_Warning = 2;

/// This diagnostic indicates that the code is ill-formed.
static const int CXDiagnostic_Error = 3;

/// This diagnostic indicates that the code is ill-formed such
/// that future parser recovery is unlikely to produce useful
/// results.
static const int CXDiagnostic_Fatal = 4;
}

/// Flags that control the creation of translation units.
///
/// The enumerators in this enumeration type are meant to be bitwise
Expand Down Expand Up @@ -2641,8 +2679,10 @@ abstract class CXChildVisitResult {
/// The visitor should return one of the \c CXChildVisitResult values
/// to direct clang_visitCursorChildren().
typedef CXCursorVisitor
= ffi.Pointer<ffi.NativeFunction<CXCursorVisitor_function>>;
typedef CXCursorVisitor_function = ffi.Int32 Function(
= ffi.Pointer<ffi.NativeFunction<CXCursorVisitorFunction>>;
typedef CXCursorVisitorFunction = ffi.Int32 Function(
CXCursor cursor, CXCursor parent, CXClientData client_data);
typedef DartCXCursorVisitorFunction = int Function(
CXCursor cursor, CXCursor parent, CXClientData client_data);

/// Opaque pointer representing client data that will be passed through
Expand Down
5 changes: 5 additions & 0 deletions pkgs/ffigen/lib/src/header_parser/data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ List<Constant> _unnamedEnumConstants = [];
ObjCBuiltInFunctions get objCBuiltInFunctions => _objCBuiltInFunctions;
late ObjCBuiltInFunctions _objCBuiltInFunctions;

/// Tracks if any source error/warning has occured which can potentially cause
/// invalid generated bindings.
mannprerak2 marked this conversation as resolved.
Show resolved Hide resolved
bool hasSourceErrors = false;

void initializeGlobals({required Config config}) {
_config = config;
_clang = Clang(DynamicLibrary.open(config.libclangDylib));
Expand All @@ -54,4 +58,5 @@ void initializeGlobals({required Config config}) {
_cursorIndex = CursorIndex();
_bindingsIndex = BindingsIndex();
_objCBuiltInFunctions = ObjCBuiltInFunctions();
hasSourceErrors = false;
}
19 changes: 17 additions & 2 deletions pkgs/ffigen/lib/src/header_parser/parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import 'utils.dart';
Library parse(Config c) {
initParser(c);

final bindings = parseToBindings();
final bindings = parseToBindings(c);

final library = Library(
bindings: bindings,
Expand Down Expand Up @@ -52,7 +52,7 @@ void initParser(Config c) {
}

/// Parses source files and adds generated bindings to [bindings].
List<Binding> parseToBindings() {
List<Binding> parseToBindings(Config c) {
final index = clang.clang_createIndex(0, 0);

Pointer<Pointer<Utf8>> clangCmdArgs = nullptr;
Expand Down Expand Up @@ -113,6 +113,21 @@ List<Binding> parseToBindings() {
tuList.add(tu);
}

if (hasSourceErrors) {
_logger.warning(
"The compiler found some warnings/errors in source files. This might generate invalid bindings due to a wrong compiler guess.");
mannprerak2 marked this conversation as resolved.
Show resolved Hide resolved
if (config.ignoreSourceErrors) {
_logger.warning(
"Ignored source errors. (User supplied --ignore-source-errors)");
} else if (config.language == Language.objc) {
_logger.warning("Ignored source errors. (ObjC)");
} else {
_logger.severe(
"Skipped generating bindings due to errors in source files. Either resolve or ignore them (Set --ignore-source-errors on cmd or ignore-source-errors:true in config.");
mannprerak2 marked this conversation as resolved.
Show resolved Hide resolved
exit(1);
}
}

final tuCursors =
tuList.map((tu) => clang.clang_getTranslationUnitCursor(tu));

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 @@ -39,6 +39,10 @@ void logTuDiagnostics(
logger.log(logLevel, 'Header $header: Total errors/warnings: $total.');
for (var i = 0; i < total; i++) {
final diag = clang.clang_getDiagnostic(tu, i);
if (clang.clang_getDiagnosticSeverity(diag) >=
clang_types.CXDiagnosticSeverity.CXDiagnostic_Warning) {
hasSourceErrors = true;
}
final cxstring = clang.clang_formatDiagnostic(
diag,
clang_types
Expand Down
1 change: 1 addition & 0 deletions pkgs/ffigen/lib/src/strings.dart
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ const supportedNativeType_mappings = <String, SupportedNativeType>{
const sort = 'sort';
const useSupportedTypedefs = 'use-supported-typedefs';
const useDartHandle = 'use-dart-handle';
const ignoreSourceErrors = 'ignore-source-errors';

const comments = 'comments';
// Sub-fields of comments.
Expand Down
2 changes: 1 addition & 1 deletion pkgs/ffigen/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# BSD-style license that can be found in the LICENSE file.

name: ffigen
version: 10.0.0
version: 10.1.0
description: >
Generator for FFI bindings, using LibClang to parse C, Objective-C, and Swift
files.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ ${strings.output}: 'unused'
${strings.headers}:
${strings.entryPoints}:
- 'test/header_parser_tests/forward_decl.h'
${strings.ignoreSourceErrors}: true
'''),
);
});
Expand Down
1 change: 1 addition & 0 deletions pkgs/ffigen/test/header_parser_tests/unions_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ ${strings.output}: 'unused'
${strings.headers}:
${strings.entryPoints}:
- 'test/header_parser_tests/unions.h'
${strings.ignoreSourceErrors}: true
'''),
);
});
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 @@ -61,6 +61,7 @@ functions:
- clang_disposeIndex
- clang_getNumDiagnostics
- clang_getDiagnostic
- clang_getDiagnosticSeverity
mannprerak2 marked this conversation as resolved.
Show resolved Hide resolved
- clang_disposeDiagnostic
- clang_parseTranslationUnit
- clang_disposeTranslationUnit
Expand Down