From 132b960e8320d7e607fd912142cff7da05d49d92 Mon Sep 17 00:00:00 2001 From: Johan Andersson Date: Sun, 17 Apr 2022 12:16:59 +0200 Subject: [PATCH] Upgrade from our standard lints v4 -> v6 (#861) --- .cargo/config | 83 +++++++++++++++++++ .../src/compile_result.rs | 2 +- .../src/builder/builder_methods.rs | 21 ++--- .../src/builder/ext_inst.rs | 13 ++- .../src/builder/spirv_asm.rs | 62 +++++++------- .../src/codegen_cx/entry.rs | 15 ++-- crates/rustc_codegen_spirv/src/lib.rs | 69 --------------- crates/spirv-builder/src/lib.rs | 7 +- crates/spirv-builder/src/watch.rs | 59 +++++++------ examples/runners/wgpu/src/bin/wgpu_runner.rs | 2 +- examples/runners/wgpu/src/graphics.rs | 3 +- examples/shaders/compute-shader/src/lib.rs | 2 - tests/src/main.rs | 1 + 13 files changed, 173 insertions(+), 166 deletions(-) diff --git a/.cargo/config b/.cargo/config index 2b1a6f47ca..22dcbe588c 100644 --- a/.cargo/config +++ b/.cargo/config @@ -1,2 +1,85 @@ [alias] compiletest = "run --release -p compiletests --" + +[target.'cfg(all())'] +rustflags = [ + # BEGIN - Embark standard lints v6 for Rust 1.55+ + # do not change or add/remove here, but one can add exceptions after this section + # for more info see: + "-Dunsafe_code", + "-Wclippy::all", + "-Wclippy::await_holding_lock", + "-Wclippy::char_lit_as_u8", + "-Wclippy::checked_conversions", + "-Wclippy::dbg_macro", + "-Wclippy::debug_assert_with_mut_call", + "-Wclippy::doc_markdown", + "-Wclippy::empty_enum", + "-Wclippy::enum_glob_use", + "-Wclippy::exit", + "-Wclippy::expl_impl_clone_on_copy", + "-Wclippy::explicit_deref_methods", + "-Wclippy::explicit_into_iter_loop", + "-Wclippy::fallible_impl_from", + "-Wclippy::filter_map_next", + "-Wclippy::flat_map_option", + "-Wclippy::float_cmp_const", + "-Wclippy::fn_params_excessive_bools", + "-Wclippy::from_iter_instead_of_collect", + "-Wclippy::if_let_mutex", + "-Wclippy::implicit_clone", + "-Wclippy::imprecise_flops", + "-Wclippy::inefficient_to_string", + "-Wclippy::invalid_upcast_comparisons", + "-Wclippy::large_digit_groups", + "-Wclippy::large_stack_arrays", + "-Wclippy::large_types_passed_by_value", + "-Wclippy::let_unit_value", + "-Wclippy::linkedlist", + "-Wclippy::lossy_float_literal", + "-Wclippy::macro_use_imports", + "-Wclippy::manual_ok_or", + "-Wclippy::map_err_ignore", + "-Wclippy::map_flatten", + "-Wclippy::map_unwrap_or", + "-Wclippy::match_on_vec_items", + "-Wclippy::match_same_arms", + "-Wclippy::match_wild_err_arm", + "-Wclippy::match_wildcard_for_single_variants", + "-Wclippy::mem_forget", + "-Wclippy::mismatched_target_os", + "-Wclippy::missing_enforced_import_renames", + "-Wclippy::mut_mut", + "-Wclippy::mutex_integer", + "-Wclippy::needless_borrow", + "-Wclippy::needless_continue", + "-Wclippy::needless_for_each", + "-Wclippy::option_option", + "-Wclippy::path_buf_push_overwrite", + "-Wclippy::ptr_as_ptr", + "-Wclippy::rc_mutex", + "-Wclippy::ref_option_ref", + "-Wclippy::rest_pat_in_fully_bound_structs", + "-Wclippy::same_functions_in_if_condition", + "-Wclippy::semicolon_if_nothing_returned", + "-Wclippy::single_match_else", + "-Wclippy::string_add_assign", + "-Wclippy::string_add", + "-Wclippy::string_lit_as_bytes", + "-Wclippy::string_to_string", + "-Wclippy::todo", + "-Wclippy::trait_duplication_in_bounds", + "-Wclippy::unimplemented", + "-Wclippy::unnested_or_patterns", + "-Wclippy::unused_self", + "-Wclippy::useless_transmute", + "-Wclippy::verbose_file_reads", + "-Wclippy::zero_sized_map_values", + "-Wfuture_incompatible", + "-Wnonstandard_style", + "-Wrust_2018_idioms", + # END - Embark standard lints v6 for Rust 1.55+ + + # repo specific lints + "-Aunsafe_code", +] diff --git a/crates/rustc_codegen_spirv-types/src/compile_result.rs b/crates/rustc_codegen_spirv-types/src/compile_result.rs index 90eb948e02..2f7dc1149d 100644 --- a/crates/rustc_codegen_spirv-types/src/compile_result.rs +++ b/crates/rustc_codegen_spirv-types/src/compile_result.rs @@ -72,7 +72,7 @@ impl<'a> Trie<'a> { children.sort_unstable_by(|(k1, _), (k2, _)| k1.cmp(k2)); for (child_name, child) in children { let full_child_name = if full_name.is_empty() { - child_name.to_string() + (*child_name).to_string() } else { format!("{}::{}", full_name, child_name) }; diff --git a/crates/rustc_codegen_spirv/src/builder/builder_methods.rs b/crates/rustc_codegen_spirv/src/builder/builder_methods.rs index c32985572e..a40da4cc5b 100644 --- a/crates/rustc_codegen_spirv/src/builder/builder_methods.rs +++ b/crates/rustc_codegen_spirv/src/builder/builder_methods.rs @@ -2134,18 +2134,15 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> { return_type, arguments, } => ( - match callee.kind { - SpirvValueKind::FnAddr { function } => { - assert_ty_eq!(self, callee_ty, pointee); - function - } - - // Truly indirect call. - _ => { - let fn_ptr_val = callee.def(self); - self.zombie(fn_ptr_val, "indirect calls are not supported in SPIR-V"); - fn_ptr_val - } + if let SpirvValueKind::FnAddr { function } = callee.kind { + assert_ty_eq!(self, callee_ty, pointee); + function + } + // Truly indirect call. + else { + let fn_ptr_val = callee.def(self); + self.zombie(fn_ptr_val, "indirect calls are not supported in SPIR-V"); + fn_ptr_val }, return_type, arguments, diff --git a/crates/rustc_codegen_spirv/src/builder/ext_inst.rs b/crates/rustc_codegen_spirv/src/builder/ext_inst.rs index 504090fd54..5e60a0304a 100644 --- a/crates/rustc_codegen_spirv/src/builder/ext_inst.rs +++ b/crates/rustc_codegen_spirv/src/builder/ext_inst.rs @@ -14,13 +14,12 @@ pub struct ExtInst { impl ExtInst { pub fn import_glsl<'a, 'tcx>(&mut self, bx: &Builder<'a, 'tcx>) -> Word { - match self.glsl { - Some(id) => id, - None => { - let id = bx.emit_global().ext_inst_import(GLSL_STD_450); - self.glsl = Some(id); - id - } + if let Some(id) = self.glsl { + id + } else { + let id = bx.emit_global().ext_inst_import(GLSL_STD_450); + self.glsl = Some(id); + id } } diff --git a/crates/rustc_codegen_spirv/src/builder/spirv_asm.rs b/crates/rustc_codegen_spirv/src/builder/spirv_asm.rs index 7c1598d4dd..0b23c889ae 100644 --- a/crates/rustc_codegen_spirv/src/builder/spirv_asm.rs +++ b/crates/rustc_codegen_spirv/src/builder/spirv_asm.rs @@ -427,19 +427,16 @@ impl<'cx, 'tcx> Builder<'cx, 'tcx> { Some(result_id) => result_id, None => return, }; - match tokens.next() { - Some(Token::Word("=")) => (), - _ => { - self.err("expected equals after result id specifier"); - return; - } + if let Some(Token::Word("=")) = tokens.next() { + } else { + self.err("expected equals after result id specifier"); + return; } - first_token = match tokens.next() { - Some(tok) => tok, - None => { - self.err("expected instruction after equals"); - return; - } + first_token = if let Some(tok) = tokens.next() { + tok + } else { + self.err("expected instruction after equals"); + return; }; Some(result_id) } else { @@ -461,12 +458,11 @@ impl<'cx, 'tcx> Builder<'cx, 'tcx> { let inst_class = inst_name .strip_prefix("Op") .and_then(|n| self.instruction_table.table.get(n)); - let inst_class = match inst_class { - Some(inst) => inst, - None => { - self.err(&format!("unknown spirv instruction {}", inst_name)); - return; - } + let inst_class = if let Some(inst) = inst_class { + inst + } else { + self.err(&format!("unknown spirv instruction {}", inst_name)); + return; }; let result_id = match out_register { Some(OutRegister::Regular(reg)) => Some(reg), @@ -808,19 +804,20 @@ impl<'cx, 'tcx> Builder<'cx, 'tcx> { token: Token<'a, 'cx, 'tcx>, ) -> Option> { match token { - Token::Word(word) => match word.strip_prefix('%') { - Some(id) => Some(OutRegister::Regular({ - let num = *id_map.entry(id).or_insert_with(|| self.emit().id()); - if !defined_ids.insert(num) { - self.err(&format!("%{} is defined more than once", id)); - } - num - })), - None => { + Token::Word(word) => { + if let Some(id) = word.strip_prefix('%') { + Some(OutRegister::Regular({ + let num = *id_map.entry(id).or_insert_with(|| self.emit().id()); + if !defined_ids.insert(num) { + self.err(&format!("%{} is defined more than once", id)); + } + num + })) + } else { self.err("expected ID"); None } - }, + } Token::String(_) => { self.err("expected ID, not string"); None @@ -896,13 +893,14 @@ impl<'cx, 'tcx> Builder<'cx, 'tcx> { token: Token<'a, 'cx, 'tcx>, ) -> Option { match token { - Token::Word(word) => match word.strip_prefix('%') { - Some(id) => Some(*id_map.entry(id).or_insert_with(|| self.emit().id())), - None => { + Token::Word(word) => { + if let Some(id) = word.strip_prefix('%') { + Some(*id_map.entry(id).or_insert_with(|| self.emit().id())) + } else { self.err("expected ID"); None } - }, + } Token::String(_) => { self.err("expected ID, not string"); None diff --git a/crates/rustc_codegen_spirv/src/codegen_cx/entry.rs b/crates/rustc_codegen_spirv/src/codegen_cx/entry.rs index 7ac3638141..ae13b03363 100644 --- a/crates/rustc_codegen_spirv/src/codegen_cx/entry.rs +++ b/crates/rustc_codegen_spirv/src/codegen_cx/entry.rs @@ -33,14 +33,13 @@ impl<'tcx> CodegenCx<'tcx> { ) { let span = self.tcx.def_span(instance.def_id()); let hir_params = { - let fn_local_def_id = match instance.def_id().as_local() { - Some(id) => id, - None => { - self.tcx - .sess - .span_err(span, &format!("Cannot declare {} as an entry point", name)); - return; - } + let fn_local_def_id = if let Some(id) = instance.def_id().as_local() { + id + } else { + self.tcx + .sess + .span_err(span, &format!("Cannot declare {} as an entry point", name)); + return; }; let fn_hir_id = self.tcx.hir().local_def_id_to_hir_id(fn_local_def_id); let body = self.tcx.hir().body(self.tcx.hir().body_owned_by(fn_hir_id)); diff --git a/crates/rustc_codegen_spirv/src/lib.rs b/crates/rustc_codegen_spirv/src/lib.rs index ce97afb9d4..8300883ab7 100644 --- a/crates/rustc_codegen_spirv/src/lib.rs +++ b/crates/rustc_codegen_spirv/src/lib.rs @@ -18,75 +18,6 @@ #![feature(rustc_private)] #![feature(assert_matches)] #![feature(once_cell)] -// BEGIN - Embark standard lints v0.4 -// do not change or add/remove here, but one can add exceptions after this section -// for more info see: -#![deny(unsafe_code)] -#![warn( - clippy::all, - clippy::await_holding_lock, - clippy::char_lit_as_u8, - clippy::checked_conversions, - clippy::dbg_macro, - clippy::debug_assert_with_mut_call, - clippy::doc_markdown, - clippy::empty_enum, - clippy::enum_glob_use, - clippy::exit, - clippy::expl_impl_clone_on_copy, - clippy::explicit_deref_methods, - clippy::explicit_into_iter_loop, - clippy::fallible_impl_from, - clippy::filter_map_next, - clippy::float_cmp_const, - clippy::fn_params_excessive_bools, - clippy::if_let_mutex, - clippy::implicit_clone, - clippy::imprecise_flops, - clippy::inefficient_to_string, - clippy::invalid_upcast_comparisons, - clippy::large_types_passed_by_value, - clippy::let_unit_value, - clippy::linkedlist, - clippy::lossy_float_literal, - clippy::macro_use_imports, - clippy::manual_ok_or, - clippy::map_err_ignore, - clippy::map_flatten, - clippy::map_unwrap_or, - clippy::match_on_vec_items, - clippy::match_same_arms, - clippy::match_wildcard_for_single_variants, - clippy::mem_forget, - clippy::mismatched_target_os, - clippy::mut_mut, - clippy::mutex_integer, - clippy::needless_borrow, - clippy::needless_continue, - clippy::option_option, - clippy::path_buf_push_overwrite, - clippy::ptr_as_ptr, - clippy::ref_option_ref, - clippy::rest_pat_in_fully_bound_structs, - clippy::same_functions_in_if_condition, - clippy::semicolon_if_nothing_returned, - clippy::string_add_assign, - clippy::string_add, - clippy::string_lit_as_bytes, - clippy::string_to_string, - clippy::todo, - clippy::trait_duplication_in_bounds, - clippy::unimplemented, - clippy::unnested_or_patterns, - clippy::unused_self, - clippy::useless_transmute, - clippy::verbose_file_reads, - clippy::zero_sized_map_values, - future_incompatible, - nonstandard_style, - rust_2018_idioms -)] -// END - Embark standard lints v0.4 // crate-specific exceptions: #![allow( unsafe_code, // rustc_codegen_ssa requires unsafe functions in traits to be impl'd diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 998c662a1b..ea7a8ae283 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -544,9 +544,10 @@ struct RustcOutput { fn get_last_artifact(out: &str) -> Option { let last = out .lines() - .filter_map(|line| match serde_json::from_str::(line) { - Ok(line) => Some(line), - Err(_) => { + .filter_map(|line| { + if let Ok(line) = serde_json::from_str::(line) { + Some(line) + } else { // Pass through invalid lines println!("{}", line); None diff --git a/crates/spirv-builder/src/watch.rs b/crates/spirv-builder/src/watch.rs index ca4624cb2b..b905da34fa 100644 --- a/crates/spirv-builder/src/watch.rs +++ b/crates/spirv-builder/src/watch.rs @@ -21,36 +21,35 @@ impl SpirvBuilder { } let metadata_result = crate::invoke_rustc(&self); // Load the dependencies of the thing - let metadata_file = match metadata_result { - Ok(path) => path, - Err(_) => { - let (tx, rx) = sync_channel(0); - // Fall back to watching from the crate root if the inital compilation fails - let mut watcher = - notify::recommended_watcher(move |event: notify::Result| match event { - Ok(e) => match e.kind { - notify::EventKind::Access(_) => (), - notify::EventKind::Any - | notify::EventKind::Create(_) - | notify::EventKind::Modify(_) - | notify::EventKind::Remove(_) - | notify::EventKind::Other => { - let _ = tx.try_send(()); - } - }, - Err(e) => println!("notify error: {:?}", e), - }) - .expect("Could create watcher"); - // This is likely to notice changes in the `target` dir, however, given that `cargo watch` doesn't seem to handle that, - watcher - .watch(&self.path_to_crate, RecursiveMode::Recursive) - .expect("Could watch crate root"); - loop { - rx.recv().expect("Watcher still alive"); - let metadata_file = crate::invoke_rustc(&self); - if let Ok(f) = metadata_file { - break f; - } + let metadata_file = if let Ok(path) = metadata_result { + path + } else { + let (tx, rx) = sync_channel(0); + // Fall back to watching from the crate root if the inital compilation fails + let mut watcher = + notify::recommended_watcher(move |event: notify::Result| match event { + Ok(e) => match e.kind { + notify::EventKind::Access(_) => (), + notify::EventKind::Any + | notify::EventKind::Create(_) + | notify::EventKind::Modify(_) + | notify::EventKind::Remove(_) + | notify::EventKind::Other => { + let _ = tx.try_send(()); + } + }, + Err(e) => println!("notify error: {:?}", e), + }) + .expect("Could create watcher"); + // This is likely to notice changes in the `target` dir, however, given that `cargo watch` doesn't seem to handle that, + watcher + .watch(&self.path_to_crate, RecursiveMode::Recursive) + .expect("Could watch crate root"); + loop { + rx.recv().expect("Watcher still alive"); + let metadata_file = crate::invoke_rustc(&self); + if let Ok(f) = metadata_file { + break f; } } }; diff --git a/examples/runners/wgpu/src/bin/wgpu_runner.rs b/examples/runners/wgpu/src/bin/wgpu_runner.rs index c91dd9dc84..120e8b9e0f 100644 --- a/examples/runners/wgpu/src/bin/wgpu_runner.rs +++ b/examples/runners/wgpu/src/bin/wgpu_runner.rs @@ -1,3 +1,3 @@ fn main() { - example_runner_wgpu::main() + example_runner_wgpu::main(); } diff --git a/examples/runners/wgpu/src/graphics.rs b/examples/runners/wgpu/src/graphics.rs index 0f5a09036d..1a8ee5c67f 100644 --- a/examples/runners/wgpu/src/graphics.rs +++ b/examples/runners/wgpu/src/graphics.rs @@ -323,6 +323,7 @@ fn create_pipeline( }) } +#[allow(clippy::match_wild_err_arm)] pub fn start(options: &Options) { // Build the shader before we pop open a window, since it might take a while. let event_loop = EventLoop::with_user_event(); @@ -332,7 +333,7 @@ pub fn start(options: &Options) { Some(Box::new(move |res| match proxy.send_event(res) { Ok(it) => it, // ShaderModuleDescriptor is not `Debug`, so can't use unwrap/expect - Err(_) => panic!("Event loop dead"), + Err(_err) => panic!("Event loop dead"), })), ); diff --git a/examples/shaders/compute-shader/src/lib.rs b/examples/shaders/compute-shader/src/lib.rs index 602c3bda8c..d5bc237fc3 100644 --- a/examples/shaders/compute-shader/src/lib.rs +++ b/examples/shaders/compute-shader/src/lib.rs @@ -7,8 +7,6 @@ // HACK(eddyb) can't easily see warnings otherwise from `spirv-builder` builds. #![deny(warnings)] -extern crate spirv_std; - use glam::UVec3; use spirv_std::glam; #[cfg(not(target_arch = "spirv"))] diff --git a/tests/src/main.rs b/tests/src/main.rs index 1e89625320..a03538038e 100644 --- a/tests/src/main.rs +++ b/tests/src/main.rs @@ -94,6 +94,7 @@ struct Runner { impl Runner { /// Runs the given `mode` on the directory that matches that name, using the /// backend provided by `codegen_backend_path`. + #[allow(clippy::string_add)] fn run_mode(&self, mode: &'static str) { /// RUSTFLAGS passed to all test files. fn test_rustc_flags(