Skip to content

Commit

Permalink
Rollup merge of #73297 - ehuss:tool-warnings, r=Mark-Simulacrum
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Manishearth committed Jun 26, 2020
2 parents 2aee608 + 75983e1 commit 10d655b
Show file tree
Hide file tree
Showing 22 changed files with 100 additions and 82 deletions.
5 changes: 3 additions & 2 deletions src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -759,6 +759,7 @@ impl<'a> Builder<'a> {
&self,
compiler: Compiler,
mode: Mode,
source_type: SourceType,
target: Interned<String>,
cmd: &str,
) -> Cargo {
Expand Down Expand Up @@ -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.
Expand Down
29 changes: 22 additions & 7 deletions src/bootstrap/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand All @@ -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>,
Expand Down Expand Up @@ -145,7 +156,7 @@ macro_rules! tool_check_step {
target,
cargo_subcommand(builder.kind),
$path,
SourceType::InTree,
$source_type,
&[],
);

Expand Down Expand Up @@ -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.
Expand Down
10 changes: 5 additions & 5 deletions src/bootstrap/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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!(
Expand Down
5 changes: 3 additions & 2 deletions src/bootstrap/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 \
Expand Down
19 changes: 12 additions & 7 deletions src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
6 changes: 4 additions & 2 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
19 changes: 10 additions & 9 deletions src/bootstrap/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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
},
})
}
}
Expand All @@ -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, {
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/doctree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 9 additions & 9 deletions src/librustdoc/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand Down
Loading

0 comments on commit 10d655b

Please sign in to comment.