Auto merge of #138961 - meithecatte:expr-use-visitor, r=<try>

Make closure capturing have consistent and correct behaviour around patterns

Author: rust-bors[bot]

compiler/rustc_hir_typeck/src/expr_use_visitor.rs

@@ -761,6 +761,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     ///       ],
     /// }
     /// ```
+    #[instrument(level = "debug", skip(self))]
     fn compute_min_captures(
         &self,
         closure_def_id: LocalDefId,
@@ -2030,6 +2031,7 @@ struct InferBorrowKind<'tcx> {
 }
 
 impl<'tcx> euv::Delegate<'tcx> for InferBorrowKind<'tcx> {
+    #[instrument(skip(self), level = "debug")]
     fn fake_read(
         &mut self,
         place_with_id: &PlaceWithHirId<'tcx>,
@@ -2120,6 +2122,7 @@ impl<'tcx> euv::Delegate<'tcx> for InferBorrowKind<'tcx> {
 }
 
 /// Rust doesn't permit moving fields out of a type that implements drop
+#[instrument(skip(fcx), ret, level = "debug")]
 fn restrict_precision_for_drop_types<'a, 'tcx>(
     fcx: &'a FnCtxt<'a, 'tcx>,
     mut place: Place<'tcx>,
@@ -2180,6 +2183,7 @@ fn restrict_precision_for_unsafe(
 /// - No unsafe block is required to capture `place`.
 ///
 /// Returns the truncated place and updated capture mode.
+#[instrument(ret, level = "debug")]
 fn restrict_capture_precision(
     place: Place<'_>,
     curr_mode: ty::UpvarCapture,
@@ -2209,6 +2213,7 @@ fn restrict_capture_precision(
 }
 
 /// Truncate deref of any reference.
+#[instrument(ret, level = "debug")]
 fn adjust_for_move_closure(
     mut place: Place<'_>,
     mut kind: ty::UpvarCapture,
@@ -2223,6 +2228,7 @@ fn adjust_for_move_closure(
 }
 
 /// Truncate deref of any reference.
+#[instrument(ret, level = "debug")]
 fn adjust_for_use_closure(
     mut place: Place<'_>,
     mut kind: ty::UpvarCapture,
@@ -2238,6 +2244,7 @@ fn adjust_for_use_closure(
 
 /// Adjust closure capture just that if taking ownership of data, only move data
 /// from enclosing stack frame.
+#[instrument(ret, level = "debug")]
 fn adjust_for_non_move_closure(
     mut place: Place<'_>,
     mut kind: ty::UpvarCapture,
@@ -2560,6 +2567,7 @@ fn determine_place_ancestry_relation<'tcx>(
 ///     // it is constrained to `'a`
 /// }
 /// ```
+#[instrument(ret, level = "debug")]
 fn truncate_capture_for_optimization(
     mut place: Place<'_>,
     mut curr_mode: ty::UpvarCapture,

compiler/rustc_hir_typeck/src/upvar.rs

@@ -339,6 +339,12 @@ impl<'tcx> MatchPairTree<'tcx> {
 
         if let Some(test_case) = test_case {
             // This pattern is refutable, so push a new match-pair node.
+            //
+            // Note: unless test_case is TestCase::Or, place must not be None.
+            // This means that the closure capture analysis in
+            // rustc_hir_typeck::upvar, and in particular the pattern handling
+            // code of ExprUseVisitor, must capture all of the places we'll use.
+            // Make sure to keep these two parts in sync!
             match_pairs.push(MatchPairTree {
                 place,
                 test_case,

compiler/rustc_mir_build/src/builder/matches/match_pair.rs

@@ -888,7 +888,7 @@ declare_clippy_lint! {
     /// ```
     #[clippy::version = "pre 1.29.0"]
     pub SEARCH_IS_SOME,
-    complexity,
+    nursery,
     "using an iterator or string search followed by `is_some()` or `is_none()`, which is more succinctly expressed as a call to `any()` or `contains()` (with negation in case of `is_none()`)"
 }
 

src/tools/clippy/clippy_lints/src/methods/mod.rs

@@ -1,3 +1,4 @@
+#![warn(clippy::search_is_some)]
 pub struct Thing;
 //@no-rustfix
 pub fn has_thing(things: &[Thing]) -> bool {

src/tools/clippy/tests/ui/crashes/ice-9041.rs

@@ -1,5 +1,5 @@
 error: called `is_some()` after searching an `Iterator` with `find`
-  --> tests/ui/crashes/ice-9041.rs:5:19
+  --> tests/ui/crashes/ice-9041.rs:6:19
    |
 LL |     things.iter().find(|p| is_thing_ready(p)).is_some()
    |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `any(|p| is_thing_ready(&p))`

src/tools/clippy/tests/ui/crashes/ice-9041.stderr

@@ -311,19 +311,23 @@ mod issue9120 {
     }
 }
 
+// skip this test due to rust-lang/rust-clippy#16086
+/*
 #[allow(clippy::match_like_matches_macro)]
 fn issue15102() {
     let values = [None, Some(3)];
-    let has_even = values.iter().any(|v| matches!(&v, Some(x) if x % 2 == 0));
-    //~^ search_is_some
+    let has_even = values.iter().find(|v| matches!(v, Some(x) if x % 2 == 0)).is_some();
+    ~^ search_is_some
     println!("{has_even}");
 
     let has_even = values
         .iter()
-        .any(|v| match &v {
-            //~^ search_is_some
+        .find(|v| match v {
+            ~^ search_is_some
             Some(x) if x % 2 == 0 => true,
             _ => false,
-        });
+        })
+        .is_some();
     println!("{has_even}");
 }
+*/

src/tools/clippy/tests/ui/search_is_some_fixable_some.fixed

@@ -322,20 +322,23 @@ mod issue9120 {
     }
 }
 
+// skip this test due to rust-lang/rust-clippy#16086
+/*
 #[allow(clippy::match_like_matches_macro)]
 fn issue15102() {
     let values = [None, Some(3)];
     let has_even = values.iter().find(|v| matches!(v, Some(x) if x % 2 == 0)).is_some();
-    //~^ search_is_some
+    ~^ search_is_some
     println!("{has_even}");
 
     let has_even = values
         .iter()
         .find(|v| match v {
-            //~^ search_is_some
+            ~^ search_is_some
             Some(x) if x % 2 == 0 => true,
             _ => false,
         })
         .is_some();
     println!("{has_even}");
 }
+*/

src/tools/clippy/tests/ui/search_is_some_fixable_some.rs

@@ -346,32 +346,5 @@ error: called `is_some()` after searching an `Iterator` with `find`
 LL |         let _ = v.iter().find(|x: &&u32| (*arg_no_deref_dyn)(x)).is_some();
    |                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `any(|x: &u32| (*arg_no_deref_dyn)(&x))`
 
-error: called `is_some()` after searching an `Iterator` with `find`
-  --> tests/ui/search_is_some_fixable_some.rs:328:34
-   |
-LL |     let has_even = values.iter().find(|v| matches!(v, Some(x) if x % 2 == 0)).is_some();
-   |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `any(|v| matches!(&v, Some(x) if x % 2 == 0))`
-
-error: called `is_some()` after searching an `Iterator` with `find`
-  --> tests/ui/search_is_some_fixable_some.rs:334:10
-   |
-LL |           .find(|v| match v {
-   |  __________^
-LL | |
-LL | |             Some(x) if x % 2 == 0 => true,
-LL | |             _ => false,
-LL | |         })
-LL | |         .is_some();
-   | |__________________^
-   |
-help: consider using
-   |
-LL ~         .any(|v| match &v {
-LL +
-LL +             Some(x) if x % 2 == 0 => true,
-LL +             _ => false,
-LL ~         });
-   |
-
-error: aborting due to 51 previous errors
+error: aborting due to 49 previous errors
 

src/tools/clippy/tests/ui/search_is_some_fixable_some.stderr

@@ -0,0 +1,20 @@
+// This test serves to document the change in semantics introduced by
+// rust-lang/rust#138961.
+//
+// A corollary of partial-pattern.rs: while the tuple access testcase makes
+// it clear why these semantics are useful, it is actually the dereference
+// being performed by the pattern that matters.
+
+fn main() {
+    // the inner reference is dangling
+    let x: &&u32 = unsafe {
+        let x: u32 = 42;
+        &&* &raw const x
+    };
+
+    let _ = || { //~ ERROR: encountered a dangling reference
+        match x {
+            &&_y => {},
+        }
+    };
+}