Rollup merge of #73297 - ehuss:tool-warnings, r=Mark-Simulacrum

Support configurable deny-warnings for all in-tree crates.

This removes the hard-coded `deny(warnings)` on all in-tree tools, and allows it to be configured from the config.  This is just a personal preference, as I find `deny(warnings)` frustrating during development or doing small tests.

This also fixes some regressions in terms of warning handling.  Warnings used to be dependent on `SourceType`, but in #64316 it was changed to be based on `Mode`. This means tools like rustdoc no longer used the same settings as the rest of the tree. It also made `SourceType` useless since the only thing it was used for was warnings. I think it would be better for everything in the tree to use the same settings.

Fixes #64523

Author: Manishearth

src/bootstrap/builder.rs

@@ -23,7 +23,7 @@ use crate::install;
 use crate::native;
 use crate::run;
 use crate::test;
-use crate::tool;
+use crate::tool::{self, SourceType};
 use crate::util::{self, add_dylib_path, add_link_lib_path, exe, libdir};
 use crate::{Build, DocTests, GitRepo, Mode};
 
@@ -759,6 +759,7 @@ impl<'a> Builder<'a> {
         &self,
         compiler: Compiler,
         mode: Mode,
+        source_type: SourceType,
         target: Interned<String>,
         cmd: &str,
     ) -> Cargo {
@@ -1125,7 +1126,7 @@ impl<'a> Builder<'a> {
 
         cargo.env("RUSTC_VERBOSE", self.verbosity.to_string());
 
-        if !mode.is_tool() {
+        if source_type == SourceType::InTree {
             // When extending this list, add the new lints to the RUSTFLAGS of the
             // build_bootstrap function of src/bootstrap/bootstrap.py as well as
             // some code doesn't go through this `rustc` wrapper.

src/bootstrap/check.rs

@@ -44,7 +44,13 @@ impl Step for Std {
         let target = self.target;
         let compiler = builder.compiler(0, builder.config.build);
 
-        let mut cargo = builder.cargo(compiler, Mode::Std, target, cargo_subcommand(builder.kind));
+        let mut cargo = builder.cargo(
+            compiler,
+            Mode::Std,
+            SourceType::InTree,
+            target,
+            cargo_subcommand(builder.kind),
+        );
         std_cargo(builder, target, compiler.stage, &mut cargo);
 
         builder.info(&format!("Checking std artifacts ({} -> {})", &compiler.host, target));
@@ -92,8 +98,13 @@ impl Step for Rustc {
 
         builder.ensure(Std { target });
 
-        let mut cargo =
-            builder.cargo(compiler, Mode::Rustc, target, cargo_subcommand(builder.kind));
+        let mut cargo = builder.cargo(
+            compiler,
+            Mode::Rustc,
+            SourceType::InTree,
+            target,
+            cargo_subcommand(builder.kind),
+        );
         rustc_cargo(builder, &mut cargo, target);
 
         builder.info(&format!("Checking compiler artifacts ({} -> {})", &compiler.host, target));
@@ -113,7 +124,7 @@ impl Step for Rustc {
 }
 
 macro_rules! tool_check_step {
-    ($name:ident, $path:expr) => {
+    ($name:ident, $path:expr, $source_type:expr) => {
         #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
         pub struct $name {
             pub target: Interned<String>,
@@ -145,7 +156,7 @@ macro_rules! tool_check_step {
                     target,
                     cargo_subcommand(builder.kind),
                     $path,
-                    SourceType::InTree,
+                    $source_type,
                     &[],
                 );
 
@@ -184,8 +195,12 @@ macro_rules! tool_check_step {
     };
 }
 
-tool_check_step!(Rustdoc, "src/tools/rustdoc");
-tool_check_step!(Clippy, "src/tools/clippy");
+tool_check_step!(Rustdoc, "src/tools/rustdoc", SourceType::InTree);
+// Clippy is a hybrid. It is an external tool, but uses a git subtree instead
+// of a submodule. Since the SourceType only drives the deny-warnings
+// behavior, treat it as in-tree so that any new warnings in clippy will be
+// rejected.
+tool_check_step!(Clippy, "src/tools/clippy", SourceType::InTree);
 
 /// Cargo's output path for the standard library in a given stage, compiled
 /// by a particular compiler for the specified target.

src/bootstrap/compile.rs

@@ -20,14 +20,14 @@ use filetime::FileTime;
 use serde::Deserialize;
 
 use crate::builder::Cargo;
+use crate::builder::{Builder, Kind, RunConfig, ShouldRun, Step};
+use crate::cache::{Interned, INTERNER};
 use crate::dist;
 use crate::native;
+use crate::tool::SourceType;
 use crate::util::{exe, is_dylib, symlink_dir};
 use crate::{Compiler, DependencyType, GitRepo, Mode};
 
-use crate::builder::{Builder, Kind, RunConfig, ShouldRun, Step};
-use crate::cache::{Interned, INTERNER};
-
 #[derive(Debug, PartialOrd, Ord, Copy, Clone, PartialEq, Eq, Hash)]
 pub struct Std {
     pub target: Interned<String>,
@@ -87,7 +87,7 @@ impl Step for Std {
         target_deps.extend(copy_third_party_objects(builder, &compiler, target));
         target_deps.extend(copy_self_contained_objects(builder, &compiler, target));
 
-        let mut cargo = builder.cargo(compiler, Mode::Std, target, "build");
+        let mut cargo = builder.cargo(compiler, Mode::Std, SourceType::InTree, target, "build");
         std_cargo(builder, target, compiler.stage, &mut cargo);
 
         builder.info(&format!(
@@ -513,7 +513,7 @@ impl Step for Rustc {
             target: builder.config.build,
         });
 
-        let mut cargo = builder.cargo(compiler, Mode::Rustc, target, "build");
+        let mut cargo = builder.cargo(compiler, Mode::Rustc, SourceType::InTree, target, "build");
         rustc_cargo(builder, &mut cargo, target);
 
         builder.info(&format!(

src/bootstrap/doc.rs

@@ -435,7 +435,8 @@ impl Step for Std {
         t!(fs::copy(builder.src.join("src/doc/rust.css"), out.join("rust.css")));
 
         let run_cargo_rustdoc_for = |package: &str| {
-            let mut cargo = builder.cargo(compiler, Mode::Std, target, "rustdoc");
+            let mut cargo =
+                builder.cargo(compiler, Mode::Std, SourceType::InTree, target, "rustdoc");
             compile::std_cargo(builder, target, compiler.stage, &mut cargo);
 
             // Keep a whitelist so we do not build internal stdlib crates, these will be
@@ -534,7 +535,7 @@ impl Step for Rustc {
         t!(symlink_dir_force(&builder.config, &out, &out_dir));
 
         // Build cargo command.
-        let mut cargo = builder.cargo(compiler, Mode::Rustc, target, "doc");
+        let mut cargo = builder.cargo(compiler, Mode::Rustc, SourceType::InTree, target, "doc");
         cargo.env(
             "RUSTDOCFLAGS",
             "--document-private-items \

src/bootstrap/lib.rs

@@ -301,16 +301,21 @@ pub enum Mode {
     /// Build codegen libraries, placing output in the "stageN-codegen" directory
     Codegen,
 
-    /// Build some tools, placing output in the "stageN-tools" directory. The
-    /// "other" here is for miscellaneous sets of tools that are built using the
-    /// bootstrap compiler in its entirety (target libraries and all).
-    /// Typically these tools compile with stable Rust.
+    /// Build a tool, placing output in the "stage0-bootstrap-tools"
+    /// directory. This is for miscellaneous sets of tools that are built
+    /// using the bootstrap stage0 compiler in its entirety (target libraries
+    /// and all). Typically these tools compile with stable Rust.
     ToolBootstrap,
 
-    /// Compile a tool which uses all libraries we compile (up to rustc).
-    /// Doesn't use the stage0 compiler libraries like "other", and includes
-    /// tools like rustdoc, cargo, rls, etc.
+    /// Build a tool which uses the locally built std, placing output in the
+    /// "stageN-tools" directory. Its usage is quite rare, mainly used by
+    /// compiletest which needs libtest.
     ToolStd,
+
+    /// Build a tool which uses the locally built rustc and the target std,
+    /// placing the output in the "stageN-tools" directory. This is used for
+    /// anything that needs a fully functional rustc, such as rustdoc, clippy,
+    /// cargo, rls, rustfmt, miri, etc.
     ToolRustc,
 }
 

src/bootstrap/test.rs

@@ -367,7 +367,8 @@ impl Step for Miri {
             extra_features: Vec::new(),
         });
         if let (Some(miri), Some(_cargo_miri)) = (miri, cargo_miri) {
-            let mut cargo = builder.cargo(compiler, Mode::ToolRustc, host, "install");
+            let mut cargo =
+                builder.cargo(compiler, Mode::ToolRustc, SourceType::Submodule, host, "install");
             cargo.arg("xargo");
             // Configure `cargo install` path. cargo adds a `bin/`.
             cargo.env("CARGO_INSTALL_ROOT", &builder.out);
@@ -1696,7 +1697,8 @@ impl Step for Crate {
         // we're working with automatically.
         let compiler = builder.compiler_for(compiler.stage, compiler.host, target);
 
-        let mut cargo = builder.cargo(compiler, mode, target, test_kind.subcommand());
+        let mut cargo =
+            builder.cargo(compiler, mode, SourceType::InTree, target, test_kind.subcommand());
         match mode {
             Mode::Std => {
                 compile::std_cargo(builder, target, compiler.stage, &mut cargo);

src/bootstrap/tool.rs

@@ -16,7 +16,7 @@ use crate::util::{add_dylib_path, exe, CiEnv};
 use crate::Compiler;
 use crate::Mode;
 
-#[derive(Debug, Clone, Hash, PartialEq, Eq)]
+#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
 pub enum SourceType {
     InTree,
     Submodule,
@@ -226,14 +226,10 @@ pub fn prepare_tool_cargo(
     source_type: SourceType,
     extra_features: &[String],
 ) -> CargoCommand {
-    let mut cargo = builder.cargo(compiler, mode, target, command);
+    let mut cargo = builder.cargo(compiler, mode, source_type, target, command);
     let dir = builder.src.join(path);
     cargo.arg("--manifest-path").arg(dir.join("Cargo.toml"));
 
-    if source_type == SourceType::Submodule {
-        cargo.env("RUSTC_EXTERNAL_TOOL", "1");
-    }
-
     let mut features = extra_features.to_vec();
     if builder.build.config.cargo_native_static {
         if path.ends_with("cargo")
@@ -596,6 +592,7 @@ macro_rules! tool_extended {
        $path:expr,
        $tool_name:expr,
        stable = $stable:expr,
+       $(in_tree = $in_tree:expr,)*
        $extra_deps:block;)+) => {
         $(
             #[derive(Debug, Clone, Hash, PartialEq, Eq)]
@@ -647,7 +644,11 @@ macro_rules! tool_extended {
                     path: $path,
                     extra_features: $sel.extra_features,
                     is_optional_tool: true,
-                    source_type: SourceType::Submodule,
+                    source_type: if false $(|| $in_tree)* {
+                        SourceType::InTree
+                    } else {
+                        SourceType::Submodule
+                    },
                 })
             }
         }
@@ -659,8 +660,8 @@ macro_rules! tool_extended {
 // to make `./x.py build <tool>` work.
 tool_extended!((self, builder),
     Cargofmt, rustfmt, "src/tools/rustfmt", "cargo-fmt", stable=true, {};
-    CargoClippy, clippy, "src/tools/clippy", "cargo-clippy", stable=true, {};
-    Clippy, clippy, "src/tools/clippy", "clippy-driver", stable=true, {};
+    CargoClippy, clippy, "src/tools/clippy", "cargo-clippy", stable=true, in_tree=true, {};
+    Clippy, clippy, "src/tools/clippy", "clippy-driver", stable=true, in_tree=true, {};
     Miri, miri, "src/tools/miri", "miri", stable=false, {};
     CargoMiri, miri, "src/tools/miri/cargo-miri", "cargo-miri", stable=false, {};
     Rls, rls, "src/tools/rls", "rls", stable=true, {

src/librustdoc/clean/inline.rs

@@ -278,7 +278,7 @@ fn build_type_alias_type(cx: &DocContext<'_>, did: DefId) -> Option<clean::Type>
     type_.def_id().and_then(|did| build_ty(cx, did))
 }
 
-pub fn build_ty(cx: &DocContext, did: DefId) -> Option<clean::Type> {
+pub fn build_ty(cx: &DocContext<'_>, did: DefId) -> Option<clean::Type> {
     match cx.tcx.def_kind(did) {
         DefKind::Struct | DefKind::Union | DefKind::Enum | DefKind::Const | DefKind::Static => {
             Some(cx.tcx.type_of(did).clean(cx))

src/librustdoc/clean/utils.rs

@@ -328,7 +328,7 @@ pub fn strip_path(path: &Path) -> Path {
     Path { global: path.global, res: path.res, segments }
 }
 
-pub fn qpath_to_string(p: &hir::QPath) -> String {
+pub fn qpath_to_string(p: &hir::QPath<'_>) -> String {
     let segments = match *p {
         hir::QPath::Resolved(_, ref path) => &path.segments,
         hir::QPath::TypeRelative(_, ref segment) => return segment.ident.to_string(),
@@ -417,7 +417,7 @@ impl ToSource for rustc_span::Span {
     }
 }
 
-pub fn name_from_pat(p: &hir::Pat) -> String {
+pub fn name_from_pat(p: &hir::Pat<'_>) -> String {
     use rustc_hir::*;
     debug!("trying to get a name from pattern: {:?}", p);
 

src/librustdoc/doctree.rs

@@ -262,7 +262,7 @@ pub struct ProcMacro<'hir> {
     pub whence: Span,
 }
 
-pub fn struct_type_from_def(vdata: &hir::VariantData) -> StructType {
+pub fn struct_type_from_def(vdata: &hir::VariantData<'_>) -> StructType {
     match *vdata {
         hir::VariantData::Struct(..) => Plain,
         hir::VariantData::Tuple(..) => Tuple,

src/librustdoc/test.rs

@@ -166,7 +166,7 @@ pub fn run(options: Options) -> Result<(), String> {
 }
 
 // Look for `#![doc(test(no_crate_inject))]`, used by crates in the std facade.
-fn scrape_test_config(krate: &::rustc_hir::Crate) -> TestOptions {
+fn scrape_test_config(krate: &::rustc_hir::Crate<'_>) -> TestOptions {
     use rustc_ast_pretty::pprust;
 
     let mut opts =
@@ -973,7 +973,7 @@ impl<'a, 'hir, 'tcx> intravisit::Visitor<'hir> for HirCollector<'a, 'hir, 'tcx>
         intravisit::NestedVisitorMap::All(self.map)
     }
 
-    fn visit_item(&mut self, item: &'hir hir::Item) {
+    fn visit_item(&mut self, item: &'hir hir::Item<'_>) {
         let name = if let hir::ItemKind::Impl { ref self_ty, .. } = item.kind {
             rustc_hir_pretty::id_to_string(&self.map, self_ty.hir_id)
         } else {
@@ -985,42 +985,42 @@ impl<'a, 'hir, 'tcx> intravisit::Visitor<'hir> for HirCollector<'a, 'hir, 'tcx>
         });
     }
 
-    fn visit_trait_item(&mut self, item: &'hir hir::TraitItem) {
+    fn visit_trait_item(&mut self, item: &'hir hir::TraitItem<'_>) {
         self.visit_testable(item.ident.to_string(), &item.attrs, item.hir_id, item.span, |this| {
             intravisit::walk_trait_item(this, item);
         });
     }
 
-    fn visit_impl_item(&mut self, item: &'hir hir::ImplItem) {
+    fn visit_impl_item(&mut self, item: &'hir hir::ImplItem<'_>) {
         self.visit_testable(item.ident.to_string(), &item.attrs, item.hir_id, item.span, |this| {
             intravisit::walk_impl_item(this, item);
         });
     }
 
-    fn visit_foreign_item(&mut self, item: &'hir hir::ForeignItem) {
+    fn visit_foreign_item(&mut self, item: &'hir hir::ForeignItem<'_>) {
         self.visit_testable(item.ident.to_string(), &item.attrs, item.hir_id, item.span, |this| {
             intravisit::walk_foreign_item(this, item);
         });
     }
 
     fn visit_variant(
         &mut self,
-        v: &'hir hir::Variant,
-        g: &'hir hir::Generics,
+        v: &'hir hir::Variant<'_>,
+        g: &'hir hir::Generics<'_>,
         item_id: hir::HirId,
     ) {
         self.visit_testable(v.ident.to_string(), &v.attrs, v.id, v.span, |this| {
             intravisit::walk_variant(this, v, g, item_id);
         });
     }
 
-    fn visit_struct_field(&mut self, f: &'hir hir::StructField) {
+    fn visit_struct_field(&mut self, f: &'hir hir::StructField<'_>) {
         self.visit_testable(f.ident.to_string(), &f.attrs, f.hir_id, f.span, |this| {
             intravisit::walk_struct_field(this, f);
         });
     }
 
-    fn visit_macro_def(&mut self, macro_def: &'hir hir::MacroDef) {
+    fn visit_macro_def(&mut self, macro_def: &'hir hir::MacroDef<'_>) {
         self.visit_testable(
             macro_def.ident.to_string(),
             &macro_def.attrs,