From fb343f32c46596770aecdf7faf77fb10cfb3b804 Mon Sep 17 00:00:00 2001 From: Dinu Blanovschi Date: Sun, 19 Nov 2023 14:45:03 +0100 Subject: [PATCH] add some test cases from rustc's test suite --- tests/ui/needless_move.fixed | 335 ++++++++++++++++++++++++++++++++++ tests/ui/needless_move.rs | 335 ++++++++++++++++++++++++++++++++++ tests/ui/needless_move.stderr | 79 ++++++-- 3 files changed, 733 insertions(+), 16 deletions(-) diff --git a/tests/ui/needless_move.fixed b/tests/ui/needless_move.fixed index f9bc51c3a718..7d3508dc7863 100644 --- a/tests/ui/needless_move.fixed +++ b/tests/ui/needless_move.fixed @@ -3,6 +3,9 @@ //! any of the `move`s are removed. #![warn(clippy::needless_move)] +#![allow(unused)] +#![allow(clippy::useless_format)] +#![allow(clippy::let_and_return)] #[derive(Copy, Clone)] struct Copy; @@ -370,4 +373,336 @@ fn main() { a.non_copy = NonCopy; with_owned(a); }); + + // below are a few tests from rustc's testsuite that use move closures, + // which might uncover edge cases + + // rust/$DIR/closures/2229_closure_analysis/migrations/no_migrations.rs + + fn _no_migrations() { + // Set of test cases that don't need migrations + + #![deny(rust_2021_incompatible_closure_captures)] + + // Copy types as copied by the closure instead of being moved into the closure + // Therefore their drop order isn't tied to the closure and won't be requiring any + // migrations. + fn test1_only_copy_types() { + let t = (0i32, 0i32); + + let c = || { + let _t = t.0; + }; + + c(); + } + + // Same as test1 but using a move closure + fn test2_only_copy_types_move_closure() { + let t = (0i32, 0i32); + + let c = move || { + println!("{}", t.0); + }; + + c(); + } + + // Don't need to migrate if captured by ref + fn test3_only_copy_types_move_closure() { + let t = (String::new(), String::new()); + + let c = || { + println!("{}", t.0); + }; + + c(); + } + + // Test migration analysis in case of Insignificant Drop + Non Drop aggregates. + // Note in this test the closure captures a non Drop type and therefore the variable + // is only captured by ref. + fn test4_insignificant_drop_non_drop_aggregate() { + let t = (String::new(), 0i32); + + let c = || { + let _t = t.1; + }; + + c(); + } + + struct Foo(i32); + impl Drop for Foo { + fn drop(&mut self) { + println!("{:?} dropped", self.0); + } + } + + // Test migration analysis in case of Significant Drop + Non Drop aggregates. + // Note in this test the closure captures a non Drop type and therefore the variable + // is only captured by ref. + fn test5_significant_drop_non_drop_aggregate() { + let t = (Foo(0), 0i32); + + let c = || { + let _t = t.1; + }; + + c(); + } + + fn main() { + test1_only_copy_types(); + test2_only_copy_types_move_closure(); + test3_only_copy_types_move_closure(); + test4_insignificant_drop_non_drop_aggregate(); + test5_significant_drop_non_drop_aggregate(); + } + } + + // rust/$DIR/closures/2229_closure_analysis/run_pass/issue-88476.rs + + fn _issue_88476() { + use std::rc::Rc; + + // Test that we restrict precision when moving not-`Copy` types, if any of the parent paths + // implement `Drop`. This is to ensure that we don't move out of a type that implements Drop. + pub fn test1() { + struct Foo(Rc); + + impl Drop for Foo { + fn drop(self: &mut Foo) {} + } + + let f = Foo(Rc::new(1)); + let x = move || { + println!("{:?}", f.0); + }; + + x(); + } + + // Test that we don't restrict precision when moving `Copy` types(i.e. when copying), + // even if any of the parent paths implement `Drop`. + pub fn test2() { + struct Character { + hp: u32, + name: String, + } + + impl Drop for Character { + fn drop(&mut self) {} + } + + let character = Character { + hp: 100, + name: format!("A"), + }; + + let c = move || println!("{}", character.hp); + + c(); + + println!("{}", character.name); + } + + fn main() {} + } + + // rust/$DIR/closures/2229_closure_analysis/preserve_field_drop_order2.rs + + fn _preserve_field_drop_order2() { + #[derive(Debug)] + struct Dropable(&'static str); + + impl Drop for Dropable { + fn drop(&mut self) { + println!("Dropping {}", self.0) + } + } + + #[derive(Debug)] + struct A { + x: Dropable, + y: Dropable, + } + + #[derive(Debug)] + struct B { + c: A, + d: A, + } + + #[derive(Debug)] + struct R<'a> { + c: &'a A, + d: &'a A, + } + + fn main() { + let a = A { + x: Dropable("x"), + y: Dropable("y"), + }; + + let c = move || println!("{:?} {:?}", a.y, a.x); + + c(); + + let b = B { + c: A { + x: Dropable("b.c.x"), + y: Dropable("b.c.y"), + }, + d: A { + x: Dropable("b.d.x"), + y: Dropable("b.d.y"), + }, + }; + + let d = move || println!("{:?} {:?} {:?} {:?}", b.d.y, b.d.x, b.c.y, b.c.x); + + d(); + + let r = R { + c: &A { + x: Dropable("r.c.x"), + y: Dropable("r.c.y"), + }, + d: &A { + x: Dropable("r.d.x"), + y: Dropable("r.d.y"), + }, + }; + + let e = move || println!("{:?} {:?} {:?} {:?}", r.d.y, r.d.x, r.c.y, r.c.x); + + e(); + } + } + + // rust/$DIR/closures/issue-72408-nested-closures-exponential.rs + + fn _issue_72408_nested_closures_exponential() { + + /* + // commented out because it takes forever to run with this + + // Closures include captured types twice in a type tree. + // + // Wrapping one closure with another leads to doubling + // the amount of types in the type tree. + // + // This test ensures that rust can handle + // deeply nested type trees with a lot + // of duplicated subtrees. + + fn dup(f: impl Fn(i32) -> i32) -> impl Fn(i32) -> i32 { + move |a| f(a * 2) + } + + fn main() { + let f = |a| a; + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + // Compiler dies around here if it tries + // to walk the tree exhaustively. + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + println!("Type size was at least {}", f(1)); + } + + */ + } + + // rust/$DIR/closures/issue-97607.rs + + fn _issue_97607() { + #[allow(unused)] + + fn test(f: F) -> Box U + 'static> + where + F: 'static + Fn(T) -> U, + for<'a> U: 'a, // < This is the problematic line, see #97607 + { + Box::new(f) + } + + fn main() {} + } + + // rust/$DIR/closures/once-move-out-on-heap.rs + + fn _once_move_out_on_heap() { + // Testing guarantees provided by once functions. + + use std::sync::Arc; + + fn foo(blk: F) { + blk(); + } + + pub fn main() { + let x = Arc::new(true); + foo(|| { + assert!(*x); + drop(x); + }); + } + } + + // rust/$DIR/closures/supertrait-hint-references-assoc-ty.rs + + fn _supertrait_hint_references_assoc_ty() { + pub trait Fn0: Fn(i32) -> Self::Out { + type Out; + } + + impl Fn0 for F { + type Out = (); + } + + pub fn closure_typer(_: impl Fn0) {} + + fn main() { + closure_typer(|x| { + let _: i64 = x.into(); + }); + } + } } diff --git a/tests/ui/needless_move.rs b/tests/ui/needless_move.rs index da2d1c1e9a7e..aa5154edd871 100644 --- a/tests/ui/needless_move.rs +++ b/tests/ui/needless_move.rs @@ -3,6 +3,9 @@ //! any of the `move`s are removed. #![warn(clippy::needless_move)] +#![allow(unused)] +#![allow(clippy::useless_format)] +#![allow(clippy::let_and_return)] #[derive(Copy, Clone)] struct Copy; @@ -370,4 +373,336 @@ fn main() { a.non_copy = NonCopy; with_owned(a); }); + + // below are a few tests from rustc's testsuite that use move closures, + // which might uncover edge cases + + // rust/tests/ui/closures/2229_closure_analysis/migrations/no_migrations.rs + + fn _no_migrations() { + // Set of test cases that don't need migrations + + #![deny(rust_2021_incompatible_closure_captures)] + + // Copy types as copied by the closure instead of being moved into the closure + // Therefore their drop order isn't tied to the closure and won't be requiring any + // migrations. + fn test1_only_copy_types() { + let t = (0i32, 0i32); + + let c = || { + let _t = t.0; + }; + + c(); + } + + // Same as test1 but using a move closure + fn test2_only_copy_types_move_closure() { + let t = (0i32, 0i32); + + let c = move || { + println!("{}", t.0); + }; + + c(); + } + + // Don't need to migrate if captured by ref + fn test3_only_copy_types_move_closure() { + let t = (String::new(), String::new()); + + let c = || { + println!("{}", t.0); + }; + + c(); + } + + // Test migration analysis in case of Insignificant Drop + Non Drop aggregates. + // Note in this test the closure captures a non Drop type and therefore the variable + // is only captured by ref. + fn test4_insignificant_drop_non_drop_aggregate() { + let t = (String::new(), 0i32); + + let c = || { + let _t = t.1; + }; + + c(); + } + + struct Foo(i32); + impl Drop for Foo { + fn drop(&mut self) { + println!("{:?} dropped", self.0); + } + } + + // Test migration analysis in case of Significant Drop + Non Drop aggregates. + // Note in this test the closure captures a non Drop type and therefore the variable + // is only captured by ref. + fn test5_significant_drop_non_drop_aggregate() { + let t = (Foo(0), 0i32); + + let c = || { + let _t = t.1; + }; + + c(); + } + + fn main() { + test1_only_copy_types(); + test2_only_copy_types_move_closure(); + test3_only_copy_types_move_closure(); + test4_insignificant_drop_non_drop_aggregate(); + test5_significant_drop_non_drop_aggregate(); + } + } + + // rust/tests/ui/closures/2229_closure_analysis/run_pass/issue-88476.rs + + fn _issue_88476() { + use std::rc::Rc; + + // Test that we restrict precision when moving not-`Copy` types, if any of the parent paths + // implement `Drop`. This is to ensure that we don't move out of a type that implements Drop. + pub fn test1() { + struct Foo(Rc); + + impl Drop for Foo { + fn drop(self: &mut Foo) {} + } + + let f = Foo(Rc::new(1)); + let x = move || { + println!("{:?}", f.0); + }; + + x(); + } + + // Test that we don't restrict precision when moving `Copy` types(i.e. when copying), + // even if any of the parent paths implement `Drop`. + pub fn test2() { + struct Character { + hp: u32, + name: String, + } + + impl Drop for Character { + fn drop(&mut self) {} + } + + let character = Character { + hp: 100, + name: format!("A"), + }; + + let c = move || println!("{}", character.hp); + + c(); + + println!("{}", character.name); + } + + fn main() {} + } + + // rust/tests/ui/closures/2229_closure_analysis/preserve_field_drop_order2.rs + + fn _preserve_field_drop_order2() { + #[derive(Debug)] + struct Dropable(&'static str); + + impl Drop for Dropable { + fn drop(&mut self) { + println!("Dropping {}", self.0) + } + } + + #[derive(Debug)] + struct A { + x: Dropable, + y: Dropable, + } + + #[derive(Debug)] + struct B { + c: A, + d: A, + } + + #[derive(Debug)] + struct R<'a> { + c: &'a A, + d: &'a A, + } + + fn main() { + let a = A { + x: Dropable("x"), + y: Dropable("y"), + }; + + let c = move || println!("{:?} {:?}", a.y, a.x); + + c(); + + let b = B { + c: A { + x: Dropable("b.c.x"), + y: Dropable("b.c.y"), + }, + d: A { + x: Dropable("b.d.x"), + y: Dropable("b.d.y"), + }, + }; + + let d = move || println!("{:?} {:?} {:?} {:?}", b.d.y, b.d.x, b.c.y, b.c.x); + + d(); + + let r = R { + c: &A { + x: Dropable("r.c.x"), + y: Dropable("r.c.y"), + }, + d: &A { + x: Dropable("r.d.x"), + y: Dropable("r.d.y"), + }, + }; + + let e = move || println!("{:?} {:?} {:?} {:?}", r.d.y, r.d.x, r.c.y, r.c.x); + + e(); + } + } + + // rust/tests/ui/closures/issue-72408-nested-closures-exponential.rs + + fn _issue_72408_nested_closures_exponential() { + + /* + // commented out because it takes forever to run with this + + // Closures include captured types twice in a type tree. + // + // Wrapping one closure with another leads to doubling + // the amount of types in the type tree. + // + // This test ensures that rust can handle + // deeply nested type trees with a lot + // of duplicated subtrees. + + fn dup(f: impl Fn(i32) -> i32) -> impl Fn(i32) -> i32 { + move |a| f(a * 2) + } + + fn main() { + let f = |a| a; + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + // Compiler dies around here if it tries + // to walk the tree exhaustively. + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + println!("Type size was at least {}", f(1)); + } + + */ + } + + // rust/tests/ui/closures/issue-97607.rs + + fn _issue_97607() { + #[allow(unused)] + + fn test(f: F) -> Box U + 'static> + where + F: 'static + Fn(T) -> U, + for<'a> U: 'a, // < This is the problematic line, see #97607 + { + Box::new(move |t| f(t)) + } + + fn main() {} + } + + // rust/tests/ui/closures/once-move-out-on-heap.rs + + fn _once_move_out_on_heap() { + // Testing guarantees provided by once functions. + + use std::sync::Arc; + + fn foo(blk: F) { + blk(); + } + + pub fn main() { + let x = Arc::new(true); + foo(move || { + assert!(*x); + drop(x); + }); + } + } + + // rust/tests/ui/closures/supertrait-hint-references-assoc-ty.rs + + fn _supertrait_hint_references_assoc_ty() { + pub trait Fn0: Fn(i32) -> Self::Out { + type Out; + } + + impl ()> Fn0 for F { + type Out = (); + } + + pub fn closure_typer(_: impl Fn0) {} + + fn main() { + closure_typer(move |x| { + let _: i64 = x.into(); + }); + } + } } diff --git a/tests/ui/needless_move.stderr b/tests/ui/needless_move.stderr index 34a559c7cecc..4818378c44fe 100644 --- a/tests/ui/needless_move.stderr +++ b/tests/ui/needless_move.stderr @@ -1,5 +1,14 @@ +error: unneeded unit return type + --> $DIR/needless_move.rs:696:24 + | +LL | impl ()> Fn0 for F { + | ^^^^^^ help: remove the `-> ()` + | + = note: `-D clippy::unused-unit` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unused_unit)]` + error: you seem to use `move`, but the `move` is unnecessary - --> $DIR/needless_move.rs:56:33 + --> $DIR/needless_move.rs:59:33 | LL | let closure = assert_static(move || {}); | -----^^^^^ @@ -11,7 +20,7 @@ LL | let closure = assert_static(move || {}); = help: to override `-D warnings` add `#[allow(clippy::needless_move)]` error: you seem to use `move`, but the `move` is unnecessary - --> $DIR/needless_move.rs:57:29 + --> $DIR/needless_move.rs:60:29 | LL | let fut = assert_static(async move {}); | ^^^^^^-----^^ @@ -21,7 +30,7 @@ LL | let fut = assert_static(async move {}); = note: there were no captured variables, so the `move` is unnecessary error: you seem to use `move`, but the `move` is unnecessary - --> $DIR/needless_move.rs:61:33 + --> $DIR/needless_move.rs:64:33 | LL | let closure = assert_static(move || { | ^---- @@ -35,7 +44,7 @@ LL | | }); = note: there were consumed variables, but no borrowed variables, so the `move` is unnecessary error: you seem to use `move`, but the `move` is unnecessary - --> $DIR/needless_move.rs:107:29 + --> $DIR/needless_move.rs:110:29 | LL | let fut = assert_static(async move { | ^ ----- help: remove the `move` @@ -48,7 +57,7 @@ LL | | }); = note: there were consumed variables, but no borrowed variables, so the `move` is unnecessary error: you seem to use `move`, but the `move` is unnecessary - --> $DIR/needless_move.rs:144:33 + --> $DIR/needless_move.rs:147:33 | LL | let closure = assert_static(move || { | ^---- @@ -63,7 +72,7 @@ LL | | }); = note: there were consumed variables, but no borrowed variables, so the `move` is unnecessary error: you seem to use `move`, but the `move` is unnecessary - --> $DIR/needless_move.rs:158:33 + --> $DIR/needless_move.rs:161:33 | LL | let closure = assert_static(move || { | ^---- @@ -78,7 +87,7 @@ LL | | }); = note: there were consumed variables, but no borrowed variables, so the `move` is unnecessary error: you seem to use `move`, but the `move` is unnecessary - --> $DIR/needless_move.rs:173:33 + --> $DIR/needless_move.rs:176:33 | LL | let closure = assert_static(move || { | ^---- @@ -94,7 +103,7 @@ LL | | }); = note: there were consumed variables, but no borrowed variables, so the `move` is unnecessary error: you seem to use `move`, but the `move` is unnecessary - --> $DIR/needless_move.rs:192:33 + --> $DIR/needless_move.rs:195:33 | LL | let closure = assert_static(move || { | ^---- @@ -108,7 +117,7 @@ LL | | }); = note: there were consumed variables, but no borrowed variables, so the `move` is unnecessary error: you seem to use `move`, but the `move` is unnecessary - --> $DIR/needless_move.rs:230:33 + --> $DIR/needless_move.rs:233:33 | LL | let closure = assert_static(move || { | ^---- @@ -122,7 +131,7 @@ LL | | }); = note: there were consumed variables, but no borrowed variables, so the `move` is unnecessary error: you seem to use `move`, but the `move` is unnecessary - --> $DIR/needless_move.rs:302:33 + --> $DIR/needless_move.rs:305:33 | LL | let closure = assert_static(move || { | ^---- @@ -137,7 +146,7 @@ LL | | }); = note: there were consumed variables, but no borrowed variables, so the `move` is unnecessary error: you seem to use `move`, but the `move` is unnecessary - --> $DIR/needless_move.rs:309:33 + --> $DIR/needless_move.rs:312:33 | LL | let closure = assert_static(move || { | ^---- @@ -152,7 +161,7 @@ LL | | }); = note: there were consumed variables, but no borrowed variables, so the `move` is unnecessary error: you seem to use `move`, but the `move` is unnecessary - --> $DIR/needless_move.rs:316:33 + --> $DIR/needless_move.rs:319:33 | LL | let closure = assert_static(move || { | ^---- @@ -167,7 +176,7 @@ LL | | }); = note: there were consumed variables, but no borrowed variables, so the `move` is unnecessary error: you seem to use `move`, but the `move` is unnecessary - --> $DIR/needless_move.rs:355:29 + --> $DIR/needless_move.rs:358:29 | LL | let fut = assert_static(async move { | ^ ----- help: remove the `move` @@ -181,7 +190,7 @@ LL | | }); = note: there were consumed variables, but no borrowed variables, so the `move` is unnecessary error: you seem to use `move`, but the `move` is unnecessary - --> $DIR/needless_move.rs:362:29 + --> $DIR/needless_move.rs:365:29 | LL | let fut = assert_static(async move { | ^ ----- help: remove the `move` @@ -195,7 +204,7 @@ LL | | }); = note: there were consumed variables, but no borrowed variables, so the `move` is unnecessary error: you seem to use `move`, but the `move` is unnecessary - --> $DIR/needless_move.rs:369:29 + --> $DIR/needless_move.rs:372:29 | LL | let fut = assert_static(async move { | ^ ----- help: remove the `move` @@ -208,5 +217,43 @@ LL | | }); | = note: there were consumed variables, but no borrowed variables, so the `move` is unnecessary -error: aborting due to 15 previous errors +error: redundant closure + --> $DIR/needless_move.rs:663:22 + | +LL | Box::new(move |t| f(t)) + | ^^^^^^^^^^^^^ help: replace the closure with the function itself: `f` + | + = note: `-D clippy::redundant-closure` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::redundant_closure)]` + +error: you seem to use `move`, but the `move` is unnecessary + --> $DIR/needless_move.rs:682:17 + | +LL | foo(move || { + | ^---- + | | + | _________________help: remove the `move` + | | +LL | | assert!(*x); +LL | | drop(x); +LL | | }); + | |_____________^ + | + = note: there were consumed variables, but no borrowed variables, so the `move` is unnecessary + +error: you seem to use `move`, but the `move` is unnecessary + --> $DIR/needless_move.rs:703:27 + | +LL | closure_typer(move |x| { + | ^---- + | | + | ___________________________help: remove the `move` + | | +LL | | let _: i64 = x.into(); +LL | | }); + | |_____________^ + | + = note: there were no captured variables, so the `move` is unnecessary + +error: aborting due to 19 previous errors