From 51b172fc6fb0b78c048c1286cbe9c9933b345f9b Mon Sep 17 00:00:00 2001 From: Filipe Casal Date: Wed, 17 Jan 2024 11:28:26 +0000 Subject: [PATCH 1/6] Add function name to the non local effect lint. --- .../src/lib.rs | 16 +++++++++------- .../ui/main.stderr | 14 +++++++------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/examples/general/non_local_effect_before_error_return/src/lib.rs b/examples/general/non_local_effect_before_error_return/src/lib.rs index 2e7ccad0f..e6a6c20e0 100644 --- a/examples/general/non_local_effect_before_error_return/src/lib.rs +++ b/examples/general/non_local_effect_before_error_return/src/lib.rs @@ -150,22 +150,21 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalEffectBeforeErrorReturn { |path, contributing_calls, span| { // smoelius: The path is from a return to the start block. for (i, &index) in path.iter().enumerate() { - let basic_block = &mir.basic_blocks[index]; - if_chain! { if !contributing_calls.contains(index); - if let Some(call_span) = is_call_with_mut_ref(cx, mir, &path[i..]); + if let Some(func_and_span) = is_call_with_mut_ref(cx, mir, &path[i..]); then { span_lint_and_then( cx, NON_LOCAL_EFFECT_BEFORE_ERROR_RETURN, - call_span, - "call with mutable reference before error return", + func_and_span.1, + format!("call to {} with mutable reference before error return", func_and_span.0).as_str(), error_note(span), ); } } + let basic_block = &mir.basic_blocks[index]; for statement in basic_block.statements.iter().rev() { if let Some(assign_span) = is_deref_assign(statement) { span_lint_and_then( @@ -200,11 +199,13 @@ fn is_result(cx: &LateContext<'_>, ty: ty::Ty) -> bool { } } +struct FunctionStringAndSpan(String, Span); + fn is_call_with_mut_ref<'tcx>( cx: &LateContext<'tcx>, mir: &'tcx Body<'tcx>, path: &[BasicBlock], -) -> Option { +) -> Option { let index = path[0]; let basic_block = &mir[index]; let terminator = basic_block.terminator(); @@ -223,7 +224,8 @@ fn is_call_with_mut_ref<'tcx>( if locals.iter().any(|local| is_mut_ref_arg(mir, local)) || constants.iter().any(|constant| is_const_ref(constant)); then { - Some(*fn_span) + let func_string = format!("{:#?}", func); + Some(FunctionStringAndSpan(func_string, *fn_span)) } else { None } diff --git a/examples/general/non_local_effect_before_error_return/ui/main.stderr b/examples/general/non_local_effect_before_error_return/ui/main.stderr index ad3d4f057..21721f7d0 100644 --- a/examples/general/non_local_effect_before_error_return/ui/main.stderr +++ b/examples/general/non_local_effect_before_error_return/ui/main.stderr @@ -12,7 +12,7 @@ LL | Err(VarError::NotPresent) = note: `-D non-local-effect-before-error-return` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(non_local_effect_before_error_return)]` -error: call with mutable reference before error return +error: call to std::vec::Vec::::push with mutable reference before error return --> $DIR/main.rs:28:8 | LL | xs.push(0); @@ -36,7 +36,7 @@ note: error is determined here LL | let _ = var("X")?; | ^^^^^^^^^ -error: call with mutable reference before error return +error: call to std::vec::Vec::::push with mutable reference before error return --> $DIR/main.rs:39:8 | LL | xs.push(0); @@ -60,7 +60,7 @@ note: error is determined here LL | let result = Err(VarError::NotPresent); | ^^^^^^^^^^^^^^^^^^^^^^^^^ -error: call with mutable reference before error return +error: call to std::vec::Vec::::push with mutable reference before error return --> $DIR/main.rs:64:8 | LL | xs.push(0); @@ -84,7 +84,7 @@ note: error is determined here LL | match result { | ^^^^^^^^^^^^ -error: call with mutable reference before error return +error: call to std::vec::Vec::::push with mutable reference before error return --> $DIR/main.rs:106:16 | LL | xs.push(0); @@ -120,7 +120,7 @@ note: error is determined here LL | Err(Error::Two) | ^^^^^^^^^^^^^^^ -error: call with mutable reference before error return +error: call to bitflags::_::::insert with mutable reference before error return --> $DIR/main.rs:185:15 | LL | flags.insert(flag); @@ -132,7 +132,7 @@ note: error is determined here LL | return Err(()); | ^^^^^^^ -error: call with mutable reference before error return +error: call to std::string::String::push with mutable reference before error return --> $DIR/main.rs:202:11 | LL | s.push('x'); @@ -144,7 +144,7 @@ note: error is determined here LL | Err(()) | ^^^^^^^ -error: call with mutable reference before error return +error: call to std::process::Command::env::<&str, &str> with mutable reference before error return --> $DIR/main.rs:212:10 | LL | .env("RUST_LOG", "debug") From 6cbe98b87aecb2b2e4a24d60af06fe69a38a420e Mon Sep 17 00:00:00 2001 From: Filipe Casal Date: Wed, 17 Jan 2024 13:46:40 +0000 Subject: [PATCH 2/6] Fix warning --- .../general/non_local_effect_before_error_return/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/general/non_local_effect_before_error_return/src/lib.rs b/examples/general/non_local_effect_before_error_return/src/lib.rs index e6a6c20e0..54531abc7 100644 --- a/examples/general/non_local_effect_before_error_return/src/lib.rs +++ b/examples/general/non_local_effect_before_error_return/src/lib.rs @@ -224,7 +224,7 @@ fn is_call_with_mut_ref<'tcx>( if locals.iter().any(|local| is_mut_ref_arg(mir, local)) || constants.iter().any(|constant| is_const_ref(constant)); then { - let func_string = format!("{:#?}", func); + let func_string = format!("{func:#?}"); Some(FunctionStringAndSpan(func_string, *fn_span)) } else { None From ff04b4b9a3f99c0fec4534816d3fa9e9bb419de1 Mon Sep 17 00:00:00 2001 From: Filipe Casal Date: Thu, 18 Jan 2024 12:00:24 +0000 Subject: [PATCH 3/6] Incorporate review comments --- .../Cargo.toml | 1 + .../src/lib.rs | 13 +++++------ .../ui/main.rs | 10 +++++++++ .../ui/main.stderr | 22 ++++++++++++------- 4 files changed, 30 insertions(+), 16 deletions(-) diff --git a/examples/general/non_local_effect_before_error_return/Cargo.toml b/examples/general/non_local_effect_before_error_return/Cargo.toml index d1b9d9327..b30dfaea6 100644 --- a/examples/general/non_local_effect_before_error_return/Cargo.toml +++ b/examples/general/non_local_effect_before_error_return/Cargo.toml @@ -25,6 +25,7 @@ bitflags = "2.4" once_cell = "1.19" dylint_testing = { path = "../../../utils/testing" } +derivative = "2.2.0" [features] rlib = ["dylint_linting/constituent"] diff --git a/examples/general/non_local_effect_before_error_return/src/lib.rs b/examples/general/non_local_effect_before_error_return/src/lib.rs index 54531abc7..cf978b48f 100644 --- a/examples/general/non_local_effect_before_error_return/src/lib.rs +++ b/examples/general/non_local_effect_before_error_return/src/lib.rs @@ -152,13 +152,13 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalEffectBeforeErrorReturn { for (i, &index) in path.iter().enumerate() { if_chain! { if !contributing_calls.contains(index); - if let Some(func_and_span) = is_call_with_mut_ref(cx, mir, &path[i..]); + if let Some((func, func_span)) = is_call_with_mut_ref(cx, mir, &path[i..]); then { span_lint_and_then( cx, NON_LOCAL_EFFECT_BEFORE_ERROR_RETURN, - func_and_span.1, - format!("call to {} with mutable reference before error return", func_and_span.0).as_str(), + func_span, + &format!("call to `{:?}` with mutable reference before error return", func), error_note(span), ); } @@ -199,13 +199,11 @@ fn is_result(cx: &LateContext<'_>, ty: ty::Ty) -> bool { } } -struct FunctionStringAndSpan(String, Span); - fn is_call_with_mut_ref<'tcx>( cx: &LateContext<'tcx>, mir: &'tcx Body<'tcx>, path: &[BasicBlock], -) -> Option { +) -> Option<(&'tcx Operand<'tcx>, Span)> { let index = path[0]; let basic_block = &mir[index]; let terminator = basic_block.terminator(); @@ -224,8 +222,7 @@ fn is_call_with_mut_ref<'tcx>( if locals.iter().any(|local| is_mut_ref_arg(mir, local)) || constants.iter().any(|constant| is_const_ref(constant)); then { - let func_string = format!("{func:#?}"); - Some(FunctionStringAndSpan(func_string, *fn_span)) + Some((func, *fn_span)) } else { None } diff --git a/examples/general/non_local_effect_before_error_return/ui/main.rs b/examples/general/non_local_effect_before_error_return/ui/main.rs index a4dad73fc..8a29351d8 100644 --- a/examples/general/non_local_effect_before_error_return/ui/main.rs +++ b/examples/general/non_local_effect_before_error_return/ui/main.rs @@ -253,3 +253,13 @@ mod downcast { Ok(()) } } + +use derivative::Derivative; + +#[derive(Derivative)] +#[derivative(Debug)] +struct Foo { + foo: u8, + #[derivative(Debug = "ignore")] + bar: u8, +} diff --git a/examples/general/non_local_effect_before_error_return/ui/main.stderr b/examples/general/non_local_effect_before_error_return/ui/main.stderr index 21721f7d0..a98ea2f36 100644 --- a/examples/general/non_local_effect_before_error_return/ui/main.stderr +++ b/examples/general/non_local_effect_before_error_return/ui/main.stderr @@ -12,7 +12,7 @@ LL | Err(VarError::NotPresent) = note: `-D non-local-effect-before-error-return` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(non_local_effect_before_error_return)]` -error: call to std::vec::Vec::::push with mutable reference before error return +error: call to `std::vec::Vec::::push` with mutable reference before error return --> $DIR/main.rs:28:8 | LL | xs.push(0); @@ -36,7 +36,7 @@ note: error is determined here LL | let _ = var("X")?; | ^^^^^^^^^ -error: call to std::vec::Vec::::push with mutable reference before error return +error: call to `std::vec::Vec::::push` with mutable reference before error return --> $DIR/main.rs:39:8 | LL | xs.push(0); @@ -60,7 +60,7 @@ note: error is determined here LL | let result = Err(VarError::NotPresent); | ^^^^^^^^^^^^^^^^^^^^^^^^^ -error: call to std::vec::Vec::::push with mutable reference before error return +error: call to `std::vec::Vec::::push` with mutable reference before error return --> $DIR/main.rs:64:8 | LL | xs.push(0); @@ -84,7 +84,7 @@ note: error is determined here LL | match result { | ^^^^^^^^^^^^ -error: call to std::vec::Vec::::push with mutable reference before error return +error: call to `std::vec::Vec::::push` with mutable reference before error return --> $DIR/main.rs:106:16 | LL | xs.push(0); @@ -120,7 +120,7 @@ note: error is determined here LL | Err(Error::Two) | ^^^^^^^^^^^^^^^ -error: call to bitflags::_::::insert with mutable reference before error return +error: call to `bitflags::_::::insert` with mutable reference before error return --> $DIR/main.rs:185:15 | LL | flags.insert(flag); @@ -132,7 +132,7 @@ note: error is determined here LL | return Err(()); | ^^^^^^^ -error: call to std::string::String::push with mutable reference before error return +error: call to `std::string::String::push` with mutable reference before error return --> $DIR/main.rs:202:11 | LL | s.push('x'); @@ -144,7 +144,7 @@ note: error is determined here LL | Err(()) | ^^^^^^^ -error: call to std::process::Command::env::<&str, &str> with mutable reference before error return +error: call to `std::process::Command::env::<&str, &str>` with mutable reference before error return --> $DIR/main.rs:212:10 | LL | .env("RUST_LOG", "debug") @@ -156,5 +156,11 @@ error: assignment to dereference before error return LL | *flag = true; | ^^^^^^^^^^^^ -error: aborting due to 14 previous errors +error: call to `std::fmt::DebugStruct::<'_, '_>::field` with mutable reference before error return + --> $DIR/main.rs:261:8 + | +LL | struct Foo { + | ^^^ + +error: aborting due to 15 previous errors From 3642505eefb3abe74cfda112409e43afab205552 Mon Sep 17 00:00:00 2001 From: Filipe Casal Date: Thu, 18 Jan 2024 14:01:24 +0000 Subject: [PATCH 4/6] Sort dep --- .../general/non_local_effect_before_error_return/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/general/non_local_effect_before_error_return/Cargo.toml b/examples/general/non_local_effect_before_error_return/Cargo.toml index b30dfaea6..48f82e49a 100644 --- a/examples/general/non_local_effect_before_error_return/Cargo.toml +++ b/examples/general/non_local_effect_before_error_return/Cargo.toml @@ -22,10 +22,10 @@ dylint_linting = { path = "../../../utils/linting" } [dev-dependencies] bitflags = "2.4" +derivative = "2.2.0" once_cell = "1.19" dylint_testing = { path = "../../../utils/testing" } -derivative = "2.2.0" [features] rlib = ["dylint_linting/constituent"] From cf5245be68eba0128171431aefd08860b15b668d Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Fri, 19 Jan 2024 01:48:44 +0000 Subject: [PATCH 5/6] Update lockfiles --- examples/general/Cargo.lock | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/examples/general/Cargo.lock b/examples/general/Cargo.lock index 3bfe5f72e..3a1171803 100644 --- a/examples/general/Cargo.lock +++ b/examples/general/Cargo.lock @@ -218,6 +218,17 @@ dependencies = [ "typenum", ] +[[package]] +name = "derivative" +version = "2.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fcc3dd5e9e9c0b295d6e1e4d811fb6f157d5ffd784b8d202fc62eac8035a770b" +dependencies = [ + "proc-macro2", + "quote", + "syn 1.0.109", +] + [[package]] name = "diff" version = "0.1.13" @@ -691,6 +702,7 @@ version = "2.6.0" dependencies = [ "bitflags 2.4.1", "clippy_utils", + "derivative", "dylint_linting", "dylint_testing", "if_chain", @@ -888,7 +900,7 @@ dependencies = [ "proc-macro2", "quote", "rust-embed-utils", - "syn", + "syn 2.0.48", "walkdir", ] @@ -990,7 +1002,7 @@ checksum = "46fe8f8603d81ba86327b23a2e9cdf49e1255fb94a4c5f297f6ee0547178ea2c" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.48", ] [[package]] @@ -1024,6 +1036,17 @@ dependencies = [ "digest", ] +[[package]] +name = "syn" +version = "1.0.109" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72b64191b275b66ffe2469e8af2c1cfe3bafa67b529ead792a6d0160888b4237" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "syn" version = "2.0.48" @@ -1104,7 +1127,7 @@ checksum = "fa0faa943b50f3db30a20aa7e265dbc66076993efed8463e8de414e5d06d3471" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.48", ] [[package]] @@ -1200,7 +1223,7 @@ checksum = "34704c8d6ebcbc939824180af020566b01a7c01f80641264eba0999f6c2b6be7" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.48", ] [[package]] From 53d2f21f5107551a1ed6ea2d60ad5b4f24b75a2d Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Fri, 19 Jan 2024 01:49:07 +0000 Subject: [PATCH 6/6] Address Clippy warning --- .../general/non_local_effect_before_error_return/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/general/non_local_effect_before_error_return/src/lib.rs b/examples/general/non_local_effect_before_error_return/src/lib.rs index cf978b48f..b97d9ecc5 100644 --- a/examples/general/non_local_effect_before_error_return/src/lib.rs +++ b/examples/general/non_local_effect_before_error_return/src/lib.rs @@ -158,7 +158,7 @@ impl<'tcx> LateLintPass<'tcx> for NonLocalEffectBeforeErrorReturn { cx, NON_LOCAL_EFFECT_BEFORE_ERROR_RETURN, func_span, - &format!("call to `{:?}` with mutable reference before error return", func), + &format!("call to `{func:?}` with mutable reference before error return"), error_note(span), ); }