From 47b1d1de1e25e254b7f2691f4c8614077d3a716a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 30 Aug 2022 14:08:16 +0200 Subject: [PATCH 01/22] Handle never type in return position consistently. Fixes #779 --- src/bindgen/cdecl.rs | 36 +++++++++++++++---- src/bindgen/ir/function.rs | 32 ++--------------- src/bindgen/ir/ty.rs | 34 ++++++++++++------ tests/expectations/function_noreturn.both.c | 20 +++++++++++ .../function_noreturn.both.compat.c | 28 +++++++++++++++ tests/expectations/function_noreturn.c | 6 +++- tests/expectations/function_noreturn.compat.c | 6 +++- tests/expectations/function_noreturn.cpp | 6 +++- tests/expectations/function_noreturn.pyx | 5 ++- tests/expectations/function_noreturn.tag.c | 20 +++++++++++ .../function_noreturn.tag.compat.c | 28 +++++++++++++++ tests/expectations/function_noreturn.tag.pyx | 22 ++++++++++++ tests/rust/function_noreturn.rs | 9 +++-- 13 files changed, 198 insertions(+), 54 deletions(-) create mode 100644 tests/expectations/function_noreturn.both.c create mode 100644 tests/expectations/function_noreturn.both.compat.c create mode 100644 tests/expectations/function_noreturn.tag.c create mode 100644 tests/expectations/function_noreturn.tag.compat.c create mode 100644 tests/expectations/function_noreturn.tag.pyx diff --git a/src/bindgen/cdecl.rs b/src/bindgen/cdecl.rs index a01d81797..c909121df 100644 --- a/src/bindgen/cdecl.rs +++ b/src/bindgen/cdecl.rs @@ -20,13 +20,17 @@ enum CDeclarator { is_ref: bool, }, Array(String), - Func(Vec<(Option, CDecl)>, bool), + Func { + args: Vec<(Option, CDecl)>, + layout_vertical: bool, + never_return: bool, + }, } impl CDeclarator { fn is_ptr(&self) -> bool { match self { - CDeclarator::Ptr { .. } | CDeclarator::Func(..) => true, + CDeclarator::Ptr { .. } | CDeclarator::Func { .. } => true, _ => false, } } @@ -92,8 +96,11 @@ impl CDecl { ) }) .collect(); - self.declarators - .push(CDeclarator::Func(args, layout_vertical)); + self.declarators.push(CDeclarator::Func { + args, + layout_vertical, + never_return: f.never_return, + }); self.build_type(&f.ret, false, config); } @@ -162,6 +169,7 @@ impl CDecl { ref ret, ref args, is_nullable: _, + never_return, } => { let args = args .iter() @@ -172,7 +180,11 @@ impl CDecl { is_nullable: true, is_ref: false, }); - self.declarators.push(CDeclarator::Func(args, false)); + self.declarators.push(CDeclarator::Func { + args, + layout_vertical: false, + never_return: *never_return, + }); self.build_type(ret, false, config); } } @@ -231,7 +243,7 @@ impl CDecl { out.write("("); } } - CDeclarator::Func(..) => { + CDeclarator::Func { .. } => { if next_is_pointer { out.write("("); } @@ -262,7 +274,11 @@ impl CDecl { last_was_pointer = false; } - CDeclarator::Func(ref args, layout_vertical) => { + CDeclarator::Func { + ref args, + layout_vertical, + never_return, + } => { if last_was_pointer { out.write(")"); } @@ -300,6 +316,12 @@ impl CDecl { } out.write(")"); + if never_return && config.language != Language::Cython { + if let Some(ref no_return_attr) = config.function.no_return { + out.write_fmt(format_args!(" {}", no_return_attr)); + } + } + last_was_pointer = true; } } diff --git a/src/bindgen/ir/function.rs b/src/bindgen/ir/function.rs index 840e3d391..8119db572 100644 --- a/src/bindgen/ir/function.rs +++ b/src/bindgen/ir/function.rs @@ -12,8 +12,7 @@ use crate::bindgen::config::{Config, Language, Layout}; use crate::bindgen::declarationtyperesolver::DeclarationTypeResolver; use crate::bindgen::dependencies::Dependencies; use crate::bindgen::ir::{ - AnnotationSet, Cfg, ConditionWrite, Documentation, GenericPath, Path, PrimitiveType, - ToCondition, Type, + AnnotationSet, Cfg, ConditionWrite, Documentation, GenericPath, Path, ToCondition, Type, }; use crate::bindgen::library::Library; use crate::bindgen::monomorph::Monomorphs; @@ -55,18 +54,7 @@ impl Function { ) -> Result { let mut args = sig.inputs.iter().try_skip_map(|x| x.as_argument())?; - let mut never_return = false; - let mut ret = match sig.output { - syn::ReturnType::Default => Type::Primitive(PrimitiveType::Void), - syn::ReturnType::Type(_, ref ty) => { - if let syn::Type::Never(_) = ty.as_ref() { - never_return = true; - Type::Primitive(PrimitiveType::Void) - } else { - Type::load(ty)?.unwrap_or(Type::Primitive(PrimitiveType::Void)) - } - } - }; + let (mut ret, never_return) = Type::load_from_output(&sig.output)?; if let Some(self_path) = self_type_path { for arg in &mut args { @@ -88,10 +76,6 @@ impl Function { }) } - pub(crate) fn never_return(&self, config: &Config) -> bool { - self.never_return && config.language != Language::Cython - } - pub fn swift_name(&self, config: &Config) -> Option { if config.language == Language::Cython { return None; @@ -272,12 +256,6 @@ impl Source for Function { } } - if func.never_return(config) { - if let Some(ref no_return_attr) = config.function.no_return { - out.write_fmt(format_args!(" {}", no_return_attr)); - } - } - out.write(";"); condition.write_after(config, out); @@ -321,12 +299,6 @@ impl Source for Function { } } - if func.never_return(config) { - if let Some(ref no_return_attr) = config.function.no_return { - out.write_fmt(format_args!(" {}", no_return_attr)); - } - } - out.write(";"); condition.write_after(config, out); diff --git a/src/bindgen/ir/ty.rs b/src/bindgen/ir/ty.rs index c08020b10..5c68e4ca1 100644 --- a/src/bindgen/ir/ty.rs +++ b/src/bindgen/ir/ty.rs @@ -407,6 +407,7 @@ pub enum Type { ret: Box, args: Vec<(Option, Type)>, is_nullable: bool, + never_return: bool, }, } @@ -420,6 +421,22 @@ impl Type { } } + pub fn load_from_output(output: &syn::ReturnType) -> Result<(Type, bool), String> { + let mut never_return = false; + let ty = match output { + syn::ReturnType::Default => Type::Primitive(PrimitiveType::Void), + syn::ReturnType::Type(_, ref ty) => { + if let syn::Type::Never(_) = ty.as_ref() { + never_return = true; + Type::Primitive(PrimitiveType::Void) + } else { + Type::load(ty)?.unwrap_or(Type::Primitive(PrimitiveType::Void)) + } + } + }; + Ok((ty, never_return)) + } + pub fn load(ty: &syn::Type) -> Result, String> { let converted = match *ty { syn::Type::Reference(ref reference) => { @@ -507,21 +524,12 @@ impl Type { }) }) })?; - let ret = match function.output { - syn::ReturnType::Default => Type::Primitive(PrimitiveType::Void), - syn::ReturnType::Type(_, ref ty) => { - if let Some(x) = Type::load(ty)? { - x - } else { - Type::Primitive(PrimitiveType::Void) - } - } - }; - + let (ret, never_return) = Type::load_from_output(&function.output)?; Type::FuncPtr { ret: Box::new(ret), args, is_nullable: false, + never_return, } } syn::Type::Tuple(ref tuple) => { @@ -585,10 +593,12 @@ impl Type { ref ret, ref args, is_nullable: false, + never_return, } => Some(Type::FuncPtr { ret: ret.clone(), args: args.clone(), is_nullable: true, + never_return, }), _ => None, } @@ -784,6 +794,7 @@ impl Type { ref ret, ref args, is_nullable, + never_return, } => Type::FuncPtr { ret: Box::new(ret.specialize(mappings)), args: args @@ -792,6 +803,7 @@ impl Type { .map(|(name, ty)| (name, ty.specialize(mappings))) .collect(), is_nullable, + never_return, }, } } diff --git a/tests/expectations/function_noreturn.both.c b/tests/expectations/function_noreturn.both.c new file mode 100644 index 000000000..1a21ba67f --- /dev/null +++ b/tests/expectations/function_noreturn.both.c @@ -0,0 +1,20 @@ +#include +#include +#include +#include +#ifndef NO_RETURN_ATTR + #ifdef __GNUC__ + #define NO_RETURN_ATTR __attribute__ ((noreturn)) + #else // __GNUC__ + #define NO_RETURN_ATTR + #endif // __GNUC__ +#endif // NO_RETURN_ATTR + + +typedef struct Example { + void (*f)(uintptr_t, uintptr_t) NO_RETURN_ATTR; +} Example; + +void loop_forever(void) NO_RETURN_ATTR; + +uint8_t normal_return(struct Example arg, void (*other)(uint8_t) NO_RETURN_ATTR); diff --git a/tests/expectations/function_noreturn.both.compat.c b/tests/expectations/function_noreturn.both.compat.c new file mode 100644 index 000000000..7336e5995 --- /dev/null +++ b/tests/expectations/function_noreturn.both.compat.c @@ -0,0 +1,28 @@ +#include +#include +#include +#include +#ifndef NO_RETURN_ATTR + #ifdef __GNUC__ + #define NO_RETURN_ATTR __attribute__ ((noreturn)) + #else // __GNUC__ + #define NO_RETURN_ATTR + #endif // __GNUC__ +#endif // NO_RETURN_ATTR + + +typedef struct Example { + void (*f)(uintptr_t, uintptr_t) NO_RETURN_ATTR; +} Example; + +#ifdef __cplusplus +extern "C" { +#endif // __cplusplus + +void loop_forever(void) NO_RETURN_ATTR; + +uint8_t normal_return(struct Example arg, void (*other)(uint8_t) NO_RETURN_ATTR); + +#ifdef __cplusplus +} // extern "C" +#endif // __cplusplus diff --git a/tests/expectations/function_noreturn.c b/tests/expectations/function_noreturn.c index e715ae3b8..ba5c1167d 100644 --- a/tests/expectations/function_noreturn.c +++ b/tests/expectations/function_noreturn.c @@ -11,6 +11,10 @@ #endif // NO_RETURN_ATTR +typedef struct { + void (*f)(uintptr_t, uintptr_t) NO_RETURN_ATTR; +} Example; + void loop_forever(void) NO_RETURN_ATTR; -uint8_t normal_return(void); +uint8_t normal_return(Example arg, void (*other)(uint8_t) NO_RETURN_ATTR); diff --git a/tests/expectations/function_noreturn.compat.c b/tests/expectations/function_noreturn.compat.c index 12238b3bb..6ea836f09 100644 --- a/tests/expectations/function_noreturn.compat.c +++ b/tests/expectations/function_noreturn.compat.c @@ -11,13 +11,17 @@ #endif // NO_RETURN_ATTR +typedef struct { + void (*f)(uintptr_t, uintptr_t) NO_RETURN_ATTR; +} Example; + #ifdef __cplusplus extern "C" { #endif // __cplusplus void loop_forever(void) NO_RETURN_ATTR; -uint8_t normal_return(void); +uint8_t normal_return(Example arg, void (*other)(uint8_t) NO_RETURN_ATTR); #ifdef __cplusplus } // extern "C" diff --git a/tests/expectations/function_noreturn.cpp b/tests/expectations/function_noreturn.cpp index 9a3040c97..5adbe111a 100644 --- a/tests/expectations/function_noreturn.cpp +++ b/tests/expectations/function_noreturn.cpp @@ -12,10 +12,14 @@ #endif // NO_RETURN_ATTR +struct Example { + void (*f)(uintptr_t, uintptr_t) NO_RETURN_ATTR; +}; + extern "C" { void loop_forever() NO_RETURN_ATTR; -uint8_t normal_return(); +uint8_t normal_return(Example arg, void (*other)(uint8_t) NO_RETURN_ATTR); } // extern "C" diff --git a/tests/expectations/function_noreturn.pyx b/tests/expectations/function_noreturn.pyx index 1af76c542..21683cbd9 100644 --- a/tests/expectations/function_noreturn.pyx +++ b/tests/expectations/function_noreturn.pyx @@ -14,6 +14,9 @@ cdef extern from *: cdef extern from *: + ctypedef struct Example: + void (*f)(uintptr_t, uintptr_t); + void loop_forever(); - uint8_t normal_return(); + uint8_t normal_return(Example arg, void (*other)(uint8_t)); diff --git a/tests/expectations/function_noreturn.tag.c b/tests/expectations/function_noreturn.tag.c new file mode 100644 index 000000000..48ead97ca --- /dev/null +++ b/tests/expectations/function_noreturn.tag.c @@ -0,0 +1,20 @@ +#include +#include +#include +#include +#ifndef NO_RETURN_ATTR + #ifdef __GNUC__ + #define NO_RETURN_ATTR __attribute__ ((noreturn)) + #else // __GNUC__ + #define NO_RETURN_ATTR + #endif // __GNUC__ +#endif // NO_RETURN_ATTR + + +struct Example { + void (*f)(uintptr_t, uintptr_t) NO_RETURN_ATTR; +}; + +void loop_forever(void) NO_RETURN_ATTR; + +uint8_t normal_return(struct Example arg, void (*other)(uint8_t) NO_RETURN_ATTR); diff --git a/tests/expectations/function_noreturn.tag.compat.c b/tests/expectations/function_noreturn.tag.compat.c new file mode 100644 index 000000000..dcb0e5b0c --- /dev/null +++ b/tests/expectations/function_noreturn.tag.compat.c @@ -0,0 +1,28 @@ +#include +#include +#include +#include +#ifndef NO_RETURN_ATTR + #ifdef __GNUC__ + #define NO_RETURN_ATTR __attribute__ ((noreturn)) + #else // __GNUC__ + #define NO_RETURN_ATTR + #endif // __GNUC__ +#endif // NO_RETURN_ATTR + + +struct Example { + void (*f)(uintptr_t, uintptr_t) NO_RETURN_ATTR; +}; + +#ifdef __cplusplus +extern "C" { +#endif // __cplusplus + +void loop_forever(void) NO_RETURN_ATTR; + +uint8_t normal_return(struct Example arg, void (*other)(uint8_t) NO_RETURN_ATTR); + +#ifdef __cplusplus +} // extern "C" +#endif // __cplusplus diff --git a/tests/expectations/function_noreturn.tag.pyx b/tests/expectations/function_noreturn.tag.pyx new file mode 100644 index 000000000..6235dca0e --- /dev/null +++ b/tests/expectations/function_noreturn.tag.pyx @@ -0,0 +1,22 @@ +from libc.stdint cimport int8_t, int16_t, int32_t, int64_t, intptr_t +from libc.stdint cimport uint8_t, uint16_t, uint32_t, uint64_t, uintptr_t +cdef extern from *: + ctypedef bint bool + ctypedef struct va_list +#ifndef NO_RETURN_ATTR + #ifdef __GNUC__ + #define NO_RETURN_ATTR __attribute__ ((noreturn)) + #else // __GNUC__ + #define NO_RETURN_ATTR + #endif // __GNUC__ +#endif // NO_RETURN_ATTR + + +cdef extern from *: + + cdef struct Example: + void (*f)(uintptr_t, uintptr_t); + + void loop_forever(); + + uint8_t normal_return(Example arg, void (*other)(uint8_t)); diff --git a/tests/rust/function_noreturn.rs b/tests/rust/function_noreturn.rs index 94a23957a..d7777adb4 100644 --- a/tests/rust/function_noreturn.rs +++ b/tests/rust/function_noreturn.rs @@ -1,9 +1,14 @@ #[no_mangle] -pub extern fn loop_forever() -> ! { +pub extern "C" fn loop_forever() -> ! { loop {} } #[no_mangle] -pub extern fn normal_return() -> u8 { +pub extern "C" fn normal_return(arg: Example, other: extern "C" fn(u8) -> !) -> u8 { 0 } + +#[repr(C)] +pub struct Example { + pub f: extern "C" fn(usize, usize) -> !, +} From ae321d80ff31739e35b9d01ef20061c1bef6d6ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 30 Aug 2022 14:15:37 +0200 Subject: [PATCH 02/22] Drive-by clippy fixes. --- src/bindgen/bindings.rs | 2 +- src/bindgen/config.rs | 16 ++++++++-------- src/bindgen/ir/cfg.rs | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/bindgen/bindings.rs b/src/bindgen/bindings.rs index 845a4c69c..a03b2826d 100644 --- a/src/bindgen/bindings.rs +++ b/src/bindgen/bindings.rs @@ -34,7 +34,7 @@ pub struct Bindings { noop: bool, } -#[derive(PartialEq)] +#[derive(PartialEq, Eq)] enum NamespaceOperation { Open, Close, diff --git a/src/bindgen/config.rs b/src/bindgen/config.rs index b4a0cb789..fbf35cbe1 100644 --- a/src/bindgen/config.rs +++ b/src/bindgen/config.rs @@ -18,7 +18,7 @@ pub use crate::bindgen::rename::RenameRule; pub const VERSION: &str = env!("CARGO_PKG_VERSION"); /// A language type to generate bindings for. -#[derive(Debug, Copy, Clone, PartialEq)] +#[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum Language { Cxx, C, @@ -115,7 +115,7 @@ impl FromStr for LineEndingStyle { deserialize_enum_str!(LineEndingStyle); /// A style of braces to use for generating code. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum Braces { SameLine, NextLine, @@ -138,7 +138,7 @@ impl FromStr for Braces { deserialize_enum_str!(Braces); /// A type of layout to use when generating long lines of code. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum Layout { Horizontal, Vertical, @@ -164,7 +164,7 @@ impl FromStr for Layout { deserialize_enum_str!(Layout); /// How the comments containing documentation should be styled. -#[derive(Debug, Clone, PartialEq, Copy)] +#[derive(Debug, Clone, PartialEq, Eq, Copy)] pub enum DocumentationStyle { C, C99, @@ -213,7 +213,7 @@ impl FromStr for DocumentationLength { deserialize_enum_str!(DocumentationLength); /// A style of Style to use when generating structs and enums. -#[derive(Debug, Copy, Clone, PartialEq)] +#[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum Style { Both, Tag, @@ -270,7 +270,7 @@ impl FromStr for Style { deserialize_enum_str!(Style); /// Different item types that we can generate and filter. -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum ItemType { Constants, Globals, @@ -304,7 +304,7 @@ impl FromStr for ItemType { deserialize_enum_str!(ItemType); /// Type which specifies the sort order of functions -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SortKey { Name, None, @@ -723,7 +723,7 @@ pub struct MacroExpansionConfig { } /// Controls which Cargo profile is used for macro expansion. -#[derive(Debug, Copy, Clone, PartialEq)] +#[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum Profile { Debug, Release, diff --git a/src/bindgen/ir/cfg.rs b/src/bindgen/ir/cfg.rs index 6aa72022e..e8aa4126a 100644 --- a/src/bindgen/ir/cfg.rs +++ b/src/bindgen/ir/cfg.rs @@ -201,13 +201,13 @@ pub trait ToCondition: Sized { fn to_condition(&self, config: &Config) -> Option; } -impl<'a> ToCondition for Option { +impl ToCondition for Option { fn to_condition(&self, config: &Config) -> Option { self.as_ref()?.to_condition(config) } } -impl<'a> ToCondition for Cfg { +impl ToCondition for Cfg { fn to_condition(&self, config: &Config) -> Option { match *self { Cfg::Boolean(ref cfg_name) => { From a643506874de1b46d6b1eee90c3e20a99fd03974 Mon Sep 17 00:00:00 2001 From: Andrew Kane Date: Mon, 10 Oct 2022 14:43:01 -0700 Subject: [PATCH 03/22] Add Homebrew instructions to readme [skip ci] --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index 837f7eca4..5576de66e 100644 --- a/README.md +++ b/README.md @@ -49,6 +49,12 @@ cargo install --force cbindgen (--force just makes it update to the latest cbindgen if it's already installed) +Or with Homebrew, run + +```text +brew install cbindgen +``` + To use cbindgen you need two things: * A configuration (cbindgen.toml, which can be empty to start) From cc8cced7e8bacaffb83e818b9c41a7875f5b7c8d Mon Sep 17 00:00:00 2001 From: messense Date: Mon, 17 Oct 2022 13:21:24 +0800 Subject: [PATCH 04/22] Add maturin to examples https://github.com/PyO3/maturin/blob/190759e804e4d10ff7e2e747c8bd76c17d6ee812/src/module_writer.rs#L461-L498 --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 5576de66e..e3df3246e 100644 --- a/README.md +++ b/README.md @@ -85,6 +85,7 @@ in production: * [milksnake](https://github.com/getsentry/milksnake) * [webrender](https://searchfox.org/mozilla-central/source/gfx/webrender_bindings) ([generated header](https://searchfox.org/mozilla-central/source/__GENERATED__/gfx/webrender_bindings/webrender_ffi_generated.h)) * [stylo](https://searchfox.org/mozilla-central/source/layout/style) ([generated header](https://searchfox.org/mozilla-central/source/__GENERATED__/layout/style/ServoStyleConsts.h)) +* [maturin](https://github.com/PyO3/maturin) If you're using `cbindgen` and would like to be added to this list, please open a pull request! From a2bda0a1de3d7fd53a881e7742068619d3cdd8e4 Mon Sep 17 00:00:00 2001 From: novafacing Date: Mon, 17 Oct 2022 10:07:36 -0400 Subject: [PATCH 05/22] Correct wrong property in docs toml example --- docs.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs.md b/docs.md index 9a434aa06..325c6c947 100644 --- a/docs.md +++ b/docs.md @@ -514,7 +514,7 @@ documentation_style = "doxy" # * "full": The full documentation. # # default: "full" -documentation_style = "short" +documentation_length = "short" From 5fd38d541712928b7bf212da4f174f00acdceee9 Mon Sep 17 00:00:00 2001 From: Thibaut Lorrain Date: Fri, 21 Oct 2022 16:28:42 +0200 Subject: [PATCH 06/22] Better line breaking in function pointer types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current implementation never goes to a new line when writing a function pointer type, this can lead to long, difficult to read lines. The goal of this change is to make that a bit more sensible. Here is a rust code example ``` 1 │ pub type MyCallback = Option; 2 │ 3 │ pub type MyOtherCallback = 4 │ Option; 5 │ 6 │ #[no_mangle] 7 │ pub extern "C" fn my_function(a: MyCallback, b: MyOtherCallback) {} ``` right when generating the corresponing C header we get ``` 1 │ #include 2 │ #include 3 │ #include 4 │ #include 5 │ 6 │ typedef void (*MyCallback)(uintptr_t a, uintptr_t b); 7 │ 8 │ typedef void (*MyOtherCallback)(uintptr_t a, uintptr_t lot, uintptr_t of, uintptr_t args); 9 │ 10 │ void my_function(MyCallback a, MyOtherCallback b); ``` line 8 here is already quite long and will be even longer if we add new args to `MyOtherCallback` With the changes in this commit, we now get ``` 1 │ #include 2 │ #include 3 │ #include 4 │ #include 5 │ 6 │ typedef void (*MyCallback)(uintptr_t a, uintptr_t b); 7 │ 8 │ typedef void (*MyOtherCallback)(uintptr_t a, 9 │ uintptr_t lot, 10 │ uintptr_t of, 11 │ uintptr_t args); 12 │ 13 │ void my_function(MyCallback a, MyOtherCallback b); ``` which is way better and more scalable if new args are atted to `MyOtherCallback` The behavior is configurable using the already existing `fn.args` configuration parameter. In this case setting it to `Horizontal` gives back the same .h as previously and setting it to `Vertical` makes the generator go to a new line even for the shorter `MyCallback` declaration: ``` 1 │ #include 2 │ #include 3 │ #include 4 │ #include 5 │ 6 │ typedef void (*MyCallback)(uintptr_t a, 7 │ uintptr_t b); 8 │ 9 │ typedef void (*MyOtherCallback)(uintptr_t a, 10 │ uintptr_t lot, 11 │ uintptr_t of, 12 │ uintptr_t args); 13 │ 14 │ void my_function(MyCallback a, 15 │ MyOtherCallback b); ``` Closes #793 --- src/bindgen/cdecl.rs | 49 +++++++++++++++++++++++++++++--------- src/bindgen/ir/function.rs | 4 ++-- 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/src/bindgen/cdecl.rs b/src/bindgen/cdecl.rs index c909121df..c0afdc249 100644 --- a/src/bindgen/cdecl.rs +++ b/src/bindgen/cdecl.rs @@ -4,6 +4,7 @@ use std::io::Write; +use crate::bindgen::config::Layout; use crate::bindgen::declarationtyperesolver::DeclarationType; use crate::bindgen::ir::{ConstExpr, Function, GenericArgument, Type}; use crate::bindgen::writer::{ListType, SourceWriter}; @@ -22,7 +23,7 @@ enum CDeclarator { Array(String), Func { args: Vec<(Option, CDecl)>, - layout_vertical: bool, + layout: Layout, never_return: bool, }, } @@ -79,13 +80,13 @@ impl CDecl { cdecl } - fn from_func(f: &Function, layout_vertical: bool, config: &Config) -> CDecl { + fn from_func(f: &Function, layout: Layout, config: &Config) -> CDecl { let mut cdecl = CDecl::new(); - cdecl.build_func(f, layout_vertical, config); + cdecl.build_func(f, layout, config); cdecl } - fn build_func(&mut self, f: &Function, layout_vertical: bool, config: &Config) { + fn build_func(&mut self, f: &Function, layout: Layout, config: &Config) { let args = f .args .iter() @@ -98,7 +99,7 @@ impl CDecl { .collect(); self.declarators.push(CDeclarator::Func { args, - layout_vertical, + layout, never_return: f.never_return, }); self.build_type(&f.ret, false, config); @@ -182,7 +183,7 @@ impl CDecl { }); self.declarators.push(CDeclarator::Func { args, - layout_vertical: false, + layout: config.function.args.clone(), never_return: *never_return, }); self.build_type(ret, false, config); @@ -276,7 +277,7 @@ impl CDecl { } CDeclarator::Func { ref args, - layout_vertical, + ref layout, never_return, } => { if last_was_pointer { @@ -287,7 +288,12 @@ impl CDecl { if args.is_empty() && config.language == Language::C { out.write("void"); } - if layout_vertical { + + fn write_vertical( + out: &mut SourceWriter, + config: &Config, + args: &[(Option, CDecl)], + ) { let align_length = out.line_length_for_align(); out.push_set_spaces(align_length); for (i, &(ref arg_ident, ref arg_ty)) in args.iter().enumerate() { @@ -302,7 +308,13 @@ impl CDecl { arg_ty.write(out, arg_ident, config); } out.pop_tab(); - } else { + } + + fn write_horizontal( + out: &mut SourceWriter, + config: &Config, + args: &[(Option, CDecl)], + ) { for (i, &(ref arg_ident, ref arg_ty)) in args.iter().enumerate() { if i != 0 { out.write(", "); @@ -314,6 +326,21 @@ impl CDecl { arg_ty.write(out, arg_ident, config); } } + + match layout { + Layout::Vertical => write_vertical(out, config, args), + Layout::Horizontal => write_horizontal(out, config, args), + Layout::Auto => { + if out.line_length_for_align() + + out.measure(|out| write_horizontal(out, config, args)) + > config.line_length + { + write_vertical(out, config, args) + } else { + write_horizontal(out, config, args) + } + } + } out.write(")"); if never_return && config.language != Language::Cython { @@ -332,10 +359,10 @@ impl CDecl { pub fn write_func( out: &mut SourceWriter, f: &Function, - layout_vertical: bool, + layout: Layout, config: &Config, ) { - CDecl::from_func(f, layout_vertical, config).write(out, Some(f.path().name()), config); + CDecl::from_func(f, layout, config).write(out, Some(f.path().name()), config); } pub fn write_field(out: &mut SourceWriter, t: &Type, ident: &str, config: &Config) { diff --git a/src/bindgen/ir/function.rs b/src/bindgen/ir/function.rs index 8119db572..27df2a0b8 100644 --- a/src/bindgen/ir/function.rs +++ b/src/bindgen/ir/function.rs @@ -242,7 +242,7 @@ impl Source for Function { } } } - cdecl::write_func(out, func, false, config); + cdecl::write_func(out, func, Layout::Horizontal, config); if !func.extern_decl { if let Some(ref postfix) = postfix { @@ -285,7 +285,7 @@ impl Source for Function { } } } - cdecl::write_func(out, func, true, config); + cdecl::write_func(out, func, Layout::Vertical, config); if !func.extern_decl { if let Some(ref postfix) = postfix { out.new_line(); From e0eaecfdf77e061ab4ae985de39f8fa9c56557e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 10 Nov 2022 13:59:04 +0100 Subject: [PATCH 07/22] Fix clippy nits. --- src/bindgen/cargo/cargo_metadata.rs | 2 +- src/bindgen/ir/enumeration.rs | 6 +++--- src/bindgen/ir/structure.rs | 7 ++----- src/bindgen/mangle.rs | 4 ++-- src/main.rs | 2 +- 5 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/bindgen/cargo/cargo_metadata.rs b/src/bindgen/cargo/cargo_metadata.rs index b9d5c0111..08a2f9dcd 100644 --- a/src/bindgen/cargo/cargo_metadata.rs +++ b/src/bindgen/cargo/cargo_metadata.rs @@ -253,6 +253,6 @@ pub fn metadata( } }; - let meta: Metadata = serde_json::from_str(&*metadata)?; + let meta: Metadata = serde_json::from_str(&metadata)?; Ok(meta) } diff --git a/src/bindgen/ir/enumeration.rs b/src/bindgen/ir/enumeration.rs index 6409a7913..b1b6c044f 100644 --- a/src/bindgen/ir/enumeration.rs +++ b/src/bindgen/ir/enumeration.rs @@ -1175,7 +1175,7 @@ impl Enum { write!(out, "static {} {}(", self.export_name, variant.export_name); if let VariantBody::Body { ref body, .. } = variant.body { - let skip_fields = if body.has_tag_field { 1 } else { 0 }; + let skip_fields = body.has_tag_field as usize; let vec: Vec<_> = body .fields .iter() @@ -1202,7 +1202,7 @@ impl Enum { .. } = variant.body { - let skip_fields = if body.has_tag_field { 1 } else { 0 }; + let skip_fields = body.has_tag_field as usize; for field in body.fields.iter().skip(skip_fields) { out.new_line(); match field.ty { @@ -1257,7 +1257,7 @@ impl Enum { VariantBody::Empty(..) => return, }; - let skip_fields = if body.has_tag_field { 1 } else { 0 }; + let skip_fields = body.has_tag_field as usize; let field_count = body.fields.len() - skip_fields; if field_count == 0 { return; diff --git a/src/bindgen/ir/structure.rs b/src/bindgen/ir/structure.rs index 2cb26a044..9f77739a6 100644 --- a/src/bindgen/ir/structure.rs +++ b/src/bindgen/ir/structure.rs @@ -294,10 +294,7 @@ impl Item for Struct { // Rename the types used in fields { - let fields = self - .fields - .iter_mut() - .skip(if self.has_tag_field { 1 } else { 0 }); + let fields = self.fields.iter_mut().skip(self.has_tag_field as usize); for field in fields { field.ty.rename_for_config(config, &self.generic_params); } @@ -604,7 +601,7 @@ impl Source for Struct { out.close_brace(false); } - let skip_fields = if self.has_tag_field { 1 } else { 0 }; + let skip_fields = self.has_tag_field as usize; macro_rules! emit_op { ($op_name:expr, $op:expr, $conjuc:expr) => {{ diff --git a/src/bindgen/mangle.rs b/src/bindgen/mangle.rs index f20bb3ce9..c8769f83c 100644 --- a/src/bindgen/mangle.rs +++ b/src/bindgen/mangle.rs @@ -112,13 +112,13 @@ impl<'a> Mangler<'a> { } else { Separator::BeginMutPtr }); - self.append_mangled_type(&**ty, last); + self.append_mangled_type(ty, last); } Type::FuncPtr { ref ret, ref args, .. } => { self.push(Separator::BeginFn); - self.append_mangled_type(&**ret, args.is_empty()); + self.append_mangled_type(ret, args.is_empty()); for (i, arg) in args.iter().enumerate() { self.push(Separator::BetweenFnArg); let last = last && i == args.len() - 1; diff --git a/src/main.rs b/src/main.rs index 0004cf70e..e6dec459d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -113,7 +113,7 @@ fn load_bindings(input: &Path, matches: &ArgMatches) -> Result let binding_crate_dir = lib.find_crate_dir(&lib.binding_crate_ref()); if let Some(binding_crate_dir) = binding_crate_dir { - Config::from_root_or_default(&binding_crate_dir) + Config::from_root_or_default(binding_crate_dir) } else { // This shouldn't happen Config::from_root_or_default(input) From d8660bb01ff77a0fccfe7b2e04eac7952e70c48b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 10 Nov 2022 14:18:43 +0100 Subject: [PATCH 08/22] Avoid doing double the work when measuring. --- src/bindgen/cdecl.rs | 10 +++---- src/bindgen/ir/function.rs | 16 +++++------ src/bindgen/writer.rs | 54 ++++++++++++++++++++------------------ 3 files changed, 41 insertions(+), 39 deletions(-) diff --git a/src/bindgen/cdecl.rs b/src/bindgen/cdecl.rs index c0afdc249..7e2ad4e10 100644 --- a/src/bindgen/cdecl.rs +++ b/src/bindgen/cdecl.rs @@ -331,13 +331,11 @@ impl CDecl { Layout::Vertical => write_vertical(out, config, args), Layout::Horizontal => write_horizontal(out, config, args), Layout::Auto => { - if out.line_length_for_align() - + out.measure(|out| write_horizontal(out, config, args)) - > config.line_length - { + if !out.try_write( + |out| write_horizontal(out, config, args), + config.line_length, + ) { write_vertical(out, config, args) - } else { - write_horizontal(out, config, args) } } } diff --git a/src/bindgen/ir/function.rs b/src/bindgen/ir/function.rs index 27df2a0b8..da0920767 100644 --- a/src/bindgen/ir/function.rs +++ b/src/bindgen/ir/function.rs @@ -304,14 +304,14 @@ impl Source for Function { condition.write_after(config, out); } - let option_1 = out.measure(|out| write_1(self, config, out)); - - if (config.function.args == Layout::Auto && option_1 <= config.line_length) - || config.function.args == Layout::Horizontal - { - write_1(self, config, out); - } else { - write_2(self, config, out); + match config.function.args { + Layout::Horizontal => write_1(self, config, out), + Layout::Vertical => write_2(self, config, out), + Layout::Auto => { + if !out.try_write(|out| write_1(self, config, out), config.line_length) { + write_2(self, config, out) + } + } } } } diff --git a/src/bindgen/writer.rs b/src/bindgen/writer.rs index 3291af911..a5336e5ed 100644 --- a/src/bindgen/writer.rs +++ b/src/bindgen/writer.rs @@ -17,18 +17,6 @@ pub enum ListType<'a> { Cap(&'a str), } -/// An empty file used for creating a null source writer and measuring line -/// metrics for various code layouts. -pub struct NullFile; -impl Write for NullFile { - fn write(&mut self, buf: &[u8]) -> io::Result { - Ok(buf.len()) - } - fn flush(&mut self) -> io::Result<()> { - Ok(()) - } -} - /// A utility wrapper to write unbuffered data and correctly adjust positions. struct InnerWriter<'a, 'b: 'a, F: 'a + Write>(&'a mut SourceWriter<'b, F>); @@ -66,7 +54,7 @@ pub struct SourceWriter<'a, F: Write> { max_line_length: usize, } -pub type MeasureWriter<'a> = SourceWriter<'a, NullFile>; +pub type MeasureWriter<'a> = SourceWriter<'a, &'a mut Vec>; impl<'a, F: Write> SourceWriter<'a, F> { pub fn new(out: F, bindings: &'a Bindings) -> Self { @@ -87,23 +75,39 @@ impl<'a, F: Write> SourceWriter<'a, F> { /// Takes a function that writes source and returns the maximum line length /// written. - pub fn measure(&self, func: T) -> usize + pub fn try_write(&mut self, func: T, max_line_length: usize) -> bool where T: Fn(&mut MeasureWriter), { - let mut measurer = SourceWriter { - out: NullFile, - bindings: self.bindings, - spaces: self.spaces.clone(), - line_started: self.line_started, - line_length: self.line_length, - line_number: self.line_number, - max_line_length: self.line_length, - }; + if self.line_length > max_line_length { + return false; + } - func(&mut measurer); + let mut buffer = Vec::new(); + let line_length = { + let mut measurer = SourceWriter { + out: &mut buffer, + bindings: self.bindings, + spaces: self.spaces.clone(), + line_started: self.line_started, + line_length: self.line_length, + line_number: self.line_number, + max_line_length: self.line_length, + }; + + func(&mut measurer); + + measurer.max_line_length + }; - measurer.max_line_length + if line_length > max_line_length { + return false; + } + // We don't want the extra alignment, it's already accounted for by the + // measurer. + self.line_started = true; + InnerWriter(self).write_all(&buffer).unwrap(); + true } fn spaces(&self) -> usize { From 66bc17facfeba395482ba7202a460290283f78b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 10 Nov 2022 14:21:06 +0100 Subject: [PATCH 09/22] Add a test for function pointer text wrapping. --- tests/expectations/function_ptr.c | 14 ++++++++++++++ tests/expectations/function_ptr.compat.c | 22 ++++++++++++++++++++++ tests/expectations/function_ptr.cpp | 19 +++++++++++++++++++ tests/expectations/function_ptr.pyx | 17 +++++++++++++++++ tests/rust/function_ptr.rs | 6 ++++++ 5 files changed, 78 insertions(+) create mode 100644 tests/expectations/function_ptr.c create mode 100644 tests/expectations/function_ptr.compat.c create mode 100644 tests/expectations/function_ptr.cpp create mode 100644 tests/expectations/function_ptr.pyx create mode 100644 tests/rust/function_ptr.rs diff --git a/tests/expectations/function_ptr.c b/tests/expectations/function_ptr.c new file mode 100644 index 000000000..1041a6b34 --- /dev/null +++ b/tests/expectations/function_ptr.c @@ -0,0 +1,14 @@ +#include +#include +#include +#include + +typedef void (*MyCallback)(uintptr_t a, uintptr_t b); + +typedef void (*MyOtherCallback)(uintptr_t a, + uintptr_t lot, + uintptr_t of, + uintptr_t args, + uintptr_t and_then_some); + +void my_function(MyCallback a, MyOtherCallback b); diff --git a/tests/expectations/function_ptr.compat.c b/tests/expectations/function_ptr.compat.c new file mode 100644 index 000000000..be3870ce1 --- /dev/null +++ b/tests/expectations/function_ptr.compat.c @@ -0,0 +1,22 @@ +#include +#include +#include +#include + +typedef void (*MyCallback)(uintptr_t a, uintptr_t b); + +typedef void (*MyOtherCallback)(uintptr_t a, + uintptr_t lot, + uintptr_t of, + uintptr_t args, + uintptr_t and_then_some); + +#ifdef __cplusplus +extern "C" { +#endif // __cplusplus + +void my_function(MyCallback a, MyOtherCallback b); + +#ifdef __cplusplus +} // extern "C" +#endif // __cplusplus diff --git a/tests/expectations/function_ptr.cpp b/tests/expectations/function_ptr.cpp new file mode 100644 index 000000000..217a16081 --- /dev/null +++ b/tests/expectations/function_ptr.cpp @@ -0,0 +1,19 @@ +#include +#include +#include +#include +#include + +using MyCallback = void(*)(uintptr_t a, uintptr_t b); + +using MyOtherCallback = void(*)(uintptr_t a, + uintptr_t lot, + uintptr_t of, + uintptr_t args, + uintptr_t and_then_some); + +extern "C" { + +void my_function(MyCallback a, MyOtherCallback b); + +} // extern "C" diff --git a/tests/expectations/function_ptr.pyx b/tests/expectations/function_ptr.pyx new file mode 100644 index 000000000..371774209 --- /dev/null +++ b/tests/expectations/function_ptr.pyx @@ -0,0 +1,17 @@ +from libc.stdint cimport int8_t, int16_t, int32_t, int64_t, intptr_t +from libc.stdint cimport uint8_t, uint16_t, uint32_t, uint64_t, uintptr_t +cdef extern from *: + ctypedef bint bool + ctypedef struct va_list + +cdef extern from *: + + ctypedef void (*MyCallback)(uintptr_t a, uintptr_t b); + + ctypedef void (*MyOtherCallback)(uintptr_t a, + uintptr_t lot, + uintptr_t of, + uintptr_t args, + uintptr_t and_then_some); + + void my_function(MyCallback a, MyOtherCallback b); diff --git a/tests/rust/function_ptr.rs b/tests/rust/function_ptr.rs new file mode 100644 index 000000000..59adb52b3 --- /dev/null +++ b/tests/rust/function_ptr.rs @@ -0,0 +1,6 @@ +pub type MyCallback = Option; + +pub type MyOtherCallback = Option; + +#[no_mangle] +pub extern "C" fn my_function(a: MyCallback, b: MyOtherCallback) {} From 317412c5a981c5db25c331031d14e1fd9a71b95a Mon Sep 17 00:00:00 2001 From: Ian Hobson Date: Thu, 10 Nov 2022 11:30:06 +0100 Subject: [PATCH 10/22] Add with_cpp_compat to the builder --- src/bindgen/builder.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/bindgen/builder.rs b/src/bindgen/builder.rs index 76a142c91..6ee06e9cd 100644 --- a/src/bindgen/builder.rs +++ b/src/bindgen/builder.rs @@ -149,6 +149,12 @@ impl Builder { self } + #[allow(unused)] + pub fn with_cpp_compat(mut self, cpp_compat: bool) -> Builder { + self.config.cpp_compat = cpp_compat; + self + } + #[allow(unused)] pub fn with_style(mut self, style: Style) -> Builder { self.config.style = style; From b6e73017e679caf3b01217e62642d0722a464887 Mon Sep 17 00:00:00 2001 From: Romain Malmain Date: Sun, 13 Nov 2022 01:57:43 +0100 Subject: [PATCH 11/22] Moved expand infinite recursion fix. --- src/bindgen/builder.rs | 18 ++++++++++++++++++ src/bindgen/library.rs | 18 ------------------ 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/bindgen/builder.rs b/src/bindgen/builder.rs index 6ee06e9cd..17d4ad1df 100644 --- a/src/bindgen/builder.rs +++ b/src/bindgen/builder.rs @@ -345,6 +345,24 @@ impl Builder { } pub fn generate(self) -> Result { + // If macro expansion is enabled, then cbindgen will attempt to build the crate + // and will run its build script which may run cbindgen again. That second run may start + // infinite recursion, or overwrite previously written files with bindings. + // So if we are called recursively, we are skipping the whole generation + // and produce "noop" bindings that won't be able to overwrite anything. + if std::env::var("_CBINDGEN_IS_RUNNING").is_ok() { + return Ok(Bindings::new( + self.config, + Default::default(), + Default::default(), + Default::default(), + Default::default(), + Default::default(), + Default::default(), + true, + )); + } + let mut result = Parse::new(); if self.std_types { diff --git a/src/bindgen/library.rs b/src/bindgen/library.rs index 23ec38dc6..c00a6ebc6 100644 --- a/src/bindgen/library.rs +++ b/src/bindgen/library.rs @@ -54,24 +54,6 @@ impl Library { } pub fn generate(mut self) -> Result { - // If macro expansion is enabled, then cbindgen will attempt to build the crate - // and will run its build script which may run cbindgen again. That second run may start - // infinite recursion, or overwrite previously written files with bindings. - // So if we are called recursively, we are skipping the whole generation - // and produce "noop" bindings that won't be able to overwrite anything. - if std::env::var("_CBINDGEN_IS_RUNNING").is_ok() { - return Ok(Bindings::new( - self.config, - Default::default(), - Default::default(), - Default::default(), - Default::default(), - Default::default(), - Default::default(), - true, - )); - } - self.transfer_annotations(); self.simplify_standard_types(); From 55e24c5090e914725b026ffccc382bd8667c05a8 Mon Sep 17 00:00:00 2001 From: Alex Touchet Date: Mon, 30 Jan 2023 23:24:56 -0800 Subject: [PATCH 12/22] Replace Travis CI references and update some formatting --- Cargo.toml | 7 ++----- contributing.md | 8 ++++---- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ce91d4eea..81f4f0ff7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,20 +11,17 @@ license = "MPL-2.0" description = "A tool for generating C bindings to Rust code." keywords = ["bindings", "ffi", "code-generation"] categories = ["external-ffi-bindings", "development-tools::ffi"] -repository = "https://github.com/eqrion/cbindgen/" +repository = "https://github.com/eqrion/cbindgen" edition = "2018" exclude = [ "tests/profile.rs", # Test relies in a sub-crate, see https://github.com/rust-lang/cargo/issues/9017 ] -[badges] -travis-ci = { repository = "eqrion/cbindgen" } - [dependencies] clap = { version = "3.1", optional = true } indexmap = "1" log = "0.4" -serde = { version = "1.0.103", default-features = false, features = ["derive"]} +serde = { version = "1.0.103", default-features = false, features = ["derive"] } serde_json = "1.0" tempfile = "3.0" toml = "0.5" diff --git a/contributing.md b/contributing.md index 5cc7a1bca..5625d50d0 100644 --- a/contributing.md +++ b/contributing.md @@ -2,23 +2,23 @@ Thanks for wanting to contribute! -If you want help or mentorship, please file a Github issue and I'll be sure to provide guidance to the best of my ability. +If you want help or mentorship, please file a GitHub issue and I'll be sure to provide guidance to the best of my ability. Otherwise be sure to check out `internals.md` for an overview on the internals. ## Filing a pull request -Check out [Servo's Github workflow](https://github.com/servo/servo/wiki/Github-workflow) for an overview on creating a pull request. +Check out [Servo's GitHub workflow](https://github.com/servo/servo/wiki/Github-workflow) for an overview on creating a pull request. Don't worry about requesting code review, as there is nothing formally setup for this repository. I try and review each pull request as soon as I can. -There is continuous integration setup for `cbindgen` using [travis](https://travis-ci.org/). It automatically runs `cargo test` which runs `cbindgen` against a series of rust files from `tests/rust/` and checks that the output compiles using `gcc` or `g++`. +There is continuous integration setup for `cbindgen` using [GitHub Actions](https://github.com/eqrion/cbindgen/actions). It automatically runs `cargo test` which runs `cbindgen` against a series of Rust files from `tests/rust/` and checks that the output compiles using `gcc` or `g++`. In addition to a C/C++ compiler `cargo test` requires Python and Cython (`python -m pip install Cython`) for checking Cython bindings generated from tests (`.pyx` files). Please run `cargo test` before filing a pull request to be sure that all tests pass. This will also update the test expectations. -Rustfmt is also enforced by travis. To format your code install `rustfmt-preview` using `rustup component add rustfmt-preview` and then `cargo fmt`. Travis runs with rust nightly, so use `rustup run nightly -- cargo fmt` to guarantee consistent results. +Rustfmt is also enforced by GitHub Actions. To format your code install `rustfmt-preview` using `rustup component add rustfmt-preview` and then `cargo fmt`. GitHub Actions runs with Rust nightly, so use `rustup run nightly -- cargo fmt` to guarantee consistent results. Writing new tests with your pull requests is also appreciated. From b566c3c064641576cae3f588635141dc9d636390 Mon Sep 17 00:00:00 2001 From: Jonathan Schwender Date: Wed, 8 Mar 2023 09:53:09 +0100 Subject: [PATCH 13/22] Fix documented MSRV The MSRV was updated to 1.54 in commit d4e508d. Update the MSRV specified in the Readme badge and in the clippy.toml to match the actual MSRV. --- .clippy.toml | 2 +- README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.clippy.toml b/.clippy.toml index 4dd68bb5c..8eb9e0003 100644 --- a/.clippy.toml +++ b/.clippy.toml @@ -1,2 +1,2 @@ # Specify the minimum supported Rust version -msrv = "1.40.0" +msrv = "1.54.0" diff --git a/README.md b/README.md index e3df3246e..a67ece62f 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# `cbindgen`   [![Build Status]][actions] [![Latest Version]][crates.io] [![Api Rustdoc]][rustdoc] [![Rust](https://img.shields.io/badge/rust-1.32%2B-blue.svg?maxAge=3600)](https://github.com/eqrion/cbindgen) +# `cbindgen`   [![Build Status]][actions] [![Latest Version]][crates.io] [![Api Rustdoc]][rustdoc] [![Rust](https://img.shields.io/badge/rust-1.54%2B-blue.svg?maxAge=3600)](https://github.com/eqrion/cbindgen) [Build Status]: https://github.com/eqrion/cbindgen/workflows/cbindgen/badge.svg [actions]: https://github.com/eqrion/cbindgen/actions From fb1fdc8aed0d40fb68e56cba17d603231bfd1602 Mon Sep 17 00:00:00 2001 From: Jonathan Schwender Date: Wed, 8 Mar 2023 09:55:41 +0100 Subject: [PATCH 14/22] Fix clippy warning (1.40 -> 1.54) The clippy.toml suppresses warnings added in newer rust versions, so fixing the documented MSRV also shows new warnings. --- src/bindgen/cdecl.rs | 4 ++-- src/bindgen/dependencies.rs | 10 ++++------ src/bindgen/ir/annotation.rs | 8 ++++---- src/bindgen/ir/item.rs | 4 ++-- src/main.rs | 6 +++--- tests/tests.rs | 2 +- 6 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/bindgen/cdecl.rs b/src/bindgen/cdecl.rs index 7e2ad4e10..74ee669de 100644 --- a/src/bindgen/cdecl.rs +++ b/src/bindgen/cdecl.rs @@ -296,7 +296,7 @@ impl CDecl { ) { let align_length = out.line_length_for_align(); out.push_set_spaces(align_length); - for (i, &(ref arg_ident, ref arg_ty)) in args.iter().enumerate() { + for (i, (arg_ident, arg_ty)) in args.iter().enumerate() { if i != 0 { out.write(","); out.new_line(); @@ -315,7 +315,7 @@ impl CDecl { config: &Config, args: &[(Option, CDecl)], ) { - for (i, &(ref arg_ident, ref arg_ty)) in args.iter().enumerate() { + for (i, (arg_ident, arg_ty)) in args.iter().enumerate() { if i != 0 { out.write(", "); } diff --git a/src/bindgen/dependencies.rs b/src/bindgen/dependencies.rs index 870d12919..6a9873818 100644 --- a/src/bindgen/dependencies.rs +++ b/src/bindgen/dependencies.rs @@ -26,17 +26,15 @@ impl Dependencies { // Sort untagged enums and opaque structs into their own layers because they don't // depend on each other or anything else. let ordering = |a: &ItemContainer, b: &ItemContainer| match (a, b) { - (&ItemContainer::Enum(ref x), &ItemContainer::Enum(ref y)) + (ItemContainer::Enum(x), ItemContainer::Enum(y)) if x.tag.is_none() && y.tag.is_none() => { x.path.cmp(&y.path) } - (&ItemContainer::Enum(ref x), _) if x.tag.is_none() => Ordering::Less, - (_, &ItemContainer::Enum(ref x)) if x.tag.is_none() => Ordering::Greater, + (ItemContainer::Enum(x), _) if x.tag.is_none() => Ordering::Less, + (_, ItemContainer::Enum(x)) if x.tag.is_none() => Ordering::Greater, - (&ItemContainer::OpaqueItem(ref x), &ItemContainer::OpaqueItem(ref y)) => { - x.path.cmp(&y.path) - } + (ItemContainer::OpaqueItem(x), ItemContainer::OpaqueItem(y)) => x.path.cmp(&y.path), (&ItemContainer::OpaqueItem(_), _) => Ordering::Less, (_, &ItemContainer::OpaqueItem(_)) => Ordering::Greater, diff --git a/src/bindgen/ir/annotation.rs b/src/bindgen/ir/annotation.rs index 208a17778..c3c8db03d 100644 --- a/src/bindgen/ir/annotation.rs +++ b/src/bindgen/ir/annotation.rs @@ -130,19 +130,19 @@ impl AnnotationSet { pub fn list(&self, name: &str) -> Option> { match self.annotations.get(name) { - Some(&AnnotationValue::List(ref x)) => Some(x.clone()), + Some(AnnotationValue::List(x)) => Some(x.clone()), _ => None, } } pub fn atom(&self, name: &str) -> Option> { match self.annotations.get(name) { - Some(&AnnotationValue::Atom(ref x)) => Some(x.clone()), + Some(AnnotationValue::Atom(x)) => Some(x.clone()), _ => None, } } pub fn bool(&self, name: &str) -> Option { match self.annotations.get(name) { - Some(&AnnotationValue::Bool(ref x)) => Some(*x), + Some(AnnotationValue::Bool(x)) => Some(*x), _ => None, } } @@ -152,7 +152,7 @@ impl AnnotationSet { T: Default + FromStr, { match self.annotations.get(name) { - Some(&AnnotationValue::Atom(ref x)) => Some( + Some(AnnotationValue::Atom(x)) => Some( x.as_ref() .map_or(T::default(), |y| y.parse::().ok().unwrap()), ), diff --git a/src/bindgen/ir/item.rs b/src/bindgen/ir/item.rs index a1c3a7521..16d98f55d 100644 --- a/src/bindgen/ir/item.rs +++ b/src/bindgen/ir/item.rs @@ -219,12 +219,12 @@ impl ItemMap { F: FnMut(&T), { match self.data.get(path) { - Some(&ItemValue::Cfg(ref items)) => { + Some(ItemValue::Cfg(items)) => { for item in items { callback(item); } } - Some(&ItemValue::Single(ref item)) => { + Some(ItemValue::Single(item)) => { callback(item); } None => {} diff --git a/src/main.rs b/src/main.rs index e6dec459d..812366f49 100644 --- a/src/main.rs +++ b/src/main.rs @@ -157,7 +157,7 @@ fn main() { .long("lang") .value_name("LANGUAGE") .help("Specify the language to output bindings in") - .possible_values(&["c++", "C++", "c", "C", "cython", "Cython"]), + .possible_values(["c++", "C++", "c", "C", "cython", "Cython"]), ) .arg( Arg::new("cpp-compat") @@ -176,7 +176,7 @@ fn main() { .long("style") .value_name("STYLE") .help("Specify the declaration style to use for bindings") - .possible_values(&["Both", "both", "Tag", "tag", "Type", "type"]), + .possible_values(["Both", "both", "Tag", "tag", "Type", "type"]), ) .arg( Arg::new("d") @@ -253,7 +253,7 @@ fn main() { "Specify the profile to use when expanding macros. \ Has no effect otherwise." ) - .possible_values(&["Debug", "debug", "Release", "release"]), + .possible_values(["Debug", "debug", "Release", "release"]), ) .arg( Arg::new("quiet") diff --git a/tests/tests.rs b/tests/tests.rs index 3b44eac3c..88a1c400e 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -23,7 +23,7 @@ fn run_cbindgen( style: Option