From 6b98586ddccdd6733ea08f9cc4454c62d7a92d3a Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sun, 15 May 2022 21:59:52 -0400 Subject: [PATCH 1/3] Check clippy for --release warnings too --- CONTRIBUTING.md | 2 +- Earthfile | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 57e16af83a..54dd29a18f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -14,7 +14,7 @@ Most contributors execute the following commands befor pushing their code: ``` cargo test cargo fmt --all -- --check -cargo clippy --workspace --tests -- -D warnings +cargo clippy --workspace --tests -- --deny warnings ``` Execute `cargo fmt --all` to fix the formatting. diff --git a/Earthfile b/Earthfile index 6ad2394b0d..71b63abf80 100644 --- a/Earthfile +++ b/Earthfile @@ -69,7 +69,9 @@ check-clippy: FROM +build-rust-test RUN cargo clippy -V RUN --mount=type=cache,target=$SCCACHE_DIR \ - cargo clippy --workspace --tests -- -D warnings + cargo clippy --workspace --tests -- --deny warnings + RUN --mount=type=cache,target=$SCCACHE_DIR \ + cargo clippy --workspace --tests --release -- --deny warnings check-rustfmt: FROM +build-rust-test From 7a353f0e0c04d22706ccb08c69af290393003222 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Sun, 15 May 2022 22:09:21 -0400 Subject: [PATCH 2/3] Fix --release warnings --- compiler/load_internal/src/file.rs | 23 +++++++++-------------- compiler/unify/src/unify.rs | 24 ++++++++++++++---------- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/compiler/load_internal/src/file.rs b/compiler/load_internal/src/file.rs index 2e38ddc178..7a87c92bd4 100644 --- a/compiler/load_internal/src/file.rs +++ b/compiler/load_internal/src/file.rs @@ -16,6 +16,11 @@ use roc_constrain::module::{ ExposedModuleTypes, }; use roc_debug_flags::dbg_do; +#[cfg(debug_assertions)] +use roc_debug_flags::{ + ROC_PRINT_IR_AFTER_REFCOUNT, ROC_PRINT_IR_AFTER_RESET_REUSE, ROC_PRINT_IR_AFTER_SPECIALIZATION, + ROC_PRINT_LOAD_LOG, +}; use roc_error_macros::internal_error; use roc_module::ident::{Ident, ModuleName, QualifiedModuleName}; use roc_module::symbol::{ @@ -54,12 +59,6 @@ use std::{env, fs}; use crate::work::Dependencies; pub use crate::work::Phase; -#[cfg(debug_assertions)] -use roc_debug_flags::{ - ROC_PRINT_IR_AFTER_REFCOUNT, ROC_PRINT_IR_AFTER_RESET_REUSE, ROC_PRINT_IR_AFTER_SPECIALIZATION, - ROC_PRINT_LOAD_LOG, -}; - #[cfg(target_family = "wasm")] use crate::wasm_system_time::{Duration, SystemTime}; #[cfg(not(target_family = "wasm"))] @@ -3876,8 +3875,7 @@ fn canonicalize_and_constrain<'a>( .. } = parsed; - // NOTE: This is prefixed with underscore because it - // is unused in release builds. + // _before has an underscore because it's unused in --release builds let _before = roc_types::types::get_type_clone_count(); let mut var_store = VarStore::default(); @@ -3896,8 +3894,7 @@ fn canonicalize_and_constrain<'a>( &mut var_store, ); - // NOTE: This is prefixed with underscore because it - // is unused in release builds. + // _after has an underscore because it's unused in --release builds let _after = roc_types::types::get_type_clone_count(); log!( @@ -3928,8 +3925,7 @@ fn canonicalize_and_constrain<'a>( } }; - // NOTE: This is prefixed with underscore because it - // is unused in release builds. + // _before has an underscore because it's unused in --release builds let _before = roc_types::types::get_type_clone_count(); let mut constraints = Constraints::new(); @@ -3946,8 +3942,7 @@ fn canonicalize_and_constrain<'a>( ) }; - // NOTE: This is prefixed with underscore because it - // is unused in release builds. + // _after has an underscore because it's unused in --release builds let _after = roc_types::types::get_type_clone_count(); log!( diff --git a/compiler/unify/src/unify.rs b/compiler/unify/src/unify.rs index a43e3b9117..822223a0b1 100644 --- a/compiler/unify/src/unify.rs +++ b/compiler/unify/src/unify.rs @@ -1,5 +1,7 @@ use bitflags::bitflags; use roc_debug_flags::dbg_do; +#[cfg(debug_assertions)] +use roc_debug_flags::{ROC_PRINT_MISMATCHES, ROC_PRINT_UNIFICATIONS}; use roc_error_macros::internal_error; use roc_module::ident::{Lowercase, TagName}; use roc_module::symbol::Symbol; @@ -10,9 +12,6 @@ use roc_types::subs::{ }; use roc_types::types::{AliasKind, DoesNotImplementAbility, ErrorType, Mismatch, RecordField}; -#[cfg(debug_assertions)] -use roc_debug_flags::{ROC_PRINT_MISMATCHES, ROC_PRINT_UNIFICATIONS}; - macro_rules! mismatch { () => {{ dbg_do!(ROC_PRINT_MISMATCHES, { @@ -350,6 +349,8 @@ fn unify_context(subs: &mut Subs, pool: &mut Pool, ctx: Context) -> Outcome { #[cfg(debug_assertions)] debug_print_unified_types(subs, &ctx, None); + // This #[allow] is needed in release builds, where `result` is no longer used. + #[allow(clippy::let_and_return)] let result = match &ctx.first_desc.content { FlexVar(opt_name) => unify_flex(subs, &ctx, opt_name, None, &ctx.second_desc.content), FlexAbleVar(opt_name, ability) => unify_flex( @@ -476,7 +477,7 @@ fn unify_two_aliases( subs: &mut Subs, pool: &mut Pool, ctx: &Context, - // NOTE: symbol is unused in release builds; the underscore prefix prevents a warning. + // _symbol has an underscore because it's unused in --release builds _symbol: Symbol, args: AliasVariables, real_var: Variable, @@ -633,7 +634,7 @@ fn unify_opaque( outcome } } - // NOTE: This is prefixed with underscore because it's unused in release builds. + // _other has an underscore because it's unused in --release builds _other => { // The type on the left is an opaque, but the one on the right is not! mismatch!("Cannot unify opaque {:?} with {:?}", symbol, _other) @@ -673,7 +674,7 @@ fn unify_structure( } outcome } - // NOTE: This is prefixed with underscore because it's unused in release builds. + // _name has an underscore because it's unused in --release builds RigidVar(_name) => { // Type mismatch! Rigid can only unify with flex. mismatch!( @@ -724,7 +725,8 @@ fn unify_structure( // Unify the two flat types unify_flat_type(subs, pool, ctx, flat_type, other_flat_type) } - // NOTE: _sym is prefixed with underscore because it's unused in release builds. + + // _sym has an underscore because it's unused in --release builds Alias(_sym, _, real_var, kind) => match kind { AliasKind::Structural => { // NB: not treating this as a presence constraint seems pivotal! I @@ -1703,7 +1705,8 @@ fn unify_flat_type( unify_tag_union_new(subs, pool, ctx, tags1, *ext1, *tags2, *ext2, rec) } - // NOTE: These are prefixed with underscores because they're unused in release builds. + + // these have underscores because they're unused in --release builds (_other1, _other2) => { // any other combination is a mismatch mismatch!( @@ -1795,7 +1798,8 @@ fn unify_rigid( output.must_implement_ability.push(must_implement_ability); output } - // NOTE: These are prefixed with underscores because they're unused in release builds. + + // these have underscores because they're unused in --release builds (Some(_ability), _other) => { // For now, only allow opaque types with no type variables to implement abilities. mismatch!( @@ -1935,7 +1939,7 @@ fn unify_recursion( }, ), - // NOTE: _opaque is prefixed with underscore because it's unused in release builds. + // _opaque has an underscore because it's unused in --release builds Alias(_opaque, _, _, AliasKind::Opaque) => { mismatch!( "RecursionVar {:?} cannot be equal to opaque {:?}", From 9c72e5e8eda21901d98b81a9c823189bb5e25c3c Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 18 May 2022 08:29:40 -0400 Subject: [PATCH 3/3] Remove unnecessary -> () function return types --- bindgen/src/bindgen_rs.rs | 8 ++++---- bindgen/tests/gen_rs.rs | 12 ++++++------ cli/benches/time_bench.rs | 2 +- compiler/gen_llvm/src/run_roc.rs | 10 +++++----- compiler/solve/tests/helpers/mod.rs | 2 +- compiler/test_gen/src/helpers/mod.rs | 2 +- compiler/test_mono/src/tests.rs | 2 +- compiler/test_mono_macros/src/lib.rs | 2 +- examples/breakout/platform/src/roc.rs | 2 +- examples/false-interpreter/platform/src/lib.rs | 2 +- examples/interactive/cli-platform/src/lib.rs | 4 ++-- nightly_benches/benches/events_bench.rs | 2 +- reporting/tests/helpers/mod.rs | 2 +- 13 files changed, 26 insertions(+), 26 deletions(-) diff --git a/bindgen/src/bindgen_rs.rs b/bindgen/src/bindgen_rs.rs index a27d75a3e4..a7c828bfbb 100644 --- a/bindgen/src/bindgen_rs.rs +++ b/bindgen/src/bindgen_rs.rs @@ -318,7 +318,7 @@ impl {name} {{ r#" /// Other `into_` methods return a payload, but since the {tag_name} tag /// has no payload, this does nothing and is only here for completeness. - pub fn into_{tag_name}(self) -> () {{ + pub fn into_{tag_name}(self) {{ () }}"#, )?; @@ -329,7 +329,7 @@ impl {name} {{ r#" /// Other `as` methods return a payload, but since the {tag_name} tag /// has no payload, this does nothing and is only here for completeness. - pub unsafe fn as_{tag_name}(&self) -> () {{ + pub unsafe fn as_{tag_name}(&self) {{ () }}"#, )?; @@ -904,7 +904,7 @@ impl {name} {{ r#" /// Other `into_` methods return a payload, but since the {null_tag} tag /// has no payload, this does nothing and is only here for completeness. - pub fn into_{null_tag}(self) -> () {{ + pub fn into_{null_tag}(self) {{ () }}"#, )?; @@ -915,7 +915,7 @@ impl {name} {{ r#" /// Other `as` methods return a payload, but since the {null_tag} tag /// has no payload, this does nothing and is only here for completeness. - pub unsafe fn as_{null_tag}(&self) -> () {{ + pub unsafe fn as_{null_tag}(&self) {{ () }}"#, )?; diff --git a/bindgen/tests/gen_rs.rs b/bindgen/tests/gen_rs.rs index 4405ef2309..237d6a43ec 100644 --- a/bindgen/tests/gen_rs.rs +++ b/bindgen/tests/gen_rs.rs @@ -250,13 +250,13 @@ fn tag_union_aliased() { /// Other `into_` methods return a payload, but since the Baz tag /// has no payload, this does nothing and is only here for completeness. - pub fn into_Baz(self) -> () { + pub fn into_Baz(self) { () } /// Other `as` methods return a payload, but since the Baz tag /// has no payload, this does nothing and is only here for completeness. - pub unsafe fn as_Baz(&self) -> () { + pub unsafe fn as_Baz(&self) { () } @@ -586,13 +586,13 @@ fn cons_list_of_strings() { /// Other `into_` methods return a payload, but since the Nil tag /// has no payload, this does nothing and is only here for completeness. - pub fn into_Nil(self) -> () { + pub fn into_Nil(self) { () } /// Other `as` methods return a payload, but since the Nil tag /// has no payload, this does nothing and is only here for completeness. - pub unsafe fn as_Nil(&self) -> () { + pub unsafe fn as_Nil(&self) { () } } @@ -708,13 +708,13 @@ fn cons_list_of_ints() { /// Other `into_` methods return a payload, but since the Empty tag /// has no payload, this does nothing and is only here for completeness. - pub fn into_Empty(self) -> () { + pub fn into_Empty(self) { () } /// Other `as` methods return a payload, but since the Empty tag /// has no payload, this does nothing and is only here for completeness. - pub unsafe fn as_Empty(&self) -> () { + pub unsafe fn as_Empty(&self) { () } } diff --git a/cli/benches/time_bench.rs b/cli/benches/time_bench.rs index 0a99bfd154..7f301634b4 100644 --- a/cli/benches/time_bench.rs +++ b/cli/benches/time_bench.rs @@ -24,7 +24,7 @@ fn bench_group_wall_time(c: &mut Criterion) { group.sample_size(nr_of_runs); - let bench_funcs: Vec>) -> ()> = vec![ + let bench_funcs: Vec>)> = vec![ bench_nqueens, // queens 11 bench_cfold, // e = mkExpr 17 1 bench_deriv, // nest deriv 8 f diff --git a/compiler/gen_llvm/src/run_roc.rs b/compiler/gen_llvm/src/run_roc.rs index 812fbdede6..34f893f68e 100644 --- a/compiler/gen_llvm/src/run_roc.rs +++ b/compiler/gen_llvm/src/run_roc.rs @@ -56,11 +56,11 @@ macro_rules! run_jit_function { } unsafe { - let main: libloading::Symbol) -> ()> = - $lib.get($main_fn_name.as_bytes()) - .ok() - .ok_or(format!("Unable to JIT compile `{}`", $main_fn_name)) - .expect("errored"); + let main: libloading::Symbol)> = $lib + .get($main_fn_name.as_bytes()) + .ok() + .ok_or(format!("Unable to JIT compile `{}`", $main_fn_name)) + .expect("errored"); #[repr(C)] struct Failures { diff --git a/compiler/solve/tests/helpers/mod.rs b/compiler/solve/tests/helpers/mod.rs index ef782e17a1..9d7f565100 100644 --- a/compiler/solve/tests/helpers/mod.rs +++ b/compiler/solve/tests/helpers/mod.rs @@ -33,7 +33,7 @@ where #[inline(always)] pub fn with_larger_debug_stack(run_test: F) where - F: FnOnce() -> (), + F: FnOnce(), F: Send, F: 'static, { diff --git a/compiler/test_gen/src/helpers/mod.rs b/compiler/test_gen/src/helpers/mod.rs index 64697024d1..7285adf7df 100644 --- a/compiler/test_gen/src/helpers/mod.rs +++ b/compiler/test_gen/src/helpers/mod.rs @@ -49,7 +49,7 @@ where #[inline(always)] pub fn with_larger_debug_stack(run_test: F) where - F: FnOnce() -> (), + F: FnOnce(), F: Send, F: 'static, { diff --git a/compiler/test_mono/src/tests.rs b/compiler/test_mono/src/tests.rs index abdcc5b7b3..39f6f74155 100644 --- a/compiler/test_mono/src/tests.rs +++ b/compiler/test_mono/src/tests.rs @@ -51,7 +51,7 @@ where #[inline(always)] pub fn with_larger_debug_stack(run_test: F) where - F: FnOnce() -> (), + F: FnOnce(), F: Send, F: 'static, { diff --git a/compiler/test_mono_macros/src/lib.rs b/compiler/test_mono_macros/src/lib.rs index 00f023125a..2be3ab4ceb 100644 --- a/compiler/test_mono_macros/src/lib.rs +++ b/compiler/test_mono_macros/src/lib.rs @@ -19,7 +19,7 @@ pub fn mono_test(_args: TokenStream, item: TokenStream) -> TokenStream { let result = quote! { #[test] #(#attributes)* - #visibility fn #name(#args) -> () { + #visibility fn #name(#args) { compiles_to_ir(#name_str, #body); } diff --git a/examples/breakout/platform/src/roc.rs b/examples/breakout/platform/src/roc.rs index a8aeb73d31..3c85aef60f 100644 --- a/examples/breakout/platform/src/roc.rs +++ b/examples/breakout/platform/src/roc.rs @@ -14,7 +14,7 @@ extern "C" { // program #[link_name = "roc__programForHost_1_exposed_generic"] - fn roc_program() -> (); + fn roc_program(); #[link_name = "roc__programForHost_size"] fn roc_program_size() -> i64; diff --git a/examples/false-interpreter/platform/src/lib.rs b/examples/false-interpreter/platform/src/lib.rs index 7efee7307c..67c2dbe26d 100644 --- a/examples/false-interpreter/platform/src/lib.rs +++ b/examples/false-interpreter/platform/src/lib.rs @@ -19,7 +19,7 @@ extern "C" { fn roc_main_size() -> i64; #[link_name = "roc__mainForHost_1_Fx_caller"] - fn call_Fx(flags: *const u8, closure_data: *const u8, output: *mut u8) -> (); + fn call_Fx(flags: *const u8, closure_data: *const u8, output: *mut u8); #[allow(dead_code)] #[link_name = "roc__mainForHost_1_Fx_size"] diff --git a/examples/interactive/cli-platform/src/lib.rs b/examples/interactive/cli-platform/src/lib.rs index b0b1783072..47e6e62424 100644 --- a/examples/interactive/cli-platform/src/lib.rs +++ b/examples/interactive/cli-platform/src/lib.rs @@ -10,13 +10,13 @@ use std::os::raw::c_char; extern "C" { #[link_name = "roc__mainForHost_1_exposed_generic"] - fn roc_main(output: *mut u8) -> (); + fn roc_main(output: *mut u8); #[link_name = "roc__mainForHost_size"] fn roc_main_size() -> i64; #[link_name = "roc__mainForHost_1_Fx_caller"] - fn call_Fx(flags: *const u8, closure_data: *const u8, output: *mut u8) -> (); + fn call_Fx(flags: *const u8, closure_data: *const u8, output: *mut u8); #[allow(dead_code)] #[link_name = "roc__mainForHost_1_Fx_size"] diff --git a/nightly_benches/benches/events_bench.rs b/nightly_benches/benches/events_bench.rs index be889672f2..2e529f8c70 100644 --- a/nightly_benches/benches/events_bench.rs +++ b/nightly_benches/benches/events_bench.rs @@ -13,7 +13,7 @@ fn bench_group(c: &mut Criterion, hw_event_str: &str) { // calculate statistics based on a fixed(flat) 100 runs group.sampling_mode(SamplingMode::Flat); - let bench_funcs: Vec>) -> ()> = vec![ + let bench_funcs: Vec>)> = vec![ bench_nqueens, bench_cfold, bench_deriv, diff --git a/reporting/tests/helpers/mod.rs b/reporting/tests/helpers/mod.rs index 3582a4903a..c181483ec5 100644 --- a/reporting/tests/helpers/mod.rs +++ b/reporting/tests/helpers/mod.rs @@ -82,7 +82,7 @@ where #[inline(always)] pub fn with_larger_debug_stack(run_test: F) where - F: FnOnce() -> (), + F: FnOnce(), F: Send, F: 'static, {