Skip to content

Commit

Permalink
Fix manual_let_else and single_match_else lint
Browse files Browse the repository at this point in the history
Used this to find issues, apply suggestions, and manually fixed a clippy bug

```bash
cargo clippy --all-targets --workspace --exclude bindgen-integration --exclude tests_expectations -- -W clippy::manual_let_else -W clippy::single_match_else
```
  • Loading branch information
nyurik authored and pvdrz committed Dec 1, 2024
1 parent 8f21aa6 commit 0c3ae5c
Show file tree
Hide file tree
Showing 18 changed files with 177 additions and 242 deletions.
5 changes: 2 additions & 3 deletions bindgen-tests/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@ pub fn main() {
let manifest_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap());
let headers_dir = manifest_dir.join("tests").join("headers");

let headers = match fs::read_dir(headers_dir) {
Ok(dir) => dir,
let Ok(headers) = fs::read_dir(headers_dir) else {
// We may not have headers directory after packaging.
Err(..) => return,
return;
};

let entries =
Expand Down
52 changes: 26 additions & 26 deletions bindgen-tests/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ fn error_diff_mismatch(

if let Some(var) = env::var_os("BINDGEN_TESTS_DIFFTOOL") {
//usecase: var = "meld" -> You can hand check differences
let name = match filename.components().last() {
Some(std::path::Component::Normal(name)) => name,
_ => panic!("Why is the header variable so weird?"),
let Some(std::path::Component::Normal(name)) =
filename.components().last()
else {
panic!("Why is the header variable so weird?")
};
let actual_result_path =
PathBuf::from(env::var("OUT_DIR").unwrap()).join(name);
Expand Down Expand Up @@ -581,29 +582,28 @@ fn test_macro_fallback_non_system_dir() {

let actual = format_code(actual).unwrap();

let (expected_filename, expected) = match clang_version().parsed {
Some((9, _)) => {
let expected_filename = concat!(
env!("CARGO_MANIFEST_DIR"),
"/tests/expectations/tests/libclang-9/macro_fallback_non_system_dir.rs",
);
let expected = include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
"/tests/expectations/tests/libclang-9/macro_fallback_non_system_dir.rs",
));
(expected_filename, expected)
}
_ => {
let expected_filename = concat!(
env!("CARGO_MANIFEST_DIR"),
"/tests/expectations/tests/test_macro_fallback_non_system_dir.rs",
);
let expected = include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
"/tests/expectations/tests/test_macro_fallback_non_system_dir.rs",
));
(expected_filename, expected)
}
let (expected_filename, expected) = if let Some((9, _)) =
clang_version().parsed
{
let expected_filename = concat!(
env!("CARGO_MANIFEST_DIR"),
"/tests/expectations/tests/libclang-9/macro_fallback_non_system_dir.rs",
);
let expected = include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
"/tests/expectations/tests/libclang-9/macro_fallback_non_system_dir.rs",
));
(expected_filename, expected)
} else {
let expected_filename = concat!(
env!("CARGO_MANIFEST_DIR"),
"/tests/expectations/tests/test_macro_fallback_non_system_dir.rs",
);
let expected = include_str!(concat!(
env!("CARGO_MANIFEST_DIR"),
"/tests/expectations/tests/test_macro_fallback_non_system_dir.rs",
));
(expected_filename, expected)
};
let expected = format_code(expected).unwrap();
if expected != actual {
Expand Down
4 changes: 1 addition & 3 deletions bindgen/clang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1882,9 +1882,7 @@ impl TranslationUnit {

/// Save a translation unit to the given file.
pub(crate) fn save(&mut self, file: &str) -> Result<(), CXSaveError> {
let file = if let Ok(cstring) = CString::new(file) {
cstring
} else {
let Ok(file) = CString::new(file) else {
return Err(CXSaveError_Unknown);
};
let ret = unsafe {
Expand Down
59 changes: 25 additions & 34 deletions bindgen/codegen/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,10 @@ pub(crate) fn blob(layout: Layout) -> syn::Type {
// some things that legitimately are more than 8-byte aligned.
//
// Eventually we should be able to `unwrap` here, but...
let ty = match opaque.known_rust_type_for_array() {
Some(ty) => ty,
None => {
warn!("Found unknown alignment on code generation!");
syn::parse_quote! { u8 }
}
};
let ty = opaque.known_rust_type_for_array().unwrap_or_else(|| {
warn!("Found unknown alignment on code generation!");
syn::parse_quote! { u8 }
});

let data_len = opaque.array_size().unwrap_or(layout.size);

Expand Down Expand Up @@ -245,24 +242,21 @@ pub(crate) mod ast_ty {
(FloatKind::Float, false) => raw_type(ctx, "c_float"),
(FloatKind::Double, false) => raw_type(ctx, "c_double"),
(FloatKind::LongDouble, _) => {
match layout {
Some(layout) => {
match layout.size {
4 => syn::parse_quote! { f32 },
8 => syn::parse_quote! { f64 },
// TODO(emilio): If rust ever gains f128 we should
// use it here and below.
_ => super::integer_type(layout)
.unwrap_or(syn::parse_quote! { f64 }),
}
}
None => {
debug_assert!(
false,
"How didn't we know the layout for a primitive type?"
);
syn::parse_quote! { f64 }
if let Some(layout) = layout {
match layout.size {
4 => syn::parse_quote! { f32 },
8 => syn::parse_quote! { f64 },
// TODO(emilio): If rust ever gains f128 we should
// use it here and below.
_ => super::integer_type(layout)
.unwrap_or(syn::parse_quote! { f64 }),
}
} else {
debug_assert!(
false,
"How didn't we know the layout for a primitive type?"
);
syn::parse_quote! { f64 }
}
}
(FloatKind::Float128, _) => {
Expand Down Expand Up @@ -365,17 +359,14 @@ pub(crate) mod ast_ty {
signature
.argument_types()
.iter()
.map(|&(ref name, _ty)| match *name {
Some(ref name) => {
let name = ctx.rust_ident(name);
quote! { #name }
}
None => {
.map(|&(ref name, _ty)| {
let name = if let Some(ref name) = *name {
ctx.rust_ident(name)
} else {
unnamed_arguments += 1;
let name =
ctx.rust_ident(format!("arg{unnamed_arguments}"));
quote! { #name }
}
ctx.rust_ident(format!("arg{unnamed_arguments}"))
};
quote! { #name }
})
.collect()
}
Expand Down
7 changes: 1 addition & 6 deletions bindgen/codegen/impl_debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,7 @@ impl<'a> ImplDebug<'a> for Item {
return None;
}

let ty = match self.as_type() {
Some(ty) => ty,
None => {
return None;
}
};
let ty = self.as_type()?;

fn debug_print(
name: &str,
Expand Down
76 changes: 32 additions & 44 deletions bindgen/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1198,10 +1198,7 @@ impl CodeGenerator for Vtable<'_> {
let function_item = ctx.resolve_item(m.signature());
let function = function_item.expect_function();
let signature_item = ctx.resolve_item(function.signature());
let signature = match signature_item.expect_type().kind() {
TypeKind::Function(ref sig) => sig,
_ => panic!("Function signature type mismatch"),
};
let TypeKind::Function(ref signature) = signature_item.expect_type().kind() else { panic!("Function signature type mismatch") };

// FIXME: Is there a canonical name without the class prepended?
let function_name = function_item.canonical_name(ctx);
Expand Down Expand Up @@ -1943,16 +1940,14 @@ impl<'a> FieldCodegen<'a> for Bitfield {
let bitfield_ty_layout = bitfield_ty
.layout(ctx)
.expect("Bitfield without layout? Gah!");
let bitfield_int_ty = match helpers::integer_type(bitfield_ty_layout) {
Some(int_ty) => {
let bitfield_int_ty =
if let Some(int_ty) = helpers::integer_type(bitfield_ty_layout) {
*bitfield_representable_as_int = true;
int_ty
}
None => {
} else {
*bitfield_representable_as_int = false;
return;
}
};
};

let bitfield_ty =
bitfield_ty.to_rust_ty_or_opaque(ctx, bitfield_ty_item);
Expand Down Expand Up @@ -2974,20 +2969,18 @@ impl Method {
}
let function = function_item.expect_function();
let times_seen = function.codegen(ctx, result, function_item);
let times_seen = match times_seen {
Some(seen) => seen,
None => return,
};
let Some(times_seen) = times_seen else { return };
let signature_item = ctx.resolve_item(function.signature());
let mut name = match self.kind() {
MethodKind::Constructor => "new".into(),
MethodKind::Destructor => "destruct".into(),
_ => function.name().to_owned(),
};

let signature = match *signature_item.expect_type().kind() {
TypeKind::Function(ref sig) => sig,
_ => panic!("How in the world?"),
let TypeKind::Function(ref signature) =
*signature_item.expect_type().kind()
else {
panic!("How in the world?")
};

let supported_abi = signature.abi(ctx, Some(&*name)).is_ok();
Expand Down Expand Up @@ -3564,18 +3557,17 @@ impl CodeGenerator for Enum {
// * the representation couldn't be determined from the C source
// * it was explicitly requested as a bindgen option

let kind = match repr {
Some(repr) => match *repr.canonical_type(ctx).kind() {
let kind = if let Some(repr) = repr {
match *repr.canonical_type(ctx).kind() {
TypeKind::Int(int_kind) => int_kind,
_ => panic!("Unexpected type as enum repr"),
},
None => {
warn!(
"Guessing type of enum! Forward declarations of enums \
shouldn't be legal!"
);
IntKind::Int
}
} else {
warn!(
"Guessing type of enum! Forward declarations of enums \
shouldn't be legal!"
);
IntKind::Int
};

let signed = kind.is_signed();
Expand Down Expand Up @@ -4488,9 +4480,8 @@ impl CodeGenerator for Function {

let signature_item = ctx.resolve_item(self.signature());
let signature = signature_item.kind().expect_type().canonical_type(ctx);
let signature = match *signature.kind() {
TypeKind::Function(ref sig) => sig,
_ => panic!("Signature kind is not a Function: {signature:?}"),
let TypeKind::Function(ref signature) = *signature.kind() else {
panic!("Signature kind is not a Function: {signature:?}")
};

if is_internal {
Expand Down Expand Up @@ -4966,9 +4957,8 @@ impl CodeGenerator for ObjCInterface {
.expect_type()
.kind();

let parent = match parent {
TypeKind::ObjCInterface(ref parent) => parent,
_ => break,
let TypeKind::ObjCInterface(parent) = parent else {
break;
};
parent_class = parent.parent_class;

Expand Down Expand Up @@ -5683,12 +5673,11 @@ pub(crate) mod utils {
.map(|(name, ty)| {
let arg_ty = fnsig_argument_type(ctx, ty);

let arg_name = match *name {
Some(ref name) => ctx.rust_mangle(name).into_owned(),
None => {
unnamed_arguments += 1;
format!("arg{unnamed_arguments}")
}
let arg_name = if let Some(ref name) = *name {
ctx.rust_mangle(name).into_owned()
} else {
unnamed_arguments += 1;
format!("arg{unnamed_arguments}")
};

assert!(!arg_name.is_empty());
Expand Down Expand Up @@ -5727,12 +5716,11 @@ pub(crate) mod utils {
.argument_types()
.iter()
.map(|&(ref name, _ty)| {
let arg_name = match *name {
Some(ref name) => ctx.rust_mangle(name).into_owned(),
None => {
unnamed_arguments += 1;
format!("arg{unnamed_arguments}")
}
let arg_name = if let Some(ref name) = *name {
ctx.rust_mangle(name).into_owned()
} else {
unnamed_arguments += 1;
format!("arg{unnamed_arguments}")
};

assert!(!arg_name.is_empty());
Expand Down
7 changes: 4 additions & 3 deletions bindgen/codegen/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,10 @@ impl<'a> CSerialize<'a> for Function {
});
}

let signature = match ctx.resolve_type(self.signature()).kind() {
TypeKind::Function(signature) => signature,
_ => unreachable!(),
let TypeKind::Function(signature) =
ctx.resolve_type(self.signature()).kind()
else {
unreachable!()
};

assert!(!signature.is_variadic());
Expand Down
5 changes: 2 additions & 3 deletions bindgen/codegen/struct_layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,9 +421,8 @@ impl<'a> StructLayoutTracker<'a> {
return false;
}

let layout = match self.latest_field_layout {
Some(l) => l,
None => return false,
let Some(layout) = self.latest_field_layout else {
return false;
};

// If it was, we may or may not need to align, depending on what the
Expand Down
31 changes: 15 additions & 16 deletions bindgen/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,22 +368,21 @@ impl FromStr for RustTarget {
));
}

let (minor, patch) = match tail.split_once('.') {
Some((minor_str, patch_str)) => {
let Ok(minor) = minor_str.parse::<u64>() else {
return Err(invalid_input(input, "the minor version number must be an unsigned 64-bit integer"));
};
let Ok(patch) = patch_str.parse::<u64>() else {
return Err(invalid_input(input, "the patch version number must be an unsigned 64-bit integer"));
};
(minor, patch)
}
None => {
let Ok(minor) = tail.parse::<u64>() else {
return Err(invalid_input(input, "the minor version number must be an unsigned 64-bit integer"));
};
(minor, 0)
}
let (minor, patch) = if let Some((minor_str, patch_str)) =
tail.split_once('.')
{
let Ok(minor) = minor_str.parse::<u64>() else {
return Err(invalid_input(input, "the minor version number must be an unsigned 64-bit integer"));
};
let Ok(patch) = patch_str.parse::<u64>() else {
return Err(invalid_input(input, "the patch version number must be an unsigned 64-bit integer"));
};
(minor, patch)
} else {
let Ok(minor) = tail.parse::<u64>() else {
return Err(invalid_input(input, "the minor version number must be an unsigned 64-bit integer"));
};
(minor, 0)
};

Self::stable(minor, patch).map_err(|err| invalid_input(input, err))
Expand Down
Loading

0 comments on commit 0c3ae5c

Please sign in to comment.