Skip to content

Commit

Permalink
Add the --emit-diagnostics flag (#2436)
Browse files Browse the repository at this point in the history
* Add the `--emit-diagnostics` flag

* remove test

* add(diagnostics): var.rs and lib.rs module have a function each

* add(diagnostics): rustfmt

* add(diagnostics): docs moved to book

* add(diagnostics): wip, saving location

* add(diagnostics): wip, saving location

* add(diagnostics): add line number and origin file to slice

* add(diagnostics): unused var name cleanup

* revert(diagnostics): changes to line number, before merging Christian's

* Add API for slice locations

* Emit diagnostic for unsupported ABIs

* add(diagnostics): line number and file names to diagnostics

* chore: fmt

* add(diagnostics): label to say 'from bindgen'

* add(diagnostics): add_footer, add a warning with footer info

* add(diagnostics): add a warning with footer info

* add(diagnostics): add a warning in the Diagnostics instead of copying everywhere

* Remove IDs that are not shown and use `diagnostic` instead of `warning`

* Remove unused function

* Deduplciate log entry

* Introduce `Level`

* Fix clippy warning

* Simplify duplicated macro diagnostic

* Unify source extraction

* Simplify non-fatal rustfmt error diagnostic

* Print all diagnostic lines when using `cargo:warning`

* Put all the diagnostic logic under the experimental flag

* Avoid `bool` parameters

* Clean rebase debris

* Disable duplicated macros warning for now

* Emit warning when using a deprecated Rust target

* Document the `emit_diagnostics` method

* Update Changelog

* Remove diagnostics section from book.

Diagnostics are prone to change so keeping the book up to date might
require some automation.

* Run rustfmt

* Fix clippy warning

* Remove file added by accident

---------

Co-authored-by: Amanjeev Sethi <aj@amanjeev.com>
  • Loading branch information
pvdrz and amanjeev committed Apr 3, 2023
1 parent 040149b commit 2ab66fd
Show file tree
Hide file tree
Showing 12 changed files with 615 additions and 83 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,9 @@
which tool will be used to format the bindings. The
`Formatter::Prettyplease` variant is only available if the
`"prettyplease"` feature is enabled.
* Added the `Builder::emit_diagnostics` method and the `--emit-diagnostics`
flag to enable emission of diagnostic messages.

## Changed
* Static functions with no arguments use `void` as their single argument
instead of having no arguments when the `--wrap-static-fns` flag is used.
Expand All @@ -187,6 +190,8 @@
* The following deprecated flags were removed: `--use-msvc-mangling`,
`--rustfmt-bindings` and `--size_t-is-usize`.
* The `--no-rustfmt-bindings` flag was removed in favor of `--formatter=none`.
* The `Bindings::emit_warnings` and `Bindings::warnings` methods were removed
in favor of `--emit-diagnostics`.

## Fixed

Expand Down
26 changes: 26 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 27 additions & 6 deletions bindgen-cli/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,9 @@ struct BindgenCommand {
/// bitfields. This flag is ignored if the `--respect-cxx-access-specs` flag is used.
#[arg(long, value_name = "VISIBILITY")]
default_visibility: Option<FieldVisibilityKind>,
/// Whether to emit diagnostics or not.
#[arg(long, requires = "experimental")]
emit_diagnostics: bool,
/// Enables experimental features.
#[arg(long)]
experimental: bool,
Expand Down Expand Up @@ -495,6 +498,7 @@ where
wrap_static_fns_path,
wrap_static_fns_suffix,
default_visibility,
emit_diagnostics,
experimental: _,
version,
clang_args,
Expand Down Expand Up @@ -997,12 +1001,25 @@ where
}
}

for (custom_derives, kind) in [
(with_derive_custom, None),
(with_derive_custom_struct, Some(TypeKind::Struct)),
(with_derive_custom_enum, Some(TypeKind::Enum)),
(with_derive_custom_union, Some(TypeKind::Union)),
for (custom_derives, kind, name) in [
(with_derive_custom, None, "--with-derive-custom"),
(
with_derive_custom_struct,
Some(TypeKind::Struct),
"--with-derive-custom-struct",
),
(
with_derive_custom_enum,
Some(TypeKind::Enum),
"--with-derive-custom-enum",
),
(
with_derive_custom_union,
Some(TypeKind::Union),
"--with-derive-custom-union",
),
] {
let name = emit_diagnostics.then(|| name);
for custom_derive in custom_derives {
let (regex, derives) = custom_derive
.rsplit_once('=')
Expand All @@ -1011,7 +1028,7 @@ where

let mut regex_set = RegexSet::new();
regex_set.insert(regex);
regex_set.build(false);
regex_set.build_with_diagnostics(false, name);

builder = builder.parse_callbacks(Box::new(CustomDeriveCallback {
derives,
Expand All @@ -1037,5 +1054,9 @@ where
builder = builder.default_visibility(visibility);
}

if emit_diagnostics {
builder = builder.emit_diagnostics();
}

Ok((builder, output, verbose))
}
16 changes: 0 additions & 16 deletions bindgen-tests/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -672,22 +672,6 @@ fn dump_preprocessed_input() {
);
}

#[test]
fn allowlist_warnings() {
let header = concat!(
env!("CARGO_MANIFEST_DIR"),
"/tests/headers/allowlist_warnings.h"
);

let bindings = builder()
.header(header)
.allowlist_function("doesnt_match_anything")
.generate()
.expect("unable to generate bindings");

assert_eq!(1, bindings.warnings().len());
}

fn build_flags_output_helper(builder: &bindgen::Builder) {
let mut command_line_flags = builder.command_line_flags();
command_line_flags.insert(0, "bindgen".to_string());
Expand Down
5 changes: 3 additions & 2 deletions bindgen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ quote = { version = "1", default-features = false }
syn = { version = "2.0", features = ["full", "extra-traits", "visit-mut"]}
regex = { version = "1.5", default-features = false , features = ["std", "unicode"] }
which = { version = "4.2.1", optional = true, default-features = false }
prettyplease = { version = "0.2.0", optional = true}
prettyplease = { version = "0.2.0", optional = true }
annotate-snippets = { version = "0.9.1", features = ["color"], optional = true }
shlex = "1"
rustc-hash = "1.0.1"
proc-macro2 = { version = "1", default-features = false }
Expand All @@ -49,7 +50,7 @@ runtime = ["clang-sys/runtime"]
# Dynamically discover a `rustfmt` binary using the `which` crate
which-rustfmt = ["which"]
__cli = []
experimental = []
experimental = ["annotate-snippets"]

# These features only exist for CI testing -- don't use them if you're not hacking
# on bindgen!
Expand Down
76 changes: 70 additions & 6 deletions bindgen/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4213,23 +4213,43 @@ impl CodeGenerator for Function {
ClangAbi::Known(Abi::ThisCall)
if !ctx.options().rust_features().thiscall_abi =>
{
warn!("Skipping function with thiscall ABI that isn't supported by the configured Rust target");
unsupported_abi_diagnostic::<false>(
name,
item.location(),
"thiscall",
ctx,
);
return None;
}
ClangAbi::Known(Abi::Vectorcall)
if !ctx.options().rust_features().vectorcall_abi =>
{
warn!("Skipping function with vectorcall ABI that isn't supported by the configured Rust target");
unsupported_abi_diagnostic::<false>(
name,
item.location(),
"vectorcall",
ctx,
);
return None;
}
ClangAbi::Known(Abi::CUnwind)
if !ctx.options().rust_features().c_unwind_abi =>
{
warn!("Skipping function with C-unwind ABI that isn't supported by the configured Rust target");
unsupported_abi_diagnostic::<false>(
name,
item.location(),
"C-unwind",
ctx,
);
return None;
}
ClangAbi::Known(Abi::Win64) if signature.is_variadic() => {
warn!("Skipping variadic function with Win64 ABI that isn't supported");
unsupported_abi_diagnostic::<true>(
name,
item.location(),
"Win64",
ctx,
);
return None;
}
ClangAbi::Unknown(unknown_abi) => {
Expand Down Expand Up @@ -4316,6 +4336,51 @@ impl CodeGenerator for Function {
}
}

fn unsupported_abi_diagnostic<const VARIADIC: bool>(
fn_name: &str,
location: Option<&crate::clang::SourceLocation>,
abi: &str,
ctx: &BindgenContext,
) {
warn!(
"Skipping {}function `{}` with the {} ABI that isn't supported by the configured Rust target",
if VARIADIC { "variadic " } else { "" },
fn_name,
abi
);

#[cfg(feature = "experimental")]
if ctx.options().emit_diagnostics {
use crate::diagnostics::{get_line, Diagnostic, Level, Slice};

let mut diag = Diagnostic::default();
diag
.with_title(format!(
"The `{}` {}function uses the {} ABI which is not supported by the configured Rust target",
fn_name,
if VARIADIC { "variadic " } else { "" },
abi), Level::Warn)
.add_annotation("No code will be generated for this function.", Level::Warn)
.add_annotation(format!("The configured Rust version is {}.", String::from(ctx.options().rust_target)), Level::Note);

if let Some(loc) = location {
let (file, line, col, _) = loc.location();

if let Some(filename) = file.name() {
if let Ok(Some(source)) = get_line(&filename, line) {
let mut slice = Slice::default();
slice
.with_source(source)
.with_location(filename, line, col);
diag.add_slice(slice);
}
}
}

diag.display()
}
}

fn objc_method_codegen(
ctx: &BindgenContext,
method: &ObjCMethod,
Expand Down Expand Up @@ -4586,8 +4651,7 @@ impl CodeGenerator for ObjCInterface {

pub(crate) fn codegen(
context: BindgenContext,
) -> Result<(proc_macro2::TokenStream, BindgenOptions, Vec<String>), CodegenError>
{
) -> Result<(proc_macro2::TokenStream, BindgenOptions), CodegenError> {
context.gen(|context| {
let _t = context.timer("codegen");
let counter = Cell::new(0);
Expand Down
Loading

0 comments on commit 2ab66fd

Please sign in to comment.