From 2ab66fdffba5f2a03982f2cc3f7cdd98aa277b42 Mon Sep 17 00:00:00 2001 From: Christian Poveda Ruiz <31802960+pvdrz@users.noreply.github.com> Date: Mon, 3 Apr 2023 12:10:16 -0500 Subject: [PATCH] Add the `--emit-diagnostics` flag (#2436) * 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 --- CHANGELOG.md | 5 + Cargo.lock | 26 +++++ bindgen-cli/options.rs | 33 +++++-- bindgen-tests/tests/tests.rs | 16 --- bindgen/Cargo.toml | 5 +- bindgen/codegen/mod.rs | 76 ++++++++++++-- bindgen/diagnostics.rs | 185 +++++++++++++++++++++++++++++++++++ bindgen/ir/context.rs | 43 ++++---- bindgen/ir/var.rs | 48 ++++++++- bindgen/lib.rs | 124 ++++++++++++++++++----- bindgen/options/mod.rs | 28 ++++-- bindgen/regex_set.rs | 109 ++++++++++++++++++++- 12 files changed, 615 insertions(+), 83 deletions(-) create mode 100644 bindgen/diagnostics.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 96ecd945e8..ffdd28666d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. @@ -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 diff --git a/Cargo.lock b/Cargo.lock index 984295e86c..38cc59ff62 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11,10 +11,21 @@ dependencies = [ "memchr", ] +[[package]] +name = "annotate-snippets" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3b9d411ecbaf79885c6df4d75fff75858d5995ff25385657a28af47e82f9c36" +dependencies = [ + "unicode-width", + "yansi-term", +] + [[package]] name = "bindgen" version = "0.64.0" dependencies = [ + "annotate-snippets", "bitflags", "cexpr", "clang-sys", @@ -586,6 +597,12 @@ version = "1.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "84a22b9f218b40614adcb3f4ff08b703773ad44fa9423e4e0d346d5db86e4ebc" +[[package]] +name = "unicode-width" +version = "0.1.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c0edd1e5b14653f783770bce4a4dabb4a5108a5370a5f5d8cfe8710c361f6c8b" + [[package]] name = "version_check" version = "0.9.4" @@ -696,3 +713,12 @@ name = "windows_x86_64_msvc" version = "0.42.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "447660ad36a13288b1db4d4248e857b510e8c3a225c822ba4fb748c0aafecffd" + +[[package]] +name = "yansi-term" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fe5c30ade05e61656247b2e334a031dfd0cc466fadef865bdcdea8d537951bf1" +dependencies = [ + "winapi", +] diff --git a/bindgen-cli/options.rs b/bindgen-cli/options.rs index 468ea1cc56..3d87fc1bc6 100644 --- a/bindgen-cli/options.rs +++ b/bindgen-cli/options.rs @@ -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, + /// Whether to emit diagnostics or not. + #[arg(long, requires = "experimental")] + emit_diagnostics: bool, /// Enables experimental features. #[arg(long)] experimental: bool, @@ -495,6 +498,7 @@ where wrap_static_fns_path, wrap_static_fns_suffix, default_visibility, + emit_diagnostics, experimental: _, version, clang_args, @@ -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('=') @@ -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, @@ -1037,5 +1054,9 @@ where builder = builder.default_visibility(visibility); } + if emit_diagnostics { + builder = builder.emit_diagnostics(); + } + Ok((builder, output, verbose)) } diff --git a/bindgen-tests/tests/tests.rs b/bindgen-tests/tests/tests.rs index e27f29a6a6..7a03952196 100644 --- a/bindgen-tests/tests/tests.rs +++ b/bindgen-tests/tests/tests.rs @@ -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()); diff --git a/bindgen/Cargo.toml b/bindgen/Cargo.toml index 2b27f2a1fd..93275c1615 100644 --- a/bindgen/Cargo.toml +++ b/bindgen/Cargo.toml @@ -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 } @@ -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! diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index 508c1ed892..9d1956f37b 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -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::( + 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::( + 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::( + 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::( + name, + item.location(), + "Win64", + ctx, + ); return None; } ClangAbi::Unknown(unknown_abi) => { @@ -4316,6 +4336,51 @@ impl CodeGenerator for Function { } } +fn unsupported_abi_diagnostic( + 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, @@ -4586,8 +4651,7 @@ impl CodeGenerator for ObjCInterface { pub(crate) fn codegen( context: BindgenContext, -) -> Result<(proc_macro2::TokenStream, BindgenOptions, Vec), CodegenError> -{ +) -> Result<(proc_macro2::TokenStream, BindgenOptions), CodegenError> { context.gen(|context| { let _t = context.timer("codegen"); let counter = Cell::new(0); diff --git a/bindgen/diagnostics.rs b/bindgen/diagnostics.rs new file mode 100644 index 0000000000..82d5359d97 --- /dev/null +++ b/bindgen/diagnostics.rs @@ -0,0 +1,185 @@ +//! Types and function used to emit pretty diagnostics for `bindgen`. +//! +//! The entry point of this module is the [`Diagnostic`] type. + +use std::fmt::Write; +use std::io::{self, BufRead, BufReader}; +use std::{borrow::Cow, fs::File}; + +use annotate_snippets::{ + display_list::{DisplayList, FormatOptions}, + snippet::{Annotation, Slice as ExtSlice, Snippet}, +}; + +use annotate_snippets::snippet::AnnotationType; + +#[derive(Clone, Copy, Debug)] +pub(crate) enum Level { + Error, + Warn, + Info, + Note, + Help, +} + +impl From for AnnotationType { + fn from(level: Level) -> Self { + match level { + Level::Error => Self::Error, + Level::Warn => Self::Warning, + Level::Info => Self::Info, + Level::Note => Self::Note, + Level::Help => Self::Help, + } + } +} + +/// A `bindgen` diagnostic. +#[derive(Default)] +pub(crate) struct Diagnostic<'a> { + title: Option<(Cow<'a, str>, Level)>, + slices: Vec>, + footer: Vec<(Cow<'a, str>, Level)>, +} + +impl<'a> Diagnostic<'a> { + /// Add a title to the diagnostic and set its type. + pub(crate) fn with_title( + &mut self, + title: impl Into>, + level: Level, + ) -> &mut Self { + self.title = Some((title.into(), level)); + self + } + + /// Add a slice of source code to the diagnostic. + pub(crate) fn add_slice(&mut self, slice: Slice<'a>) -> &mut Self { + self.slices.push(slice); + self + } + + /// Add a footer annotation to the diagnostic. This annotation will have its own type. + pub(crate) fn add_annotation( + &mut self, + msg: impl Into>, + level: Level, + ) -> &mut Self { + self.footer.push((msg.into(), level)); + self + } + + /// Print this diagnostic. + /// + /// The diagnostic is printed using `cargo:warning` if `bindgen` is being invoked by a build + /// script or using `eprintln` otherwise. + pub(crate) fn display(&self) { + std::thread_local! { + static INVOKED_BY_BUILD_SCRIPT: bool = std::env::var_os("CARGO_CFG_TARGET_ARCH").is_some(); + } + + let mut title = None; + let mut footer = vec![]; + let mut slices = vec![]; + if let Some((msg, level)) = &self.title { + title = Some(Annotation { + id: Some("bindgen"), + label: Some(msg.as_ref()), + annotation_type: (*level).into(), + }) + } + + for (msg, level) in &self.footer { + footer.push(Annotation { + id: None, + label: Some(msg.as_ref()), + annotation_type: (*level).into(), + }); + } + + // add additional info that this is generated by bindgen + // so as to not confuse with rustc warnings + footer.push(Annotation { + id: None, + label: Some("This diagnostic was generated by bindgen."), + annotation_type: AnnotationType::Info, + }); + + for slice in &self.slices { + if let Some(source) = &slice.source { + slices.push(ExtSlice { + source: source.as_ref(), + line_start: slice.line.unwrap_or_default(), + origin: slice.filename.as_deref(), + annotations: vec![], + fold: false, + }) + } + } + + let snippet = Snippet { + title, + footer, + slices, + opt: FormatOptions { + color: true, + ..Default::default() + }, + }; + let dl = DisplayList::from(snippet); + + if INVOKED_BY_BUILD_SCRIPT.with(Clone::clone) { + let string = dl.to_string(); + for line in string.lines() { + println!("cargo:warning={}\n", line); + } + } else { + eprintln!("{}\n", dl); + } + } +} + +/// A slice of source code. +#[derive(Default)] +pub(crate) struct Slice<'a> { + source: Option>, + filename: Option, + line: Option, +} + +impl<'a> Slice<'a> { + /// Set the source code. + pub(crate) fn with_source( + &mut self, + source: impl Into>, + ) -> &mut Self { + self.source = Some(source.into()); + self + } + + /// Set the file, line and column. + pub(crate) fn with_location( + &mut self, + mut name: String, + line: usize, + col: usize, + ) -> &mut Self { + write!(name, ":{}:{}", line, col) + .expect("Writing to a string cannot fail"); + self.filename = Some(name); + self.line = Some(line); + self + } +} + +pub(crate) fn get_line( + filename: &str, + line: usize, +) -> io::Result> { + let file = BufReader::new(File::open(filename)?); + if let Some(line) = file.lines().nth(line.wrapping_sub(1)) { + return line.map(Some); + } + + Ok(None) +} diff --git a/bindgen/ir/context.rs b/bindgen/ir/context.rs index c6015ded72..a34f210bc7 100644 --- a/bindgen/ir/context.rs +++ b/bindgen/ir/context.rs @@ -477,9 +477,6 @@ pub(crate) struct BindgenContext { /// Populated when we enter codegen by `compute_has_float`; always `None` /// before that and `Some` after. has_float: Option>, - - /// The set of warnings raised during binding generation. - warnings: Vec, } /// A traversal of allowlisted items. @@ -595,7 +592,6 @@ If you encounter an error missing from this list, please file an issue or a PR!" have_destructor: None, has_type_param_in_array: None, has_float: None, - warnings: Vec::new(), } } @@ -1156,7 +1152,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" pub(crate) fn gen( mut self, cb: F, - ) -> Result<(Out, BindgenOptions, Vec), CodegenError> + ) -> Result<(Out, BindgenOptions), CodegenError> where F: FnOnce(&Self) -> Result, { @@ -1194,7 +1190,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" self.compute_cannot_derive_partialord_partialeq_or_eq(); let ret = cb(&self)?; - Ok((ret, self.options, self.warnings)) + Ok((ret, self.options)) } /// When the `testing_only_extra_assertions` feature is enabled, this @@ -2463,24 +2459,16 @@ If you encounter an error missing from this list, please file an issue or a PR!" self.allowlisted = Some(allowlisted); self.codegen_items = Some(codegen_items); - let mut warnings = Vec::new(); - for item in self.options().allowlisted_functions.unmatched_items() { - warnings - .push(format!("unused option: --allowlist-function {}", item)); + unused_regex_diagnostic(item, "--allowlist-function", self); } for item in self.options().allowlisted_vars.unmatched_items() { - warnings.push(format!("unused option: --allowlist-var {}", item)); + unused_regex_diagnostic(item, "--allowlist-var", self); } for item in self.options().allowlisted_types.unmatched_items() { - warnings.push(format!("unused option: --allowlist-type {}", item)); - } - - for msg in warnings { - warn!("{}", msg); - self.warnings.push(msg); + unused_regex_diagnostic(item, "--allowlist-type", self); } } @@ -2971,3 +2959,24 @@ impl TemplateParameters for PartialType { } } } + +fn unused_regex_diagnostic(item: &str, name: &str, ctx: &BindgenContext) { + warn!("unused option: {} {}", name, item); + + #[cfg(feature = "experimental")] + if ctx.options().emit_diagnostics { + use crate::diagnostics::{Diagnostic, Level, Slice}; + + let mut slice = Slice::default(); + slice.with_source(item); + + Diagnostic::default() + .with_title("unused regular expression:", Level::Warn) + .add_slice(slice) + .add_annotation( + format!("this regular expression was passed via `{}`", name), + Level::Note, + ) + .display(); + } +} diff --git a/bindgen/ir/var.rs b/bindgen/ir/var.rs index db20124cfb..c0bf4fc297 100644 --- a/bindgen/ir/var.rs +++ b/bindgen/ir/var.rs @@ -222,7 +222,7 @@ impl ClangSubItemParser for Var { if previously_defined { let name = String::from_utf8(id).unwrap(); - warn!("Duplicated macro definition: {}", name); + duplicated_macro_diagnostic(&name, cursor.location(), ctx); return Err(ParseError::Continue); } @@ -440,3 +440,49 @@ fn get_integer_literal_from_cursor(cursor: &clang::Cursor) -> Option { }); value } + +fn duplicated_macro_diagnostic( + macro_name: &str, + location: crate::clang::SourceLocation, + ctx: &BindgenContext, +) { + warn!("Duplicated macro definition: {}", macro_name); + + #[cfg(feature = "experimental")] + // FIXME (pvdrz & amanjeev): This diagnostic message shows way too often to be actually + // useful. We have to change the logic where this function is called to be able to emit this + // message only when the duplication is an actuall issue. + // + // If I understood correctly, `bindgen` ignores all `#undef` directives. Meaning that this: + // ```c + // #define FOO 1 + // #undef FOO + // #define FOO 2 + // ``` + // + // Will trigger this message even though there's nothing wrong with it. + #[allow(clippy::overly_complex_bool_expr)] + if false && ctx.options().emit_diagnostics { + use crate::diagnostics::{get_line, Diagnostic, Level, Slice}; + use std::borrow::Cow; + + let mut slice = Slice::default(); + let mut source = Cow::from(macro_name); + + let (file, line, col, _) = location.location(); + if let Some(filename) = file.name() { + if let Ok(Some(code)) = get_line(&filename, line) { + source = code.into(); + } + slice.with_location(filename, line, col); + } + + slice.with_source(source); + + Diagnostic::default() + .with_title("Duplicated macro definition", Level::Warn) + .add_slice(slice) + .add_annotation("This macro had a duplicate", Level::Note) + .display(); + } +} diff --git a/bindgen/lib.rs b/bindgen/lib.rs index 2b1d68079a..f40b7ffbf1 100644 --- a/bindgen/lib.rs +++ b/bindgen/lib.rs @@ -42,6 +42,8 @@ mod time; pub mod callbacks; mod clang; +#[cfg(feature = "experimental")] +mod diagnostics; mod features; mod ir; mod parse; @@ -404,7 +406,10 @@ impl Builder { impl BindgenOptions { fn build(&mut self) { - let regex_sets = [ + const REGEX_SETS_LEN: usize = 27; + let sets_len = REGEX_SETS_LEN + self.abi_overrides.len(); + + let regex_sets: [_; REGEX_SETS_LEN] = [ &mut self.allowlisted_vars, &mut self.allowlisted_types, &mut self.allowlisted_functions, @@ -433,10 +438,63 @@ impl BindgenOptions { &mut self.no_hash_types, &mut self.must_use_types, ]; + let record_matches = self.record_matches; + #[cfg(feature = "experimental")] + { + let names = if self.emit_diagnostics { + <[&str; 27]>::into_iter([ + "--blocklist-type", + "--blocklist-function", + "--blocklist-item", + "--blocklist-file", + "--opaque-type", + "--allowlist-type", + "--allowlist-function", + "--allowlist-var", + "--allowlist-file", + "--bitfield-enum", + "--newtype-enum", + "--newtype-global-enum", + "--rustified-enum", + "--rustified-enum-non-exhaustive", + "--constified-enum-module", + "--constified-enum", + "--type-alias", + "--new-type-alias", + "--new-type-alias-deref", + "--bindgen-wrapper-union", + "--manually-drop-union", + "--no-partialeq", + "--no-copy", + "--no-debug", + "--no-default", + "--no-hash", + "--must-use", + ]) + .chain((0..self.abi_overrides.len()).map(|_| "--override-abi")) + .map(Some) + .collect() + } else { + vec![None; sets_len] + }; + + for (regex_set, name) in + self.abi_overrides.values_mut().chain(regex_sets).zip(names) + { + regex_set.build_with_diagnostics(record_matches, name); + } + } + #[cfg(not(feature = "experimental"))] for regex_set in self.abi_overrides.values_mut().chain(regex_sets) { regex_set.build(record_matches); } + + let rust_target = self.rust_target; + #[allow(deprecated)] + if rust_target <= RustTarget::Stable_1_30 { + deprecated_target_diagnostic(rust_target, self); + } } /// Update rust target version @@ -481,6 +539,28 @@ impl BindgenOptions { } } +fn deprecated_target_diagnostic(target: RustTarget, options: &BindgenOptions) { + let target = String::from(target); + warn!("The {} Rust target is deprecated. If you have a good reason to use this target please report it at https://github.com/rust-lang/rust-bindgen/issues", target,); + + #[cfg(feature = "experimental")] + if options.emit_diagnostics { + use crate::diagnostics::{Diagnostic, Level}; + + let mut diagnostic = Diagnostic::default(); + diagnostic.with_title( + format!("The {} Rust target is deprecated.", target), + Level::Warn, + ); + diagnostic.add_annotation( + "This Rust target was passed as an argument to `--rust-target`", + Level::Info, + ); + diagnostic.add_annotation("If you have a good reason to use this target please report it at https://github.com/rust-lang/rust-bindgen/issues", Level::Help); + diagnostic.display(); + } +} + #[cfg(feature = "runtime")] fn ensure_libclang_is_loaded() { if clang_sys::is_loaded() { @@ -551,7 +631,6 @@ impl std::error::Error for BindgenError {} #[derive(Debug)] pub struct Bindings { options: BindgenOptions, - warnings: Vec, module: proc_macro2::TokenStream, } @@ -776,14 +855,10 @@ impl Bindings { parse(&mut context)?; } - let (module, options, warnings) = + let (module, options) = codegen::codegen(context).map_err(BindgenError::Codegen)?; - Ok(Bindings { - options, - warnings, - module, - }) + Ok(Bindings { options, module }) } /// Write these bindings as source text to a file. @@ -917,7 +992,10 @@ impl Bindings { "Rustfmt parsing errors.".to_string(), )), Some(3) => { - warn!("Rustfmt could not format some lines."); + rustfmt_non_fatal_error_diagnostic( + "Rustfmt could not format some lines", + &self.options, + ); Ok(bindings) } _ => Err(io::Error::new( @@ -928,22 +1006,22 @@ impl Bindings { _ => Ok(source), } } +} - /// Emit all the warning messages raised while generating the bindings in a build script. - /// - /// If you are using `bindgen` outside of a build script you should use [`Bindings::warnings`] - /// and handle the messages accordingly instead. - #[inline] - pub fn emit_warnings(&self) { - for message in &self.warnings { - println!("cargo:warning={}", message); - } - } +fn rustfmt_non_fatal_error_diagnostic(msg: &str, options: &BindgenOptions) { + warn!("{}", msg); - /// Return all the warning messages raised while generating the bindings. - #[inline] - pub fn warnings(&self) -> &[String] { - &self.warnings + #[cfg(feature = "experimental")] + if options.emit_diagnostics { + use crate::diagnostics::{Diagnostic, Level}; + + Diagnostic::default() + .with_title(msg, Level::Warn) + .add_annotation( + "The bindings will be generated but not formatted", + Level::Note, + ) + .display(); } } diff --git a/bindgen/options/mod.rs b/bindgen/options/mod.rs index d28d4aa96a..6c3de1240d 100644 --- a/bindgen/options/mod.rs +++ b/bindgen/options/mod.rs @@ -1583,13 +1583,6 @@ options! { /// /// The default target is the latest stable Rust version. pub fn rust_target(mut self, rust_target: RustTarget) -> Self { - #[allow(deprecated)] - if rust_target <= RustTarget::Stable_1_30 { - warn!( - "The {} rust target is deprecated. If you have a good reason to use this target please report it at https://github.com/rust-lang/rust-bindgen/issues", - String::from(rust_target) - ); - } self.options.set_rust_target(rust_target); self } @@ -2101,4 +2094,25 @@ options! { } }, }, + /// Whether to emit diagnostics or not. + emit_diagnostics: { + ty: bool, + methods: { + #[cfg(feature = "experimental")] + /// Emit diagnostics. + /// + /// These diagnostics are emitted to `stderr` if you are using `bindgen-cli` or printed + /// using `cargo:warning=` if you are using `bindgen` as a `build-dependency`. + /// + /// Diagnostics are not emitted by default. + /// + /// The layout and contents of these diagnostic messages are not covered by versioning + /// and can change without notice. + pub fn emit_diagnostics(mut self) -> Self { + self.options.emit_diagnostics = true; + self + } + }, + as_args: "--emit-diagnostics", + } } diff --git a/bindgen/regex_set.rs b/bindgen/regex_set.rs index f827107efd..f68ed8fa5f 100644 --- a/bindgen/regex_set.rs +++ b/bindgen/regex_set.rs @@ -32,11 +32,7 @@ impl RegexSet { where S: AsRef, { - let string = string.as_ref().to_owned(); - if string == "*" { - warn!("using wildcard patterns (`*`) is no longer considered valid. Use `.*` instead"); - } - self.items.push(string); + self.items.push(string.as_ref().to_owned()); self.matched.push(Cell::new(false)); self.set = None; } @@ -62,13 +58,56 @@ impl RegexSet { /// /// Must be called before calling `matches()`, or it will always return /// false. + #[inline] pub fn build(&mut self, record_matches: bool) { + self.build_inner(record_matches, None) + } + + #[cfg(all(feature = "__cli", feature = "experimental"))] + /// Construct a RegexSet from the set of entries we've accumulated and emit diagnostics if the + /// name of the regex set is passed to it. + /// + /// Must be called before calling `matches()`, or it will always return + /// false. + #[inline] + pub fn build_with_diagnostics( + &mut self, + record_matches: bool, + name: Option<&'static str>, + ) { + self.build_inner(record_matches, name) + } + + #[cfg(all(not(feature = "__cli"), feature = "experimental"))] + /// Construct a RegexSet from the set of entries we've accumulated and emit diagnostics if the + /// name of the regex set is passed to it. + /// + /// Must be called before calling `matches()`, or it will always return + /// false. + #[inline] + pub(crate) fn build_with_diagnostics( + &mut self, + record_matches: bool, + name: Option<&'static str>, + ) { + self.build_inner(record_matches, name) + } + + fn build_inner( + &mut self, + record_matches: bool, + name: Option<&'static str>, + ) { let items = self.items.iter().map(|item| format!("^({})$", item)); self.record_matches = record_matches; self.set = match RxSet::new(items) { Ok(x) => Some(x), Err(e) => { warn!("Invalid regex in {:?}: {:?}", self.items, e); + if let Some(name) = name { + #[cfg(feature = "experimental")] + invalid_regex_warning(self, e, name); + } None } } @@ -100,3 +139,63 @@ impl RegexSet { true } } + +#[cfg(feature = "experimental")] +fn invalid_regex_warning( + set: &RegexSet, + err: regex::Error, + name: &'static str, +) { + use crate::diagnostics::{Diagnostic, Level, Slice}; + + let mut diagnostic = Diagnostic::default(); + + match err { + regex::Error::Syntax(string) => { + if string.starts_with("regex parse error:\n") { + let mut source = String::new(); + + let mut parsing_source = true; + + for line in string.lines().skip(1) { + if parsing_source { + if line.starts_with(' ') { + source.push_str(line); + source.push('\n'); + continue; + } + parsing_source = false; + } + let error = "error: "; + if line.starts_with(error) { + let (_, msg) = line.split_at(error.len()); + diagnostic.add_annotation(msg.to_owned(), Level::Error); + } else { + diagnostic.add_annotation(line.to_owned(), Level::Info); + } + } + let mut slice = Slice::default(); + slice.with_source(source); + diagnostic.add_slice(slice); + + diagnostic.with_title("regex parse error:", Level::Warn); + } else { + diagnostic.with_title(string, Level::Warn); + } + } + err => { + let err = err.to_string(); + diagnostic.with_title(err, Level::Warn); + } + } + + diagnostic.add_annotation( + format!("this regular expression was passed via `{}`", name), + Level::Note, + ); + + if set.items.iter().any(|item| item == "*") { + diagnostic.add_annotation("using wildcard patterns \"*\" is no longer considered valid. Consider using \".*\" instead", Level::Help); + } + diagnostic.display(); +}