Auto merge of #154842 - JonathanBrouwer:rollup-nuHAPsT, r=JonathanBrouwer

Rollup of 2 pull requests

Successful merges:

 - #154555 (Make sure we run the full suite, even when more specific filters are given)
 - #154674 (borrowck: Don't mention unused vars in closure outlive errors)

Author: bors

compiler/rustc_borrowck/src/diagnostics/var_name.rs

@@ -1,12 +1,15 @@
 use rustc_index::IndexSlice;
-use rustc_middle::mir::{Body, Local};
+use rustc_middle::mir::visit::{PlaceContext, VisitPlacesWith, Visitor};
+use rustc_middle::mir::{Body, Local, Place};
 use rustc_middle::ty::{self, RegionVid, TyCtxt};
 use rustc_span::{Span, Symbol};
 use tracing::debug;
 
 use crate::region_infer::RegionInferenceContext;
 
 impl<'tcx> RegionInferenceContext<'tcx> {
+    /// Find the the name and span of the variable corresponding to the given region.
+    /// The returned var will also be ensured to actually be used in `body`.
     pub(crate) fn get_var_name_and_span_for_region(
         &self,
         tcx: TyCtxt<'tcx>,
@@ -22,13 +25,20 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         self.get_upvar_index_for_region(tcx, fr)
             .map(|index| {
                 // FIXME(project-rfc-2229#8): Use place span for diagnostics
+                // We know our upvars are used thanks to `fn compute_min_captures()` in `upvar.rs`.
                 let (name, span) = self.get_upvar_name_and_span_for_region(tcx, upvars, index);
                 (Some(name), span)
             })
             .or_else(|| {
                 debug!("get_var_name_and_span_for_region: attempting argument");
-                self.get_argument_index_for_region(tcx, fr).map(|index| {
-                    self.get_argument_name_and_span_for_region(body, local_names, index)
+                self.get_argument_index_for_region(tcx, fr).and_then(|index| {
+                    let local = self.user_arg_index_to_local(body, index);
+                    if body_uses_local(body, local) {
+                        Some(self.get_argument_name_and_span_for_region(body, local_names, index))
+                    } else {
+                        debug!("get_var_name_and_span_for_region: skipping unused local {local:?}");
+                        None
+                    }
                 })
             })
     }
@@ -105,6 +115,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         Some(argument_index)
     }
 
+    /// Given the index of an argument as seen from the user (i.e. excluding
+    /// implicit inputs), returns the corresponding MIR local.
+    fn user_arg_index_to_local(&self, body: &Body<'tcx>, user_arg_index: usize) -> Local {
+        let implicit_inputs = self.universal_regions().defining_ty.implicit_inputs();
+        body.args_iter().nth(implicit_inputs + user_arg_index).unwrap()
+    }
+
     /// Given the index of an argument, finds its name (if any) and the span from where it was
     /// declared.
     pub(crate) fn get_argument_name_and_span_for_region(
@@ -113,8 +130,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         local_names: &IndexSlice<Local, Option<Symbol>>,
         argument_index: usize,
     ) -> (Option<Symbol>, Span) {
-        let implicit_inputs = self.universal_regions().defining_ty.implicit_inputs();
-        let argument_local = Local::from_usize(implicit_inputs + argument_index + 1);
+        let argument_local = self.user_arg_index_to_local(body, argument_index);
         debug!("get_argument_name_and_span_for_region: argument_local={argument_local:?}");
 
         let argument_name = local_names[argument_local];
@@ -126,3 +142,14 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         (argument_name, argument_span)
     }
 }
+
+fn body_uses_local<'tcx>(body: &Body<'tcx>, target: Local) -> bool {
+    let mut used = false;
+    VisitPlacesWith(|place: Place<'_>, context: PlaceContext| {
+        if !matches!(context, PlaceContext::NonUse(_)) && place.local == target {
+            used = true;
+        }
+    })
+    .visit_body(body);
+    used
+}

src/bootstrap/src/core/build_steps/test.rs

@@ -37,8 +37,9 @@ use crate::core::{android, debuggers};
 use crate::utils::build_stamp::{self, BuildStamp};
 use crate::utils::exec::{BootstrapCommand, command};
 use crate::utils::helpers::{
-    self, LldThreads, add_dylib_path, add_rustdoc_cargo_linker_args, dylib_path, dylib_path_var,
-    linker_args, linker_flags, t, target_supports_cranelift_backend, up_to_date,
+    self, LldThreads, TestFilterCategory, add_dylib_path, add_rustdoc_cargo_linker_args,
+    dylib_path, dylib_path_var, linker_args, linker_flags, t, target_supports_cranelift_backend,
+    up_to_date,
 };
 use crate::utils::render_tests::{add_flags_and_try_run_tests, try_run_tests};
 use crate::{CLang, CodegenBackendKind, GitRepo, Mode, PathSet, TestTarget, envify};
@@ -969,13 +970,15 @@ impl Step for Clippy {
             let paths = &builder.config.paths[..];
             let mut test_names = Vec::new();
             for path in paths {
-                if let Some(path) =
-                    helpers::is_valid_test_suite_arg(path, "src/tools/clippy/tests", builder)
-                {
-                    test_names.push(path);
-                } else if path.ends_with("src/tools/clippy") {
-                    // When src/tools/clippy is called directly, all tests should be run.
-                    break 'partially_test;
+                match helpers::is_valid_test_suite_arg(path, "src/tools/clippy/tests", builder) {
+                    TestFilterCategory::Arg(path) => {
+                        test_names.push(path);
+                    }
+                    TestFilterCategory::Fullsuite => {
+                        // When src/tools/clippy is called directly, all tests should be run.
+                        break 'partially_test;
+                    }
+                    TestFilterCategory::Uninteresting => {}
                 }
             }
             cargo.env("TESTNAME", test_names.join(","));
@@ -1104,16 +1107,30 @@ impl Step for RustdocJSStd {
             .arg(builder.doc_out(self.target))
             .arg("--test-folder")
             .arg(builder.src.join("tests/rustdoc-js-std"));
-        for path in &builder.paths {
-            if let Some(p) = helpers::is_valid_test_suite_arg(path, "tests/rustdoc-js-std", builder)
-            {
-                if !p.ends_with(".js") {
-                    eprintln!("A non-js file was given: `{}`", path.display());
-                    panic!("Cannot run rustdoc-js-std tests");
+
+        let full_suite = builder.paths.iter().any(|path| {
+            matches!(
+                helpers::is_valid_test_suite_arg(path, "tests/rustdoc-js-std", builder),
+                TestFilterCategory::Fullsuite
+            )
+        });
+
+        // If we have to also run the full suite, don't worry about the individual arguments.
+        // They will be covered by running the entire suite
+        if !full_suite {
+            for path in &builder.paths {
+                if let TestFilterCategory::Arg(p) =
+                    helpers::is_valid_test_suite_arg(path, "tests/rustdoc-js-std", builder)
+                {
+                    if !p.ends_with(".js") {
+                        eprintln!("A non-js file was given: `{}`", path.display());
+                        panic!("Cannot run rustdoc-js-std tests");
+                    }
+                    command.arg("--test-file").arg(path);
                 }
-                command.arg("--test-file").arg(path);
             }
         }
+
         builder.ensure(crate::core::build_steps::doc::Std::from_build_compiler(
             self.build_compiler,
             self.target,
@@ -1252,14 +1269,27 @@ impl Step for RustdocGUI {
 
         add_rustdoc_cargo_linker_args(&mut cmd, builder, self.test_compiler.host, LldThreads::No);
 
-        for path in &builder.paths {
-            if let Some(p) = helpers::is_valid_test_suite_arg(path, "tests/rustdoc-gui", builder) {
-                if !p.ends_with(".goml") {
-                    eprintln!("A non-goml file was given: `{}`", path.display());
-                    panic!("Cannot run rustdoc-gui tests");
-                }
-                if let Some(name) = path.file_name().and_then(|f| f.to_str()) {
-                    cmd.arg("--goml-file").arg(name);
+        let full_suite = builder.paths.iter().any(|path| {
+            matches!(
+                helpers::is_valid_test_suite_arg(path, "tests/rustdoc-js-std", builder),
+                TestFilterCategory::Fullsuite
+            )
+        });
+
+        // If we have to also run the full suite, don't worry about the individual arguments.
+        // They will be covered by running the entire suite
+        if !full_suite {
+            for path in &builder.paths {
+                if let TestFilterCategory::Arg(p) =
+                    helpers::is_valid_test_suite_arg(path, "tests/rustdoc-gui", builder)
+                {
+                    if !p.ends_with(".goml") {
+                        eprintln!("A non-goml file was given: `{}`", path.display());
+                        panic!("Cannot run rustdoc-gui tests");
+                    }
+                    if let Some(name) = path.file_name().and_then(|f| f.to_str()) {
+                        cmd.arg("--goml-file").arg(name);
+                    }
                 }
             }
         }
@@ -2270,11 +2300,23 @@ Please disable assertions with `rust.debug-assertions = false`.
             }
             paths = &paths_v;
         }
+
         // Get test-args by striping suite path
-        let mut test_args: Vec<&str> = paths
-            .iter()
-            .filter_map(|p| helpers::is_valid_test_suite_arg(p, suite_path, builder))
-            .collect();
+        let mut test_args = Vec::new();
+        for p in paths {
+            match helpers::is_valid_test_suite_arg(p, suite_path, builder) {
+                TestFilterCategory::Fullsuite => {
+                    // If we also have to run the full suite, don't append _any_ test args here,
+                    // clear the list instead and break out.
+                    // That way none of the more specific paths make it into test_args,
+                    // since running the whole suite will run the specific ones anyway.
+                    test_args.clear();
+                    break;
+                }
+                TestFilterCategory::Arg(a) => test_args.push(a),
+                TestFilterCategory::Uninteresting => {}
+            }
+        }
 
         test_args.append(&mut builder.config.test_args());
 

src/bootstrap/src/utils/helpers.rs

@@ -240,18 +240,30 @@ pub fn target_supports_cranelift_backend(target: TargetSelection) -> bool {
     }
 }
 
+/// Value returned from [`is_valid_test_suite_arg`], which figures out which paths start with the
+/// suite name (and therefore which should be run).
+pub enum TestFilterCategory<'a> {
+    /// If a path is equal to the name of the suite, this is returned.
+    Fullsuite,
+    /// If a path starts with the suite, the suite prefix is stripped and the rest is returned as
+    /// this variant.
+    Arg(&'a str),
+    /// For paths that don't start with the suite.
+    Uninteresting,
+}
+
 pub fn is_valid_test_suite_arg<'a, P: AsRef<Path>>(
     path: &'a Path,
     suite_path: P,
     builder: &Builder<'_>,
-) -> Option<&'a str> {
+) -> TestFilterCategory<'a> {
     let suite_path = suite_path.as_ref();
     let path = match path.strip_prefix(".") {
         Ok(p) => p,
         Err(_) => path,
     };
     if !path.starts_with(suite_path) {
-        return None;
+        return TestFilterCategory::Uninteresting;
     }
     let abs_path = builder.src.join(path);
     let exists = abs_path.is_dir() || abs_path.is_file();
@@ -268,8 +280,8 @@ pub fn is_valid_test_suite_arg<'a, P: AsRef<Path>>(
     // flag is respected, so providing an empty --test-args conflicts with
     // any following it.
     match path.strip_prefix(suite_path).ok().and_then(|p| p.to_str()) {
-        Some(s) if !s.is_empty() => Some(s),
-        _ => None,
+        Some(s) if !s.is_empty() => TestFilterCategory::Arg(s),
+        _ => TestFilterCategory::Fullsuite,
     }
 }
 

tests/ui/borrowck/var-matching-lifetime-but-unused-not-mentioned.rs

@@ -0,0 +1,17 @@
+//! Regression test for <https://github.com/rust-lang/rust/issues/113121>.
+
+#![allow(unused_variables)]
+
+fn consume<T: 'static>(_: T) {}
+
+fn foo<'a>(
+    used_arg: &'a u8,
+    unused_arg: &'static u16, // Unused in closure. Must not appear in error.
+) {
+    let unused_var: &'static u32 = &42; // Unused in closure. Must not appear in error.
+
+    let c = move || used_arg;
+    consume(c); //~ ERROR: borrowed data escapes outside of function
+}
+
+fn main() {}

tests/ui/borrowck/var-matching-lifetime-but-unused-not-mentioned.stderr

@@ -0,0 +1,17 @@
+error[E0521]: borrowed data escapes outside of function
+  --> $DIR/var-matching-lifetime-but-unused-not-mentioned.rs:14:5
+   |
+LL | fn foo<'a>(
+   |        -- lifetime `'a` defined here
+LL |     used_arg: &'a u8,
+   |     -------- `used_arg` is a reference that is only valid in the function body
+...
+LL |     consume(c);
+   |     ^^^^^^^^^^
+   |     |
+   |     `used_arg` escapes the function body here
+   |     argument requires that `'a` must outlive `'static`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0521`.