From 88e755607b0bd65c223fcac40756cb703449c09f Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Fri, 13 Sep 2019 20:59:46 -0700 Subject: [PATCH 01/25] Fix build script sanitizer check. --- src/librustc_asan/build.rs | 1 + src/librustc_lsan/build.rs | 1 + src/librustc_msan/build.rs | 1 + src/librustc_tsan/build.rs | 1 + 4 files changed, 4 insertions(+) diff --git a/src/librustc_asan/build.rs b/src/librustc_asan/build.rs index cc856ba68fbb7..645707ccc0338 100644 --- a/src/librustc_asan/build.rs +++ b/src/librustc_asan/build.rs @@ -4,6 +4,7 @@ use build_helper::sanitizer_lib_boilerplate; use cmake::Config; fn main() { + println!("cargo:rerun-if-env-changed=RUSTC_BUILD_SANITIZERS"); if env::var("RUSTC_BUILD_SANITIZERS") != Ok("1".to_string()) { return; } diff --git a/src/librustc_lsan/build.rs b/src/librustc_lsan/build.rs index d5f3e37dea51c..73720d8c2d64e 100644 --- a/src/librustc_lsan/build.rs +++ b/src/librustc_lsan/build.rs @@ -4,6 +4,7 @@ use build_helper::sanitizer_lib_boilerplate; use cmake::Config; fn main() { + println!("cargo:rerun-if-env-changed=RUSTC_BUILD_SANITIZERS"); if env::var("RUSTC_BUILD_SANITIZERS") != Ok("1".to_string()) { return; } diff --git a/src/librustc_msan/build.rs b/src/librustc_msan/build.rs index de1676f489a46..a81786ee36d04 100644 --- a/src/librustc_msan/build.rs +++ b/src/librustc_msan/build.rs @@ -4,6 +4,7 @@ use build_helper::sanitizer_lib_boilerplate; use cmake::Config; fn main() { + println!("cargo:rerun-if-env-changed=RUSTC_BUILD_SANITIZERS"); if env::var("RUSTC_BUILD_SANITIZERS") != Ok("1".to_string()) { return; } diff --git a/src/librustc_tsan/build.rs b/src/librustc_tsan/build.rs index 6df9691257455..f9333e1502327 100644 --- a/src/librustc_tsan/build.rs +++ b/src/librustc_tsan/build.rs @@ -4,6 +4,7 @@ use build_helper::sanitizer_lib_boilerplate; use cmake::Config; fn main() { + println!("cargo:rerun-if-env-changed=RUSTC_BUILD_SANITIZERS"); if env::var("RUSTC_BUILD_SANITIZERS") != Ok("1".to_string()) { return; } From b60954757eb1acd4069ddbc28a51f4a5bb7d42c9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 14 Sep 2019 10:23:10 +0200 Subject: [PATCH 02/25] std: always depend on backtrace, but only enable its features on demand --- src/bootstrap/lib.rs | 2 +- src/libstd/Cargo.toml | 18 +++++++++--------- src/libstd/build.rs | 2 +- src/libstd/panicking.rs | 6 +++--- src/libstd/rt.rs | 4 ++-- src/libstd/sys_common/backtrace.rs | 5 ++++- src/libstd/sys_common/mod.rs | 1 - src/libstd/thread/mod.rs | 4 ++-- src/libtest/Cargo.toml | 2 +- 9 files changed, 23 insertions(+), 21 deletions(-) diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 5d7581c8211be..e4f1b773a690d 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -490,7 +490,7 @@ impl Build { features.push_str(" llvm-libunwind"); } if self.config.backtrace { - features.push_str(" backtrace"); + features.push_str(" backtrace_support"); } if self.config.profiler { features.push_str(" profiler"); diff --git a/src/libstd/Cargo.toml b/src/libstd/Cargo.toml index 20442abc58890..2da73c11907de 100644 --- a/src/libstd/Cargo.toml +++ b/src/libstd/Cargo.toml @@ -27,15 +27,8 @@ hashbrown = { version = "0.5.0", features = ['rustc-dep-of-std'] } [dependencies.backtrace] version = "0.3.37" -default-features = false # don't use coresymbolication on OSX -features = [ - "rustc-dep-of-std", # enable build support for integrating into libstd - "dbghelp", # backtrace/symbolize on MSVC - "libbacktrace", # symbolize on most platforms - "libunwind", # backtrace on most platforms - "dladdr", # symbolize on platforms w/o libbacktrace -] -optional = true +default-features = false # without the libstd `backtrace` feature, stub out everything +features = [ "rustc-dep-of-std" ] # enable build support for integrating into libstd [dev-dependencies] rand = "0.7" @@ -65,6 +58,13 @@ cc = "1.0" [features] default = ["std_detect_file_io", "std_detect_dlsym_getauxval"] +backtrace_support = [ + "backtrace/dbghelp", # backtrace/symbolize on MSVC + "backtrace/libbacktrace", # symbolize on most platforms + "backtrace/libunwind", # backtrace on most platforms + "backtrace/dladdr", # symbolize on platforms w/o libbacktrace +] + panic-unwind = ["panic_unwind"] profiler = ["profiler_builtins"] compiler-builtins-c = ["alloc/compiler-builtins-c"] diff --git a/src/libstd/build.rs b/src/libstd/build.rs index 8db7bc12cd308..3209332eeb622 100644 --- a/src/libstd/build.rs +++ b/src/libstd/build.rs @@ -49,7 +49,7 @@ fn main() { println!("cargo:rustc-link-lib=zircon"); println!("cargo:rustc-link-lib=fdio"); } else if target.contains("cloudabi") { - if cfg!(feature = "backtrace") { + if cfg!(feature = "backtrace_support") { println!("cargo:rustc-link-lib=unwind"); } println!("cargo:rustc-link-lib=c"); diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index db4089c294812..93a17d6eea54e 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -157,12 +157,12 @@ pub fn take_hook() -> Box) + 'static + Sync + Send> { } fn default_hook(info: &PanicInfo<'_>) { - #[cfg(feature = "backtrace")] + #[cfg(feature = "backtrace_support")] use crate::sys_common::{backtrace as backtrace_mod}; // If this is a double panic, make sure that we print a backtrace // for this panic. Otherwise only print it if logging is enabled. - #[cfg(feature = "backtrace")] + #[cfg(feature = "backtrace_support")] let log_backtrace = { let panics = update_panic_count(0); @@ -190,7 +190,7 @@ fn default_hook(info: &PanicInfo<'_>) { let _ = writeln!(err, "thread '{}' panicked at '{}', {}", name, msg, location); - #[cfg(feature = "backtrace")] + #[cfg(feature = "backtrace_support")] { use crate::sync::atomic::{AtomicBool, Ordering}; diff --git a/src/libstd/rt.rs b/src/libstd/rt.rs index cf45eb0daba39..f73bd6c6e74fa 100644 --- a/src/libstd/rt.rs +++ b/src/libstd/rt.rs @@ -44,11 +44,11 @@ fn lang_start_internal(main: &(dyn Fn() -> i32 + Sync + crate::panic::RefUnwindS sys::args::init(argc, argv); // Let's run some code! - #[cfg(feature = "backtrace")] + #[cfg(feature = "backtrace_support")] let exit_code = panic::catch_unwind(|| { sys_common::backtrace::__rust_begin_short_backtrace(move || main()) }); - #[cfg(not(feature = "backtrace"))] + #[cfg(not(feature = "backtrace_support"))] let exit_code = panic::catch_unwind(move || main()); sys_common::cleanup(); diff --git a/src/libstd/sys_common/backtrace.rs b/src/libstd/sys_common/backtrace.rs index 1a78abf508620..52890668c35ee 100644 --- a/src/libstd/sys_common/backtrace.rs +++ b/src/libstd/sys_common/backtrace.rs @@ -7,7 +7,6 @@ use crate::io; use crate::borrow::Cow; use crate::io::prelude::*; use crate::path::{self, Path, PathBuf}; -use crate::sync::atomic::{self, Ordering}; use crate::sys::mutex::Mutex; use backtrace::{BacktraceFmt, BytesOrWideString, PrintFmt}; @@ -34,6 +33,7 @@ pub fn lock() -> impl Drop { } /// Prints the current backtrace. +#[cfg(feature = "backtrace_support")] pub fn print(w: &mut dyn Write, format: PrintFmt) -> io::Result<()> { // There are issues currently linking libbacktrace into tests, and in // general during libstd's own unit tests we're not testing this path. In @@ -129,7 +129,10 @@ where // For now logging is turned off by default, and this function checks to see // whether the magical environment variable is present to see if it's turned on. +#[cfg(feature = "backtrace_support")] pub fn log_enabled() -> Option { + use crate::sync::atomic::{self, Ordering}; + // Setting environment variables for Fuchsia components isn't a standard // or easily supported workflow. For now, always display backtraces. if cfg!(target_os = "fuchsia") { diff --git a/src/libstd/sys_common/mod.rs b/src/libstd/sys_common/mod.rs index 9190a3b0d5fc7..cba3eca538625 100644 --- a/src/libstd/sys_common/mod.rs +++ b/src/libstd/sys_common/mod.rs @@ -41,7 +41,6 @@ macro_rules! rtunwrap { pub mod alloc; pub mod at_exit_imp; -#[cfg(feature = "backtrace")] pub mod backtrace; pub mod condvar; pub mod io; diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index 764041d2f4239..85fd80e16afb3 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -465,11 +465,11 @@ impl Builder { } thread_info::set(imp::guard::current(), their_thread); - #[cfg(feature = "backtrace")] + #[cfg(feature = "backtrace_support")] let try_result = panic::catch_unwind(panic::AssertUnwindSafe(|| { crate::sys_common::backtrace::__rust_begin_short_backtrace(f) })); - #[cfg(not(feature = "backtrace"))] + #[cfg(not(feature = "backtrace_support"))] let try_result = panic::catch_unwind(panic::AssertUnwindSafe(f)); *their_packet.get() = Some(try_result); }; diff --git a/src/libtest/Cargo.toml b/src/libtest/Cargo.toml index 170fbb984cf9b..f0041bcf67cb0 100644 --- a/src/libtest/Cargo.toml +++ b/src/libtest/Cargo.toml @@ -23,7 +23,7 @@ proc_macro = { path = "../libproc_macro" } # Forward features to the `std` crate as necessary [features] -backtrace = ["std/backtrace"] +backtrace_support = ["std/backtrace_support"] compiler-builtins-c = ["std/compiler-builtins-c"] llvm-libunwind = ["std/llvm-libunwind"] panic-unwind = ["std/panic_unwind"] From dac0a158eb78419e3593fa888f9cf1ab81153030 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 14 Sep 2019 12:12:32 +0200 Subject: [PATCH 03/25] rename the crate, not the feature --- src/bootstrap/lib.rs | 2 +- src/libstd/Cargo.toml | 13 +++++++------ src/libstd/backtrace.rs | 1 + src/libstd/build.rs | 2 +- src/libstd/panicking.rs | 8 ++++---- src/libstd/rt.rs | 4 ++-- src/libstd/sys_common/backtrace.rs | 10 +++++----- src/libstd/thread/mod.rs | 4 ++-- src/libtest/Cargo.toml | 2 +- 9 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index e4f1b773a690d..5d7581c8211be 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -490,7 +490,7 @@ impl Build { features.push_str(" llvm-libunwind"); } if self.config.backtrace { - features.push_str(" backtrace_support"); + features.push_str(" backtrace"); } if self.config.profiler { features.push_str(" profiler"); diff --git a/src/libstd/Cargo.toml b/src/libstd/Cargo.toml index 2da73c11907de..af1d2402f88e7 100644 --- a/src/libstd/Cargo.toml +++ b/src/libstd/Cargo.toml @@ -25,7 +25,8 @@ profiler_builtins = { path = "../libprofiler_builtins", optional = true } unwind = { path = "../libunwind" } hashbrown = { version = "0.5.0", features = ['rustc-dep-of-std'] } -[dependencies.backtrace] +[dependencies.backtrace_rs] +package = "backtrace" version = "0.3.37" default-features = false # without the libstd `backtrace` feature, stub out everything features = [ "rustc-dep-of-std" ] # enable build support for integrating into libstd @@ -58,11 +59,11 @@ cc = "1.0" [features] default = ["std_detect_file_io", "std_detect_dlsym_getauxval"] -backtrace_support = [ - "backtrace/dbghelp", # backtrace/symbolize on MSVC - "backtrace/libbacktrace", # symbolize on most platforms - "backtrace/libunwind", # backtrace on most platforms - "backtrace/dladdr", # symbolize on platforms w/o libbacktrace +backtrace = [ + "backtrace_rs/dbghelp", # backtrace/symbolize on MSVC + "backtrace_rs/libbacktrace", # symbolize on most platforms + "backtrace_rs/libunwind", # backtrace on most platforms + "backtrace_rs/dladdr", # symbolize on platforms w/o libbacktrace ] panic-unwind = ["panic_unwind"] diff --git a/src/libstd/backtrace.rs b/src/libstd/backtrace.rs index 5d46ef7dbb10a..61c42a56071e6 100644 --- a/src/libstd/backtrace.rs +++ b/src/libstd/backtrace.rs @@ -97,6 +97,7 @@ use crate::sync::atomic::{AtomicUsize, Ordering::SeqCst}; use crate::sync::Mutex; use crate::sys_common::backtrace::{output_filename, lock}; use crate::vec::Vec; +use backtrace_rs as backtrace; use backtrace::BytesOrWideString; /// A captured OS thread stack backtrace. diff --git a/src/libstd/build.rs b/src/libstd/build.rs index 3209332eeb622..8db7bc12cd308 100644 --- a/src/libstd/build.rs +++ b/src/libstd/build.rs @@ -49,7 +49,7 @@ fn main() { println!("cargo:rustc-link-lib=zircon"); println!("cargo:rustc-link-lib=fdio"); } else if target.contains("cloudabi") { - if cfg!(feature = "backtrace_support") { + if cfg!(feature = "backtrace") { println!("cargo:rustc-link-lib=unwind"); } println!("cargo:rustc-link-lib=c"); diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index 93a17d6eea54e..e7755af7f07da 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -157,17 +157,17 @@ pub fn take_hook() -> Box) + 'static + Sync + Send> { } fn default_hook(info: &PanicInfo<'_>) { - #[cfg(feature = "backtrace_support")] + #[cfg(feature = "backtrace")] use crate::sys_common::{backtrace as backtrace_mod}; // If this is a double panic, make sure that we print a backtrace // for this panic. Otherwise only print it if logging is enabled. - #[cfg(feature = "backtrace_support")] + #[cfg(feature = "backtrace")] let log_backtrace = { let panics = update_panic_count(0); if panics >= 2 { - Some(backtrace::PrintFmt::Full) + Some(backtrace_rs::PrintFmt::Full) } else { backtrace_mod::log_enabled() } @@ -190,7 +190,7 @@ fn default_hook(info: &PanicInfo<'_>) { let _ = writeln!(err, "thread '{}' panicked at '{}', {}", name, msg, location); - #[cfg(feature = "backtrace_support")] + #[cfg(feature = "backtrace")] { use crate::sync::atomic::{AtomicBool, Ordering}; diff --git a/src/libstd/rt.rs b/src/libstd/rt.rs index f73bd6c6e74fa..cf45eb0daba39 100644 --- a/src/libstd/rt.rs +++ b/src/libstd/rt.rs @@ -44,11 +44,11 @@ fn lang_start_internal(main: &(dyn Fn() -> i32 + Sync + crate::panic::RefUnwindS sys::args::init(argc, argv); // Let's run some code! - #[cfg(feature = "backtrace_support")] + #[cfg(feature = "backtrace")] let exit_code = panic::catch_unwind(|| { sys_common::backtrace::__rust_begin_short_backtrace(move || main()) }); - #[cfg(not(feature = "backtrace_support"))] + #[cfg(not(feature = "backtrace"))] let exit_code = panic::catch_unwind(move || main()); sys_common::cleanup(); diff --git a/src/libstd/sys_common/backtrace.rs b/src/libstd/sys_common/backtrace.rs index 52890668c35ee..f49adc01659ff 100644 --- a/src/libstd/sys_common/backtrace.rs +++ b/src/libstd/sys_common/backtrace.rs @@ -9,7 +9,7 @@ use crate::io::prelude::*; use crate::path::{self, Path, PathBuf}; use crate::sys::mutex::Mutex; -use backtrace::{BacktraceFmt, BytesOrWideString, PrintFmt}; +use backtrace_rs::{BacktraceFmt, BytesOrWideString, PrintFmt}; /// Max number of frames to print. const MAX_NB_FRAMES: usize = 100; @@ -33,7 +33,7 @@ pub fn lock() -> impl Drop { } /// Prints the current backtrace. -#[cfg(feature = "backtrace_support")] +#[cfg(feature = "backtrace")] pub fn print(w: &mut dyn Write, format: PrintFmt) -> io::Result<()> { // There are issues currently linking libbacktrace into tests, and in // general during libstd's own unit tests we're not testing this path. In @@ -74,14 +74,14 @@ unsafe fn _print_fmt(fmt: &mut fmt::Formatter<'_>, print_fmt: PrintFmt) -> fmt:: bt_fmt.add_context()?; let mut idx = 0; let mut res = Ok(()); - backtrace::trace_unsynchronized(|frame| { + backtrace_rs::trace_unsynchronized(|frame| { if print_fmt == PrintFmt::Short && idx > MAX_NB_FRAMES { return false; } let mut hit = false; let mut stop = false; - backtrace::resolve_frame_unsynchronized(frame, |symbol| { + backtrace_rs::resolve_frame_unsynchronized(frame, |symbol| { hit = true; if print_fmt == PrintFmt::Short { if let Some(sym) = symbol.name().and_then(|s| s.as_str()) { @@ -129,7 +129,7 @@ where // For now logging is turned off by default, and this function checks to see // whether the magical environment variable is present to see if it's turned on. -#[cfg(feature = "backtrace_support")] +#[cfg(feature = "backtrace")] pub fn log_enabled() -> Option { use crate::sync::atomic::{self, Ordering}; diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index 85fd80e16afb3..764041d2f4239 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -465,11 +465,11 @@ impl Builder { } thread_info::set(imp::guard::current(), their_thread); - #[cfg(feature = "backtrace_support")] + #[cfg(feature = "backtrace")] let try_result = panic::catch_unwind(panic::AssertUnwindSafe(|| { crate::sys_common::backtrace::__rust_begin_short_backtrace(f) })); - #[cfg(not(feature = "backtrace_support"))] + #[cfg(not(feature = "backtrace"))] let try_result = panic::catch_unwind(panic::AssertUnwindSafe(f)); *their_packet.get() = Some(try_result); }; diff --git a/src/libtest/Cargo.toml b/src/libtest/Cargo.toml index f0041bcf67cb0..170fbb984cf9b 100644 --- a/src/libtest/Cargo.toml +++ b/src/libtest/Cargo.toml @@ -23,7 +23,7 @@ proc_macro = { path = "../libproc_macro" } # Forward features to the `std` crate as necessary [features] -backtrace_support = ["std/backtrace_support"] +backtrace = ["std/backtrace"] compiler-builtins-c = ["std/compiler-builtins-c"] llvm-libunwind = ["std/llvm-libunwind"] panic-unwind = ["std/panic_unwind"] From ee83402918b25172665ba5cc09bcf8adc5f55380 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 14 Sep 2019 12:49:04 +0200 Subject: [PATCH 04/25] when BUILD_MANIFEST_DISABLE_SIGNING is set, we don't need gpg-password-file --- src/bootstrap/dist.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/bootstrap/dist.rs b/src/bootstrap/dist.rs index 500d5766a899e..076bcd878df71 100644 --- a/src/bootstrap/dist.rs +++ b/src/bootstrap/dist.rs @@ -2000,6 +2000,8 @@ impl Step for HashSign { } fn run(self, builder: &Builder<'_>) { + // This gets called by `promote-release` + // (https://github.com/rust-lang/rust-central-station/tree/master/promote-release). let mut cmd = builder.tool_cmd(Tool::BuildManifest); if builder.config.dry_run { return; @@ -2010,10 +2012,14 @@ impl Step for HashSign { let addr = builder.config.dist_upload_addr.as_ref().unwrap_or_else(|| { panic!("\n\nfailed to specify `dist.upload-addr` in `config.toml`\n\n") }); - let file = builder.config.dist_gpg_password_file.as_ref().unwrap_or_else(|| { - panic!("\n\nfailed to specify `dist.gpg-password-file` in `config.toml`\n\n") - }); - let pass = t!(fs::read_to_string(&file)); + let pass = if env::var("BUILD_MANIFEST_DISABLE_SIGNING").is_err() { + let file = builder.config.dist_gpg_password_file.as_ref().unwrap_or_else(|| { + panic!("\n\nfailed to specify `dist.gpg-password-file` in `config.toml`\n\n") + }); + t!(fs::read_to_string(&file)) + } else { + String::new() + }; let today = output(Command::new("date").arg("+%Y-%m-%d")); From 766c4a556e95b4c47f811932a710e326c263634c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 14 Sep 2019 12:57:39 +0200 Subject: [PATCH 05/25] build-manifest: when Miri tests are not passing, do not add Miri component --- Cargo.lock | 2 ++ src/tools/build-manifest/Cargo.toml | 2 ++ src/tools/build-manifest/src/main.rs | 54 +++++++++++++++++++++++++--- 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f2eaf470658b3..9ec76d6c3cf8b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -201,7 +201,9 @@ dependencies = [ name = "build-manifest" version = "0.1.0" dependencies = [ + "reqwest", "serde", + "serde_json", "toml", ] diff --git a/src/tools/build-manifest/Cargo.toml b/src/tools/build-manifest/Cargo.toml index c364479d8db13..63ae445f99b60 100644 --- a/src/tools/build-manifest/Cargo.toml +++ b/src/tools/build-manifest/Cargo.toml @@ -7,3 +7,5 @@ edition = "2018" [dependencies] toml = "0.5" serde = { version = "1.0", features = ["derive"] } +serde_json = "1.0" +reqwest = "0.9" diff --git a/src/tools/build-manifest/src/main.rs b/src/tools/build-manifest/src/main.rs index 9ffa9391c820b..c2d642bb136be 100644 --- a/src/tools/build-manifest/src/main.rs +++ b/src/tools/build-manifest/src/main.rs @@ -1,12 +1,19 @@ +//! Build a dist manifest, hash and sign everything. +//! This gets called by `promote-release` +//! (https://github.com/rust-lang/rust-central-station/tree/master/promote-release) +//! via `x.py dist hash-and-sign`; the cmdline arguments are set up +//! by rustbuild (in `src/bootstrap/dist.rs`). + use toml; use serde::Serialize; use std::collections::BTreeMap; use std::env; use std::fs; -use std::io::{self, Read, Write}; +use std::io::{self, Read, Write, BufRead, BufReader}; use std::path::{PathBuf, Path}; use std::process::{Command, Stdio}; +use std::collections::HashMap; static HOSTS: &[&str] = &[ "aarch64-unknown-linux-gnu", @@ -146,6 +153,9 @@ static MINGW: &[&str] = &[ "x86_64-pc-windows-gnu", ]; +static TOOLSTATE: &str = + "https://github.com/raw/rust-lang-nursery/rust-toolstate/master/history/linux.tsv"; + #[derive(Serialize)] #[serde(rename_all = "kebab-case")] struct Manifest { @@ -270,6 +280,7 @@ fn main() { // Do not ask for a passphrase while manually testing let mut passphrase = String::new(); if should_sign { + // `x.py` passes the passphrase via stdin. t!(io::stdin().read_to_string(&mut passphrase)); } @@ -353,6 +364,7 @@ impl Builder { self.lldb_git_commit_hash = self.git_commit_hash("lldb", "x86_64-unknown-linux-gnu"); self.miri_git_commit_hash = self.git_commit_hash("miri", "x86_64-unknown-linux-gnu"); + self.check_toolstate(); self.digest_and_sign(); let manifest = self.build_manifest(); self.write_channel_files(&self.rust_release, &manifest); @@ -362,6 +374,37 @@ impl Builder { } } + /// If a tool does not pass its tests, don't ship it. + /// Right now, we do this only for Miri. + fn check_toolstate(&mut self) { + // Get the toolstate for this rust revision. + let rev = self.rust_git_commit_hash.as_ref().expect("failed to determine rust git hash"); + let toolstates = reqwest::get(TOOLSTATE).expect("failed to get toolstates"); + let toolstates = BufReader::new(toolstates); + let toolstate = toolstates.lines() + .find_map(|line| { + let line = line.expect("failed to read toolstate lines"); + let mut pieces = line.splitn(2, '\t'); + let commit = pieces.next().expect("malformed toolstate line"); + if commit != rev { + // Not the right commit. + return None; + } + // Return the 2nd piece, the JSON. + Some(pieces.next().expect("malformed toolstate line").to_owned()) + }) + .expect("failed to find toolstate for rust commit"); + let toolstate: HashMap = + serde_json::from_str(&toolstate).expect("toolstate is malformed JSON"); + // Mark some tools as missing based on toolstate. + if toolstate.get("miri").map(|s| &*s as &str) != Some("test-pass") { + println!("Miri tests are not passing, removing component"); + self.miri_version = None; + self.miri_git_commit_hash = None; + } + } + + /// Hash all files, compute their signatures, and collect the hashes in `self.digests`. fn digest_and_sign(&mut self) { for file in t!(self.input.read_dir()).map(|e| t!(e).path()) { let filename = file.file_name().unwrap().to_str().unwrap(); @@ -532,19 +575,20 @@ impl Builder { .as_ref() .cloned() .map(|version| (version, true)) - .unwrap_or_default(); + .unwrap_or_default(); // `is_present` defaults to `false` here. - // miri needs to build std with xargo, which doesn't allow stable/beta: - // + // Miri is nightly-only; never ship it for other trains. if pkgname == "miri-preview" && self.rust_release != "nightly" { - is_present = false; // ignore it + is_present = false; // Pretend the component is entirely missing. } let targets = targets.iter().map(|name| { if is_present { + // The component generally exists, but it might still be missing for this target. let filename = self.filename(pkgname, name); let digest = match self.digests.remove(&filename) { Some(digest) => digest, + // This component does not exist for this target -- skip it. None => return (name.to_string(), Target::unavailable()), }; let xz_filename = filename.replace(".tar.gz", ".tar.xz"); From 613769193397374485485fed2d9ba3ea2eee5077 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 27 Aug 2019 19:54:54 +0200 Subject: [PATCH 06/25] const interning: move mutability computation into intern_shallow, and always intern constants as immutable --- src/librustc_mir/interpret/intern.rs | 125 +++++++++--------- .../mutable_references_ice.stderr | 2 +- 2 files changed, 60 insertions(+), 67 deletions(-) diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index 4cbbc0ffe17cc..e05b31477e175 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -3,7 +3,7 @@ //! After a const evaluation has computed a value, before we destroy the const evaluator's session //! memory, we need to extract all memory allocations to the global memory pool so they stay around. -use rustc::ty::{Ty, TyCtxt, ParamEnv, self}; +use rustc::ty::{Ty, ParamEnv, self}; use rustc::mir::interpret::{InterpResult, ErrorHandled}; use rustc::hir; use rustc::hir::def_id::DefId; @@ -11,10 +11,9 @@ use super::validity::RefTracking; use rustc_data_structures::fx::FxHashSet; use syntax::ast::Mutability; -use syntax_pos::Span; use super::{ - ValueVisitor, MemoryKind, Pointer, AllocId, MPlaceTy, Scalar, + ValueVisitor, MemoryKind, AllocId, MPlaceTy, Scalar, }; use crate::const_eval::{CompileTimeInterpreter, CompileTimeEvalContext}; @@ -27,12 +26,10 @@ struct InternVisitor<'rt, 'mir, 'tcx> { /// for sanity assertions that will ICE when `const_qualif` screws up. mode: InternMode, /// This field stores the mutability of the value *currently* being checked. - /// It is set to mutable when an `UnsafeCell` is encountered - /// When recursing across a reference, we don't recurse but store the - /// value to be checked in `ref_tracking` together with the mutability at which we are checking - /// the value. - /// When encountering an immutable reference, we treat everything as immutable that is behind - /// it. + /// When encountering a mutable reference, we determine the pointee mutability + /// taking into account the mutability of the context: `& &mut i32` is entirely immutable, + /// despite the nested mutable reference! + /// The field gets updated when an `UnsafeCell` is encountered. mutability: Mutability, /// A list of all encountered relocations. After type-based interning, we traverse this list to /// also intern allocations that are only referenced by a raw pointer or inside a union. @@ -45,9 +42,10 @@ enum InternMode { /// `static`. In a `static mut` we start out as mutable and thus can also contain further `&mut` /// that will actually be treated as mutable. Static, - /// UnsafeCell is OK in the value of a constant, but not behind references in a constant + /// UnsafeCell is OK in the value of a constant: `const FOO = Cell::new(0)` creates + /// a new cell every time it is used. ConstBase, - /// `UnsafeCell` ICEs + /// `UnsafeCell` ICEs. Const, } @@ -56,26 +54,31 @@ enum InternMode { struct IsStaticOrFn; impl<'rt, 'mir, 'tcx> InternVisitor<'rt, 'mir, 'tcx> { - /// Intern an allocation without looking at its children + /// Intern an allocation without looking at its children. + /// `mutablity` is the mutability of the place to be interned; even if that says + /// `immutable` things might become mutable if `ty` is not frozen. fn intern_shallow( &mut self, - ptr: Pointer, + alloc_id: AllocId, mutability: Mutability, + ty: Option>, ) -> InterpResult<'tcx, Option> { trace!( "InternVisitor::intern {:?} with {:?}", - ptr, mutability, + alloc_id, mutability, ); // remove allocation let tcx = self.ecx.tcx; let memory = self.ecx.memory_mut(); - let (kind, mut alloc) = match memory.alloc_map.remove(&ptr.alloc_id) { + let (kind, mut alloc) = match memory.alloc_map.remove(&alloc_id) { Some(entry) => entry, None => { - // if the pointer is dangling (neither in local nor global memory), we leave it + // Pointer not found in local memory map. It is either a pointer to the global + // map, or dangling. + // If the pointer is dangling (neither in local nor global memory), we leave it // to validation to error. The `delay_span_bug` ensures that we don't forget such // a check in validation. - if tcx.alloc_map.lock().get(ptr.alloc_id).is_none() { + if tcx.alloc_map.lock().get(alloc_id).is_none() { tcx.sess.delay_span_bug(self.ecx.tcx.span, "tried to intern dangling pointer"); } // treat dangling pointers like other statics @@ -88,14 +91,35 @@ impl<'rt, 'mir, 'tcx> InternVisitor<'rt, 'mir, 'tcx> { match kind { MemoryKind::Stack | MemoryKind::Vtable => {}, } - // Ensure llvm knows to only put this into immutable memory if the value is immutable either - // by being behind a reference or by being part of a static or const without interior - // mutability - alloc.mutability = mutability; + // Set allocation mutability as appropriate. This is used by LLVM to put things into + // read-only memory, and also by Miri when evluating other constants/statics that + // access this one. + if self.mode == InternMode::Static { + let frozen = ty.map_or(true, |ty| ty.is_freeze( + self.ecx.tcx.tcx, + self.param_env, + self.ecx.tcx.span, + )); + // For statics, allocation mutability is the combination of the place mutability and + // the type mutability. + // The entire allocation needs to be mutable if it contains an `UnsafeCell` anywhere. + if mutability == Mutability::Immutable && frozen { + alloc.mutability = Mutability::Immutable; + } else { + // Just making sure we are not "upgrading" an immutable allocation to mutable. + assert_eq!(alloc.mutability, Mutability::Mutable); + } + } else { + // We *could* be non-frozen at `ConstBase`, for constants like `Cell::new(0)`. + // But we still intern that as immutable as the memory cannot be changed once the + // initial value was computed. + // Constants are never mutable. + alloc.mutability = Mutability::Immutable; + }; // link the alloc id to the actual allocation let alloc = tcx.intern_const_alloc(alloc); self.leftover_relocations.extend(alloc.relocations().iter().map(|&(_, ((), reloc))| reloc)); - tcx.alloc_map.lock().set_alloc_id_memory(ptr.alloc_id, alloc); + tcx.alloc_map.lock().set_alloc_id_memory(alloc_id, alloc); Ok(None) } } @@ -119,14 +143,16 @@ for ) -> InterpResult<'tcx> { if let Some(def) = mplace.layout.ty.ty_adt_def() { if Some(def.did) == self.ecx.tcx.lang_items().unsafe_cell_type() { - // We are crossing over an `UnsafeCell`, we can mutate again + // We are crossing over an `UnsafeCell`, we can mutate again. This means that + // References we encounter inside here are interned as pointing to mutable + // allocations. let old = std::mem::replace(&mut self.mutability, Mutability::Mutable); assert_ne!( self.mode, InternMode::Const, "UnsafeCells are not allowed behind references in constants. This should have \ been prevented statically by const qualification. If this were allowed one \ - would be able to change a constant at one use site and other use sites may \ - arbitrarily decide to change, too.", + would be able to change a constant at one use site and other use sites could \ + observe that mutation.", ); let walked = self.walk_aggregate(mplace, fields); self.mutability = old; @@ -150,7 +176,7 @@ for if let Ok(vtable) = meta.unwrap().to_ptr() { // explitly choose `Immutable` here, since vtables are immutable, even // if the reference of the fat pointer is mutable - self.intern_shallow(vtable, Mutability::Immutable)?; + self.intern_shallow(vtable.alloc_id, Mutability::Immutable, None)?; } } } @@ -195,21 +221,13 @@ for (Mutability::Mutable, hir::Mutability::MutMutable) => Mutability::Mutable, _ => Mutability::Immutable, }; - // Compute the mutability of the allocation - let intern_mutability = intern_mutability( - self.ecx.tcx.tcx, - self.param_env, - mplace.layout.ty, - self.ecx.tcx.span, - mutability, - ); // Recursing behind references changes the intern mode for constants in order to // cause assertions to trigger if we encounter any `UnsafeCell`s. let mode = match self.mode { InternMode::ConstBase => InternMode::Const, other => other, }; - match self.intern_shallow(ptr, intern_mutability)? { + match self.intern_shallow(ptr.alloc_id, mutability, Some(mplace.layout.ty))? { // No need to recurse, these are interned already and statics may have // cycles, so we don't want to recurse there Some(IsStaticOrFn) => {}, @@ -224,23 +242,6 @@ for } } -/// Figure out the mutability of the allocation. -/// Mutable if it has interior mutability *anywhere* in the type. -fn intern_mutability<'tcx>( - tcx: TyCtxt<'tcx>, - param_env: ParamEnv<'tcx>, - ty: Ty<'tcx>, - span: Span, - mutability: Mutability, -) -> Mutability { - let has_interior_mutability = !ty.is_freeze(tcx, param_env, span); - if has_interior_mutability { - Mutability::Mutable - } else { - mutability - } -} - pub fn intern_const_alloc_recursive( ecx: &mut CompileTimeEvalContext<'mir, 'tcx>, def_id: DefId, @@ -251,7 +252,7 @@ pub fn intern_const_alloc_recursive( ) -> InterpResult<'tcx> { let tcx = ecx.tcx; // this `mutability` is the mutability of the place, ignoring the type - let (mutability, base_intern_mode) = match tcx.static_mutability(def_id) { + let (base_mutability, base_intern_mode) = match tcx.static_mutability(def_id) { Some(hir::Mutability::MutImmutable) => (Mutability::Immutable, InternMode::Static), None => (Mutability::Immutable, InternMode::ConstBase), // `static mut` doesn't care about interior mutability, it's mutable anyway @@ -259,17 +260,9 @@ pub fn intern_const_alloc_recursive( }; // type based interning - let mut ref_tracking = RefTracking::new((ret, mutability, base_intern_mode)); + let mut ref_tracking = RefTracking::new((ret, base_mutability, base_intern_mode)); let leftover_relocations = &mut FxHashSet::default(); - // This mutability is the combination of the place mutability and the type mutability. If either - // is mutable, `alloc_mutability` is mutable. This exists because the entire allocation needs - // to be mutable if it contains an `UnsafeCell` anywhere. The other `mutability` exists so that - // the visitor does not treat everything outside the `UnsafeCell` as mutable. - let alloc_mutability = intern_mutability( - tcx.tcx, param_env, ret.layout.ty, tcx.span, mutability, - ); - // start with the outermost allocation InternVisitor { ref_tracking: &mut ref_tracking, @@ -277,8 +270,8 @@ pub fn intern_const_alloc_recursive( mode: base_intern_mode, leftover_relocations, param_env, - mutability, - }.intern_shallow(ret.ptr.to_ptr()?, alloc_mutability)?; + mutability: base_mutability, + }.intern_shallow(ret.ptr.to_ptr()?.alloc_id, base_mutability, Some(ret.layout.ty))?; while let Some(((mplace, mutability, mode), _)) = ref_tracking.todo.pop() { let interned = InternVisitor { @@ -312,8 +305,8 @@ pub fn intern_const_alloc_recursive( let mut todo: Vec<_> = leftover_relocations.iter().cloned().collect(); while let Some(alloc_id) = todo.pop() { if let Some((_, alloc)) = ecx.memory_mut().alloc_map.remove(&alloc_id) { - // We can't call the `intern` method here, as its logic is tailored to safe references. - // So we hand-roll the interning logic here again + // We can't call the `intern_shallow` method here, as its logic is tailored to safe + // references. So we hand-roll the interning logic here again. let alloc = tcx.intern_const_alloc(alloc); tcx.alloc_map.lock().set_alloc_id_memory(alloc_id, alloc); for &(_, ((), reloc)) in alloc.relocations().iter() { diff --git a/src/test/ui/consts/miri_unleashed/mutable_references_ice.stderr b/src/test/ui/consts/miri_unleashed/mutable_references_ice.stderr index 82569e260143c..28cf3537d605a 100644 --- a/src/test/ui/consts/miri_unleashed/mutable_references_ice.stderr +++ b/src/test/ui/consts/miri_unleashed/mutable_references_ice.stderr @@ -6,7 +6,7 @@ LL | *MUH.x.get() = 99; thread 'rustc' panicked at 'assertion failed: `(left != right)` left: `Const`, - right: `Const`: UnsafeCells are not allowed behind references in constants. This should have been prevented statically by const qualification. If this were allowed one would be able to change a constant at one use site and other use sites may arbitrarily decide to change, too.', src/librustc_mir/interpret/intern.rs:LL:CC + right: `Const`: UnsafeCells are not allowed behind references in constants. This should have been prevented statically by const qualification. If this were allowed one would be able to change a constant at one use site and other use sites could observe that mutation.', src/librustc_mir/interpret/intern.rs:LL:CC note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace. error: internal compiler error: unexpected panic From 7b8693eff85d39d7b836fa45cdcf30507d7f8731 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 27 Aug 2019 20:59:34 +0200 Subject: [PATCH 07/25] all memory behind a constant must be immutable --- src/librustc_mir/interpret/intern.rs | 7 ++++- .../ui/consts/miri_unleashed/mutable_const.rs | 20 ++++++++++++++ .../miri_unleashed/mutable_const.stderr | 27 +++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/consts/miri_unleashed/mutable_const.rs create mode 100644 src/test/ui/consts/miri_unleashed/mutable_const.stderr diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index e05b31477e175..6333fce2c47f2 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -304,9 +304,14 @@ pub fn intern_const_alloc_recursive( let mut todo: Vec<_> = leftover_relocations.iter().cloned().collect(); while let Some(alloc_id) = todo.pop() { - if let Some((_, alloc)) = ecx.memory_mut().alloc_map.remove(&alloc_id) { + if let Some((_, mut alloc)) = ecx.memory_mut().alloc_map.remove(&alloc_id) { // We can't call the `intern_shallow` method here, as its logic is tailored to safe // references. So we hand-roll the interning logic here again. + if base_intern_mode != InternMode::Static { + // If it's not a static, it *must* be immutable. + // We cannot have mutable memory inside a constant. + alloc.mutability = Mutability::Immutable; + } let alloc = tcx.intern_const_alloc(alloc); tcx.alloc_map.lock().set_alloc_id_memory(alloc_id, alloc); for &(_, ((), reloc)) in alloc.relocations().iter() { diff --git a/src/test/ui/consts/miri_unleashed/mutable_const.rs b/src/test/ui/consts/miri_unleashed/mutable_const.rs new file mode 100644 index 0000000000000..b476e04529a52 --- /dev/null +++ b/src/test/ui/consts/miri_unleashed/mutable_const.rs @@ -0,0 +1,20 @@ +// compile-flags: -Zunleash-the-miri-inside-of-you + +#![feature(const_raw_ptr_deref)] +#![deny(const_err)] + +use std::cell::UnsafeCell; + +// make sure we do not just intern this as mutable +const MUTABLE_BEHIND_RAW: *mut i32 = &UnsafeCell::new(42) as *const _ as *mut _; + +const MUTATING_BEHIND_RAW: () = { + // Test that `MUTABLE_BEHIND_RAW` is actually immutable, by doing this at const time. + unsafe { + *MUTABLE_BEHIND_RAW = 99 //~ WARN skipping const checks + //~^ ERROR any use of this value will cause an error + //~^^ tried to modify constant memory + } +}; + +fn main() {} diff --git a/src/test/ui/consts/miri_unleashed/mutable_const.stderr b/src/test/ui/consts/miri_unleashed/mutable_const.stderr new file mode 100644 index 0000000000000..507d4823a111d --- /dev/null +++ b/src/test/ui/consts/miri_unleashed/mutable_const.stderr @@ -0,0 +1,27 @@ +warning: skipping const checks + --> $DIR/mutable_const.rs:14:9 + | +LL | *MUTABLE_BEHIND_RAW = 99 + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: any use of this value will cause an error + --> $DIR/mutable_const.rs:14:9 + | +LL | / const MUTATING_BEHIND_RAW: () = { +LL | | // Test that `MUTABLE_BEHIND_RAW` is actually immutable, by doing this at const time. +LL | | unsafe { +LL | | *MUTABLE_BEHIND_RAW = 99 + | | ^^^^^^^^^^^^^^^^^^^^^^^^ tried to modify constant memory +... | +LL | | } +LL | | }; + | |__- + | +note: lint level defined here + --> $DIR/mutable_const.rs:4:9 + | +LL | #![deny(const_err)] + | ^^^^^^^^^ + +error: aborting due to previous error + From 75c82b4dd8152562ff2056f2172b20aa9c46def2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 27 Aug 2019 21:13:05 +0200 Subject: [PATCH 08/25] drop redundant ParamEnv, and avoid constructing InternVisitor without visiting --- src/librustc_mir/const_eval.rs | 6 +- src/librustc_mir/interpret/intern.rs | 170 +++++++++++++++------------ 2 files changed, 95 insertions(+), 81 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 57ddaa4eff038..3f53f842f314f 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -134,9 +134,8 @@ fn eval_body_using_ecx<'mir, 'tcx>( ecx: &mut CompileTimeEvalContext<'mir, 'tcx>, cid: GlobalId<'tcx>, body: &'mir mir::Body<'tcx>, - param_env: ty::ParamEnv<'tcx>, ) -> InterpResult<'tcx, MPlaceTy<'tcx>> { - debug!("eval_body_using_ecx: {:?}, {:?}", cid, param_env); + debug!("eval_body_using_ecx: {:?}, {:?}", cid, ecx.param_env); let tcx = ecx.tcx.tcx; let layout = ecx.layout_of(body.return_ty().subst(tcx, cid.instance.substs))?; assert!(!layout.is_unsized()); @@ -162,7 +161,6 @@ fn eval_body_using_ecx<'mir, 'tcx>( ecx, cid.instance.def_id(), ret, - param_env, )?; debug!("eval_body_using_ecx done: {:?}", *ret); @@ -658,7 +656,7 @@ pub fn const_eval_raw_provider<'tcx>( let res = ecx.load_mir(cid.instance.def, cid.promoted); res.and_then( - |body| eval_body_using_ecx(&mut ecx, cid, body, key.param_env) + |body| eval_body_using_ecx(&mut ecx, cid, body) ).and_then(|place| { Ok(RawConst { alloc_id: place.ptr.assert_ptr().alloc_id, diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index 6333fce2c47f2..b657a33db516e 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -3,7 +3,7 @@ //! After a const evaluation has computed a value, before we destroy the const evaluator's session //! memory, we need to extract all memory allocations to the global memory pool so they stay around. -use rustc::ty::{Ty, ParamEnv, self}; +use rustc::ty::{Ty, self}; use rustc::mir::interpret::{InterpResult, ErrorHandled}; use rustc::hir; use rustc::hir::def_id::DefId; @@ -18,10 +18,10 @@ use super::{ use crate::const_eval::{CompileTimeInterpreter, CompileTimeEvalContext}; struct InternVisitor<'rt, 'mir, 'tcx> { - /// previously encountered safe references - ref_tracking: &'rt mut RefTracking<(MPlaceTy<'tcx>, Mutability, InternMode)>, + /// The ectx from which we intern. ecx: &'rt mut CompileTimeEvalContext<'mir, 'tcx>, - param_env: ParamEnv<'tcx>, + /// Previously encountered safe references. + ref_tracking: &'rt mut RefTracking<(MPlaceTy<'tcx>, Mutability, InternMode)>, /// The root node of the value that we're looking at. This field is never mutated and only used /// for sanity assertions that will ICE when `const_qualif` screws up. mode: InternMode, @@ -53,74 +53,93 @@ enum InternMode { /// into the memory of other constants or statics struct IsStaticOrFn; +/// Intern an allocation without looking at its children. +/// `mode` is the mode of the environment where we found this pointer. +/// `mutablity` is the mutability of the place to be interned; even if that says +/// `immutable` things might become mutable if `ty` is not frozen. +fn intern_shallow<'rt, 'mir, 'tcx>( + ecx: &'rt mut CompileTimeEvalContext<'mir, 'tcx>, + leftover_relocations: &'rt mut FxHashSet, + mode: InternMode, + alloc_id: AllocId, + mutability: Mutability, + ty: Option>, +) -> InterpResult<'tcx, Option> { + trace!( + "InternVisitor::intern {:?} with {:?}", + alloc_id, mutability, + ); + // remove allocation + let tcx = ecx.tcx; + let memory = ecx.memory_mut(); + let (kind, mut alloc) = match memory.alloc_map.remove(&alloc_id) { + Some(entry) => entry, + None => { + // Pointer not found in local memory map. It is either a pointer to the global + // map, or dangling. + // If the pointer is dangling (neither in local nor global memory), we leave it + // to validation to error. The `delay_span_bug` ensures that we don't forget such + // a check in validation. + if tcx.alloc_map.lock().get(alloc_id).is_none() { + tcx.sess.delay_span_bug(ecx.tcx.span, "tried to intern dangling pointer"); + } + // treat dangling pointers like other statics + // just to stop trying to recurse into them + return Ok(Some(IsStaticOrFn)); + }, + }; + // This match is just a canary for future changes to `MemoryKind`, which most likely need + // changes in this function. + match kind { + MemoryKind::Stack | MemoryKind::Vtable => {}, + } + // Set allocation mutability as appropriate. This is used by LLVM to put things into + // read-only memory, and also by Miri when evluating other constants/statics that + // access this one. + if mode == InternMode::Static { + let frozen = ty.map_or(true, |ty| ty.is_freeze( + ecx.tcx.tcx, + ecx.param_env, + ecx.tcx.span, + )); + // For statics, allocation mutability is the combination of the place mutability and + // the type mutability. + // The entire allocation needs to be mutable if it contains an `UnsafeCell` anywhere. + if mutability == Mutability::Immutable && frozen { + alloc.mutability = Mutability::Immutable; + } else { + // Just making sure we are not "upgrading" an immutable allocation to mutable. + assert_eq!(alloc.mutability, Mutability::Mutable); + } + } else { + // We *could* be non-frozen at `ConstBase`, for constants like `Cell::new(0)`. + // But we still intern that as immutable as the memory cannot be changed once the + // initial value was computed. + // Constants are never mutable. + alloc.mutability = Mutability::Immutable; + }; + // link the alloc id to the actual allocation + let alloc = tcx.intern_const_alloc(alloc); + leftover_relocations.extend(alloc.relocations().iter().map(|&(_, ((), reloc))| reloc)); + tcx.alloc_map.lock().set_alloc_id_memory(alloc_id, alloc); + Ok(None) +} + impl<'rt, 'mir, 'tcx> InternVisitor<'rt, 'mir, 'tcx> { - /// Intern an allocation without looking at its children. - /// `mutablity` is the mutability of the place to be interned; even if that says - /// `immutable` things might become mutable if `ty` is not frozen. fn intern_shallow( &mut self, alloc_id: AllocId, mutability: Mutability, ty: Option>, ) -> InterpResult<'tcx, Option> { - trace!( - "InternVisitor::intern {:?} with {:?}", - alloc_id, mutability, - ); - // remove allocation - let tcx = self.ecx.tcx; - let memory = self.ecx.memory_mut(); - let (kind, mut alloc) = match memory.alloc_map.remove(&alloc_id) { - Some(entry) => entry, - None => { - // Pointer not found in local memory map. It is either a pointer to the global - // map, or dangling. - // If the pointer is dangling (neither in local nor global memory), we leave it - // to validation to error. The `delay_span_bug` ensures that we don't forget such - // a check in validation. - if tcx.alloc_map.lock().get(alloc_id).is_none() { - tcx.sess.delay_span_bug(self.ecx.tcx.span, "tried to intern dangling pointer"); - } - // treat dangling pointers like other statics - // just to stop trying to recurse into them - return Ok(Some(IsStaticOrFn)); - }, - }; - // This match is just a canary for future changes to `MemoryKind`, which most likely need - // changes in this function. - match kind { - MemoryKind::Stack | MemoryKind::Vtable => {}, - } - // Set allocation mutability as appropriate. This is used by LLVM to put things into - // read-only memory, and also by Miri when evluating other constants/statics that - // access this one. - if self.mode == InternMode::Static { - let frozen = ty.map_or(true, |ty| ty.is_freeze( - self.ecx.tcx.tcx, - self.param_env, - self.ecx.tcx.span, - )); - // For statics, allocation mutability is the combination of the place mutability and - // the type mutability. - // The entire allocation needs to be mutable if it contains an `UnsafeCell` anywhere. - if mutability == Mutability::Immutable && frozen { - alloc.mutability = Mutability::Immutable; - } else { - // Just making sure we are not "upgrading" an immutable allocation to mutable. - assert_eq!(alloc.mutability, Mutability::Mutable); - } - } else { - // We *could* be non-frozen at `ConstBase`, for constants like `Cell::new(0)`. - // But we still intern that as immutable as the memory cannot be changed once the - // initial value was computed. - // Constants are never mutable. - alloc.mutability = Mutability::Immutable; - }; - // link the alloc id to the actual allocation - let alloc = tcx.intern_const_alloc(alloc); - self.leftover_relocations.extend(alloc.relocations().iter().map(|&(_, ((), reloc))| reloc)); - tcx.alloc_map.lock().set_alloc_id_memory(alloc_id, alloc); - Ok(None) + intern_shallow( + self.ecx, + self.leftover_relocations, + self.mode, + alloc_id, + mutability, + ty, + ) } } @@ -171,7 +190,8 @@ for // Handle trait object vtables if let Ok(meta) = value.to_meta() { if let ty::Dynamic(..) = - self.ecx.tcx.struct_tail_erasing_lifetimes(referenced_ty, self.param_env).sty + self.ecx.tcx.struct_tail_erasing_lifetimes( + referenced_ty, self.ecx.param_env).sty { if let Ok(vtable) = meta.unwrap().to_ptr() { // explitly choose `Immutable` here, since vtables are immutable, even @@ -203,7 +223,7 @@ for (InternMode::Const, hir::Mutability::MutMutable) => { match referenced_ty.sty { ty::Array(_, n) - if n.eval_usize(self.ecx.tcx.tcx, self.param_env) == 0 => {} + if n.eval_usize(self.ecx.tcx.tcx, self.ecx.param_env) == 0 => {} ty::Slice(_) if value.to_meta().unwrap().unwrap().to_usize(self.ecx)? == 0 => {} _ => bug!("const qualif failed to prevent mutable references"), @@ -246,9 +266,6 @@ pub fn intern_const_alloc_recursive( ecx: &mut CompileTimeEvalContext<'mir, 'tcx>, def_id: DefId, ret: MPlaceTy<'tcx>, - // FIXME(oli-obk): can we scrap the param env? I think we can, the final value of a const eval - // must always be monomorphic, right? - param_env: ty::ParamEnv<'tcx>, ) -> InterpResult<'tcx> { let tcx = ecx.tcx; // this `mutability` is the mutability of the place, ignoring the type @@ -264,14 +281,14 @@ pub fn intern_const_alloc_recursive( let leftover_relocations = &mut FxHashSet::default(); // start with the outermost allocation - InternVisitor { - ref_tracking: &mut ref_tracking, + intern_shallow( ecx, - mode: base_intern_mode, leftover_relocations, - param_env, - mutability: base_mutability, - }.intern_shallow(ret.ptr.to_ptr()?.alloc_id, base_mutability, Some(ret.layout.ty))?; + base_intern_mode, + ret.ptr.to_ptr()?.alloc_id, + base_mutability, + Some(ret.layout.ty) + )?; while let Some(((mplace, mutability, mode), _)) = ref_tracking.todo.pop() { let interned = InternVisitor { @@ -279,7 +296,6 @@ pub fn intern_const_alloc_recursive( ecx, mode, leftover_relocations, - param_env, mutability, }.visit_value(mplace); if let Err(error) = interned { From b1d4d2bfeaf45c9f3132b863867888ec89d1d40e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 27 Aug 2019 21:15:55 +0200 Subject: [PATCH 09/25] relocations -> allocations --- src/librustc_mir/interpret/intern.rs | 31 ++++++++++++++++------------ 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index b657a33db516e..85bc88c86c277 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -22,6 +22,9 @@ struct InternVisitor<'rt, 'mir, 'tcx> { ecx: &'rt mut CompileTimeEvalContext<'mir, 'tcx>, /// Previously encountered safe references. ref_tracking: &'rt mut RefTracking<(MPlaceTy<'tcx>, Mutability, InternMode)>, + /// A list of all encountered allocations. After type-based interning, we traverse this list to + /// also intern allocations that are only referenced by a raw pointer or inside a union. + leftover_allocations: &'rt mut FxHashSet, /// The root node of the value that we're looking at. This field is never mutated and only used /// for sanity assertions that will ICE when `const_qualif` screws up. mode: InternMode, @@ -31,9 +34,6 @@ struct InternVisitor<'rt, 'mir, 'tcx> { /// despite the nested mutable reference! /// The field gets updated when an `UnsafeCell` is encountered. mutability: Mutability, - /// A list of all encountered relocations. After type-based interning, we traverse this list to - /// also intern allocations that are only referenced by a raw pointer or inside a union. - leftover_relocations: &'rt mut FxHashSet, } #[derive(Copy, Clone, Debug, PartialEq, Hash, Eq)] @@ -59,7 +59,7 @@ struct IsStaticOrFn; /// `immutable` things might become mutable if `ty` is not frozen. fn intern_shallow<'rt, 'mir, 'tcx>( ecx: &'rt mut CompileTimeEvalContext<'mir, 'tcx>, - leftover_relocations: &'rt mut FxHashSet, + leftover_allocations: &'rt mut FxHashSet, mode: InternMode, alloc_id: AllocId, mutability: Mutability, @@ -120,7 +120,7 @@ fn intern_shallow<'rt, 'mir, 'tcx>( }; // link the alloc id to the actual allocation let alloc = tcx.intern_const_alloc(alloc); - leftover_relocations.extend(alloc.relocations().iter().map(|&(_, ((), reloc))| reloc)); + leftover_allocations.extend(alloc.relocations().iter().map(|&(_, ((), reloc))| reloc)); tcx.alloc_map.lock().set_alloc_id_memory(alloc_id, alloc); Ok(None) } @@ -134,7 +134,7 @@ impl<'rt, 'mir, 'tcx> InternVisitor<'rt, 'mir, 'tcx> { ) -> InterpResult<'tcx, Option> { intern_shallow( self.ecx, - self.leftover_relocations, + self.leftover_allocations, self.mode, alloc_id, mutability, @@ -276,14 +276,18 @@ pub fn intern_const_alloc_recursive( Some(hir::Mutability::MutMutable) => (Mutability::Mutable, InternMode::Static), }; - // type based interning + // Type based interning. + // `ref_tracking` tracks typed references we have seen and still need to crawl for + // more typed information inside them. + // `leftover_allocations` collects *all* allocations we see, because some might not + // be available in a typed way. They get interned at the end. let mut ref_tracking = RefTracking::new((ret, base_mutability, base_intern_mode)); - let leftover_relocations = &mut FxHashSet::default(); + let leftover_allocations = &mut FxHashSet::default(); // start with the outermost allocation intern_shallow( ecx, - leftover_relocations, + leftover_allocations, base_intern_mode, ret.ptr.to_ptr()?.alloc_id, base_mutability, @@ -295,7 +299,7 @@ pub fn intern_const_alloc_recursive( ref_tracking: &mut ref_tracking, ecx, mode, - leftover_relocations, + leftover_allocations, mutability, }.visit_value(mplace); if let Err(error) = interned { @@ -318,11 +322,12 @@ pub fn intern_const_alloc_recursive( // Intern the rest of the allocations as mutable. These might be inside unions, padding, raw // pointers, ... So we can't intern them according to their type rules - let mut todo: Vec<_> = leftover_relocations.iter().cloned().collect(); + let mut todo: Vec<_> = leftover_allocations.iter().cloned().collect(); while let Some(alloc_id) = todo.pop() { if let Some((_, mut alloc)) = ecx.memory_mut().alloc_map.remove(&alloc_id) { // We can't call the `intern_shallow` method here, as its logic is tailored to safe - // references. So we hand-roll the interning logic here again. + // references and a `leftover_allocations` set (where we only have a todo-list here). + // So we hand-roll the interning logic here again. if base_intern_mode != InternMode::Static { // If it's not a static, it *must* be immutable. // We cannot have mutable memory inside a constant. @@ -331,7 +336,7 @@ pub fn intern_const_alloc_recursive( let alloc = tcx.intern_const_alloc(alloc); tcx.alloc_map.lock().set_alloc_id_memory(alloc_id, alloc); for &(_, ((), reloc)) in alloc.relocations().iter() { - if leftover_relocations.insert(reloc) { + if leftover_allocations.insert(reloc) { todo.push(reloc); } } From 14e3506738e16ffdd1d913c371c9691e36e85b13 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 27 Aug 2019 21:24:30 +0200 Subject: [PATCH 10/25] note a FIXME --- src/librustc_mir/interpret/intern.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index 85bc88c86c277..cf4f3fda090f3 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -331,6 +331,8 @@ pub fn intern_const_alloc_recursive( if base_intern_mode != InternMode::Static { // If it's not a static, it *must* be immutable. // We cannot have mutable memory inside a constant. + // FIXME: ideally we would assert that they already are immutable, to double- + // check our static checks. alloc.mutability = Mutability::Immutable; } let alloc = tcx.intern_const_alloc(alloc); From 342481185255acef5e44af8023bab372314afe51 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 28 Aug 2019 08:31:51 +0200 Subject: [PATCH 11/25] assert that nobody asks for mutable constants --- src/librustc_mir/interpret/intern.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index cf4f3fda090f3..606f506434579 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -116,6 +116,10 @@ fn intern_shallow<'rt, 'mir, 'tcx>( // But we still intern that as immutable as the memory cannot be changed once the // initial value was computed. // Constants are never mutable. + assert_eq!( + mutability, Mutability::Immutable, + "Something went very wrong: mutability requested for a constant" + ); alloc.mutability = Mutability::Immutable; }; // link the alloc id to the actual allocation From 224e2e5e9e1d03db1a80455d08bb1d6d8686ec3a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 28 Aug 2019 09:44:48 +0200 Subject: [PATCH 12/25] explain ty == None --- src/librustc_mir/interpret/intern.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index 606f506434579..0031dbc4d0d40 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -57,6 +57,8 @@ struct IsStaticOrFn; /// `mode` is the mode of the environment where we found this pointer. /// `mutablity` is the mutability of the place to be interned; even if that says /// `immutable` things might become mutable if `ty` is not frozen. +/// `ty` can be `None` if there is no potential interior mutability +/// to account for (e.g. for vtables). fn intern_shallow<'rt, 'mir, 'tcx>( ecx: &'rt mut CompileTimeEvalContext<'mir, 'tcx>, leftover_allocations: &'rt mut FxHashSet, @@ -97,6 +99,7 @@ fn intern_shallow<'rt, 'mir, 'tcx>( // read-only memory, and also by Miri when evluating other constants/statics that // access this one. if mode == InternMode::Static { + // When `ty` is `None`, we assume no interior mutability. let frozen = ty.map_or(true, |ty| ty.is_freeze( ecx.tcx.tcx, ecx.param_env, From 5462ecb4b131a513b2d821d52b9af491781bd898 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 28 Aug 2019 09:54:30 +0200 Subject: [PATCH 13/25] update intern classification comment --- src/librustc_mir/interpret/intern.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index 0031dbc4d0d40..95647ce642c5b 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -278,9 +278,10 @@ pub fn intern_const_alloc_recursive( // this `mutability` is the mutability of the place, ignoring the type let (base_mutability, base_intern_mode) = match tcx.static_mutability(def_id) { Some(hir::Mutability::MutImmutable) => (Mutability::Immutable, InternMode::Static), - None => (Mutability::Immutable, InternMode::ConstBase), // `static mut` doesn't care about interior mutability, it's mutable anyway Some(hir::Mutability::MutMutable) => (Mutability::Mutable, InternMode::Static), + // consts, promoteds. FIXME: what about array lengths, array initializers? + None => (Mutability::Immutable, InternMode::ConstBase), }; // Type based interning. From 5355a16150a89503b67edbb3da545f8d653a4126 Mon Sep 17 00:00:00 2001 From: Guanqun Lu Date: Fri, 13 Sep 2019 17:36:35 +0800 Subject: [PATCH 14/25] use println!() --- src/librustc_codegen_llvm/lib.rs | 6 ++-- src/libstd/process.rs | 2 +- .../debuginfo/function-arg-initialization.rs | 4 +-- src/test/ui/drop/dropck_legal_cycles.rs | 30 +++++++++---------- src/test/ui/for/for-c-in-str.rs | 2 +- src/tools/compiletest/src/main.rs | 4 +-- src/tools/compiletest/src/runtest.rs | 4 +-- 7 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/librustc_codegen_llvm/lib.rs b/src/librustc_codegen_llvm/lib.rs index 9f2c303145dc8..34e39af3c39fc 100644 --- a/src/librustc_codegen_llvm/lib.rs +++ b/src/librustc_codegen_llvm/lib.rs @@ -226,21 +226,21 @@ impl CodegenBackend for LlvmCodegenBackend { for &(name, _) in back::write::RELOC_MODEL_ARGS.iter() { println!(" {}", name); } - println!(""); + println!(); } PrintRequest::CodeModels => { println!("Available code models:"); for &(name, _) in back::write::CODE_GEN_MODEL_ARGS.iter(){ println!(" {}", name); } - println!(""); + println!(); } PrintRequest::TlsModels => { println!("Available TLS models:"); for &(name, _) in back::write::TLS_MODEL_ARGS.iter(){ println!(" {}", name); } - println!(""); + println!(); } req => llvm_util::print(req, sess), } diff --git a/src/libstd/process.rs b/src/libstd/process.rs index c50025ab7d1de..b8d57cfafea0a 100644 --- a/src/libstd/process.rs +++ b/src/libstd/process.rs @@ -422,7 +422,7 @@ impl fmt::Debug for ChildStderr { /// // Execute `ls` in the current directory of the program. /// list_dir.status().expect("process failed to execute"); /// -/// println!(""); +/// println!(); /// /// // Change `ls` to execute in the root directory. /// list_dir.current_dir("/"); diff --git a/src/test/debuginfo/function-arg-initialization.rs b/src/test/debuginfo/function-arg-initialization.rs index fef62534b304d..8c86d2cf435b6 100644 --- a/src/test/debuginfo/function-arg-initialization.rs +++ b/src/test/debuginfo/function-arg-initialization.rs @@ -242,12 +242,12 @@ fn non_immediate_args(a: BigStruct, b: BigStruct) { fn binding(a: i64, b: u64, c: f64) { let x = 0; // #break - println!("") + println!() } fn assignment(mut a: u64, b: u64, c: f64) { a = b; // #break - println!("") + println!() } fn function_call(x: u64, y: u64, z: f64) { diff --git a/src/test/ui/drop/dropck_legal_cycles.rs b/src/test/ui/drop/dropck_legal_cycles.rs index a4f4c2666ac9a..fb13fd764bfaf 100644 --- a/src/test/ui/drop/dropck_legal_cycles.rs +++ b/src/test/ui/drop/dropck_legal_cycles.rs @@ -143,7 +143,7 @@ pub fn main() { v[0].descend_into_self(&mut c); assert!(!c.saw_prev_marked); // <-- different from below, b/c acyclic above - if PRINT { println!(""); } + if PRINT { println!(); } // Cycle 1: { v[0] -> v[1], v[1] -> v[0] }; // does not exercise `v` itself @@ -158,7 +158,7 @@ pub fn main() { v[0].descend_into_self(&mut c); assert!(c.saw_prev_marked); - if PRINT { println!(""); } + if PRINT { println!(); } // Cycle 2: { v[0] -> v, v[1] -> v } let v: V = Named::new("v"); @@ -171,7 +171,7 @@ pub fn main() { v.descend_into_self(&mut c); assert!(c.saw_prev_marked); - if PRINT { println!(""); } + if PRINT { println!(); } // Cycle 3: { hk0 -> hv0, hv0 -> hk0, hk1 -> hv1, hv1 -> hk1 }; // does not exercise `h` itself @@ -193,7 +193,7 @@ pub fn main() { assert!(c.saw_prev_marked); } - if PRINT { println!(""); } + if PRINT { println!(); } // Cycle 4: { h -> (hmk0,hmv0,hmk1,hmv1), {hmk0,hmv0,hmk1,hmv1} -> h } @@ -216,7 +216,7 @@ pub fn main() { // break; } - if PRINT { println!(""); } + if PRINT { println!(); } // Cycle 5: { vd[0] -> vd[1], vd[1] -> vd[0] }; // does not exercise vd itself @@ -232,7 +232,7 @@ pub fn main() { vd[0].descend_into_self(&mut c); assert!(c.saw_prev_marked); - if PRINT { println!(""); } + if PRINT { println!(); } // Cycle 6: { vd -> (vd0, vd1), {vd0, vd1} -> vd } let mut vd: VecDeque = VecDeque::new(); @@ -247,7 +247,7 @@ pub fn main() { vd[0].descend_into_self(&mut c); assert!(c.saw_prev_marked); - if PRINT { println!(""); } + if PRINT { println!(); } // Cycle 7: { vm -> (vm0, vm1), {vm0, vm1} -> vm } let mut vm: HashMap = HashMap::new(); @@ -262,7 +262,7 @@ pub fn main() { vm[&0].descend_into_self(&mut c); assert!(c.saw_prev_marked); - if PRINT { println!(""); } + if PRINT { println!(); } // Cycle 8: { ll -> (ll0, ll1), {ll0, ll1} -> ll } let mut ll: LinkedList = LinkedList::new(); @@ -282,7 +282,7 @@ pub fn main() { // break; } - if PRINT { println!(""); } + if PRINT { println!(); } // Cycle 9: { bh -> (bh0, bh1), {bh0, bh1} -> bh } let mut bh: BinaryHeap = BinaryHeap::new(); @@ -302,7 +302,7 @@ pub fn main() { // break; } - if PRINT { println!(""); } + if PRINT { println!(); } // Cycle 10: { btm -> (btk0, btv1), {bt0, bt1} -> btm } let mut btm: BTreeMap = BTreeMap::new(); @@ -323,7 +323,7 @@ pub fn main() { // break; } - if PRINT { println!(""); } + if PRINT { println!(); } // Cycle 10: { bts -> (bts0, bts1), {bts0, bts1} -> btm } let mut bts: BTreeSet = BTreeSet::new(); @@ -343,7 +343,7 @@ pub fn main() { // break; } - if PRINT { println!(""); } + if PRINT { println!(); } // Cycle 11: { rc0 -> (rc1, rc2), rc1 -> (), rc2 -> rc0 } let (rc0, rc1, rc2): (RCRC, RCRC, RCRC); @@ -361,7 +361,7 @@ pub fn main() { rc0.descend_into_self(&mut c); assert!(c.saw_prev_marked); - if PRINT { println!(""); } + if PRINT { println!(); } // We want to take the previous Rc case and generalize it to Arc. // @@ -395,7 +395,7 @@ pub fn main() { arc0.descend_into_self(&mut c); assert!(c.saw_prev_marked); - if PRINT { println!(""); } + if PRINT { println!(); } // Cycle 13: { arc0 -> (arc1, arc2), arc1 -> (), arc2 -> arc0 }, rwlocks let (arc0, arc1, arc2): (ARCRW, ARCRW, ARCRW); @@ -413,7 +413,7 @@ pub fn main() { arc0.descend_into_self(&mut c); assert!(c.saw_prev_marked); - if PRINT { println!(""); } + if PRINT { println!(); } // Cycle 14: { arc0 -> (arc1, arc2), arc1 -> (), arc2 -> arc0 }, mutexs let (arc0, arc1, arc2): (ARCM, ARCM, ARCM); diff --git a/src/test/ui/for/for-c-in-str.rs b/src/test/ui/for/for-c-in-str.rs index 43b1a04c22d04..0fbc796d7c0d3 100644 --- a/src/test/ui/for/for-c-in-str.rs +++ b/src/test/ui/for/for-c-in-str.rs @@ -6,6 +6,6 @@ fn main() { //~| NOTE `&str` is not an iterator //~| HELP the trait `std::iter::Iterator` is not implemented for `&str` //~| NOTE required by `std::iter::IntoIterator::into_iter` - println!(""); + println!(); } } diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 467b7771c152e..7c51de5df2267 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -253,7 +253,7 @@ pub fn parse_config(args: Vec) -> Config { if args.len() == 1 || args[1] == "-h" || args[1] == "--help" { let message = format!("Usage: {} [OPTIONS] [TESTNAME...]", argv0); println!("{}", opts.usage(&message)); - println!(""); + println!(); panic!() } @@ -265,7 +265,7 @@ pub fn parse_config(args: Vec) -> Config { if matches.opt_present("h") || matches.opt_present("help") { let message = format!("Usage: {} [OPTIONS] [TESTNAME...]", argv0); println!("{}", opts.usage(&message)); - println!(""); + println!(); panic!() } diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index aff554678a3f4..baed27dd15152 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -2593,7 +2593,7 @@ impl<'test> TestCx<'test> { " actual: {}", codegen_units_to_str(&actual_item.codegen_units) ); - println!(""); + println!(); } } @@ -3526,7 +3526,7 @@ impl<'test> TestCx<'test> { } } } - println!(""); + println!(); } } } From a678e3191197f145451c97c6cc884e15cae38186 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 14 Sep 2019 16:17:57 -0400 Subject: [PATCH 15/25] Hide diagnostics emitted during --cfg parsing The early error is more than sufficient for fixing the problem. --- src/librustc/session/config.rs | 13 ++++++++++++- .../ui/conditional-compilation/cfg-arg-invalid-6.rs | 3 +++ .../cfg-arg-invalid-6.stderr | 2 ++ 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/conditional-compilation/cfg-arg-invalid-6.rs create mode 100644 src/test/ui/conditional-compilation/cfg-arg-invalid-6.stderr diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index c74b2fee41d6c..723855c7c29cf 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -7,6 +7,7 @@ use crate::session::{early_error, early_warn, Session}; use crate::session::search_paths::SearchPath; use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::sync::Lrc; use rustc_target::spec::{LinkerFlavor, MergeFunctions, PanicStrategy, RelroLevel}; use rustc_target::spec::{Target, TargetTriple}; @@ -19,6 +20,7 @@ use syntax::parse::{ParseSess, new_parser_from_source_str}; use syntax::parse::token; use syntax::symbol::{sym, Symbol}; use syntax::feature_gate::UnstableFeatures; +use syntax::source_map::SourceMap; use errors::emitter::HumanReadableErrorType; use errors::{ColorConfig, FatalError, Handler}; @@ -1850,11 +1852,20 @@ pub fn rustc_optgroups() -> Vec { opts } +struct NullEmitter; + +impl errors::emitter::Emitter for NullEmitter { + fn emit_diagnostic(&mut self, _: &errors::DiagnosticBuilder<'_>) {} +} + // Converts strings provided as `--cfg [cfgspec]` into a `crate_cfg`. pub fn parse_cfgspecs(cfgspecs: Vec) -> FxHashSet<(String, Option)> { syntax::with_default_globals(move || { let cfg = cfgspecs.into_iter().map(|s| { - let sess = ParseSess::new(FilePathMapping::empty()); + + let cm = Lrc::new(SourceMap::new(FilePathMapping::empty())); + let handler = Handler::with_emitter(false, None, Box::new(NullEmitter)); + let sess = ParseSess::with_span_handler(handler, cm); let filename = FileName::cfg_spec_source_code(&s); let mut parser = new_parser_from_source_str(&sess, filename, s.to_string()); diff --git a/src/test/ui/conditional-compilation/cfg-arg-invalid-6.rs b/src/test/ui/conditional-compilation/cfg-arg-invalid-6.rs new file mode 100644 index 0000000000000..9fa726f93e3ea --- /dev/null +++ b/src/test/ui/conditional-compilation/cfg-arg-invalid-6.rs @@ -0,0 +1,3 @@ +// compile-flags: --cfg a{ +// error-pattern: invalid `--cfg` argument: `a{` (expected `key` or `key="value"`) +fn main() {} diff --git a/src/test/ui/conditional-compilation/cfg-arg-invalid-6.stderr b/src/test/ui/conditional-compilation/cfg-arg-invalid-6.stderr new file mode 100644 index 0000000000000..7d2087b4b71f7 --- /dev/null +++ b/src/test/ui/conditional-compilation/cfg-arg-invalid-6.stderr @@ -0,0 +1,2 @@ +error: invalid `--cfg` argument: `a{` (expected `key` or `key="value"`) + From bb229865fa6f1178dcb2c87279336f9bf36d7282 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 16 Sep 2019 13:27:38 +1000 Subject: [PATCH 16/25] Don't print the "total" `-Ztime-passes` output if `--prints=...` is also given. Fixes #64339. --- src/librustc_driver/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index a912ea3c35821..dc2592d164098 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -132,8 +132,11 @@ pub struct TimePassesCallbacks { impl Callbacks for TimePassesCallbacks { fn config(&mut self, config: &mut interface::Config) { + // If a --prints=... option has been given, we don't print the "total" + // time because it will mess up the --prints output. See #64339. self.time_passes = - config.opts.debugging_opts.time_passes || config.opts.debugging_opts.time; + config.opts.prints.is_empty() && + (config.opts.debugging_opts.time_passes || config.opts.debugging_opts.time); } } From 163892cf50a0f5ffaf4367475e7d4a412b008794 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 16 Sep 2019 15:00:28 +1000 Subject: [PATCH 17/25] Use `Symbol` in two more functions. --- src/libsyntax/ext/tt/macro_rules.rs | 44 ++++++++++++++--------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/libsyntax/ext/tt/macro_rules.rs b/src/libsyntax/ext/tt/macro_rules.rs index 46ffa52f7f572..b27e9c543377a 100644 --- a/src/libsyntax/ext/tt/macro_rules.rs +++ b/src/libsyntax/ext/tt/macro_rules.rs @@ -877,9 +877,9 @@ fn check_matcher_core( // Now `last` holds the complete set of NT tokens that could // end the sequence before SUFFIX. Check that every one works with `suffix`. 'each_last: for token in &last.tokens { - if let TokenTree::MetaVarDecl(_, ref name, ref frag_spec) = *token { + if let TokenTree::MetaVarDecl(_, name, frag_spec) = *token { for next_token in &suffix_first.tokens { - match is_in_follow(next_token, &frag_spec.as_str()) { + match is_in_follow(next_token, frag_spec.name) { IsInFollow::Invalid(msg, help) => { sess.span_diagnostic .struct_span_err(next_token.span(), &msg) @@ -948,7 +948,7 @@ fn check_matcher_core( fn token_can_be_followed_by_any(tok: "ed::TokenTree) -> bool { if let quoted::TokenTree::MetaVarDecl(_, _, frag_spec) = *tok { - frag_can_be_followed_by_any(&frag_spec.as_str()) + frag_can_be_followed_by_any(frag_spec.name) } else { // (Non NT's can always be followed by anthing in matchers.) true @@ -963,15 +963,15 @@ fn token_can_be_followed_by_any(tok: "ed::TokenTree) -> bool { /// specifier which consumes at most one token tree can be followed by /// a fragment specifier (indeed, these fragments can be followed by /// ANYTHING without fear of future compatibility hazards). -fn frag_can_be_followed_by_any(frag: &str) -> bool { +fn frag_can_be_followed_by_any(frag: Symbol) -> bool { match frag { - "item" | // always terminated by `}` or `;` - "block" | // exactly one token tree - "ident" | // exactly one token tree - "literal" | // exactly one token tree - "meta" | // exactly one token tree - "lifetime" | // exactly one token tree - "tt" => // exactly one token tree + sym::item | // always terminated by `}` or `;` + sym::block | // exactly one token tree + sym::ident | // exactly one token tree + sym::literal | // exactly one token tree + sym::meta | // exactly one token tree + sym::lifetime | // exactly one token tree + sym::tt => // exactly one token tree true, _ => @@ -993,7 +993,7 @@ enum IsInFollow { /// break macros that were relying on that binary operator as a /// separator. // when changing this do not forget to update doc/book/macros.md! -fn is_in_follow(tok: "ed::TokenTree, frag: &str) -> IsInFollow { +fn is_in_follow(tok: "ed::TokenTree, frag: Symbol) -> IsInFollow { use quoted::TokenTree; if let TokenTree::Token(Token { kind: token::CloseDelim(_), .. }) = *tok { @@ -1002,17 +1002,17 @@ fn is_in_follow(tok: "ed::TokenTree, frag: &str) -> IsInFollow { IsInFollow::Yes } else { match frag { - "item" => { + sym::item => { // since items *must* be followed by either a `;` or a `}`, we can // accept anything after them IsInFollow::Yes } - "block" => { + sym::block => { // anything can follow block, the braces provide an easy boundary to // maintain IsInFollow::Yes } - "stmt" | "expr" => { + sym::stmt | sym::expr => { const TOKENS: &[&str] = &["`=>`", "`,`", "`;`"]; match tok { TokenTree::Token(token) => match token.kind { @@ -1022,7 +1022,7 @@ fn is_in_follow(tok: "ed::TokenTree, frag: &str) -> IsInFollow { _ => IsInFollow::No(TOKENS), } } - "pat" => { + sym::pat => { const TOKENS: &[&str] = &["`=>`", "`,`", "`=`", "`|`", "`if`", "`in`"]; match tok { TokenTree::Token(token) => match token.kind { @@ -1033,7 +1033,7 @@ fn is_in_follow(tok: "ed::TokenTree, frag: &str) -> IsInFollow { _ => IsInFollow::No(TOKENS), } } - "path" | "ty" => { + sym::path | sym::ty => { const TOKENS: &[&str] = &[ "`{`", "`[`", "`=>`", "`,`", "`>`", "`=`", "`:`", "`;`", "`|`", "`as`", "`where`", @@ -1061,20 +1061,20 @@ fn is_in_follow(tok: "ed::TokenTree, frag: &str) -> IsInFollow { _ => IsInFollow::No(TOKENS), } } - "ident" | "lifetime" => { + sym::ident | sym::lifetime => { // being a single token, idents and lifetimes are harmless IsInFollow::Yes } - "literal" => { + sym::literal => { // literals may be of a single token, or two tokens (negative numbers) IsInFollow::Yes } - "meta" | "tt" => { + sym::meta | sym::tt => { // being either a single token or a delimited sequence, tt is // harmless IsInFollow::Yes } - "vis" => { + sym::vis => { // Explicitly disallow `priv`, on the off chance it comes back. const TOKENS: &[&str] = &["`,`", "an ident", "a type"]; match tok { @@ -1099,7 +1099,7 @@ fn is_in_follow(tok: "ed::TokenTree, frag: &str) -> IsInFollow { _ => IsInFollow::No(TOKENS), } } - "" => IsInFollow::Yes, // kw::Invalid + kw::Invalid => IsInFollow::Yes, _ => IsInFollow::Invalid( format!("invalid fragment specifier `{}`", frag), VALID_FRAGMENT_NAMES_MSG, From d3d7b58c3743b6146b5fb818508d9eda958e8cd9 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Tue, 3 Sep 2019 11:33:29 +0200 Subject: [PATCH 18/25] ci: ensure all tool maintainers are assignable on issues GitHub only allows people explicitly listed as collaborators on the repository or who commented on the issue/PR to be assignees, failing to create the issue if non-assignable people are assigned. This adds an extra check on CI to make sure all the people listed as tool maintainers can be assigned to toolstate issues. The check won't be executed on PR builds due to the lack of a valid token. --- src/ci/azure-pipelines/steps/run.yml | 7 ++++ src/tools/publish_toolstate.py | 60 ++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/src/ci/azure-pipelines/steps/run.yml b/src/ci/azure-pipelines/steps/run.yml index ac6b344a45e66..da0a899ac85eb 100644 --- a/src/ci/azure-pipelines/steps/run.yml +++ b/src/ci/azure-pipelines/steps/run.yml @@ -147,8 +147,15 @@ steps: git clone --depth=1 https://github.com/rust-lang-nursery/rust-toolstate.git cd rust-toolstate python2.7 "$BUILD_SOURCESDIRECTORY/src/tools/publish_toolstate.py" "$(git rev-parse HEAD)" "$(git log --format=%s -n1 HEAD)" "" "" + # Only check maintainers if this build is supposed to publish toolstate. + # Builds that are not supposed to publish don't have the access token. + if [ -n "${TOOLSTATE_PUBLISH+is_set}" ]; then + TOOLSTATE_VALIDATE_MAINTAINERS_REPO=rust-lang/rust python2.7 "${BUILD_SOURCESDIRECTORY}/src/tools/publish_toolstate.py" + fi cd .. rm -rf rust-toolstate + env: + TOOLSTATE_REPO_ACCESS_TOKEN: $(TOOLSTATE_REPO_ACCESS_TOKEN) condition: and(succeeded(), not(variables.SKIP_JOB), eq(variables['IMAGE'], 'mingw-check')) displayName: Verify the publish_toolstate script works diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 2e2505b7f0246..bc00fbc90bf96 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -7,6 +7,8 @@ ## It is set as callback for `src/ci/docker/x86_64-gnu-tools/repo.sh` by the CI scripts ## when a new commit lands on `master` (i.e., after it passed all checks on `auto`). +from __future__ import print_function + import sys import re import os @@ -20,6 +22,8 @@ import urllib.request as urllib2 # List of people to ping when the status of a tool or a book changed. +# These should be collaborators of the rust-lang/rust repository (with at least +# read privileges on it). CI will fail otherwise. MAINTAINERS = { 'miri': '@oli-obk @RalfJung @eddyb', 'clippy-driver': '@Manishearth @llogiq @mcarton @oli-obk @phansch @flip1995 @yaahc', @@ -52,6 +56,53 @@ } +def validate_maintainers(repo, github_token): + '''Ensure all maintainers are assignable on a GitHub repo''' + next_link_re = re.compile(r'<([^>]+)>; rel="next"') + + # Load the list of assignable people in the GitHub repo + assignable = [] + url = 'https://github.com/gitapi/repos/%s/collaborators?per_page=100' % repo + while url is not None: + response = urllib2.urlopen(urllib2.Request(url, headers={ + 'Authorization': 'token ' + github_token, + # Properly load nested teams. + 'Accept': 'application/vnd.github.hellcat-preview+json', + })) + for user in json.loads(response.read()): + assignable.append(user['login']) + # Load the next page if available + if 'Link' in response.headers: + matches = next_link_re.match(response.headers['Link']) + if matches is not None: + url = matches.group(1) + else: + url = None + + errors = False + for tool, maintainers in MAINTAINERS.items(): + for maintainer in maintainers.split(' '): + if maintainer.startswith('@'): + maintainer = maintainer[1:] + if maintainer not in assignable: + errors = True + print( + "error: %s maintainer @%s is not assignable in the %s repo" + % (tool, maintainer, repo), + ) + + if errors: + print() + print(" To be assignable, a person needs to be explicitly listed as a") + print(" collaborator in the repository settings. The simple way to") + print(" fix this is to ask someone with 'admin' privileges on the repo") + print(" to add the person or whole team as a collaborator with 'read'") + print(" privileges. Those privileges don't grant any extra permissions") + print(" so it's safe to apply them.") + print() + print("The build will fail due to this.") + exit(1) + def read_current_status(current_commit, path): '''Reads build status of `current_commit` from content of `history/*.tsv` ''' @@ -200,6 +251,15 @@ def update_latest( if __name__ == '__main__': + if 'TOOLSTATE_VALIDATE_MAINTAINERS_REPO' in os.environ: + repo = os.environ['TOOLSTATE_VALIDATE_MAINTAINERS_REPO'] + if 'TOOLSTATE_REPO_ACCESS_TOKEN' in os.environ: + github_token = os.environ['TOOLSTATE_REPO_ACCESS_TOKEN'] + validate_maintainers(repo, github_token) + else: + print('skipping toolstate maintainers validation since no GitHub token is present') + exit(0) + cur_commit = sys.argv[1] cur_datetime = datetime.datetime.utcnow().strftime('%Y-%m-%dT%H:%M:%SZ') cur_commit_msg = sys.argv[2] From eb97b1bfdede51981dbed39bb3fe483311f8f65d Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Tue, 3 Sep 2019 12:04:22 +0200 Subject: [PATCH 19/25] ci: rename Gankro to Gankra in toolstate --- src/tools/publish_toolstate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index bc00fbc90bf96..cd7a182da2706 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -30,7 +30,7 @@ 'rls': '@Xanewok', 'rustfmt': '@topecongiro', 'book': '@carols10cents @steveklabnik', - 'nomicon': '@frewsxcv @Gankro', + 'nomicon': '@frewsxcv @Gankra', 'reference': '@steveklabnik @Havvy @matthewjasper @ehuss', 'rust-by-example': '@steveklabnik @marioidival @projektir', 'embedded-book': ( From f968c1a4f51b1e57f1f613055332b3bf63f5864e Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Wed, 4 Sep 2019 10:08:54 +0200 Subject: [PATCH 20/25] ci: address publish_toolstate review comments --- src/tools/publish_toolstate.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index cd7a182da2706..5211d1141c7e0 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -69,15 +69,14 @@ def validate_maintainers(repo, github_token): # Properly load nested teams. 'Accept': 'application/vnd.github.hellcat-preview+json', })) - for user in json.loads(response.read()): - assignable.append(user['login']) + assignable.extend(user['login'] for user in json.load(response)) # Load the next page if available - if 'Link' in response.headers: - matches = next_link_re.match(response.headers['Link']) + url = None + link_header = response.headers.get('Link') + if link_header: + matches = next_link_re.match(link_header) if matches is not None: url = matches.group(1) - else: - url = None errors = False for tool, maintainers in MAINTAINERS.items(): @@ -251,13 +250,14 @@ def update_latest( if __name__ == '__main__': - if 'TOOLSTATE_VALIDATE_MAINTAINERS_REPO' in os.environ: - repo = os.environ['TOOLSTATE_VALIDATE_MAINTAINERS_REPO'] - if 'TOOLSTATE_REPO_ACCESS_TOKEN' in os.environ: - github_token = os.environ['TOOLSTATE_REPO_ACCESS_TOKEN'] + repo = os.environ.get('TOOLSTATE_VALIDATE_MAINTAINERS_REPO') + if repo: + github_token = os.environ.get('TOOLSTATE_REPO_ACCESS_TOKEN') + if github_token: validate_maintainers(repo, github_token) else: print('skipping toolstate maintainers validation since no GitHub token is present') + # When validating maintainers don't run the full script. exit(0) cur_commit = sys.argv[1] From f2576c821d2e15b23dcea8af9b3ed08cf4a5795f Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Wed, 4 Sep 2019 10:21:23 +0200 Subject: [PATCH 21/25] ci: convert maintainer list in publish_toolstate to a set --- src/tools/publish_toolstate.py | 45 +++++++++++++++++----------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 5211d1141c7e0..217f8a7f6f4c7 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -25,20 +25,23 @@ # These should be collaborators of the rust-lang/rust repository (with at least # read privileges on it). CI will fail otherwise. MAINTAINERS = { - 'miri': '@oli-obk @RalfJung @eddyb', - 'clippy-driver': '@Manishearth @llogiq @mcarton @oli-obk @phansch @flip1995 @yaahc', - 'rls': '@Xanewok', - 'rustfmt': '@topecongiro', - 'book': '@carols10cents @steveklabnik', - 'nomicon': '@frewsxcv @Gankra', - 'reference': '@steveklabnik @Havvy @matthewjasper @ehuss', - 'rust-by-example': '@steveklabnik @marioidival @projektir', - 'embedded-book': ( - '@adamgreig @andre-richter @jamesmunns @korken89 ' - '@ryankurte @thejpster @therealprof' - ), - 'edition-guide': '@ehuss @Centril @steveklabnik', - 'rustc-guide': '@mark-i-m @spastorino @amanjeev' + 'miri': {'oli-obk', 'RalfJung', 'eddyb'}, + 'clippy-driver': { + 'Manishearth', 'llogiq', 'mcarton', 'oli-obk', 'phansch', 'flip1995', + 'yaahc', + }, + 'rls': {'Xanewok'}, + 'rustfmt': {'topecongiro'}, + 'book': {'carols10cents', 'steveklabnik'}, + 'nomicon': {'frewsxcv', 'Gankra'}, + 'reference': {'steveklabnik', 'Havvy', 'matthewjasper', 'ehuss'}, + 'rust-by-example': {'steveklabnik', 'marioidival', 'projektir'}, + 'embedded-book': { + 'adamgreig', 'andre-richter', 'jamesmunns', 'korken89', + 'ryankurte', 'thejpster', 'therealprof', + }, + 'edition-guide': {'ehuss', 'Centril', 'steveklabnik'}, + 'rustc-guide': {'mark-i-m', 'spastorino', 'amanjeev'}, } REPOS = { @@ -80,9 +83,7 @@ def validate_maintainers(repo, github_token): errors = False for tool, maintainers in MAINTAINERS.items(): - for maintainer in maintainers.split(' '): - if maintainer.startswith('@'): - maintainer = maintainer[1:] + for maintainer in maintainers: if maintainer not in assignable: errors = True print( @@ -123,13 +124,12 @@ def maybe_delink(message): def issue( tool, status, - maintainers, + assignees, relevant_pr_number, relevant_pr_user, pr_reviewer, ): # Open an issue about the toolstate failure. - assignees = [x.strip() for x in maintainers.split('@') if x != ''] if status == 'test-fail': status_description = 'has failing tests' else: @@ -150,7 +150,7 @@ def issue( REPOS.get(tool), relevant_pr_user, pr_reviewer )), 'title': '`{}` no longer builds after {}'.format(tool, relevant_pr_number), - 'assignees': assignees, + 'assignees': list(assignees), 'labels': ['T-compiler', 'I-nominated'], }) print("Creating issue:\n{}".format(request)) @@ -200,18 +200,19 @@ def update_latest( old = status[os] new = s.get(tool, old) status[os] = new + maintainers = ' '.join('@'+name for name in MAINTAINERS[tool]) if new > old: # comparing the strings, but they are ordered appropriately! # things got fixed or at least the status quo improved changed = True message += '🎉 {} on {}: {} → {} (cc {}, @rust-lang/infra).\n' \ - .format(tool, os, old, new, MAINTAINERS.get(tool)) + .format(tool, os, old, new, maintainers) elif new < old: # tests or builds are failing and were not failing before changed = True title = '💔 {} on {}: {} → {}' \ .format(tool, os, old, new) message += '{} (cc {}, @rust-lang/infra).\n' \ - .format(title, MAINTAINERS.get(tool)) + .format(title, maintainers) # Most tools only create issues for build failures. # Other failures can be spurious. if new == 'build-fail' or (tool == 'miri' and new == 'test-fail'): From ce451b9b269207feb565f566cdbd34d7ec369b4b Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Mon, 9 Sep 2019 16:03:57 +0200 Subject: [PATCH 22/25] ci: remove projektir from toolstate notifications They don't contribute to rust-by-example anymore. --- src/tools/publish_toolstate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 217f8a7f6f4c7..4060b90d952bd 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -35,7 +35,7 @@ 'book': {'carols10cents', 'steveklabnik'}, 'nomicon': {'frewsxcv', 'Gankra'}, 'reference': {'steveklabnik', 'Havvy', 'matthewjasper', 'ehuss'}, - 'rust-by-example': {'steveklabnik', 'marioidival', 'projektir'}, + 'rust-by-example': {'steveklabnik', 'marioidival'}, 'embedded-book': { 'adamgreig', 'andre-richter', 'jamesmunns', 'korken89', 'ryankurte', 'thejpster', 'therealprof', From 49854c4f71eb8470c2a4483cbad3f03eb99e67cb Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 16 Sep 2019 16:37:44 +0200 Subject: [PATCH 23/25] avoid #[cfg] in favor of cfg! --- src/libstd/panicking.rs | 18 +++++++----------- src/libstd/sys_common/backtrace.rs | 2 -- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index e7755af7f07da..28fb40244043e 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -17,8 +17,7 @@ use crate::ptr; use crate::raw; use crate::sys::stdio::panic_output; use crate::sys_common::rwlock::RWLock; -use crate::sys_common::thread_info; -use crate::sys_common::util; +use crate::sys_common::{thread_info, util, backtrace}; use crate::thread; #[cfg(not(test))] @@ -157,20 +156,18 @@ pub fn take_hook() -> Box) + 'static + Sync + Send> { } fn default_hook(info: &PanicInfo<'_>) { - #[cfg(feature = "backtrace")] - use crate::sys_common::{backtrace as backtrace_mod}; - // If this is a double panic, make sure that we print a backtrace // for this panic. Otherwise only print it if logging is enabled. - #[cfg(feature = "backtrace")] - let log_backtrace = { + let log_backtrace = if cfg!(feature = "backtrace") { let panics = update_panic_count(0); if panics >= 2 { Some(backtrace_rs::PrintFmt::Full) } else { - backtrace_mod::log_enabled() + backtrace::log_enabled() } + } else { + None }; // The current implementation always returns `Some`. @@ -190,14 +187,13 @@ fn default_hook(info: &PanicInfo<'_>) { let _ = writeln!(err, "thread '{}' panicked at '{}', {}", name, msg, location); - #[cfg(feature = "backtrace")] - { + if cfg!(feature = "backtrace") { use crate::sync::atomic::{AtomicBool, Ordering}; static FIRST_PANIC: AtomicBool = AtomicBool::new(true); if let Some(format) = log_backtrace { - let _ = backtrace_mod::print(err, format); + let _ = backtrace::print(err, format); } else if FIRST_PANIC.compare_and_swap(true, false, Ordering::SeqCst) { let _ = writeln!(err, "note: run with `RUST_BACKTRACE=1` \ environment variable to display a backtrace."); diff --git a/src/libstd/sys_common/backtrace.rs b/src/libstd/sys_common/backtrace.rs index f49adc01659ff..01711d415d86c 100644 --- a/src/libstd/sys_common/backtrace.rs +++ b/src/libstd/sys_common/backtrace.rs @@ -33,7 +33,6 @@ pub fn lock() -> impl Drop { } /// Prints the current backtrace. -#[cfg(feature = "backtrace")] pub fn print(w: &mut dyn Write, format: PrintFmt) -> io::Result<()> { // There are issues currently linking libbacktrace into tests, and in // general during libstd's own unit tests we're not testing this path. In @@ -129,7 +128,6 @@ where // For now logging is turned off by default, and this function checks to see // whether the magical environment variable is present to see if it's turned on. -#[cfg(feature = "backtrace")] pub fn log_enabled() -> Option { use crate::sync::atomic::{self, Ordering}; From d5fe5831ecc82d2705110810261b79ae4040c403 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 29 Aug 2019 11:32:38 +0200 Subject: [PATCH 24/25] Const-stabilize `Vec::new`. --- src/liballoc/lib.rs | 2 +- src/liballoc/raw_vec.rs | 27 ++++++++++++++++++- src/liballoc/vec.rs | 4 +-- src/test/ui/collections-const-new.rs | 4 +-- ...ure-gate-unleash_the_miri_inside_of_you.rs | 3 ++- ...gate-unleash_the_miri_inside_of_you.stderr | 10 +------ 6 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 370e5cf4b303f..9e6ed92ffb567 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -117,7 +117,7 @@ #![feature(allocator_internals)] #![feature(on_unimplemented)] #![feature(rustc_const_unstable)] -#![feature(const_vec_new)] +#![cfg_attr(bootstrap, feature(const_vec_new))] #![feature(slice_partition_dedup)] #![feature(maybe_uninit_extra, maybe_uninit_slice)] #![feature(alloc_layout_extra)] diff --git a/src/liballoc/raw_vec.rs b/src/liballoc/raw_vec.rs index cf025eee4358b..ee75fc288fee5 100644 --- a/src/liballoc/raw_vec.rs +++ b/src/liballoc/raw_vec.rs @@ -113,13 +113,38 @@ impl RawVec { } impl RawVec { + /// HACK(Centril): This exists because `#[unstable]` `const fn`s needn't conform + /// to `min_const_fn` and so they cannot be called in `min_const_fn`s either. + /// + /// If you change `RawVec::new` or dependencies, please take care to not + /// introduce anything that would truly violate `min_const_fn`. + /// + /// NOTE: We could avoid this hack and check conformance with some + /// `#[rustc_force_min_const_fn]` attribute which requires conformance + /// with `min_const_fn` but does not necessarily allow calling it in + /// `stable(...) const fn` / user code not enabling `foo` when + /// `#[rustc_const_unstable(feature = "foo", ..)]` is present. + pub const NEW: Self = Self::new(); + /// Creates the biggest possible `RawVec` (on the system heap) /// without allocating. If `T` has positive size, then this makes a /// `RawVec` with capacity `0`. If `T` is zero-sized, then it makes a /// `RawVec` with capacity `usize::MAX`. Useful for implementing /// delayed allocation. pub const fn new() -> Self { - Self::new_in(Global) + // FIXME(Centril): Reintegrate this with `fn new_in` when we can. + + // `!0` is `usize::MAX`. This branch should be stripped at compile time. + // FIXME(mark-i-m): use this line when `if`s are allowed in `const`: + //let cap = if mem::size_of::() == 0 { !0 } else { 0 }; + + // `Unique::empty()` doubles as "unallocated" and "zero-sized allocation". + RawVec { + ptr: Unique::empty(), + // FIXME(mark-i-m): use `cap` when ifs are allowed in const + cap: [0, !0][(mem::size_of::() == 0) as usize], + a: Global, + } } /// Creates a `RawVec` (on the system heap) with exactly the diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index c513658c842e5..405969a550b88 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -314,10 +314,10 @@ impl Vec { /// ``` #[inline] #[stable(feature = "rust1", since = "1.0.0")] - #[rustc_const_unstable(feature = "const_vec_new")] + #[cfg_attr(bootstrap, rustc_const_unstable(feature = "const_vec_new"))] pub const fn new() -> Vec { Vec { - buf: RawVec::new(), + buf: RawVec::NEW, len: 0, } } diff --git a/src/test/ui/collections-const-new.rs b/src/test/ui/collections-const-new.rs index e01b0dfa14d6e..15c5c65ff81f3 100644 --- a/src/test/ui/collections-const-new.rs +++ b/src/test/ui/collections-const-new.rs @@ -1,15 +1,13 @@ // run-pass -#![allow(dead_code)] // Test several functions can be used for constants // 1. Vec::new() // 2. String::new() -#![feature(const_vec_new)] #![feature(const_string_new)] const MY_VEC: Vec = Vec::new(); const MY_STRING: String = String::new(); -pub fn main() {} +fn main() {} diff --git a/src/test/ui/consts/miri_unleashed/feature-gate-unleash_the_miri_inside_of_you.rs b/src/test/ui/consts/miri_unleashed/feature-gate-unleash_the_miri_inside_of_you.rs index 5fb92535502a5..8b17f6885ad3e 100644 --- a/src/test/ui/consts/miri_unleashed/feature-gate-unleash_the_miri_inside_of_you.rs +++ b/src/test/ui/consts/miri_unleashed/feature-gate-unleash_the_miri_inside_of_you.rs @@ -14,8 +14,9 @@ trait Bar> { impl Foo for () { const X: u32 = 42; } + impl Foo> for String { - const X: Vec = Vec::new(); //~ ERROR not yet stable as a const fn + const X: Vec = Vec::new(); } impl Bar for () {} diff --git a/src/test/ui/consts/miri_unleashed/feature-gate-unleash_the_miri_inside_of_you.stderr b/src/test/ui/consts/miri_unleashed/feature-gate-unleash_the_miri_inside_of_you.stderr index c56ebf60df481..5bc7b70638c80 100644 --- a/src/test/ui/consts/miri_unleashed/feature-gate-unleash_the_miri_inside_of_you.stderr +++ b/src/test/ui/consts/miri_unleashed/feature-gate-unleash_the_miri_inside_of_you.stderr @@ -4,13 +4,5 @@ error[E0493]: destructors cannot be evaluated at compile-time LL | const F: u32 = (U::X, 42).1; | ^^^^^^^^^^ constants cannot evaluate destructors -error: `std::vec::Vec::::new` is not yet stable as a const fn - --> $DIR/feature-gate-unleash_the_miri_inside_of_you.rs:18:25 - | -LL | const X: Vec = Vec::new(); - | ^^^^^^^^^^ - | - = help: add `#![feature(const_vec_new)]` to the crate attributes to enable - -error: aborting due to 2 previous errors +error: aborting due to previous error From 9b3e11f635c0354040ce515ac0ed4fade6fe928f Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 29 Aug 2019 12:36:07 +0200 Subject: [PATCH 25/25] Const-stabilize `String::new`. --- src/liballoc/string.rs | 2 +- src/test/ui/collections-const-new.rs | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/liballoc/string.rs b/src/liballoc/string.rs index b65f191836e9d..1166e7b5df295 100644 --- a/src/liballoc/string.rs +++ b/src/liballoc/string.rs @@ -369,7 +369,7 @@ impl String { /// ``` #[inline] #[stable(feature = "rust1", since = "1.0.0")] - #[rustc_const_unstable(feature = "const_string_new")] + #[cfg_attr(bootstrap, rustc_const_unstable(feature = "const_string_new"))] pub const fn new() -> String { String { vec: Vec::new() } } diff --git a/src/test/ui/collections-const-new.rs b/src/test/ui/collections-const-new.rs index 15c5c65ff81f3..a93f9a136db23 100644 --- a/src/test/ui/collections-const-new.rs +++ b/src/test/ui/collections-const-new.rs @@ -1,11 +1,9 @@ -// run-pass +// check-pass // Test several functions can be used for constants // 1. Vec::new() // 2. String::new() -#![feature(const_string_new)] - const MY_VEC: Vec = Vec::new(); const MY_STRING: String = String::new();