Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollup of bare_trait_objects PRs #52336

Merged
merged 13 commits into from
Jul 27, 2018
4 changes: 3 additions & 1 deletion src/bootstrap/bin/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,8 +296,10 @@ fn main() {
cmd.arg("--color=always");
}

if env::var_os("RUSTC_DENY_WARNINGS").is_some() {
if env::var_os("RUSTC_DENY_WARNINGS").is_some() && env::var_os("RUSTC_EXTERNAL_TOOL").is_none()
{
cmd.arg("-Dwarnings");
cmd.arg("-Dbare_trait_objects");
}

if verbose > 1 {
Expand Down
3 changes: 2 additions & 1 deletion src/bootstrap/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,8 @@ impl Step for Rustdoc {
Mode::ToolRustc,
target,
"check",
"src/tools/rustdoc");
"src/tools/rustdoc",
false);

let _folder = builder.fold_output(|| format!("stage{}-rustdoc", compiler.stage));
println!("Checking rustdoc artifacts ({} -> {})", &compiler.host, target);
Expand Down
1 change: 1 addition & 0 deletions src/bootstrap/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,7 @@ impl Step for Rustdoc {
target,
"doc",
"src/tools/rustdoc",
false
);

cargo.env("RUSTDOCFLAGS", "--document-private-items");
Expand Down
55 changes: 28 additions & 27 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,17 +222,18 @@ impl Step for Cargo {
compiler,
target: self.host,
});
let mut cargo = builder.cargo(compiler, Mode::ToolRustc, self.host, "test");
cargo
.arg("--manifest-path")
.arg(builder.src.join("src/tools/cargo/Cargo.toml"));
let mut cargo = tool::prepare_tool_cargo(builder,
compiler,
Mode::ToolRustc,
self.host,
"test",
"src/tools/cargo",
true);

if !builder.fail_fast {
cargo.arg("--no-fail-fast");
}

// Don't build tests dynamically, just a pain to work with
cargo.env("RUSTC_NO_PREFER_DYNAMIC", "1");

// Don't run cross-compile tests, we may not have cross-compiled libstd libs
// available.
cargo.env("CFG_DISABLE_CROSS_TESTS", "1");
Expand Down Expand Up @@ -286,10 +287,8 @@ impl Step for Rls {
Mode::ToolRustc,
host,
"test",
"src/tools/rls");

// Don't build tests dynamically, just a pain to work with
cargo.env("RUSTC_NO_PREFER_DYNAMIC", "1");
"src/tools/rls",
true);

builder.add_rustc_lib_path(compiler, &mut cargo);

Expand Down Expand Up @@ -341,10 +340,9 @@ impl Step for Rustfmt {
Mode::ToolRustc,
host,
"test",
"src/tools/rustfmt");
"src/tools/rustfmt",
true);

// Don't build tests dynamically, just a pain to work with
cargo.env("RUSTC_NO_PREFER_DYNAMIC", "1");
let dir = testdir(builder, compiler.host);
t!(fs::create_dir_all(&dir));
cargo.env("RUSTFMT_TEST_DIR", dir);
Expand Down Expand Up @@ -392,13 +390,14 @@ impl Step for Miri {
extra_features: Vec::new(),
});
if let Some(miri) = miri {
let mut cargo = builder.cargo(compiler, Mode::ToolRustc, host, "test");
cargo
.arg("--manifest-path")
.arg(builder.src.join("src/tools/miri/Cargo.toml"));
let mut cargo = tool::prepare_tool_cargo(builder,
compiler,
Mode::ToolRustc,
host,
"test",
"src/tools/miri",
true);

// Don't build tests dynamically, just a pain to work with
cargo.env("RUSTC_NO_PREFER_DYNAMIC", "1");
// miri tests need to know about the stage sysroot
cargo.env("MIRI_SYSROOT", builder.sysroot(compiler));
cargo.env("RUSTC_TEST_SUITE", builder.rustc(compiler));
Expand Down Expand Up @@ -450,13 +449,14 @@ impl Step for Clippy {
extra_features: Vec::new(),
});
if let Some(clippy) = clippy {
let mut cargo = builder.cargo(compiler, Mode::ToolRustc, host, "test");
cargo
.arg("--manifest-path")
.arg(builder.src.join("src/tools/clippy/Cargo.toml"));
let mut cargo = tool::prepare_tool_cargo(builder,
compiler,
Mode::ToolRustc,
host,
"test",
"src/tools/clippy",
true);

// Don't build tests dynamically, just a pain to work with
cargo.env("RUSTC_NO_PREFER_DYNAMIC", "1");
// clippy tests need to know about the stage sysroot
cargo.env("SYSROOT", builder.sysroot(compiler));
cargo.env("RUSTC_TEST_SUITE", builder.rustc(compiler));
Expand Down Expand Up @@ -1739,7 +1739,8 @@ impl Step for CrateRustdoc {
Mode::ToolRustc,
target,
test_kind.subcommand(),
"src/tools/rustdoc");
"src/tools/rustdoc",
false);
if test_kind.subcommand() == "test" && !builder.fail_fast {
cargo.arg("--no-fail-fast");
}
Expand Down
55 changes: 39 additions & 16 deletions src/bootstrap/tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ struct ToolBuild {
tool: &'static str,
path: &'static str,
mode: Mode,
is_ext_tool: bool,
is_optional_tool: bool,
is_external_tool: bool,
extra_features: Vec<String>,
}

Expand All @@ -102,7 +103,7 @@ impl Step for ToolBuild {
let target = self.target;
let tool = self.tool;
let path = self.path;
let is_ext_tool = self.is_ext_tool;
let is_optional_tool = self.is_optional_tool;

match self.mode {
Mode::ToolRustc => {
Expand All @@ -115,7 +116,15 @@ impl Step for ToolBuild {
_ => panic!("unexpected Mode for tool build")
}

let mut cargo = prepare_tool_cargo(builder, compiler, self.mode, target, "build", path);
let mut cargo = prepare_tool_cargo(
builder,
compiler,
self.mode,
target,
"build",
path,
self.is_external_tool,
);
cargo.arg("--features").arg(self.extra_features.join(" "));

let _folder = builder.fold_output(|| format!("stage{}-{}", compiler.stage, tool));
Expand Down Expand Up @@ -216,7 +225,7 @@ impl Step for ToolBuild {
});

if !is_expected {
if !is_ext_tool {
if !is_optional_tool {
exit(1);
} else {
return None;
Expand All @@ -238,6 +247,7 @@ pub fn prepare_tool_cargo(
target: Interned<String>,
command: &'static str,
path: &'static str,
is_external_tool: bool,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this bool an enum instead? Every time I've gone to review this PR I've had to look up what the true/false here means; it'd be good to have an enum ExternalTool { Yes, No } with the variants documented as to when to use which.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, having an enum just representing a bool sounds like tautology. Should I just add a doc comment instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enums representing bools are a common an excellent design pattern; the doc comment wouldn't help because the problem is at call sites, not here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you dislike Yes, No you could make it enum Tool { Normal, External, OptionalExternal }

) -> Command {
let mut cargo = builder.cargo(compiler, mode, target, command);
let dir = builder.src.join(path);
Expand All @@ -247,6 +257,10 @@ pub fn prepare_tool_cargo(
// stages and such and it's just easier if they're not dynamically linked.
cargo.env("RUSTC_NO_PREFER_DYNAMIC", "1");

if is_external_tool {
cargo.env("RUSTC_EXTERNAL_TOOL", "1");
}

if let Some(dir) = builder.openssl_install_dir(target) {
cargo.env("OPENSSL_STATIC", "1");
cargo.env("OPENSSL_DIR", dir);
Expand Down Expand Up @@ -274,7 +288,8 @@ pub fn prepare_tool_cargo(
}

macro_rules! tool {
($($name:ident, $path:expr, $tool_name:expr, $mode:expr $(,llvm_tools = $llvm:expr)*;)+) => {
($($name:ident, $path:expr, $tool_name:expr, $mode:expr
$(,llvm_tools = $llvm:expr)* $(,external_tool = $external:expr)*;)+) => {
#[derive(Copy, PartialEq, Eq, Clone)]
pub enum Tool {
$(
Expand Down Expand Up @@ -351,7 +366,8 @@ macro_rules! tool {
tool: $tool_name,
mode: $mode,
path: $path,
is_ext_tool: false,
is_optional_tool: false,
is_external_tool: false $(|| $external)*,
extra_features: Vec::new(),
}).expect("expected to build -- essential tool")
}
Expand All @@ -370,7 +386,8 @@ tool!(
Compiletest, "src/tools/compiletest", "compiletest", Mode::ToolBootstrap, llvm_tools = true;
BuildManifest, "src/tools/build-manifest", "build-manifest", Mode::ToolBootstrap;
RemoteTestClient, "src/tools/remote-test-client", "remote-test-client", Mode::ToolBootstrap;
RustInstaller, "src/tools/rust-installer", "fabricate", Mode::ToolBootstrap;
RustInstaller, "src/tools/rust-installer", "fabricate", Mode::ToolBootstrap,
external_tool = true;
RustdocTheme, "src/tools/rustdoc-themes", "rustdoc-themes", Mode::ToolBootstrap;
);

Expand Down Expand Up @@ -401,7 +418,8 @@ impl Step for RemoteTestServer {
tool: "remote-test-server",
mode: Mode::ToolStd,
path: "src/tools/remote-test-server",
is_ext_tool: false,
is_optional_tool: false,
is_external_tool: false,
extra_features: Vec::new(),
}).expect("expected to build -- essential tool")
}
Expand Down Expand Up @@ -449,12 +467,15 @@ impl Step for Rustdoc {
target: builder.config.build,
});

let mut cargo = prepare_tool_cargo(builder,
build_compiler,
Mode::ToolRustc,
target,
"build",
"src/tools/rustdoc");
let mut cargo = prepare_tool_cargo(
builder,
build_compiler,
Mode::ToolRustc,
target,
"build",
"src/tools/rustdoc",
false,
);

// Most tools don't get debuginfo, but rustdoc should.
cargo.env("RUSTC_DEBUGINFO", builder.config.rust_debuginfo.to_string())
Expand Down Expand Up @@ -525,7 +546,8 @@ impl Step for Cargo {
tool: "cargo",
mode: Mode::ToolRustc,
path: "src/tools/cargo",
is_ext_tool: false,
is_optional_tool: false,
is_external_tool: true,
extra_features: Vec::new(),
}).expect("expected to build -- essential tool")
}
Expand Down Expand Up @@ -574,7 +596,8 @@ macro_rules! tool_extended {
mode: Mode::ToolRustc,
path: $path,
extra_features: $sel.extra_features,
is_ext_tool: true,
is_optional_tool: true,
is_external_tool: true,
})
}
}
Expand Down
2 changes: 0 additions & 2 deletions src/build_helper/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![deny(bare_trait_objects)]

use std::fs::File;
use std::path::{Path, PathBuf};
use std::process::{Command, Stdio};
Expand Down
1 change: 0 additions & 1 deletion src/liballoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@
test(no_crate_inject, attr(allow(unused_variables), deny(warnings))))]
#![no_std]
#![needs_allocator]
#![deny(bare_trait_objects)]
#![deny(missing_debug_implementations)]

#![cfg_attr(test, allow(deprecated))] // rand
Expand Down
6 changes: 3 additions & 3 deletions src/liballoc/tests/arc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn uninhabited() {
a = a.clone();
assert!(a.upgrade().is_none());

let mut a: Weak<Any> = a; // Unsizing
let mut a: Weak<dyn Any> = a; // Unsizing
a = a.clone();
assert!(a.upgrade().is_none());
}
Expand All @@ -39,7 +39,7 @@ fn slice() {
#[test]
fn trait_object() {
let a: Arc<u32> = Arc::new(4);
let a: Arc<Any> = a; // Unsizing
let a: Arc<dyn Any> = a; // Unsizing

// Exercise is_dangling() with a DST
let mut a = Arc::downgrade(&a);
Expand All @@ -49,7 +49,7 @@ fn trait_object() {
let mut b = Weak::<u32>::new();
b = b.clone();
assert!(b.upgrade().is_none());
let mut b: Weak<Any> = b; // Unsizing
let mut b: Weak<dyn Any> = b; // Unsizing
b = b.clone();
assert!(b.upgrade().is_none());
}
2 changes: 1 addition & 1 deletion src/liballoc/tests/btree/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ fn test_hash() {
}

fn check<F>(a: &[i32], b: &[i32], expected: &[i32], f: F)
where F: FnOnce(&BTreeSet<i32>, &BTreeSet<i32>, &mut FnMut(&i32) -> bool) -> bool
where F: FnOnce(&BTreeSet<i32>, &BTreeSet<i32>, &mut dyn FnMut(&i32) -> bool) -> bool
{
let mut set_a = BTreeSet::new();
let mut set_b = BTreeSet::new();
Expand Down
2 changes: 1 addition & 1 deletion src/liballoc/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ fn test_boxed_hasher() {
5u32.hash(&mut hasher_1);
assert_eq!(ordinary_hash, hasher_1.finish());

let mut hasher_2 = Box::new(DefaultHasher::new()) as Box<Hasher>;
let mut hasher_2 = Box::new(DefaultHasher::new()) as Box<dyn Hasher>;
5u32.hash(&mut hasher_2);
assert_eq!(ordinary_hash, hasher_2.finish());
}
6 changes: 3 additions & 3 deletions src/liballoc/tests/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn uninhabited() {
a = a.clone();
assert!(a.upgrade().is_none());

let mut a: Weak<Any> = a; // Unsizing
let mut a: Weak<dyn Any> = a; // Unsizing
a = a.clone();
assert!(a.upgrade().is_none());
}
Expand All @@ -39,7 +39,7 @@ fn slice() {
#[test]
fn trait_object() {
let a: Rc<u32> = Rc::new(4);
let a: Rc<Any> = a; // Unsizing
let a: Rc<dyn Any> = a; // Unsizing

// Exercise is_dangling() with a DST
let mut a = Rc::downgrade(&a);
Expand All @@ -49,7 +49,7 @@ fn trait_object() {
let mut b = Weak::<u32>::new();
b = b.clone();
assert!(b.upgrade().is_none());
let mut b: Weak<Any> = b; // Unsizing
let mut b: Weak<dyn Any> = b; // Unsizing
b = b.clone();
assert!(b.upgrade().is_none());
}
1 change: 0 additions & 1 deletion src/liballoc_jemalloc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

#![no_std]
#![allow(unused_attributes)]
#![deny(bare_trait_objects)]
#![unstable(feature = "alloc_jemalloc",
reason = "implementation detail of std, does not provide any public API",
issue = "0")]
Expand Down
1 change: 0 additions & 1 deletion src/liballoc_system/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

#![no_std]
#![allow(unused_attributes)]
#![deny(bare_trait_objects)]
#![unstable(feature = "alloc_system",
reason = "this library is unlikely to be stabilized in its current \
form or name",
Expand Down
1 change: 0 additions & 1 deletion src/libarena/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
#![cfg_attr(test, feature(test))]

#![allow(deprecated)]
#![deny(bare_trait_objects)]

extern crate alloc;
extern crate rustc_data_structures;
Expand Down
Loading