Skip to content

Commit

Permalink
Fix needless_pass_by_value lint
Browse files Browse the repository at this point in the history
This also gets rid of some unneeded cloning
  • Loading branch information
nyurik authored and emilio committed Jan 9, 2025
1 parent b660237 commit 5d40c68
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 60 deletions.
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ wildcard_imports = "allow"
# TODO
borrow_as_ptr = "allow"
trivially_copy_pass_by_ref = "allow"
needless_pass_by_value = "allow"
unused_self = "allow"

# Theese seem to be ok to ignore for now
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ pub fn test_item_discovery_callback() {
),
]);

compare_item_caches(info.borrow().clone(), expected);
compare_item_caches(&info.borrow(), &expected);
}

pub fn compare_item_caches(generated: ItemCache, expected: ItemCache) {
pub fn compare_item_caches(generated: &ItemCache, expected: &ItemCache) {
// We can't use a simple Eq::eq comparison because of two reasons:
// - anonymous structs/unions will have a final name generated by bindgen which may change
// if the header file or the bindgen logic is altered
Expand All @@ -89,8 +89,8 @@ pub fn compare_item_caches(generated: ItemCache, expected: ItemCache) {
compare_item_info(
expected_item,
generated_item,
&expected,
&generated,
expected,
generated,
)
});

Expand Down
5 changes: 3 additions & 2 deletions bindgen-tests/tests/quickchecking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ static CONTEXT: Mutex<Context> = Mutex::new(Context { output_path: None });
// Passes fuzzed header to the `csmith-fuzzing/predicate.py` script, returns
// output of the associated command.
fn run_predicate_script(
header: fuzzers::HeaderC,
header: &fuzzers::HeaderC,
) -> Result<Output, Box<dyn Error>> {
let dir = Builder::new().prefix("bindgen_prop").tempdir()?;
let header_path = dir.path().join("prop_test.h");
Expand Down Expand Up @@ -77,8 +77,9 @@ fn run_predicate_script(
// Generatable property. Pass generated headers off to run through the
// `csmith-fuzzing/predicate.py` script. Success is measured by the success
// status of that command.
#[allow(clippy::needless_pass_by_value)]
fn bindgen_prop(header: fuzzers::HeaderC) -> TestResult {
match run_predicate_script(header) {
match run_predicate_script(&header) {
Ok(o) => TestResult::from_bool(o.status.success()),
Err(e) => {
println!("{e:?}");
Expand Down
18 changes: 9 additions & 9 deletions bindgen/codegen/dyngen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl DynamicItems {

pub(crate) fn get_tokens(
&self,
lib_ident: Ident,
lib_ident: &Ident,
ctx: &BindgenContext,
) -> TokenStream {
let struct_members = &self.struct_members;
Expand Down Expand Up @@ -130,15 +130,15 @@ impl DynamicItems {
#[allow(clippy::too_many_arguments)]
pub(crate) fn push_func(
&mut self,
ident: Ident,
ident: &Ident,
abi: ClangAbi,
is_variadic: bool,
is_required: bool,
args: Vec<TokenStream>,
args_identifiers: Vec<TokenStream>,
ret: TokenStream,
ret_ty: TokenStream,
attributes: Vec<TokenStream>,
args: &[TokenStream],
args_identifiers: &[TokenStream],
ret: &TokenStream,
ret_ty: &TokenStream,
attributes: &[TokenStream],
ctx: &BindgenContext,
) {
if !is_variadic {
Expand Down Expand Up @@ -205,8 +205,8 @@ impl DynamicItems {

pub fn push_var(
&mut self,
ident: Ident,
ty: TokenStream,
ident: &Ident,
ty: &TokenStream,
is_required: bool,
wrap_unsafe_ops: bool,
) {
Expand Down
2 changes: 1 addition & 1 deletion bindgen/codegen/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub(crate) mod attributes {
}
}

pub(crate) fn doc(comment: String) -> TokenStream {
pub(crate) fn doc(comment: &str) -> TokenStream {
if comment.is_empty() {
quote!()
} else {
Expand Down
10 changes: 5 additions & 5 deletions bindgen/codegen/impl_debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ impl<'a> ImplDebug<'a> for Item {

fn debug_print(
name: &str,
name_ident: proc_macro2::TokenStream,
name_ident: &proc_macro2::TokenStream,
) -> Option<(String, Vec<proc_macro2::TokenStream>)> {
Some((
format!("{name}: {{:?}}"),
Expand All @@ -154,13 +154,13 @@ impl<'a> ImplDebug<'a> for Item {
TypeKind::ObjCInterface(..) |
TypeKind::ObjCId |
TypeKind::Comp(..) |
TypeKind::ObjCSel => debug_print(name, quote! { #name_ident }),
TypeKind::ObjCSel => debug_print(name, &quote! { #name_ident }),

TypeKind::TemplateInstantiation(ref inst) => {
if inst.is_opaque(ctx, self) {
Some((format!("{name}: opaque"), vec![]))
} else {
debug_print(name, quote! { #name_ident })
debug_print(name, &quote! { #name_ident })
}
}

Expand All @@ -177,7 +177,7 @@ impl<'a> ImplDebug<'a> for Item {
ctx.options().rust_features().larger_arrays
{
// The simple case
debug_print(name, quote! { #name_ident })
debug_print(name, &quote! { #name_ident })
} else if ctx.options().use_core {
// There is no String in core; reducing field visibility to avoid breaking
// no_std setups.
Expand Down Expand Up @@ -233,7 +233,7 @@ impl<'a> ImplDebug<'a> for Item {
{
Some((format!("{name}: FunctionPointer"), vec![]))
}
_ => debug_print(name, quote! { #name_ident }),
_ => debug_print(name, &quote! { #name_ident }),
}
}

Expand Down
8 changes: 4 additions & 4 deletions bindgen/codegen/impl_partialeq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ fn gen_field(
name: &str,
) -> proc_macro2::TokenStream {
fn quote_equals(
name_ident: proc_macro2::Ident,
name_ident: &proc_macro2::Ident,
) -> proc_macro2::TokenStream {
quote! { self.#name_ident == other.#name_ident }
}
Expand All @@ -100,23 +100,23 @@ fn gen_field(
TypeKind::Comp(..) |
TypeKind::Pointer(_) |
TypeKind::Function(..) |
TypeKind::Opaque => quote_equals(name_ident),
TypeKind::Opaque => quote_equals(&name_ident),

TypeKind::TemplateInstantiation(ref inst) => {
if inst.is_opaque(ctx, ty_item) {
quote! {
&self. #name_ident [..] == &other. #name_ident [..]
}
} else {
quote_equals(name_ident)
quote_equals(&name_ident)
}
}

TypeKind::Array(_, len) => {
if len <= RUST_DERIVE_IN_ARRAY_LIMIT ||
ctx.options().rust_features().larger_arrays
{
quote_equals(name_ident)
quote_equals(&name_ident)
} else {
quote! {
&self. #name_ident [..] == &other. #name_ident [..]
Expand Down
Loading

0 comments on commit 5d40c68

Please sign in to comment.