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

pkgs/ffigen/CHANGELOG.md

@@ -1,3 +1,10 @@
+## 11.0.0-wip
+
+- Any compiler errors/warnings in source header files will now result in
+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

pkgs/ffigen/README.md

@@ -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>

pkgs/ffigen/doc/errors.md

@@ -0,0 +1,26 @@
+# Errors in ffigen
+
+This file documents various errors and their potential fixes related to ffigen.
+
+## Errors in source header files
+
+FFIgen uses libclang to parse header files. Any compiler warnings/errors should be logged (with SEVERE level).
+Compiler errors and warnings should be resolved as they can potentially generate invalid bindings that might cause silent errors and crashes at runtime.
+
+> You can pass in args to libclang using  `compiler-opts` via cmd line or yaml config or both.
+
+Here we'll list some common usecases. You can find the full list of [supported args here](https://clang.llvm.org/docs/ClangCommandLineReference.html#id5).
+
+### Missing headers
+
+These are the most common source file errors. You can specify [include paths to clang](https://clang.llvm.org/docs/ClangCommandLineReference.html#id6) like this 
+```yaml
+compiler-opts:
+  - "-I/path/to/folder"
+```
+
+### Ignoring source errors
+
+As a last resort, you can pass in `--ignore-source-errors` or set `ignore-source-errors: true` in yaml config.
+
+**Warning: This will likely lead to incorrect bindings!**

pkgs/ffigen/ffigen.schema.json

@@ -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,
@@ -73,6 +73,9 @@
         "entry-points"
       ]
     },
+    "ignore-source-errors": {
+      "type": "boolean"
+    },
     "compiler-opts": {
       "$oneOf": [
         {

pkgs/ffigen/lib/src/config_provider/config.dart

@@ -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.
@@ -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>>(

pkgs/ffigen/lib/src/config_provider/config_spec.dart

@@ -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,

pkgs/ffigen/lib/src/executables/ffigen.dart

@@ -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';
@@ -87,6 +88,10 @@ Config getConfig(ArgResults result, PackageConfig? packageConfig) {
         highPriority: true);
   }
 
+  if (result.wasParsed(ignoreSourceErrors)) {
+    config.ignoreSourceErrors = true;
+  }
+
   return config;
 }
 
@@ -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 {

pkgs/ffigen/lib/src/header_parser/clang_bindings/clang_bindings.dart

@@ -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
@@ -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
@@ -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

pkgs/ffigen/lib/src/header_parser/data.dart

@@ -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.
+bool hasSourceErrors = false;
+
 void initializeGlobals({required Config config}) {
   _config = config;
   _clang = Clang(DynamicLibrary.open(config.libclangDylib));
@@ -54,4 +58,5 @@ void initializeGlobals({required Config config}) {
   _cursorIndex = CursorIndex();
   _bindingsIndex = BindingsIndex();
   _objCBuiltInFunctions = ObjCBuiltInFunctions();
+  hasSourceErrors = false;
 }

pkgs/ffigen/lib/src/header_parser/parser.dart

@@ -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,
@@ -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;
@@ -113,6 +113,21 @@ List<Binding> parseToBindings() {
     tuList.add(tu);
   }
 
+  if (hasSourceErrors) {
+    _logger.warning("The compiler found warnings/errors in source files.");
+    _logger.warning("This will likely generate invalid bindings.");
+    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. See https://github.com/dart-lang/native/blob/main/pkgs/ffigen/doc/errors.md.");
+      exit(1);
+    }
+  }
+
   final tuCursors =
       tuList.map((tu) => clang.clang_getTranslationUnitCursor(tu));
 

pkgs/ffigen/lib/src/header_parser/utils.dart

@@ -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

pkgs/ffigen/lib/src/strings.dart

@@ -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.

pkgs/ffigen/pubspec.yaml

@@ -3,7 +3,7 @@
 # BSD-style license that can be found in the LICENSE file.
 
 name: ffigen
-version: 10.0.0
+version: 11.0.0-wip
 description: >
   Generator for FFI bindings, using LibClang to parse C, Objective-C, and Swift
   files.

pkgs/ffigen/test/header_parser_tests/forward_decl_test.dart

@@ -23,6 +23,7 @@ ${strings.output}: 'unused'
 ${strings.headers}:
   ${strings.entryPoints}:
     - 'test/header_parser_tests/forward_decl.h'
+${strings.ignoreSourceErrors}: true
         '''),
       );
     });

pkgs/ffigen/test/header_parser_tests/unions_test.dart

@@ -23,6 +23,7 @@ ${strings.output}: 'unused'
 ${strings.headers}:
   ${strings.entryPoints}:
     - 'test/header_parser_tests/unions.h'
+${strings.ignoreSourceErrors}: true
         '''),
       );
     });

pkgs/ffigen/tool/libclang_config.yaml

@@ -61,6 +61,7 @@ functions:
     - clang_disposeIndex
     - clang_getNumDiagnostics
     - clang_getDiagnostic
+    - clang_getDiagnosticSeverity
     - clang_disposeDiagnostic
     - clang_parseTranslationUnit
     - clang_disposeTranslationUnit