Merge pull request #3076 from rtfeldman/fix-release-warnings

Fix --release warnings

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.
 

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

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) {{
         ()
     }}"#,
         )?;

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) {
                         ()
                     }
                 }

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<fn(Option<&mut BenchmarkGroup<WallTime>>) -> ()> = vec![
+    let bench_funcs: Vec<fn(Option<&mut BenchmarkGroup<WallTime>>)> = vec![
         bench_nqueens,   // queens 11
         bench_cfold,     // e = mkExpr 17 1
         bench_deriv,     // nest deriv 8 f

compiler/gen_llvm/src/run_roc.rs

@@ -56,11 +56,11 @@ macro_rules! run_jit_function {
         }
 
         unsafe {
-            let main: libloading::Symbol<unsafe extern "C" fn(*mut RocCallResult<$ty>) -> ()> =
+            let main: libloading::Symbol<unsafe extern "C" fn(*mut RocCallResult<$ty>)> = $lib
-                $lib.get($main_fn_name.as_bytes())
+                .get($main_fn_name.as_bytes())
-                    .ok()
+                .ok()
-                    .ok_or(format!("Unable to JIT compile `{}`", $main_fn_name))
+                .ok_or(format!("Unable to JIT compile `{}`", $main_fn_name))
-                    .expect("errored");
+                .expect("errored");
 
             #[repr(C)]
             struct Failures {

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"))]
@@ -3854,8 +3853,7 @@ fn canonicalize_and_constrain<'a>(
         ..
     } = parsed;
 
-    // NOTE: This is prefixed with underscore because it
+    // _before has an underscore because it's unused in --release builds
-    // is unused in release builds.
     let _before = roc_types::types::get_type_clone_count();
 
     let mut var_store = VarStore::default();
@@ -3874,8 +3872,7 @@ fn canonicalize_and_constrain<'a>(
         &mut var_store,
     );
 
-    // NOTE: This is prefixed with underscore because it
+    // _after has an underscore because it's unused in --release builds
-    // is unused in release builds.
     let _after = roc_types::types::get_type_clone_count();
 
     log!(
@@ -3906,8 +3903,7 @@ fn canonicalize_and_constrain<'a>(
         }
     };
 
-    // NOTE: This is prefixed with underscore because it
+    // _before has an underscore because it's unused in --release builds
-    // is unused in release builds.
     let _before = roc_types::types::get_type_clone_count();
 
     let mut constraints = Constraints::new();
@@ -3924,8 +3920,7 @@ fn canonicalize_and_constrain<'a>(
         )
     };
 
-    // NOTE: This is prefixed with underscore because it
+    // _after has an underscore because it's unused in --release builds
-    // is unused in release builds.
     let _after = roc_types::types::get_type_clone_count();
 
     log!(

compiler/solve/tests/helpers/mod.rs

@@ -33,7 +33,7 @@ where
 #[inline(always)]
 pub fn with_larger_debug_stack<F>(run_test: F)
 where
-    F: FnOnce() -> (),
+    F: FnOnce(),
     F: Send,
     F: 'static,
 {

compiler/test_gen/src/helpers/mod.rs

@@ -49,7 +49,7 @@ where
 #[inline(always)]
 pub fn with_larger_debug_stack<F>(run_test: F)
 where
-    F: FnOnce() -> (),
+    F: FnOnce(),
     F: Send,
     F: 'static,
 {

compiler/test_mono/src/tests.rs

@@ -51,7 +51,7 @@ where
 #[inline(always)]
 pub fn with_larger_debug_stack<F>(run_test: F)
 where
-    F: FnOnce() -> (),
+    F: FnOnce(),
     F: Send,
     F: 'static,
 {

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);
 
         }

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 {:?}",

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;

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"]

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"]

nightly_benches/benches/events_bench.rs

@@ -13,7 +13,7 @@ fn bench_group(c: &mut Criterion<Perf>, hw_event_str: &str) {
     // calculate statistics based on a fixed(flat) 100 runs
     group.sampling_mode(SamplingMode::Flat);
 
-    let bench_funcs: Vec<fn(Option<&mut BenchmarkGroup<Perf>>) -> ()> = vec![
+    let bench_funcs: Vec<fn(Option<&mut BenchmarkGroup<Perf>>)> = vec![
         bench_nqueens,
         bench_cfold,
         bench_deriv,

reporting/tests/helpers/mod.rs

@@ -82,7 +82,7 @@ where
 #[inline(always)]
 pub fn with_larger_debug_stack<F>(run_test: F)
 where
-    F: FnOnce() -> (),
+    F: FnOnce(),
     F: Send,
     F: 'static,
 {