From 5d9eeff062053f87ab900fc7ebde6eb13a2a1645 Mon Sep 17 00:00:00 2001 From: Mohsen Zohrevandi Date: Wed, 21 Apr 2021 13:45:57 -0700 Subject: [PATCH 01/26] Ensure TLS destructors run before thread joins in SGX --- library/std/src/sys/sgx/abi/mod.rs | 6 ++- library/std/src/sys/sgx/thread.rs | 69 +++++++++++++++++++++++---- library/std/src/thread/local/tests.rs | 55 ++++++++++++++++++++- 3 files changed, 119 insertions(+), 11 deletions(-) diff --git a/library/std/src/sys/sgx/abi/mod.rs b/library/std/src/sys/sgx/abi/mod.rs index a5e453034762c..f9536c4203df2 100644 --- a/library/std/src/sys/sgx/abi/mod.rs +++ b/library/std/src/sys/sgx/abi/mod.rs @@ -62,10 +62,12 @@ unsafe extern "C" fn tcs_init(secondary: bool) { extern "C" fn entry(p1: u64, p2: u64, p3: u64, secondary: bool, p4: u64, p5: u64) -> EntryReturn { // FIXME: how to support TLS in library mode? let tls = Box::new(tls::Tls::new()); - let _tls_guard = unsafe { tls.activate() }; + let tls_guard = unsafe { tls.activate() }; if secondary { - super::thread::Thread::entry(); + let join_notifier = super::thread::Thread::entry(); + drop(tls_guard); + drop(join_notifier); EntryReturn(0, 0) } else { diff --git a/library/std/src/sys/sgx/thread.rs b/library/std/src/sys/sgx/thread.rs index 55ef460cc90c5..67e2e8b59d397 100644 --- a/library/std/src/sys/sgx/thread.rs +++ b/library/std/src/sys/sgx/thread.rs @@ -9,26 +9,37 @@ pub struct Thread(task_queue::JoinHandle); pub const DEFAULT_MIN_STACK_SIZE: usize = 4096; +pub use self::task_queue::JoinNotifier; + mod task_queue { - use crate::sync::mpsc; + use super::wait_notify; use crate::sync::{Mutex, MutexGuard, Once}; - pub type JoinHandle = mpsc::Receiver<()>; + pub type JoinHandle = wait_notify::Waiter; + + pub struct JoinNotifier(Option); + + impl Drop for JoinNotifier { + fn drop(&mut self) { + self.0.take().unwrap().notify(); + } + } pub(super) struct Task { p: Box, - done: mpsc::Sender<()>, + done: JoinNotifier, } impl Task { pub(super) fn new(p: Box) -> (Task, JoinHandle) { - let (done, recv) = mpsc::channel(); + let (done, recv) = wait_notify::new(); + let done = JoinNotifier(Some(done)); (Task { p, done }, recv) } - pub(super) fn run(self) { + pub(super) fn run(self) -> JoinNotifier { (self.p)(); - let _ = self.done.send(()); + self.done } } @@ -47,6 +58,48 @@ mod task_queue { } } +/// This module provides a synchronization primitive that does not use thread +/// local variables. This is needed for signaling that a thread has finished +/// execution. The signal is sent once all TLS destructors have finished at +/// which point no new thread locals should be created. +pub mod wait_notify { + use super::super::waitqueue::{SpinMutex, WaitQueue, WaitVariable}; + use crate::sync::Arc; + + pub struct Notifier(Arc>>); + + impl Notifier { + /// Notify the waiter. The waiter is either notified right away (if + /// currently blocked in `Waiter::wait()`) or later when it calls the + /// `Waiter::wait()` method. + pub fn notify(self) { + let mut guard = self.0.lock(); + *guard.lock_var_mut() = true; + let _ = WaitQueue::notify_one(guard); + } + } + + pub struct Waiter(Arc>>); + + impl Waiter { + /// Wait for a notification. If `Notifier::notify()` has already been + /// called, this will return immediately, otherwise the current thread + /// is blocked until notified. + pub fn wait(self) { + let guard = self.0.lock(); + if *guard.lock_var() { + return; + } + WaitQueue::wait(guard, || {}); + } + } + + pub fn new() -> (Notifier, Waiter) { + let inner = Arc::new(SpinMutex::new(WaitVariable::new(false))); + (Notifier(inner.clone()), Waiter(inner)) + } +} + impl Thread { // unsafe: see thread::Builder::spawn_unchecked for safety requirements pub unsafe fn new(_stack: usize, p: Box) -> io::Result { @@ -57,7 +110,7 @@ impl Thread { Ok(Thread(handle)) } - pub(super) fn entry() { + pub(super) fn entry() -> JoinNotifier { let mut pending_tasks = task_queue::lock(); let task = rtunwrap!(Some, pending_tasks.pop()); drop(pending_tasks); // make sure to not hold the task queue lock longer than necessary @@ -78,7 +131,7 @@ impl Thread { } pub fn join(self) { - let _ = self.0.recv(); + self.0.wait(); } } diff --git a/library/std/src/thread/local/tests.rs b/library/std/src/thread/local/tests.rs index 80e6798d847b1..98f525eafb0f6 100644 --- a/library/std/src/thread/local/tests.rs +++ b/library/std/src/thread/local/tests.rs @@ -1,5 +1,6 @@ use crate::cell::{Cell, UnsafeCell}; -use crate::sync::mpsc::{channel, Sender}; +use crate::sync::atomic::{AtomicBool, Ordering}; +use crate::sync::mpsc::{self, channel, Sender}; use crate::thread::{self, LocalKey}; use crate::thread_local; @@ -207,3 +208,55 @@ fn dtors_in_dtors_in_dtors_const_init() { }); rx.recv().unwrap(); } + +// This test tests that TLS destructors have run before the thread joins. The +// test has no false positives (meaning: if the test fails, there's actually +// an ordering problem). It may have false negatives, where the test passes but +// join is not guaranteed to be after the TLS destructors. However, false +// negatives should be exceedingly rare due to judicious use of +// thread::yield_now and running the test several times. +#[test] +fn join_orders_after_tls_destructors() { + static THREAD2_LAUNCHED: AtomicBool = AtomicBool::new(false); + + for _ in 0..10 { + let (tx, rx) = mpsc::sync_channel(0); + THREAD2_LAUNCHED.store(false, Ordering::SeqCst); + + let jh = thread::spawn(move || { + struct RecvOnDrop(Cell>>); + + impl Drop for RecvOnDrop { + fn drop(&mut self) { + let rx = self.0.take().unwrap(); + while !THREAD2_LAUNCHED.load(Ordering::SeqCst) { + thread::yield_now(); + } + rx.recv().unwrap(); + } + } + + thread_local! { + static TL_RX: RecvOnDrop = RecvOnDrop(Cell::new(None)); + } + + TL_RX.with(|v| v.0.set(Some(rx))) + }); + + let tx_clone = tx.clone(); + let jh2 = thread::spawn(move || { + THREAD2_LAUNCHED.store(true, Ordering::SeqCst); + jh.join().unwrap(); + tx_clone.send(()).expect_err( + "Expecting channel to be closed because thread 1 TLS destructors must've run", + ); + }); + + while !THREAD2_LAUNCHED.load(Ordering::SeqCst) { + thread::yield_now(); + } + thread::yield_now(); + tx.send(()).expect("Expecting channel to be live because thread 2 must block on join"); + jh2.join().unwrap(); + } +} From 8a0a4b1493264901e5e41c4285fc3cc9f8419c28 Mon Sep 17 00:00:00 2001 From: Mohsen Zohrevandi Date: Thu, 29 Apr 2021 08:44:45 -0700 Subject: [PATCH 02/26] Use atomics in join_orders_after_tls_destructors test std::sync::mpsc uses thread locals and depending on the order TLS dtors are run `rx.recv()` can panic when used in a TLS dtor. --- library/std/src/thread/local/tests.rs | 122 +++++++++++++++++++------- 1 file changed, 88 insertions(+), 34 deletions(-) diff --git a/library/std/src/thread/local/tests.rs b/library/std/src/thread/local/tests.rs index 98f525eafb0f6..494ad4e5fda9a 100644 --- a/library/std/src/thread/local/tests.rs +++ b/library/std/src/thread/local/tests.rs @@ -1,6 +1,6 @@ use crate::cell::{Cell, UnsafeCell}; -use crate::sync::atomic::{AtomicBool, Ordering}; -use crate::sync::mpsc::{self, channel, Sender}; +use crate::sync::atomic::{AtomicU8, Ordering}; +use crate::sync::mpsc::{channel, Sender}; use crate::thread::{self, LocalKey}; use crate::thread_local; @@ -217,46 +217,100 @@ fn dtors_in_dtors_in_dtors_const_init() { // thread::yield_now and running the test several times. #[test] fn join_orders_after_tls_destructors() { - static THREAD2_LAUNCHED: AtomicBool = AtomicBool::new(false); + // We emulate a synchronous MPSC rendezvous channel using only atomics and + // thread::yield_now. We can't use std::mpsc as the implementation itself + // may rely on thread locals. + // + // The basic state machine for an SPSC rendezvous channel is: + // FRESH -> THREAD1_WAITING -> MAIN_THREAD_RENDEZVOUS + // where the first transition is done by the “receiving” thread and the 2nd + // transition is done by the “sending” thread. + // + // We add an additional state `THREAD2_LAUNCHED` between `FRESH` and + // `THREAD1_WAITING` to block until all threads are actually running. + // + // A thread that joins on the “receiving” thread completion should never + // observe the channel in the `THREAD1_WAITING` state. If this does occur, + // we switch to the “poison” state `THREAD2_JOINED` and panic all around. + // (This is equivalent to “sending” from an alternate producer thread.) + const FRESH: u8 = 0; + const THREAD2_LAUNCHED: u8 = 1; + const THREAD1_WAITING: u8 = 2; + const MAIN_THREAD_RENDEZVOUS: u8 = 3; + const THREAD2_JOINED: u8 = 4; + static SYNC_STATE: AtomicU8 = AtomicU8::new(FRESH); for _ in 0..10 { - let (tx, rx) = mpsc::sync_channel(0); - THREAD2_LAUNCHED.store(false, Ordering::SeqCst); + SYNC_STATE.store(FRESH, Ordering::SeqCst); + + let jh = thread::Builder::new() + .name("thread1".into()) + .spawn(move || { + struct TlDrop; + + impl Drop for TlDrop { + fn drop(&mut self) { + loop { + match SYNC_STATE.load(Ordering::SeqCst) { + FRESH => thread::yield_now(), + THREAD2_LAUNCHED => break, + v => unreachable!("sync state: {}", v), + } + } + let mut sync_state = SYNC_STATE.swap(THREAD1_WAITING, Ordering::SeqCst); + loop { + match sync_state { + THREAD2_LAUNCHED | THREAD1_WAITING => thread::yield_now(), + MAIN_THREAD_RENDEZVOUS => break, + THREAD2_JOINED => panic!( + "Thread 1 still running after thread 2 joined on thread 1" + ), + v => unreachable!("sync state: {}", v), + } + sync_state = SYNC_STATE.load(Ordering::SeqCst); + } + } + } - let jh = thread::spawn(move || { - struct RecvOnDrop(Cell>>); + thread_local! { + static TL_DROP: TlDrop = TlDrop; + } - impl Drop for RecvOnDrop { - fn drop(&mut self) { - let rx = self.0.take().unwrap(); - while !THREAD2_LAUNCHED.load(Ordering::SeqCst) { - thread::yield_now(); + TL_DROP.with(|_| {}) + }) + .unwrap(); + + let jh2 = thread::Builder::new() + .name("thread2".into()) + .spawn(move || { + assert_eq!(SYNC_STATE.swap(THREAD2_LAUNCHED, Ordering::SeqCst), FRESH); + jh.join().unwrap(); + match SYNC_STATE.swap(THREAD2_JOINED, Ordering::SeqCst) { + MAIN_THREAD_RENDEZVOUS => return, + THREAD2_LAUNCHED | THREAD1_WAITING => { + panic!("Thread 2 running after thread 1 join before main thread rendezvous") } - rx.recv().unwrap(); + v => unreachable!("sync state: {:?}", v), } + }) + .unwrap(); + + loop { + match SYNC_STATE.compare_exchange_weak( + THREAD1_WAITING, + MAIN_THREAD_RENDEZVOUS, + Ordering::SeqCst, + Ordering::SeqCst, + ) { + Ok(_) => break, + Err(FRESH) => thread::yield_now(), + Err(THREAD2_LAUNCHED) => thread::yield_now(), + Err(THREAD2_JOINED) => { + panic!("Main thread rendezvous after thread 2 joined thread 1") + } + v => unreachable!("sync state: {:?}", v), } - - thread_local! { - static TL_RX: RecvOnDrop = RecvOnDrop(Cell::new(None)); - } - - TL_RX.with(|v| v.0.set(Some(rx))) - }); - - let tx_clone = tx.clone(); - let jh2 = thread::spawn(move || { - THREAD2_LAUNCHED.store(true, Ordering::SeqCst); - jh.join().unwrap(); - tx_clone.send(()).expect_err( - "Expecting channel to be closed because thread 1 TLS destructors must've run", - ); - }); - - while !THREAD2_LAUNCHED.load(Ordering::SeqCst) { - thread::yield_now(); } - thread::yield_now(); - tx.send(()).expect("Expecting channel to be live because thread 2 must block on join"); jh2.join().unwrap(); } } From 051f9ec694ec8c173b551d674cd17b8b989a9ee8 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Sat, 24 Apr 2021 01:20:48 +0000 Subject: [PATCH 03/26] Add --run flag to compiletest This controls whether run-* tests actually get run. --- src/tools/compiletest/src/common.rs | 3 +++ src/tools/compiletest/src/main.rs | 7 +++++++ src/tools/compiletest/src/runtest.rs | 21 ++++++++++++++++----- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index b7693a3cb1431..9a2166675d8ab 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -249,6 +249,9 @@ pub struct Config { /// Force the pass mode of a check/build/run-pass test to this mode. pub force_pass_mode: Option, + /// Explicitly enable or disable running. + pub run: Option, + /// Write out a parseable log of tests that were run pub logfile: Option, diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 480916018619d..5bf1b55e45b4a 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -87,6 +87,7 @@ pub fn parse_config(args: Vec) -> Config { "force {check,build,run}-pass tests to this mode.", "check | build | run", ) + .optopt("", "run", "whether to execute run-* tests", "auto | always | never") .optflag("", "ignored", "run tests marked as ignored") .optflag("", "exact", "filters match exactly") .optopt( @@ -234,6 +235,12 @@ pub fn parse_config(args: Vec) -> Config { mode.parse::() .unwrap_or_else(|_| panic!("unknown `--pass` option `{}` given", mode)) }), + run: matches.opt_str("run").and_then(|mode| match mode.as_str() { + "auto" => None, + "always" => Some(true), + "never" => Some(false), + _ => panic!("unknown `--run` option `{}` given", mode), + }), logfile: matches.opt_str("logfile").map(|s| PathBuf::from(&s)), runtool: matches.opt_str("runtool"), host_rustcflags: matches.opt_str("host-rustcflags"), diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index ecbaccf744dcd..c758b977573ae 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -317,6 +317,7 @@ enum TestOutput { enum WillExecute { Yes, No, + Disabled, } /// Should `--emit metadata` be used? @@ -357,13 +358,22 @@ impl<'test> TestCx<'test> { } fn should_run(&self, pm: Option) -> WillExecute { - match self.config.mode { + let test_should_run = match self.config.mode { Ui if pm == Some(PassMode::Run) || self.props.fail_mode == Some(FailMode::Run) => { - WillExecute::Yes + true } - MirOpt if pm == Some(PassMode::Run) => WillExecute::Yes, - Ui | MirOpt => WillExecute::No, + MirOpt if pm == Some(PassMode::Run) => true, + Ui | MirOpt => false, mode => panic!("unimplemented for mode {:?}", mode), + }; + let enabled = self.config.run.unwrap_or_else(|| { + // Auto-detect whether to run based on the platform. + !self.config.target.ends_with("-fuchsia") + }); + match (test_should_run, enabled) { + (false, _) => WillExecute::No, + (true, true) => WillExecute::Yes, + (true, false) => WillExecute::Disabled, } } @@ -1531,7 +1541,8 @@ impl<'test> TestCx<'test> { // Only use `make_exe_name` when the test ends up being executed. let output_file = match will_execute { WillExecute::Yes => TargetLocation::ThisFile(self.make_exe_name()), - WillExecute::No => TargetLocation::ThisDirectory(self.output_base_dir()), + WillExecute::No | WillExecute::Disabled => + TargetLocation::ThisDirectory(self.output_base_dir()), }; let allow_unused = match self.config.mode { From 09783815b29fe6a8d0299bf883dba733f8a6fd1d Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 28 Apr 2021 23:50:16 +0000 Subject: [PATCH 04/26] Add run flag to bootstrap test --- src/bootstrap/builder/tests.rs | 3 +++ src/bootstrap/flags.rs | 15 +++++++++++++++ src/bootstrap/test.rs | 5 +++++ 3 files changed, 23 insertions(+) diff --git a/src/bootstrap/builder/tests.rs b/src/bootstrap/builder/tests.rs index a881512e988ed..4d7c207e3ab8b 100644 --- a/src/bootstrap/builder/tests.rs +++ b/src/bootstrap/builder/tests.rs @@ -489,6 +489,7 @@ mod dist { compare_mode: None, rustfix_coverage: false, pass: None, + run: None, }; let build = Build::new(config); @@ -529,6 +530,7 @@ mod dist { compare_mode: None, rustfix_coverage: false, pass: None, + run: None, }; let build = Build::new(config); @@ -584,6 +586,7 @@ mod dist { compare_mode: None, rustfix_coverage: false, pass: None, + run: None, }; // Make sure rustfmt binary not being found isn't an error. config.channel = "beta".to_string(); diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 6044899c237e2..13d11cded303a 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -103,6 +103,7 @@ pub enum Subcommand { bless: bool, compare_mode: Option, pass: Option, + run: Option, test_args: Vec, rustc_args: Vec, fail_fast: bool, @@ -293,6 +294,12 @@ To learn more about a subcommand, run `./x.py -h`", "force {check,build,run}-pass tests to this mode.", "check | build | run", ); + opts.optopt( + "", + "run", + "whether to execute run-* tests", + "auto | always | never", + ); opts.optflag( "", "rustfix-coverage", @@ -556,6 +563,7 @@ Arguments: bless: matches.opt_present("bless"), compare_mode: matches.opt_str("compare-mode"), pass: matches.opt_str("pass"), + run: matches.opt_str("run"), test_args: matches.opt_strs("test-args"), rustc_args: matches.opt_strs("rustc-args"), fail_fast: !matches.opt_present("no-fail-fast"), @@ -742,6 +750,13 @@ impl Subcommand { } } + pub fn run(&self) -> Option<&str> { + match *self { + Subcommand::Test { ref run, .. } => run.as_ref().map(|s| &s[..]), + _ => None, + } + } + pub fn open(&self) -> bool { match *self { Subcommand::Doc { open, .. } => open, diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index b9d7ecf8c0eb1..1eccfe102be8f 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -1207,6 +1207,11 @@ note: if you're sure you want to do this, please open an issue as to why. In the cmd.arg(pass); } + if let Some(ref run) = builder.config.cmd.run() { + cmd.arg("--run"); + cmd.arg(run); + } + if let Some(ref nodejs) = builder.config.nodejs { cmd.arg("--nodejs").arg(nodejs); } From 0b2e908691a2d5d35ebd877a2c3339b230b81eb0 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 29 Apr 2021 00:13:56 +0000 Subject: [PATCH 05/26] Add support for --run for non-ui tests --- src/bootstrap/flags.rs | 7 +--- src/tools/compiletest/src/runtest.rs | 58 ++++++++++++++++++++-------- 2 files changed, 43 insertions(+), 22 deletions(-) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 13d11cded303a..dd1e48a14684f 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -294,12 +294,7 @@ To learn more about a subcommand, run `./x.py -h`", "force {check,build,run}-pass tests to this mode.", "check | build | run", ); - opts.optopt( - "", - "run", - "whether to execute run-* tests", - "auto | always | never", - ); + opts.optopt("", "run", "whether to execute run-* tests", "auto | always | never"); opts.optflag( "", "rustfix-coverage", diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index c758b977573ae..a044c4425b4e6 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -359,22 +359,20 @@ impl<'test> TestCx<'test> { fn should_run(&self, pm: Option) -> WillExecute { let test_should_run = match self.config.mode { - Ui if pm == Some(PassMode::Run) || self.props.fail_mode == Some(FailMode::Run) => { - true - } + Ui if pm == Some(PassMode::Run) || self.props.fail_mode == Some(FailMode::Run) => true, MirOpt if pm == Some(PassMode::Run) => true, Ui | MirOpt => false, mode => panic!("unimplemented for mode {:?}", mode), }; + if test_should_run { self.run_if_enabled() } else { WillExecute::No } + } + + fn run_if_enabled(&self) -> WillExecute { let enabled = self.config.run.unwrap_or_else(|| { // Auto-detect whether to run based on the platform. !self.config.target.ends_with("-fuchsia") }); - match (test_should_run, enabled) { - (false, _) => WillExecute::No, - (true, true) => WillExecute::Yes, - (true, false) => WillExecute::Disabled, - } + if enabled { WillExecute::Yes } else { WillExecute::Disabled } } fn should_run_successfully(&self, pm: Option) -> bool { @@ -449,12 +447,17 @@ impl<'test> TestCx<'test> { fn run_rfail_test(&self) { let pm = self.pass_mode(); - let proc_res = self.compile_test(WillExecute::Yes, self.should_emit_metadata(pm)); + let should_run = self.run_if_enabled(); + let proc_res = self.compile_test(should_run, self.should_emit_metadata(pm)); if !proc_res.status.success() { self.fatal_proc_rec("compilation failed!", &proc_res); } + if let WillExecute::Disabled = should_run { + return; + } + let proc_res = self.exec_compiled_test(); // The value our Makefile configures valgrind to return on failure @@ -493,12 +496,17 @@ impl<'test> TestCx<'test> { fn run_rpass_test(&self) { let emit_metadata = self.should_emit_metadata(self.pass_mode()); - let proc_res = self.compile_test(WillExecute::Yes, emit_metadata); + let should_run = self.run_if_enabled(); + let proc_res = self.compile_test(should_run, emit_metadata); if !proc_res.status.success() { self.fatal_proc_rec("compilation failed!", &proc_res); } + if let WillExecute::Disabled = should_run { + return; + } + // FIXME(#41968): Move this check to tidy? let expected_errors = errors::load_errors(&self.testpaths.file, self.revision); assert!( @@ -520,12 +528,17 @@ impl<'test> TestCx<'test> { return self.run_rpass_test(); } - let mut proc_res = self.compile_test(WillExecute::Yes, EmitMetadata::No); + let should_run = self.run_if_enabled(); + let mut proc_res = self.compile_test(should_run, EmitMetadata::No); if !proc_res.status.success() { self.fatal_proc_rec("compilation failed!", &proc_res); } + if let WillExecute::Disabled = should_run { + return; + } + let mut new_config = self.config.clone(); new_config.runtool = new_config.valgrind_path.clone(); let new_cx = TestCx { config: &new_config, ..*self }; @@ -742,10 +755,14 @@ impl<'test> TestCx<'test> { fn run_debuginfo_cdb_test_no_opt(&self) { // compile test file (it should have 'compile-flags:-g' in the header) - let compile_result = self.compile_test(WillExecute::Yes, EmitMetadata::No); + let should_run = self.run_if_enabled(); + let compile_result = self.compile_test(should_run, EmitMetadata::No); if !compile_result.status.success() { self.fatal_proc_rec("compilation failed!", &compile_result); } + if let WillExecute::Disabled = should_run { + return; + } let exe_file = self.make_exe_name(); @@ -836,10 +853,14 @@ impl<'test> TestCx<'test> { let mut cmds = commands.join("\n"); // compile test file (it should have 'compile-flags:-g' in the header) - let compiler_run_result = self.compile_test(WillExecute::Yes, EmitMetadata::No); + let should_run = self.run_if_enabled(); + let compiler_run_result = self.compile_test(should_run, EmitMetadata::No); if !compiler_run_result.status.success() { self.fatal_proc_rec("compilation failed!", &compiler_run_result); } + if let WillExecute::Disabled = should_run { + return; + } let exe_file = self.make_exe_name(); @@ -1054,10 +1075,14 @@ impl<'test> TestCx<'test> { fn run_debuginfo_lldb_test_no_opt(&self) { // compile test file (it should have 'compile-flags:-g' in the header) - let compile_result = self.compile_test(WillExecute::Yes, EmitMetadata::No); + let should_run = self.run_if_enabled(); + let compile_result = self.compile_test(should_run, EmitMetadata::No); if !compile_result.status.success() { self.fatal_proc_rec("compilation failed!", &compile_result); } + if let WillExecute::Disabled = should_run { + return; + } let exe_file = self.make_exe_name(); @@ -1541,8 +1566,9 @@ impl<'test> TestCx<'test> { // Only use `make_exe_name` when the test ends up being executed. let output_file = match will_execute { WillExecute::Yes => TargetLocation::ThisFile(self.make_exe_name()), - WillExecute::No | WillExecute::Disabled => - TargetLocation::ThisDirectory(self.output_base_dir()), + WillExecute::No | WillExecute::Disabled => { + TargetLocation::ThisDirectory(self.output_base_dir()) + } }; let allow_unused = match self.config.mode { From e282fd045a96793fb060c224887b9807370bd9d1 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 29 Apr 2021 00:20:21 +0000 Subject: [PATCH 06/26] Include --run in stamp hash --- src/tools/compiletest/src/runtest.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index a044c4425b4e6..c87a0c8c8d9ec 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -259,6 +259,7 @@ pub fn run(config: Config, testpaths: &TestPaths, revision: Option<&str>) { pub fn compute_stamp_hash(config: &Config) -> String { let mut hash = DefaultHasher::new(); config.stage_id.hash(&mut hash); + config.run.hash(&mut hash); match config.debugger { Some(Debugger::Cdb) => { From f64c45a7d21f69251b05d796ba52a04e6201eefd Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 29 Apr 2021 01:02:07 +0000 Subject: [PATCH 07/26] Add needs-run-enabled directive for should-fail tests I was wary of doing any automatic disabling here, since should-fail is how we test compiletest itself. --- src/test/debuginfo/should-fail.rs | 1 + src/test/ui/meta/revision-bad.rs | 1 + src/tools/compiletest/src/common.rs | 9 +++++++++ src/tools/compiletest/src/header.rs | 4 ++++ src/tools/compiletest/src/runtest.rs | 6 +----- 5 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/test/debuginfo/should-fail.rs b/src/test/debuginfo/should-fail.rs index 1e0d22cbce404..eef6d99d2a91c 100644 --- a/src/test/debuginfo/should-fail.rs +++ b/src/test/debuginfo/should-fail.rs @@ -2,6 +2,7 @@ // == Test [gdb|lldb]-[command|check] are parsed correctly === // should-fail +// needs-run-enabled // compile-flags:-g // === GDB TESTS =================================================================================== diff --git a/src/test/ui/meta/revision-bad.rs b/src/test/ui/meta/revision-bad.rs index 01f1518c1c6ed..37ddbe99a9f03 100644 --- a/src/test/ui/meta/revision-bad.rs +++ b/src/test/ui/meta/revision-bad.rs @@ -4,6 +4,7 @@ // run-fail // revisions: foo bar // should-fail +// needs-run-enabled //[foo] error-pattern:bar //[bar] error-pattern:foo diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index 9a2166675d8ab..2a14d8a39990a 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -351,6 +351,15 @@ pub struct Config { pub npm: Option, } +impl Config { + pub fn run_enabled(&self) -> bool { + self.run.unwrap_or_else(|| { + // Auto-detect whether to run based on the platform. + !self.target.ends_with("-fuchsia") + }) + } +} + #[derive(Debug, Clone)] pub struct TestPaths { pub file: PathBuf, // e.g., compile-test/foo/bar/baz.rs diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index f31a24738df6c..56527420c0d08 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -85,6 +85,10 @@ impl EarlyProps { props.ignore = true; } + if !config.run_enabled() && config.parse_name_directive(ln, "needs-run-enabled") { + props.ignore = true; + } + if !rustc_has_sanitizer_support && config.parse_name_directive(ln, "needs-sanitizer-support") { diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index c87a0c8c8d9ec..c606aa1dfbfd4 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -369,11 +369,7 @@ impl<'test> TestCx<'test> { } fn run_if_enabled(&self) -> WillExecute { - let enabled = self.config.run.unwrap_or_else(|| { - // Auto-detect whether to run based on the platform. - !self.config.target.ends_with("-fuchsia") - }); - if enabled { WillExecute::Yes } else { WillExecute::Disabled } + if self.config.run_enabled() { WillExecute::Yes } else { WillExecute::Disabled } } fn should_run_successfully(&self, pm: Option) -> bool { From 1e46b18fec554664f35c73079bec0964429b9fa8 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 28 Apr 2021 23:54:39 +0000 Subject: [PATCH 08/26] Fix help for profile flags --- src/bootstrap/flags.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index dd1e48a14684f..d961e067db37c 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -223,8 +223,8 @@ To learn more about a subcommand, run `./x.py -h`", VALUE overrides the skip-rebuild option in config.toml.", "VALUE", ); - opts.optopt("", "rust-profile-generate", "rustc error format", "FORMAT"); - opts.optopt("", "rust-profile-use", "rustc error format", "FORMAT"); + opts.optopt("", "rust-profile-generate", "generate PGO profile with rustc build", "FORMAT"); + opts.optopt("", "rust-profile-use", "use PGO profile for rustc build", "FORMAT"); // We can't use getopt to parse the options until we have completed specifying which // options are valid, but under the current implementation, some options are conditional on From 4a63e1e991cadfdfceb7f421c0dec9ba823b42c8 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 30 Apr 2021 14:54:18 +0000 Subject: [PATCH 09/26] Allow using `core::` in intra-doc links within core itself I came up with this idea ages ago, but rustdoc used to ICE on it. Now it doesn't. --- library/core/src/intrinsics.rs | 20 ++++++++++---------- library/core/src/lib.rs | 4 ++++ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 1ba0b23ae5be3..f5bfc98c71938 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -723,7 +723,7 @@ extern "rust-intrinsic" { /// macro, which panics when it is executed, it is *undefined behavior* to /// reach code marked with this function. /// - /// The stabilized version of this intrinsic is [`core::hint::unreachable_unchecked`](crate::hint::unreachable_unchecked). + /// The stabilized version of this intrinsic is [`core::hint::unreachable_unchecked`]. #[rustc_const_unstable(feature = "const_unreachable_unchecked", issue = "53188")] pub fn unreachable() -> !; @@ -768,13 +768,13 @@ extern "rust-intrinsic" { /// More specifically, this is the offset in bytes between successive /// items of the same type, including alignment padding. /// - /// The stabilized version of this intrinsic is [`core::mem::size_of`](crate::mem::size_of). + /// The stabilized version of this intrinsic is [`core::mem::size_of`]. #[rustc_const_stable(feature = "const_size_of", since = "1.40.0")] pub fn size_of() -> usize; /// The minimum alignment of a type. /// - /// The stabilized version of this intrinsic is [`core::mem::align_of`](crate::mem::align_of). + /// The stabilized version of this intrinsic is [`core::mem::align_of`]. #[rustc_const_stable(feature = "const_min_align_of", since = "1.40.0")] pub fn min_align_of() -> usize; /// The preferred alignment of a type. @@ -790,13 +790,13 @@ extern "rust-intrinsic" { pub fn size_of_val(_: *const T) -> usize; /// The required alignment of the referenced value. /// - /// The stabilized version of this intrinsic is [`core::mem::align_of_val`](crate::mem::align_of_val). + /// The stabilized version of this intrinsic is [`core::mem::align_of_val`]. #[rustc_const_unstable(feature = "const_align_of_val", issue = "46571")] pub fn min_align_of_val(_: *const T) -> usize; /// Gets a static string slice containing the name of a type. /// - /// The stabilized version of this intrinsic is [`core::any::type_name`](crate::any::type_name). + /// The stabilized version of this intrinsic is [`core::any::type_name`]. #[rustc_const_unstable(feature = "const_type_name", issue = "63084")] pub fn type_name() -> &'static str; @@ -804,7 +804,7 @@ extern "rust-intrinsic" { /// function will return the same value for a type regardless of whichever /// crate it is invoked in. /// - /// The stabilized version of this intrinsic is [`core::any::TypeId::of`](crate::any::TypeId::of). + /// The stabilized version of this intrinsic is [`core::any::TypeId::of`]. #[rustc_const_unstable(feature = "const_type_id", issue = "77125")] pub fn type_id() -> u64; @@ -829,7 +829,7 @@ extern "rust-intrinsic" { /// Gets a reference to a static `Location` indicating where it was called. /// - /// Consider using [`core::panic::Location::caller`](crate::panic::Location::caller) instead. + /// Consider using [`core::panic::Location::caller`] instead. #[rustc_const_unstable(feature = "const_caller_location", issue = "76156")] pub fn caller_location() -> &'static crate::panic::Location<'static>; @@ -1158,11 +1158,11 @@ extern "rust-intrinsic" { /// Performs a volatile load from the `src` pointer. /// - /// The stabilized version of this intrinsic is [`core::ptr::read_volatile`](crate::ptr::read_volatile). + /// The stabilized version of this intrinsic is [`core::ptr::read_volatile`]. pub fn volatile_load(src: *const T) -> T; /// Performs a volatile store to the `dst` pointer. /// - /// The stabilized version of this intrinsic is [`core::ptr::write_volatile`](crate::ptr::write_volatile). + /// The stabilized version of this intrinsic is [`core::ptr::write_volatile`]. pub fn volatile_store(dst: *mut T, val: T); /// Performs a volatile load from the `src` pointer @@ -1703,7 +1703,7 @@ extern "rust-intrinsic" { /// Returns the value of the discriminant for the variant in 'v'; /// if `T` has no discriminant, returns `0`. /// - /// The stabilized version of this intrinsic is [`core::mem::discriminant`](crate::mem::discriminant). + /// The stabilized version of this intrinsic is [`core::mem::discriminant`]. #[rustc_const_unstable(feature = "const_discriminant", issue = "69821")] pub fn discriminant_value(v: &T) -> ::Discriminant; diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 0e2c140c367a9..cbabf55d17b66 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -169,6 +169,10 @@ #![feature(int_error_matching)] #![deny(unsafe_op_in_unsafe_fn)] +// allow using `core::` in intra-doc links +#[allow(unused_extern_crates)] +extern crate self as core; + #[prelude_import] #[allow(unused)] use prelude::v1::*; From d7cd6e24264d75b7771e63fc15bfcb40a0678d92 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 3 May 2021 18:36:06 +0200 Subject: [PATCH 10/26] Fix RESTRICTED_DEPENDENCY_CRATES to list rustc_driver instead of rustc_middle --- src/tools/tidy/src/deps.rs | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 5a843ea61ec6f..4d9a19535ade9 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -49,7 +49,7 @@ const EXCEPTIONS: &[(&str, &str)] = &[ const RUNTIME_CRATES: &[&str] = &["std", "core", "alloc", "test", "panic_abort", "panic_unwind"]; /// Crates whose dependencies must be explicitly permitted. -const RESTRICTED_DEPENDENCY_CRATES: &[&str] = &["rustc_middle", "rustc_codegen_llvm"]; +const RESTRICTED_DEPENDENCY_CRATES: &[&str] = &["rustc_driver", "rustc_codegen_llvm"]; /// Crates rustc is allowed to depend on. Avoid adding to the list if possible. /// @@ -72,7 +72,10 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "cc", "cfg-if", "chalk-derive", + "chalk-engine", "chalk-ir", + "chalk-solve", + "chrono", "cmake", "compiler_builtins", "cpuid-bool", @@ -92,6 +95,7 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "expect-test", "fake-simd", "filetime", + "fixedbitset", "flate2", "fortanix-sgx-abi", "fuchsia-zircon", @@ -107,6 +111,7 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "indexmap", "instant", "itertools", + "itoa", "jobserver", "kernel32-sys", "lazy_static", @@ -114,6 +119,7 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "libz-sys", "lock_api", "log", + "matchers", "maybe-uninit", "md-5", "measureme", @@ -123,6 +129,8 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "memoffset", "miniz_oxide", "num_cpus", + "num-integer", + "num-traits", "object", "once_cell", "opaque-debug", @@ -130,6 +138,7 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "parking_lot_core", "pathdiff", "perf-event-open-sys", + "petgraph", "pin-project-lite", "pkg-config", "polonius-engine", @@ -147,22 +156,28 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "rand_xorshift", "redox_syscall", "regex", + "regex-automata", "regex-syntax", "remove_dir_all", + "rls-data", + "rls-span", "rustc-demangle", "rustc-hash", "rustc-rayon", "rustc-rayon-core", "rustc_version", + "ryu", "scoped-tls", "scopeguard", "semver", "semver-parser", "serde", "serde_derive", + "serde_json", "sha-1", "sha2", "smallvec", + "sharded-slab", "snap", "stable_deref_trait", "stacker", @@ -172,9 +187,15 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "termcolor", "termize", "thread_local", + "time", + "tinyvec", "tracing", "tracing-attributes", "tracing-core", + "tracing-log", + "tracing-serde", + "tracing-subscriber", + "tracing-tree", "typenum", "unicode-normalization", "unicode-script", From 2fa18b8864be68948fdd5df2170bf75e5bb0b158 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 3 May 2021 18:36:38 +0200 Subject: [PATCH 11/26] Remove obsolete crate exceptions --- src/tools/tidy/src/deps.rs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 4d9a19535ade9..86231946bb725 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -244,13 +244,6 @@ fn check_exceptions(metadata: &Metadata, bad: &mut bool) { } // Check that the license hasn't changed. for pkg in metadata.packages.iter().filter(|p| p.name == *name) { - if pkg.name == "fuchsia-cprng" { - // This package doesn't declare a license expression. Manual - // inspection of the license file is necessary, which appears - // to be BSD-3-Clause. - assert!(pkg.license.is_none()); - continue; - } match &pkg.license { None => { tidy_error!( @@ -261,14 +254,6 @@ fn check_exceptions(metadata: &Metadata, bad: &mut bool) { } Some(pkg_license) => { if pkg_license.as_str() != *license { - if *name == "crossbeam-queue" - && *license == "MIT/Apache-2.0 AND BSD-2-Clause" - { - // We have two versions of crossbeam-queue and both - // are fine. - continue; - } - println!("dependency exception `{}` license has changed", name); println!(" previously `{}` now `{}`", license, pkg_license); println!(" update EXCEPTIONS for the new license"); From 5db01aae99a82675565c333ece511daa31d545cb Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 3 May 2021 18:38:52 +0200 Subject: [PATCH 12/26] Take build dependencies into account during license checks The comment says that build dependencies shouldn't matter unless they do some kind of codegen. It is safer to always check it though. --- src/tools/tidy/src/deps.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 86231946bb725..91008fc9fb7f9 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -460,16 +460,7 @@ fn normal_deps_of_r<'a>( .iter() .find(|n| &n.id == pkg_id) .unwrap_or_else(|| panic!("could not find `{}` in resolve", pkg_id)); - // Don't care about dev-dependencies. - // Build dependencies *shouldn't* matter unless they do some kind of - // codegen. For now we'll assume they don't. - let deps = node.deps.iter().filter(|node_dep| { - node_dep - .dep_kinds - .iter() - .any(|kind_info| kind_info.kind == cargo_metadata::DependencyKind::Normal) - }); - for dep in deps { + for dep in &node.deps { normal_deps_of_r(resolve, &dep.pkg, result); } } From 24def637b36df2945f3a54d06ba5f791a3d78dd7 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 3 May 2021 18:40:07 +0200 Subject: [PATCH 13/26] Wire up tidy dependency checks for cg_clif --- src/tools/tidy/src/deps.rs | 134 +++++++++++++++++++++++++++++++------ 1 file changed, 113 insertions(+), 21 deletions(-) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 91008fc9fb7f9..aa27ce6a72316 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -44,6 +44,23 @@ const EXCEPTIONS: &[(&str, &str)] = &[ ("fortanix-sgx-abi", "MPL-2.0"), // libstd but only for `sgx` target ]; +const EXCEPTIONS_CRANELIFT: &[(&str, &str)] = &[ + ("cranelift-bforest", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-codegen", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-codegen-meta", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-codegen-shared", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-entity", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-frontend", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-jit", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-module", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-native", "Apache-2.0 WITH LLVM-exception"), + ("cranelift-object", "Apache-2.0 WITH LLVM-exception"), + ("libloading", "ISC"), + ("mach", "BSD-2-Clause"), + ("regalloc", "Apache-2.0 WITH LLVM-exception"), + ("target-lexicon", "Apache-2.0 WITH LLVM-exception"), +]; + /// These are the root crates that are part of the runtime. The licenses for /// these and all their dependencies *must not* be in the exception list. const RUNTIME_CRATES: &[&str] = &["std", "core", "alloc", "test", "panic_abort", "panic_unwind"]; @@ -212,6 +229,59 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "winapi-x86_64-pc-windows-gnu", ]; +const PERMITTED_CRANELIFT_DEPENDENCIES: &[&str] = &[ + "anyhow", + "ar", + "autocfg", + "bitflags", + "byteorder", + "cfg-if", + "cranelift-bforest", + "cranelift-codegen", + "cranelift-codegen-meta", + "cranelift-codegen-shared", + "cranelift-entity", + "cranelift-frontend", + "cranelift-jit", + "cranelift-module", + "cranelift-native", + "cranelift-object", + "crc32fast", + "errno", + "errno-dragonfly", + "gcc", + "gimli", + "hashbrown", + "indexmap", + "libc", + "libloading", + "log", + "mach", + "object", + "proc-macro2", + "quote", + "regalloc", + "region", + "rustc-hash", + "smallvec", + "syn", + "target-lexicon", + "thiserror", + "thiserror-impl", + "unicode-xid", + "winapi", + "winapi-i686-pc-windows-gnu", + "winapi-x86_64-pc-windows-gnu", +]; + +const FORBIDDEN_TO_HAVE_DUPLICATES: &[&str] = &[ + // These two crates take quite a long time to build, so don't allow two versions of them + // to accidentally sneak into our dependency graph, in order to ensure we keep our CI times + // under control. + "cargo", + "rustc-ap-rustc_ast", +]; + /// Dependency checks. /// /// `root` is path to the directory with the root `Cargo.toml` (for the workspace). `cargo` is path @@ -222,17 +292,39 @@ pub fn check(root: &Path, cargo: &Path, bad: &mut bool) { .manifest_path(root.join("Cargo.toml")) .features(cargo_metadata::CargoOpt::AllFeatures); let metadata = t!(cmd.exec()); - check_exceptions(&metadata, bad); - check_dependencies(&metadata, bad); - check_crate_duplicate(&metadata, bad); + let runtime_ids = compute_runtime_crates(&metadata); + check_exceptions(&metadata, EXCEPTIONS, runtime_ids, bad); + check_dependencies(&metadata, PERMITTED_DEPENDENCIES, RESTRICTED_DEPENDENCY_CRATES, bad); + check_crate_duplicate(&metadata, FORBIDDEN_TO_HAVE_DUPLICATES, bad); + + // Check rustc_codegen_cranelift independently as it has it's own workspace. + let mut cmd = cargo_metadata::MetadataCommand::new(); + cmd.cargo_path(cargo) + .manifest_path(root.join("compiler/rustc_codegen_cranelift/Cargo.toml")) + .features(cargo_metadata::CargoOpt::AllFeatures); + let metadata = t!(cmd.exec()); + let runtime_ids = HashSet::new(); + check_exceptions(&metadata, EXCEPTIONS_CRANELIFT, runtime_ids, bad); + check_dependencies( + &metadata, + PERMITTED_CRANELIFT_DEPENDENCIES, + &["rustc_codegen_cranelift"], + bad, + ); + check_crate_duplicate(&metadata, &[], bad); } /// Check that all licenses are in the valid list in `LICENSES`. /// /// Packages listed in `EXCEPTIONS` are allowed for tools. -fn check_exceptions(metadata: &Metadata, bad: &mut bool) { +fn check_exceptions( + metadata: &Metadata, + exceptions: &[(&str, &str)], + runtime_ids: HashSet<&PackageId>, + bad: &mut bool, +) { // Validate the EXCEPTIONS list hasn't changed. - for (name, license) in EXCEPTIONS { + for (name, license) in exceptions { // Check that the package actually exists. if !metadata.packages.iter().any(|p| p.name == *name) { tidy_error!( @@ -264,8 +356,7 @@ fn check_exceptions(metadata: &Metadata, bad: &mut bool) { } } - let exception_names: Vec<_> = EXCEPTIONS.iter().map(|(name, _license)| *name).collect(); - let runtime_ids = compute_runtime_crates(metadata); + let exception_names: Vec<_> = exceptions.iter().map(|(name, _license)| *name).collect(); // Check if any package does not have a valid license. for pkg in &metadata.packages { @@ -300,9 +391,14 @@ fn check_exceptions(metadata: &Metadata, bad: &mut bool) { /// `true` if a check failed. /// /// Specifically, this checks that the dependencies are on the `PERMITTED_DEPENDENCIES`. -fn check_dependencies(metadata: &Metadata, bad: &mut bool) { +fn check_dependencies( + metadata: &Metadata, + permitted_dependencies: &[&'static str], + restricted_dependency_crates: &[&'static str], + bad: &mut bool, +) { // Check that the PERMITTED_DEPENDENCIES does not have unused entries. - for name in PERMITTED_DEPENDENCIES { + for name in permitted_dependencies { if !metadata.packages.iter().any(|p| p.name == *name) { tidy_error!( bad, @@ -313,12 +409,12 @@ fn check_dependencies(metadata: &Metadata, bad: &mut bool) { } } // Get the list in a convenient form. - let permitted_dependencies: HashSet<_> = PERMITTED_DEPENDENCIES.iter().cloned().collect(); + let permitted_dependencies: HashSet<_> = permitted_dependencies.iter().cloned().collect(); // Check dependencies. let mut visited = BTreeSet::new(); let mut unapproved = BTreeSet::new(); - for &krate in RESTRICTED_DEPENDENCY_CRATES.iter() { + for &krate in restricted_dependency_crates.iter() { let pkg = pkg_from_name(metadata, krate); let mut bad = check_crate_dependencies(&permitted_dependencies, metadata, &mut visited, pkg); @@ -371,16 +467,12 @@ fn check_crate_dependencies<'a>( } /// Prevents multiple versions of some expensive crates. -fn check_crate_duplicate(metadata: &Metadata, bad: &mut bool) { - const FORBIDDEN_TO_HAVE_DUPLICATES: &[&str] = &[ - // These two crates take quite a long time to build, so don't allow two versions of them - // to accidentally sneak into our dependency graph, in order to ensure we keep our CI times - // under control. - "cargo", - "rustc-ap-rustc_ast", - ]; - - for &name in FORBIDDEN_TO_HAVE_DUPLICATES { +fn check_crate_duplicate( + metadata: &Metadata, + forbidden_to_have_duplicates: &[&str], + bad: &mut bool, +) { + for &name in forbidden_to_have_duplicates { let matches: Vec<_> = metadata.packages.iter().filter(|pkg| pkg.name == name).collect(); match matches.len() { 0 => { From 6b64202d5eebdaddcc246412a795ec32dac0f8f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 3 May 2021 23:48:56 -0700 Subject: [PATCH 14/26] Handle incorrect placement of parentheses in trait bounds more gracefully Fix #84772. --- compiler/rustc_parse/src/parser/ty.rs | 39 +++++++++- src/test/ui/parser/trait-object-delimiters.rs | 17 ++++ .../ui/parser/trait-object-delimiters.stderr | 77 +++++++++++++++++++ 3 files changed, 130 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/parser/trait-object-delimiters.rs create mode 100644 src/test/ui/parser/trait-object-delimiters.stderr diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs index 0f7b8ebd376b9..d537741c749c5 100644 --- a/compiler/rustc_parse/src/parser/ty.rs +++ b/compiler/rustc_parse/src/parser/ty.rs @@ -470,7 +470,7 @@ impl<'a> Parser<'a> { /// Is a `dyn B0 + ... + Bn` type allowed here? fn is_explicit_dyn_type(&mut self) -> bool { self.check_keyword(kw::Dyn) - && (self.token.uninterpolated_span().rust_2018() + && (!self.token.uninterpolated_span().rust_2015() || self.look_ahead(1, |t| { t.can_begin_bound() && !can_continue_type_after_non_fn_ident(t) })) @@ -539,7 +539,21 @@ impl<'a> Parser<'a> { ) -> PResult<'a, GenericBounds> { let mut bounds = Vec::new(); let mut negative_bounds = Vec::new(); - while self.can_begin_bound() { + + while self.can_begin_bound() || self.token.is_keyword(kw::Dyn) { + if self.token.is_keyword(kw::Dyn) { + // Account for `&dyn Trait + dyn Other`. + self.struct_span_err(self.token.span, "invalid `dyn` keyword") + .help("`dyn` is only needed at the start of a trait `+`-separated list") + .span_suggestion( + self.token.span, + "remove this keyword", + String::new(), + Applicability::MachineApplicable, + ) + .emit(); + self.bump(); + } match self.parse_generic_bound()? { Ok(bound) => bounds.push(bound), Err(neg_sp) => negative_bounds.push(neg_sp), @@ -721,7 +735,26 @@ impl<'a> Parser<'a> { let lifetime_defs = self.parse_late_bound_lifetime_defs()?; let path = self.parse_path(PathStyle::Type)?; if has_parens { - self.expect(&token::CloseDelim(token::Paren))?; + if self.token.is_like_plus() { + // Someone has written something like `&dyn (Trait + Other)`. The correct code + // would be `&(dyn Trait + Other)`, but we don't have access to the appropriate + // span to suggest that. When written as `&dyn Trait + Other`, an appropriate + // suggestion is given. + let bounds = vec![]; + self.parse_remaining_bounds(bounds, true)?; + self.expect(&token::CloseDelim(token::Paren))?; + let sp = vec![lo, self.prev_token.span]; + let sugg: Vec<_> = sp.iter().map(|sp| (*sp, String::new())).collect(); + self.struct_span_err(sp, "incorrect braces around trait bounds") + .multipart_suggestion( + "remove the parentheses", + sugg, + Applicability::MachineApplicable, + ) + .emit(); + } else { + self.expect(&token::CloseDelim(token::Paren))?; + } } let modifier = modifiers.to_trait_bound_modifier(); diff --git a/src/test/ui/parser/trait-object-delimiters.rs b/src/test/ui/parser/trait-object-delimiters.rs new file mode 100644 index 0000000000000..650ab57226187 --- /dev/null +++ b/src/test/ui/parser/trait-object-delimiters.rs @@ -0,0 +1,17 @@ +// edition:2018 + +fn foo1(_: &dyn Drop + AsRef) {} //~ ERROR ambiguous `+` in a type +//~^ ERROR only auto traits can be used as additional traits in a trait object + +fn foo2(_: &dyn (Drop + AsRef)) {} //~ ERROR incorrect braces around trait bounds + +fn foo3(_: &dyn {Drop + AsRef}) {} //~ ERROR expected parameter name, found `{` +//~^ ERROR expected one of `!`, `(`, `)`, `,`, `?`, `for`, lifetime, or path, found `{` +//~| ERROR at least one trait is required for an object type + +fn foo4(_: &dyn >) {} //~ ERROR expected identifier, found `<` + +fn foo5(_: &(dyn Drop + dyn AsRef)) {} //~ ERROR invalid `dyn` keyword +//~^ ERROR only auto traits can be used as additional traits in a trait object + +fn main() {} diff --git a/src/test/ui/parser/trait-object-delimiters.stderr b/src/test/ui/parser/trait-object-delimiters.stderr new file mode 100644 index 0000000000000..18b1b24122ecf --- /dev/null +++ b/src/test/ui/parser/trait-object-delimiters.stderr @@ -0,0 +1,77 @@ +error: ambiguous `+` in a type + --> $DIR/trait-object-delimiters.rs:3:13 + | +LL | fn foo1(_: &dyn Drop + AsRef) {} + | ^^^^^^^^^^^^^^^^^^^^^ help: use parentheses to disambiguate: `(dyn Drop + AsRef)` + +error: incorrect braces around trait bounds + --> $DIR/trait-object-delimiters.rs:6:17 + | +LL | fn foo2(_: &dyn (Drop + AsRef)) {} + | ^ ^ + | +help: remove the parentheses + | +LL | fn foo2(_: &dyn Drop + AsRef) {} + | -- -- + +error: expected parameter name, found `{` + --> $DIR/trait-object-delimiters.rs:8:17 + | +LL | fn foo3(_: &dyn {Drop + AsRef}) {} + | ^ expected parameter name + +error: expected one of `!`, `(`, `)`, `,`, `?`, `for`, lifetime, or path, found `{` + --> $DIR/trait-object-delimiters.rs:8:17 + | +LL | fn foo3(_: &dyn {Drop + AsRef}) {} + | -^ expected one of 8 possible tokens + | | + | help: missing `,` + +error: expected identifier, found `<` + --> $DIR/trait-object-delimiters.rs:12:17 + | +LL | fn foo4(_: &dyn >) {} + | ^ expected identifier + +error: invalid `dyn` keyword + --> $DIR/trait-object-delimiters.rs:14:25 + | +LL | fn foo5(_: &(dyn Drop + dyn AsRef)) {} + | ^^^ help: remove this keyword + | + = help: `dyn` is only needed at the start of a trait `+`-separated list + +error[E0225]: only auto traits can be used as additional traits in a trait object + --> $DIR/trait-object-delimiters.rs:3:24 + | +LL | fn foo1(_: &dyn Drop + AsRef) {} + | ---- ^^^^^^^^^^ additional non-auto trait + | | + | first non-auto trait + | + = help: consider creating a new trait with all of these as super-traits and using that trait here instead: `trait NewTrait: Drop + AsRef {}` + = note: auto-traits like `Send` and `Sync` are traits that have special properties; for more information on them, visit + +error[E0224]: at least one trait is required for an object type + --> $DIR/trait-object-delimiters.rs:8:13 + | +LL | fn foo3(_: &dyn {Drop + AsRef}) {} + | ^^^ + +error[E0225]: only auto traits can be used as additional traits in a trait object + --> $DIR/trait-object-delimiters.rs:14:29 + | +LL | fn foo5(_: &(dyn Drop + dyn AsRef)) {} + | ---- ^^^^^^^^^^ additional non-auto trait + | | + | first non-auto trait + | + = help: consider creating a new trait with all of these as super-traits and using that trait here instead: `trait NewTrait: Drop + AsRef {}` + = note: auto-traits like `Send` and `Sync` are traits that have special properties; for more information on them, visit + +error: aborting due to 9 previous errors + +Some errors have detailed explanations: E0224, E0225. +For more information about an error, try `rustc --explain E0224`. From 0b94338a267fce3c25a25d09ab18d00d0dc21268 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 4 May 2021 13:43:50 +0200 Subject: [PATCH 15/26] =?UTF-8?q?CTFE=20engine:=20rename=20copy=20?= =?UTF-8?q?=E2=86=92=20copy=5Fintrinsic,=20move=20to=20intrinsics.rs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../rustc_mir/src/interpret/intrinsics.rs | 34 ++++++++++++++++++- compiler/rustc_mir/src/interpret/step.rs | 34 +------------------ 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_mir/src/interpret/intrinsics.rs b/compiler/rustc_mir/src/interpret/intrinsics.rs index dea1b11331549..292306f6cde6e 100644 --- a/compiler/rustc_mir/src/interpret/intrinsics.rs +++ b/compiler/rustc_mir/src/interpret/intrinsics.rs @@ -323,7 +323,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.write_scalar(result, dest)?; } sym::copy => { - self.copy(&args[0], &args[1], &args[2], /*nonoverlapping*/ false)?; + self.copy_intrinsic(&args[0], &args[1], &args[2], /*nonoverlapping*/ false)?; } sym::offset => { let ptr = self.read_scalar(&args[0])?.check_init()?; @@ -530,4 +530,36 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { )?; Ok(offset_ptr) } + + /// Copy `count*size_of::()` many bytes from `*src` to `*dst`. + pub(crate) fn copy_intrinsic( + &mut self, + src: &OpTy<'tcx, >::PointerTag>, + dst: &OpTy<'tcx, >::PointerTag>, + count: &OpTy<'tcx, >::PointerTag>, + nonoverlapping: bool, + ) -> InterpResult<'tcx> { + let count = self.read_scalar(&count)?.to_machine_usize(self)?; + let layout = self.layout_of(src.layout.ty.builtin_deref(true).unwrap().ty)?; + let (size, align) = (layout.size, layout.align.abi); + let size = size.checked_mul(count, self).ok_or_else(|| { + err_ub_format!( + "overflow computing total size of `{}`", + if nonoverlapping { "copy_nonoverlapping" } else { "copy" } + ) + })?; + + // Make sure we check both pointers for an access of the total size and aligment, + // *even if* the total size is 0. + let src = + self.memory.check_ptr_access(self.read_scalar(&src)?.check_init()?, size, align)?; + + let dst = + self.memory.check_ptr_access(self.read_scalar(&dst)?.check_init()?, size, align)?; + + if let (Some(src), Some(dst)) = (src, dst) { + self.memory.copy(src, dst, size, nonoverlapping)?; + } + Ok(()) + } } diff --git a/compiler/rustc_mir/src/interpret/step.rs b/compiler/rustc_mir/src/interpret/step.rs index 6084f67abd78e..5a10ffe6d6199 100644 --- a/compiler/rustc_mir/src/interpret/step.rs +++ b/compiler/rustc_mir/src/interpret/step.rs @@ -2,7 +2,6 @@ //! //! The main entry point is the `step` method. -use crate::interpret::OpTy; use rustc_middle::mir; use rustc_middle::mir::interpret::{InterpResult, Scalar}; use rustc_target::abi::LayoutOf; @@ -119,7 +118,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let src = self.eval_operand(src, None)?; let dst = self.eval_operand(dst, None)?; let count = self.eval_operand(count, None)?; - self.copy(&src, &dst, &count, /* nonoverlapping */ true)?; + self.copy_intrinsic(&src, &dst, &count, /* nonoverlapping */ true)?; } // Statements we do not track. @@ -149,37 +148,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(()) } - pub(crate) fn copy( - &mut self, - src: &OpTy<'tcx, >::PointerTag>, - dst: &OpTy<'tcx, >::PointerTag>, - count: &OpTy<'tcx, >::PointerTag>, - nonoverlapping: bool, - ) -> InterpResult<'tcx> { - let count = self.read_scalar(&count)?.to_machine_usize(self)?; - let layout = self.layout_of(src.layout.ty.builtin_deref(true).unwrap().ty)?; - let (size, align) = (layout.size, layout.align.abi); - let size = size.checked_mul(count, self).ok_or_else(|| { - err_ub_format!( - "overflow computing total size of `{}`", - if nonoverlapping { "copy_nonoverlapping" } else { "copy" } - ) - })?; - - // Make sure we check both pointers for an access of the total size and aligment, - // *even if* the total size is 0. - let src = - self.memory.check_ptr_access(self.read_scalar(&src)?.check_init()?, size, align)?; - - let dst = - self.memory.check_ptr_access(self.read_scalar(&dst)?.check_init()?, size, align)?; - - if let (Some(src), Some(dst)) = (src, dst) { - self.memory.copy(src, dst, size, nonoverlapping)?; - } - Ok(()) - } - /// Evaluate an assignment statement. /// /// There is no separate `eval_rvalue` function. Instead, the code for handling each rvalue From 3584c1dd0cb501916c8bc6fd452864b261068beb Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 3 May 2021 08:47:43 -0700 Subject: [PATCH 16/26] Disallows `#![feature(no_coverage)]` on stable and beta using allow_internal_unstable (as recommended) Fixes: #84836 ```shell $ ./build/x86_64-unknown-linux-gnu/stage1/bin/rustc src/test/run-make-fulldeps/coverage/no_cov_crate.rs error[E0554]: `#![feature]` may not be used on the dev release channel --> src/test/run-make-fulldeps/coverage/no_cov_crate.rs:2:1 | 2 | #![feature(no_coverage)] | ^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error For more information about this error, try `rustc --explain E0554`. ``` --- .../src/deriving/cmp/eq.rs | 14 ++-------- compiler/rustc_feature/src/builtin_attrs.rs | 8 +----- compiler/rustc_typeck/src/collect.rs | 28 +------------------ library/core/src/cmp.rs | 5 ++-- library/core/src/lib.rs | 1 + .../expected_show_coverage.no_cov_func.txt | 19 ------------- .../run-make-fulldeps/coverage/no_cov_func.rs | 18 ------------ .../feature-gates/feature-gate-no_coverage.rs | 13 ++++++--- .../feature-gate-no_coverage.stderr | 3 +- 9 files changed, 18 insertions(+), 91 deletions(-) delete mode 100644 src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.no_cov_func.txt delete mode 100644 src/test/run-make-fulldeps/coverage/no_cov_func.rs diff --git a/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs b/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs index 5a4e7fd9d07b4..54ab88dc3ffc9 100644 --- a/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs +++ b/compiler/rustc_builtin_macros/src/deriving/cmp/eq.rs @@ -15,20 +15,12 @@ pub fn expand_deriving_eq( item: &Annotatable, push: &mut dyn FnMut(Annotatable), ) { + let span = cx.with_def_site_ctxt(span); let inline = cx.meta_word(span, sym::inline); - let no_coverage_ident = - rustc_ast::attr::mk_nested_word_item(Ident::new(sym::no_coverage, span)); - let no_coverage_feature = - rustc_ast::attr::mk_list_item(Ident::new(sym::feature, span), vec![no_coverage_ident]); - let no_coverage = cx.meta_word(span, sym::no_coverage); let hidden = rustc_ast::attr::mk_nested_word_item(Ident::new(sym::hidden, span)); let doc = rustc_ast::attr::mk_list_item(Ident::new(sym::doc, span), vec![hidden]); - let attrs = vec![ - cx.attribute(inline), - cx.attribute(no_coverage_feature), - cx.attribute(no_coverage), - cx.attribute(doc), - ]; + let no_coverage = cx.meta_word(span, sym::no_coverage); + let attrs = vec![cx.attribute(inline), cx.attribute(doc), cx.attribute(no_coverage)]; let trait_def = TraitDef { span, attributes: Vec::new(), diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 5474fea9c7857..a8719be84c2a4 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -273,13 +273,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ template!(List: "address, memory, thread"), experimental!(no_sanitize) ), - ungated!( - // Not exclusively gated at the crate level (though crate-level is - // supported). The feature can alternatively be enabled on individual - // functions. - no_coverage, AssumedUsed, - template!(Word), - ), + gated!(no_coverage, AssumedUsed, template!(Word), experimental!(no_coverage)), // FIXME: #14408 assume docs are used since rustdoc looks at them. ungated!(doc, AssumedUsed, template!(List: "hidden|inline|...", NameValueStr: "string")), diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index 190c9d35934f9..0528f8812f920 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -2661,8 +2661,6 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs { let mut inline_span = None; let mut link_ordinal_span = None; let mut no_sanitize_span = None; - let mut no_coverage_feature_enabled = false; - let mut no_coverage_attr = None; for attr in attrs.iter() { if tcx.sess.check_name(attr, sym::cold) { codegen_fn_attrs.flags |= CodegenFnAttrFlags::COLD; @@ -2726,15 +2724,8 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs { codegen_fn_attrs.flags |= CodegenFnAttrFlags::NAKED; } else if tcx.sess.check_name(attr, sym::no_mangle) { codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_MANGLE; - } else if attr.has_name(sym::feature) { - if let Some(list) = attr.meta_item_list() { - if list.iter().any(|nested_meta_item| nested_meta_item.has_name(sym::no_coverage)) { - tcx.sess.mark_attr_used(attr); - no_coverage_feature_enabled = true; - } - } } else if tcx.sess.check_name(attr, sym::no_coverage) { - no_coverage_attr = Some(attr); + codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_COVERAGE; } else if tcx.sess.check_name(attr, sym::rustc_std_internal_symbol) { codegen_fn_attrs.flags |= CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL; } else if tcx.sess.check_name(attr, sym::used) { @@ -2945,23 +2936,6 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs { } } - if let Some(no_coverage_attr) = no_coverage_attr { - if tcx.sess.features_untracked().no_coverage || no_coverage_feature_enabled { - codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_COVERAGE - } else { - let mut err = feature_err( - &tcx.sess.parse_sess, - sym::no_coverage, - no_coverage_attr.span, - "the `#[no_coverage]` attribute is an experimental feature", - ); - if tcx.sess.parse_sess.unstable_features.is_nightly_build() { - err.help("or, alternatively, add `#[feature(no_coverage)]` to the function"); - } - err.emit(); - } - } - codegen_fn_attrs.inline = attrs.iter().fold(InlineAttr::None, |ia, attr| { if !attr.has_name(sym::inline) { return ia; diff --git a/library/core/src/cmp.rs b/library/core/src/cmp.rs index 0a3e5789e8bed..f8b16b6f9275c 100644 --- a/library/core/src/cmp.rs +++ b/library/core/src/cmp.rs @@ -274,8 +274,7 @@ pub trait Eq: PartialEq { // // This should never be implemented by hand. #[doc(hidden)] - #[cfg_attr(not(bootstrap), feature(no_coverage))] - #[cfg_attr(not(bootstrap), no_coverage)] + #[cfg_attr(not(bootstrap), no_coverage)] // rust-lang/rust#84605 #[inline] #[stable(feature = "rust1", since = "1.0.0")] fn assert_receiver_is_total_eq(&self) {} @@ -284,7 +283,7 @@ pub trait Eq: PartialEq { /// Derive macro generating an impl of the trait `Eq`. #[rustc_builtin_macro] #[stable(feature = "builtin_macro_prelude", since = "1.38.0")] -#[allow_internal_unstable(core_intrinsics, derive_eq, structural_match)] +#[allow_internal_unstable(core_intrinsics, derive_eq, structural_match, no_coverage)] pub macro Eq($item:item) { /* compiler built-in */ } diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 0e2c140c367a9..d1329b8f22116 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -166,6 +166,7 @@ #![feature(const_caller_location)] #![feature(slice_ptr_get)] #![feature(no_niche)] // rust-lang/rust#68303 +#![cfg_attr(not(bootstrap), feature(no_coverage))] // rust-lang/rust#84605 #![feature(int_error_matching)] #![deny(unsafe_op_in_unsafe_fn)] diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.no_cov_func.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.no_cov_func.txt deleted file mode 100644 index 16eaf7c858c19..0000000000000 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.no_cov_func.txt +++ /dev/null @@ -1,19 +0,0 @@ - 1| |// Enables `no_coverage` on individual functions - 2| | - 3| |#[feature(no_coverage)] - 4| |#[no_coverage] - 5| |fn do_not_add_coverage_1() { - 6| | println!("called but not covered"); - 7| |} - 8| | - 9| |#[no_coverage] - 10| |#[feature(no_coverage)] - 11| |fn do_not_add_coverage_2() { - 12| | println!("called but not covered"); - 13| |} - 14| | - 15| 1|fn main() { - 16| 1| do_not_add_coverage_1(); - 17| 1| do_not_add_coverage_2(); - 18| 1|} - diff --git a/src/test/run-make-fulldeps/coverage/no_cov_func.rs b/src/test/run-make-fulldeps/coverage/no_cov_func.rs deleted file mode 100644 index e19a2c4a87200..0000000000000 --- a/src/test/run-make-fulldeps/coverage/no_cov_func.rs +++ /dev/null @@ -1,18 +0,0 @@ -// Enables `no_coverage` on individual functions - -#[feature(no_coverage)] -#[no_coverage] -fn do_not_add_coverage_1() { - println!("called but not covered"); -} - -#[no_coverage] -#[feature(no_coverage)] -fn do_not_add_coverage_2() { - println!("called but not covered"); -} - -fn main() { - do_not_add_coverage_1(); - do_not_add_coverage_2(); -} diff --git a/src/test/ui/feature-gates/feature-gate-no_coverage.rs b/src/test/ui/feature-gates/feature-gate-no_coverage.rs index c6b79f9a43171..fd4c6f76059aa 100644 --- a/src/test/ui/feature-gates/feature-gate-no_coverage.rs +++ b/src/test/ui/feature-gates/feature-gate-no_coverage.rs @@ -1,8 +1,13 @@ #![crate_type = "lib"] -#[no_coverage] -#[feature(no_coverage)] // does not have to be enabled before `#[no_coverage]` -fn no_coverage_is_enabled_on_this_function() {} +#[derive(PartialEq, Eq)] // ensure deriving `Eq` does not enable `feature(no_coverage)` +struct Foo { + a: u8, + b: u32, +} #[no_coverage] //~ ERROR the `#[no_coverage]` attribute is an experimental feature -fn requires_feature_no_coverage() {} +fn requires_feature_no_coverage() -> bool { + let bar = Foo { a: 0, b: 0 }; + bar == Foo { a: 0, b: 0 } +} diff --git a/src/test/ui/feature-gates/feature-gate-no_coverage.stderr b/src/test/ui/feature-gates/feature-gate-no_coverage.stderr index 04627be4aaf65..f7167e0b771c0 100644 --- a/src/test/ui/feature-gates/feature-gate-no_coverage.stderr +++ b/src/test/ui/feature-gates/feature-gate-no_coverage.stderr @@ -1,12 +1,11 @@ error[E0658]: the `#[no_coverage]` attribute is an experimental feature - --> $DIR/feature-gate-no_coverage.rs:7:1 + --> $DIR/feature-gate-no_coverage.rs:9:1 | LL | #[no_coverage] | ^^^^^^^^^^^^^^ | = note: see issue #84605 for more information = help: add `#![feature(no_coverage)]` to the crate attributes to enable - = help: or, alternatively, add `#[feature(no_coverage)]` to the function error: aborting due to previous error From ad4ccf966bea9bf374fa95500ad1f3d1ae6439e9 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 5 May 2021 17:28:18 +0200 Subject: [PATCH 17/26] Remove unneeded call to with_default_session_globals in rustdoc highlight --- src/librustdoc/html/highlight.rs | 37 +++++++++++++++----------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 7130a6bc1e888..f631f627fc255 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -13,7 +13,6 @@ use std::iter::Peekable; use rustc_lexer::{LiteralKind, TokenKind}; use rustc_span::edition::Edition; use rustc_span::symbol::Symbol; -use rustc_span::with_default_session_globals; use super::format::Buffer; @@ -238,28 +237,26 @@ impl<'a> Classifier<'a> { /// possibly giving it an HTML span with a class specifying what flavor of /// token is used. fn highlight(mut self, sink: &mut dyn FnMut(Highlight<'a>)) { - with_default_session_globals(|| { - loop { - if self - .tokens - .peek() - .map(|t| matches!(t.0, TokenKind::Colon | TokenKind::Ident)) - .unwrap_or(false) - { - let tokens = self.get_full_ident_path(); - for (token, start, end) in tokens { - let text = &self.src[start..end]; - self.advance(token, text, sink); - self.byte_pos += text.len() as u32; - } - } - if let Some((token, text)) = self.next() { + loop { + if self + .tokens + .peek() + .map(|t| matches!(t.0, TokenKind::Colon | TokenKind::Ident)) + .unwrap_or(false) + { + let tokens = self.get_full_ident_path(); + for (token, start, end) in tokens { + let text = &self.src[start..end]; self.advance(token, text, sink); - } else { - break; + self.byte_pos += text.len() as u32; } } - }) + if let Some((token, text)) = self.next() { + self.advance(token, text, sink); + } else { + break; + } + } } /// Single step of highlighting. This will classify `token`, but maybe also From 3c489a3482258a5788516d2d8c31e68ee26e0e15 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 5 May 2021 18:13:47 +0200 Subject: [PATCH 18/26] Update highlight tests --- src/librustdoc/html/highlight/tests.rs | 27 +++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/librustdoc/html/highlight/tests.rs b/src/librustdoc/html/highlight/tests.rs index 305cf61091dc6..a0da2c963d167 100644 --- a/src/librustdoc/html/highlight/tests.rs +++ b/src/librustdoc/html/highlight/tests.rs @@ -2,6 +2,7 @@ use super::write_code; use crate::html::format::Buffer; use expect_test::expect_file; use rustc_span::edition::Edition; +use rustc_span::with_default_session_globals; const STYLE: &str = r#"