From 95aac4487d1cb89913539619c201079d0fc2b463 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 11 Oct 2020 23:59:32 +0200 Subject: [PATCH 01/25] transmute_copy: explain that alignment is handled correctly --- library/core/src/mem/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/core/src/mem/mod.rs b/library/core/src/mem/mod.rs index a2c7da6e6958e..e84014c68a676 100644 --- a/library/core/src/mem/mod.rs +++ b/library/core/src/mem/mod.rs @@ -884,10 +884,10 @@ pub fn drop(_x: T) {} /// Interprets `src` as having type `&U`, and then reads `src` without moving /// the contained value. /// -/// This function will unsafely assume the pointer `src` is valid for -/// [`size_of::`][size_of] bytes by transmuting `&T` to `&U` and then reading -/// the `&U`. It will also unsafely create a copy of the contained value instead of -/// moving out of `src`. +/// This function will unsafely assume the pointer `src` is valid for [`size_of::`][size_of] +/// bytes by transmuting `&T` to `&U` and then reading the `&U` (except that this is done in a way +/// that is correct even when `&U` makes stricter alignment requirements than `&T`). It will also +/// unsafely create a copy of the contained value instead of moving out of `src`. /// /// It is not a compile-time error if `T` and `U` have different sizes, but it /// is highly encouraged to only invoke this function where `T` and `U` have the From 08d5e9673677d5874484b2f9837a42db9fea9331 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Tue, 20 Oct 2020 00:00:00 +0000 Subject: [PATCH 02/25] Initialize tracing subscriber in compiletest tool The logging in compiletest was migrated from log crate to a tracing, but the initialization code was never changed, so logging is non-functional. Initialize tracing subscriber using default settings. --- Cargo.lock | 2 +- src/tools/compiletest/Cargo.toml | 2 +- src/tools/compiletest/src/main.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0d2170a992747..722632e53c75d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -639,7 +639,6 @@ name = "compiletest" version = "0.0.0" dependencies = [ "diff", - "env_logger 0.7.1", "getopts", "glob", "lazy_static", @@ -650,6 +649,7 @@ dependencies = [ "serde", "serde_json", "tracing", + "tracing-subscriber", "walkdir", "winapi 0.3.9", ] diff --git a/src/tools/compiletest/Cargo.toml b/src/tools/compiletest/Cargo.toml index c601084d11917..209254a5d5e2f 100644 --- a/src/tools/compiletest/Cargo.toml +++ b/src/tools/compiletest/Cargo.toml @@ -6,9 +6,9 @@ edition = "2018" [dependencies] diff = "0.1.10" -env_logger = { version = "0.7", default-features = false } getopts = "0.2" tracing = "0.1" +tracing-subscriber = { version = "0.2.13", default-features = false, features = ["fmt", "env-filter", "smallvec", "parking_lot", "ansi"] } regex = "1.0" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 190a9c6221060..2b2a6cfa8bd6f 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -35,7 +35,7 @@ pub mod runtest; pub mod util; fn main() { - env_logger::init(); + tracing_subscriber::fmt::init(); let config = parse_config(env::args().collect()); From 2ec4d82e159c7d8437721ee53645c3620aaa69b3 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 20 Oct 2020 21:26:02 +0200 Subject: [PATCH 03/25] Add issue template link to IRLO --- .github/ISSUE_TEMPLATE/config.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index bd7dc0ac95c1f..898b983ba3790 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -3,3 +3,6 @@ contact_links: - name: Rust Programming Language Forum url: https://users.rust-lang.org about: Please ask and answer questions about Rust here. + - name: Rust Internals Forum + url: https://internals.rust-lang.org/ + about: Please discuss language feature requests here. From 95cbfb1aa458d336a0425144117b2e5262a51380 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 20 Oct 2020 21:36:39 +0200 Subject: [PATCH 04/25] Retitle forum links --- .github/ISSUE_TEMPLATE/config.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index 898b983ba3790..7d4cfaece4481 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -1,8 +1,8 @@ blank_issues_enabled: true contact_links: - - name: Rust Programming Language Forum + - name: Question url: https://users.rust-lang.org - about: Please ask and answer questions about Rust here. - - name: Rust Internals Forum + about: Please ask and answer questions about Rust on the user forum. + - name: Feature Request url: https://internals.rust-lang.org/ - about: Please discuss language feature requests here. + about: Please discuss language feature requests on the internals forum. From 57d01a9aeea505b2f4b0c420e1b9975d51690ff8 Mon Sep 17 00:00:00 2001 From: Simon Vandel Sillesen Date: Thu, 22 Oct 2020 22:22:17 +0200 Subject: [PATCH 05/25] Check which places are dead Fixes #78192 --- .../rustc_mir/src/transform/instcombine.rs | 20 +++++++++---- .../const_prop/ref_deref.main.ConstProp.diff | 2 +- .../ref_deref_project.main.ConstProp.diff | 2 +- src/test/mir-opt/inst_combine_deref.rs | 2 +- src/test/mir-opt/issue-78192.rs | 9 ++++++ .../mir-opt/issue_78192.f.InstCombine.diff | 29 +++++++++++++++++++ 6 files changed, 55 insertions(+), 9 deletions(-) create mode 100644 src/test/mir-opt/issue-78192.rs create mode 100644 src/test/mir-opt/issue_78192.f.InstCombine.diff diff --git a/compiler/rustc_mir/src/transform/instcombine.rs b/compiler/rustc_mir/src/transform/instcombine.rs index c0b3d5ff18b5c..c5c14ca7caeb5 100644 --- a/compiler/rustc_mir/src/transform/instcombine.rs +++ b/compiler/rustc_mir/src/transform/instcombine.rs @@ -119,11 +119,6 @@ impl OptimizationFinder<'b, 'tcx> { } fn find_deref_of_address(&mut self, rvalue: &Rvalue<'tcx>, location: Location) -> Option<()> { - // FIXME(#78192): This optimization can result in unsoundness. - if !self.tcx.sess.opts.debugging_opts.unsound_mir_opts { - return None; - } - // Look for the sequence // // _2 = &_1; @@ -137,6 +132,8 @@ impl OptimizationFinder<'b, 'tcx> { _ => None, }?; + let mut dead_locals_seen = vec![]; + let stmt_index = location.statement_index; // Look behind for statement that assigns the local from a address of operator. // 6 is chosen as a heuristic determined by seeing the number of times @@ -160,6 +157,11 @@ impl OptimizationFinder<'b, 'tcx> { BorrowKind::Shared, place_taken_address_of, ) => { + // Make sure that the place has not been marked dead + if dead_locals_seen.contains(&place_taken_address_of.local) { + return None; + } + self.optimizations .unneeded_deref .insert(location, *place_taken_address_of); @@ -178,13 +180,19 @@ impl OptimizationFinder<'b, 'tcx> { // Inline asm can do anything, so bail out of the optimization. rustc_middle::mir::StatementKind::LlvmInlineAsm(_) => return None, + // Remember `StorageDead`s, as the local being marked dead could be the + // place RHS we are looking for, in which case we need to abort to avoid UB + // using an uninitialized place + rustc_middle::mir::StatementKind::StorageDead(dead) => { + dead_locals_seen.push(*dead) + } + // Check that `local_being_deref` is not being used in a mutating way which can cause misoptimization. rustc_middle::mir::StatementKind::Assign(box (_, _)) | rustc_middle::mir::StatementKind::Coverage(_) | rustc_middle::mir::StatementKind::Nop | rustc_middle::mir::StatementKind::FakeRead(_, _) | rustc_middle::mir::StatementKind::StorageLive(_) - | rustc_middle::mir::StatementKind::StorageDead(_) | rustc_middle::mir::StatementKind::Retag(_, _) | rustc_middle::mir::StatementKind::AscribeUserType(_, _) | rustc_middle::mir::StatementKind::SetDiscriminant { .. } => { diff --git a/src/test/mir-opt/const_prop/ref_deref.main.ConstProp.diff b/src/test/mir-opt/const_prop/ref_deref.main.ConstProp.diff index 4fd1b8b227649..feef65f52ebe0 100644 --- a/src/test/mir-opt/const_prop/ref_deref.main.ConstProp.diff +++ b/src/test/mir-opt/const_prop/ref_deref.main.ConstProp.diff @@ -19,7 +19,7 @@ // + span: $DIR/ref_deref.rs:5:6: 5:10 // + literal: Const { ty: &i32, val: Unevaluated(WithOptConstParam { did: DefId(0:3 ~ ref_deref[317d]::main), const_param_did: None }, [], Some(promoted[0])) } _2 = _4; // scope 0 at $DIR/ref_deref.rs:5:6: 5:10 -- _1 = (*_2); // scope 0 at $DIR/ref_deref.rs:5:5: 5:10 +- _1 = (*_4); // scope 0 at $DIR/ref_deref.rs:5:5: 5:10 + _1 = const 4_i32; // scope 0 at $DIR/ref_deref.rs:5:5: 5:10 StorageDead(_2); // scope 0 at $DIR/ref_deref.rs:5:10: 5:11 StorageDead(_1); // scope 0 at $DIR/ref_deref.rs:5:10: 5:11 diff --git a/src/test/mir-opt/const_prop/ref_deref_project.main.ConstProp.diff b/src/test/mir-opt/const_prop/ref_deref_project.main.ConstProp.diff index 812c7c9771801..7ec0751263fb1 100644 --- a/src/test/mir-opt/const_prop/ref_deref_project.main.ConstProp.diff +++ b/src/test/mir-opt/const_prop/ref_deref_project.main.ConstProp.diff @@ -19,7 +19,7 @@ // + span: $DIR/ref_deref_project.rs:5:6: 5:17 // + literal: Const { ty: &(i32, i32), val: Unevaluated(WithOptConstParam { did: DefId(0:3 ~ ref_deref_project[317d]::main), const_param_did: None }, [], Some(promoted[0])) } _2 = &((*_4).1: i32); // scope 0 at $DIR/ref_deref_project.rs:5:6: 5:17 - _1 = (*_2); // scope 0 at $DIR/ref_deref_project.rs:5:5: 5:17 + _1 = ((*_4).1: i32); // scope 0 at $DIR/ref_deref_project.rs:5:5: 5:17 StorageDead(_2); // scope 0 at $DIR/ref_deref_project.rs:5:17: 5:18 StorageDead(_1); // scope 0 at $DIR/ref_deref_project.rs:5:17: 5:18 _0 = const (); // scope 0 at $DIR/ref_deref_project.rs:4:11: 6:2 diff --git a/src/test/mir-opt/inst_combine_deref.rs b/src/test/mir-opt/inst_combine_deref.rs index 78361c336607c..3be8c2f3ac732 100644 --- a/src/test/mir-opt/inst_combine_deref.rs +++ b/src/test/mir-opt/inst_combine_deref.rs @@ -1,4 +1,4 @@ -// compile-flags: -O -Zunsound-mir-opts +// compile-flags: -O // EMIT_MIR inst_combine_deref.simple_opt.InstCombine.diff fn simple_opt() -> u64 { let x = 5; diff --git a/src/test/mir-opt/issue-78192.rs b/src/test/mir-opt/issue-78192.rs new file mode 100644 index 0000000000000..906d094f72b4a --- /dev/null +++ b/src/test/mir-opt/issue-78192.rs @@ -0,0 +1,9 @@ +// EMIT_MIR issue_78192.f.InstCombine.diff +pub fn f(a: &T) -> *const T { + let b: &*const T = &(a as *const T); + *b +} + +fn main() { + f(&2); +} diff --git a/src/test/mir-opt/issue_78192.f.InstCombine.diff b/src/test/mir-opt/issue_78192.f.InstCombine.diff new file mode 100644 index 0000000000000..ec3be78525802 --- /dev/null +++ b/src/test/mir-opt/issue_78192.f.InstCombine.diff @@ -0,0 +1,29 @@ +- // MIR for `f` before InstCombine ++ // MIR for `f` after InstCombine + + fn f(_1: &T) -> *const T { + debug a => _1; // in scope 0 at $DIR/issue-78192.rs:2:13: 2:14 + let mut _0: *const T; // return place in scope 0 at $DIR/issue-78192.rs:2:23: 2:31 + let _2: &*const T; // in scope 0 at $DIR/issue-78192.rs:3:9: 3:10 + let _3: &*const T; // in scope 0 at $DIR/issue-78192.rs:3:24: 3:40 + let _4: *const T; // in scope 0 at $DIR/issue-78192.rs:3:25: 3:40 + scope 1 { + debug b => _2; // in scope 1 at $DIR/issue-78192.rs:3:9: 3:10 + } + + bb0: { + StorageLive(_2); // scope 0 at $DIR/issue-78192.rs:3:9: 3:10 + StorageLive(_3); // scope 0 at $DIR/issue-78192.rs:3:24: 3:40 + StorageLive(_4); // scope 0 at $DIR/issue-78192.rs:3:25: 3:40 + _4 = &raw const (*_1); // scope 0 at $DIR/issue-78192.rs:3:26: 3:27 + _3 = &_4; // scope 0 at $DIR/issue-78192.rs:3:24: 3:40 +- _2 = &(*_3); // scope 0 at $DIR/issue-78192.rs:3:24: 3:40 ++ _2 = _3; // scope 0 at $DIR/issue-78192.rs:3:24: 3:40 + StorageDead(_3); // scope 0 at $DIR/issue-78192.rs:3:40: 3:41 + _0 = (*_2); // scope 1 at $DIR/issue-78192.rs:4:5: 4:7 + StorageDead(_4); // scope 0 at $DIR/issue-78192.rs:5:1: 5:2 + StorageDead(_2); // scope 0 at $DIR/issue-78192.rs:5:1: 5:2 + return; // scope 0 at $DIR/issue-78192.rs:5:2: 5:2 + } + } + From d0d0e7820806bbb89a2733bae8f9aaa9c78345d0 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Tue, 4 Aug 2020 18:42:36 -0700 Subject: [PATCH 06/25] Capture output from threads spawned in tests Fixes #42474. --- compiler/rustc_interface/src/util.rs | 5 +++ library/std/src/io/impls.rs | 6 ++-- library/std/src/io/mod.rs | 4 ++- library/std/src/io/stdio.rs | 33 +++++++++++++++++--- library/std/src/panicking.rs | 2 +- library/std/src/thread/mod.rs | 5 +++ library/test/src/helpers/sink.rs | 7 +++++ src/test/ui/panic-while-printing.rs | 19 +++++++++-- src/test/ui/test-thread-capture.rs | 30 ++++++++++++++++++ src/test/ui/test-thread-capture.run.stdout | 21 +++++++++++++ src/test/ui/test-thread-nocapture.rs | 30 ++++++++++++++++++ src/test/ui/test-thread-nocapture.run.stderr | 2 ++ src/test/ui/test-thread-nocapture.run.stdout | 20 ++++++++++++ src/test/ui/threads-sendsync/task-stderr.rs | 5 +++ 14 files changed, 177 insertions(+), 12 deletions(-) create mode 100644 src/test/ui/test-thread-capture.rs create mode 100644 src/test/ui/test-thread-capture.run.stdout create mode 100644 src/test/ui/test-thread-nocapture.rs create mode 100644 src/test/ui/test-thread-nocapture.run.stderr create mode 100644 src/test/ui/test-thread-nocapture.run.stdout diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index c1b359c7d0de5..73003d8ebd6a3 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -113,6 +113,11 @@ impl Write for Sink { Ok(()) } } +impl io::LocalOutput for Sink { + fn clone_box(&self) -> Box { + Box::new(Self(self.0.clone())) + } +} /// Like a `thread::Builder::spawn` followed by a `join()`, but avoids the need /// for `'static` bounds. diff --git a/library/std/src/io/impls.rs b/library/std/src/io/impls.rs index e09e7ba978e86..66426101d278e 100644 --- a/library/std/src/io/impls.rs +++ b/library/std/src/io/impls.rs @@ -213,13 +213,13 @@ impl BufRead for Box { #[cfg(test)] /// This impl is only used by printing logic, so any error returned is always /// of kind `Other`, and should be ignored. -impl Write for Box { +impl Write for dyn ::realstd::io::LocalOutput { fn write(&mut self, buf: &[u8]) -> io::Result { - (**self).write(buf).map_err(|_| ErrorKind::Other.into()) + (*self).write(buf).map_err(|_| ErrorKind::Other.into()) } fn flush(&mut self) -> io::Result<()> { - (**self).flush().map_err(|_| ErrorKind::Other.into()) + (*self).flush().map_err(|_| ErrorKind::Other.into()) } } diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index d9d0380781925..e6efe6ec57eea 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -277,10 +277,12 @@ pub use self::stdio::{StderrLock, StdinLock, StdoutLock}; pub use self::stdio::{_eprint, _print}; #[unstable(feature = "libstd_io_internals", issue = "42788")] #[doc(no_inline, hidden)] -pub use self::stdio::{set_panic, set_print}; +pub use self::stdio::{set_panic, set_print, LocalOutput}; #[stable(feature = "rust1", since = "1.0.0")] pub use self::util::{copy, empty, repeat, sink, Empty, Repeat, Sink}; +pub(crate) use self::stdio::clone_io; + mod buffered; mod cursor; mod error; diff --git a/library/std/src/io/stdio.rs b/library/std/src/io/stdio.rs index 36b49401591f5..5b50ce4c2ee8d 100644 --- a/library/std/src/io/stdio.rs +++ b/library/std/src/io/stdio.rs @@ -18,14 +18,14 @@ use crate::thread::LocalKey; thread_local! { /// Used by the test crate to capture the output of the print! and println! macros. - static LOCAL_STDOUT: RefCell>> = { + static LOCAL_STDOUT: RefCell>> = { RefCell::new(None) } } thread_local! { /// Used by the test crate to capture the output of the eprint! and eprintln! macros, and panics. - static LOCAL_STDERR: RefCell>> = { + static LOCAL_STDERR: RefCell>> = { RefCell::new(None) } } @@ -888,6 +888,18 @@ impl fmt::Debug for StderrLock<'_> { } } +/// A writer than can be cloned to new threads. +#[unstable( + feature = "set_stdio", + reason = "this trait may disappear completely or be replaced \ + with a more general mechanism", + issue = "none" +)] +#[doc(hidden)] +pub trait LocalOutput: Write + Send { + fn clone_box(&self) -> Box; +} + /// Resets the thread-local stderr handle to the specified writer /// /// This will replace the current thread's stderr handle, returning the old @@ -903,7 +915,7 @@ impl fmt::Debug for StderrLock<'_> { issue = "none" )] #[doc(hidden)] -pub fn set_panic(sink: Option>) -> Option> { +pub fn set_panic(sink: Option>) -> Option> { use crate::mem; if sink.is_none() && !LOCAL_STREAMS.load(Ordering::Relaxed) { // LOCAL_STDERR is definitely None since LOCAL_STREAMS is false. @@ -934,7 +946,7 @@ pub fn set_panic(sink: Option>) -> Option>) -> Option> { +pub fn set_print(sink: Option>) -> Option> { use crate::mem; if sink.is_none() && !LOCAL_STREAMS.load(Ordering::Relaxed) { // LOCAL_STDOUT is definitely None since LOCAL_STREAMS is false. @@ -950,6 +962,17 @@ pub fn set_print(sink: Option>) -> Option (Option>, Option>) { + LOCAL_STDOUT.with(|stdout| { + LOCAL_STDERR.with(|stderr| { + ( + stdout.borrow().as_ref().map(|o| o.clone_box()), + stderr.borrow().as_ref().map(|o| o.clone_box()), + ) + }) + }) +} + /// Write `args` to output stream `local_s` if possible, `global_s` /// otherwise. `label` identifies the stream in a panic message. /// @@ -962,7 +985,7 @@ pub fn set_print(sink: Option>) -> Option( args: fmt::Arguments<'_>, - local_s: &'static LocalKey>>>, + local_s: &'static LocalKey>>>, global_s: fn() -> T, label: &str, ) where diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index 8dceb12de87b8..17d5398b4ef3a 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -220,7 +220,7 @@ fn default_hook(info: &PanicInfo<'_>) { if let Some(mut local) = set_panic(None) { // NB. In `cfg(test)` this uses the forwarding impl - // for `Box`. + // for `dyn ::realstd::io::LocalOutput`. write(&mut local); set_panic(Some(local)); } else if let Some(mut out) = panic_output() { diff --git a/library/std/src/thread/mod.rs b/library/std/src/thread/mod.rs index 45c10266ba255..bdb8fc7807b3a 100644 --- a/library/std/src/thread/mod.rs +++ b/library/std/src/thread/mod.rs @@ -457,11 +457,16 @@ impl Builder { let my_packet: Arc>>> = Arc::new(UnsafeCell::new(None)); let their_packet = my_packet.clone(); + let (stdout, stderr) = crate::io::clone_io(); + let main = move || { if let Some(name) = their_thread.cname() { imp::Thread::set_name(name); } + crate::io::set_print(stdout); + crate::io::set_panic(stderr); + // SAFETY: the stack guard passed is the one for the current thread. // This means the current thread's stack and the new thread's stack // are properly set and protected from each other. diff --git a/library/test/src/helpers/sink.rs b/library/test/src/helpers/sink.rs index aa7fe2487730e..dfbf0a3b72f54 100644 --- a/library/test/src/helpers/sink.rs +++ b/library/test/src/helpers/sink.rs @@ -6,6 +6,7 @@ use std::{ sync::{Arc, Mutex}, }; +#[derive(Clone)] pub struct Sink(Arc>>); impl Sink { @@ -14,6 +15,12 @@ impl Sink { } } +impl io::LocalOutput for Sink { + fn clone_box(&self) -> Box { + Box::new(self.clone()) + } +} + impl Write for Sink { fn write(&mut self, data: &[u8]) -> io::Result { Write::write(&mut *self.0.lock().unwrap(), data) diff --git a/src/test/ui/panic-while-printing.rs b/src/test/ui/panic-while-printing.rs index 7e9fa16b0847a..21fc12759f87c 100644 --- a/src/test/ui/panic-while-printing.rs +++ b/src/test/ui/panic-while-printing.rs @@ -5,7 +5,7 @@ use std::fmt; use std::fmt::{Display, Formatter}; -use std::io::set_panic; +use std::io::{self, set_panic, LocalOutput, Write}; pub struct A; @@ -15,8 +15,23 @@ impl Display for A { } } +struct Sink; +impl Write for Sink { + fn write(&mut self, buf: &[u8]) -> io::Result { + Ok(buf.len()) + } + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } +} +impl LocalOutput for Sink { + fn clone_box(&self) -> Box { + Box::new(Sink) + } +} + fn main() { - set_panic(Some(Box::new(Vec::new()))); + set_panic(Some(Box::new(Sink))); assert!(std::panic::catch_unwind(|| { eprintln!("{}", A); }) diff --git a/src/test/ui/test-thread-capture.rs b/src/test/ui/test-thread-capture.rs new file mode 100644 index 0000000000000..2f8f3163deb9e --- /dev/null +++ b/src/test/ui/test-thread-capture.rs @@ -0,0 +1,30 @@ +// compile-flags: --test +// run-fail +// run-flags: --test-threads=1 +// check-run-results +// exec-env:RUST_BACKTRACE=0 + +#[test] +fn thready_pass() { + println!("fee"); + std::thread::spawn(|| { + println!("fie"); + println!("foe"); + }) + .join() + .unwrap(); + println!("fum"); +} + +#[test] +fn thready_fail() { + println!("fee"); + std::thread::spawn(|| { + println!("fie"); + println!("foe"); + }) + .join() + .unwrap(); + println!("fum"); + panic!(); +} diff --git a/src/test/ui/test-thread-capture.run.stdout b/src/test/ui/test-thread-capture.run.stdout new file mode 100644 index 0000000000000..6a49de5f4ab9e --- /dev/null +++ b/src/test/ui/test-thread-capture.run.stdout @@ -0,0 +1,21 @@ + +running 2 tests +test thready_fail ... FAILED +test thready_pass ... ok + +failures: + +---- thready_fail stdout ---- +fee +fie +foe +fum +thread 'main' panicked at 'explicit panic', $DIR/test-thread-capture.rs:29:5 +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace + + +failures: + thready_fail + +test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out + diff --git a/src/test/ui/test-thread-nocapture.rs b/src/test/ui/test-thread-nocapture.rs new file mode 100644 index 0000000000000..c107c6c9dd327 --- /dev/null +++ b/src/test/ui/test-thread-nocapture.rs @@ -0,0 +1,30 @@ +// compile-flags: --test +// run-fail +// run-flags: --test-threads=1 --nocapture +// check-run-results +// exec-env:RUST_BACKTRACE=0 + +#[test] +fn thready_pass() { + println!("fee"); + std::thread::spawn(|| { + println!("fie"); + println!("foe"); + }) + .join() + .unwrap(); + println!("fum"); +} + +#[test] +fn thready_fail() { + println!("fee"); + std::thread::spawn(|| { + println!("fie"); + println!("foe"); + }) + .join() + .unwrap(); + println!("fum"); + panic!(); +} diff --git a/src/test/ui/test-thread-nocapture.run.stderr b/src/test/ui/test-thread-nocapture.run.stderr new file mode 100644 index 0000000000000..ce5244c13ab26 --- /dev/null +++ b/src/test/ui/test-thread-nocapture.run.stderr @@ -0,0 +1,2 @@ +thread 'main' panicked at 'explicit panic', $DIR/test-thread-nocapture.rs:29:5 +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace diff --git a/src/test/ui/test-thread-nocapture.run.stdout b/src/test/ui/test-thread-nocapture.run.stdout new file mode 100644 index 0000000000000..77b42ed88d63f --- /dev/null +++ b/src/test/ui/test-thread-nocapture.run.stdout @@ -0,0 +1,20 @@ + +running 2 tests +test thready_fail ... fee +fie +foe +fum +FAILED +test thready_pass ... fee +fie +foe +fum +ok + +failures: + +failures: + thready_fail + +test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out + diff --git a/src/test/ui/threads-sendsync/task-stderr.rs b/src/test/ui/threads-sendsync/task-stderr.rs index d474084bf20ce..bc4bedac196ec 100644 --- a/src/test/ui/threads-sendsync/task-stderr.rs +++ b/src/test/ui/threads-sendsync/task-stderr.rs @@ -16,6 +16,11 @@ impl Write for Sink { } fn flush(&mut self) -> io::Result<()> { Ok(()) } } +impl io::LocalOutput for Sink { + fn clone_box(&self) -> Box { + Box::new(Sink(self.0.clone())) + } +} fn main() { let data = Arc::new(Mutex::new(Vec::new())); From db15596c5747eed0344f2622a9d85ad9534b23f9 Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Thu, 22 Oct 2020 03:22:13 -0700 Subject: [PATCH 07/25] Only load LOCAL_STREAMS if they are being used --- library/std/src/io/stdio.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/library/std/src/io/stdio.rs b/library/std/src/io/stdio.rs index 5b50ce4c2ee8d..2eb5fb4528620 100644 --- a/library/std/src/io/stdio.rs +++ b/library/std/src/io/stdio.rs @@ -963,6 +963,11 @@ pub fn set_print(sink: Option>) -> Option (Option>, Option>) { + // Don't waste time when LOCAL_{STDOUT,STDERR} are definitely None. + if !LOCAL_STREAMS.load(Ordering::Relaxed) { + return (None, None); + } + LOCAL_STDOUT.with(|stdout| { LOCAL_STDERR.with(|stderr| { ( From 86df9039b2a49dde3a2453d75c4c58540a568a1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 21 Oct 2020 14:25:09 -0700 Subject: [PATCH 08/25] Tweak "use `.await`" suggestion --- .../src/traits/error_reporting/suggestions.rs | 10 +++++----- .../src/check/fn_ctxt/suggestions.rs | 16 ++++++---------- .../incorrect-syntax-suggestions.stderr | 9 +++++---- src/test/ui/async-await/issue-61076.stderr | 18 ++++++++++-------- .../suggest-missing-await-closure.fixed | 4 ++-- .../suggest-missing-await-closure.rs | 4 ++-- .../suggest-missing-await-closure.stderr | 9 +++++---- .../ui/async-await/suggest-missing-await.fixed | 8 ++++---- .../ui/async-await/suggest-missing-await.rs | 8 ++++---- .../async-await/suggest-missing-await.stderr | 13 +++++++------ src/test/ui/suggestions/issue-72766.stderr | 9 +++++---- 11 files changed, 55 insertions(+), 53 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index efa9bd633ba8c..fa837e04db35e 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -21,7 +21,7 @@ use rustc_middle::ty::{ }; use rustc_middle::ty::{TypeAndMut, TypeckResults}; use rustc_span::symbol::{kw, sym, Ident, Symbol}; -use rustc_span::{MultiSpan, Span, DUMMY_SP}; +use rustc_span::{BytePos, MultiSpan, Span, DUMMY_SP}; use rustc_target::spec::abi; use std::fmt; @@ -2114,10 +2114,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { if self.predicate_may_hold(&try_obligation) && impls_future { if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) { if snippet.ends_with('?') { - err.span_suggestion( - span, - "consider using `.await` here", - format!("{}.await?", snippet.trim_end_matches('?')), + err.span_suggestion_verbose( + span.with_hi(span.hi() - BytePos(1)).shrink_to_hi(), + "consider `await`ing on the `Future`", + ".await".to_string(), Applicability::MaybeIncorrect, ); } diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs index 9bad02c41b4b1..18db8d63850b2 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs @@ -497,16 +497,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if self.infcx.predicate_may_hold(&obligation) { debug!("suggest_missing_await: obligation held: {:?}", obligation); - if let Ok(code) = self.sess().source_map().span_to_snippet(sp) { - err.span_suggestion( - sp, - "consider using `.await` here", - format!("{}.await", code), - Applicability::MaybeIncorrect, - ); - } else { - debug!("suggest_missing_await: no snippet for {:?}", sp); - } + err.span_suggestion_verbose( + sp.shrink_to_hi(), + "consider `await`ing on the `Future`", + ".await".to_string(), + Applicability::MaybeIncorrect, + ); } else { debug!("suggest_missing_await: obligation did not hold: {:?}", obligation) } diff --git a/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr b/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr index 6a653fc060b1d..d90ce0f1bb482 100644 --- a/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr +++ b/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr @@ -237,13 +237,14 @@ error[E0277]: the `?` operator can only be applied to values that implement `Try --> $DIR/incorrect-syntax-suggestions.rs:16:19 | LL | let _ = await bar()?; - | ^^^^^^ - | | - | the `?` operator cannot be applied to type `impl Future` - | help: consider using `.await` here: `bar().await?` + | ^^^^^^ the `?` operator cannot be applied to type `impl Future` | = help: the trait `Try` is not implemented for `impl Future` = note: required by `into_result` +help: consider `await`ing on the `Future` + | +LL | let _ = await bar().await?; + | ^^^^^^ error[E0277]: the `?` operator can only be applied to values that implement `Try` --> $DIR/incorrect-syntax-suggestions.rs:63:19 diff --git a/src/test/ui/async-await/issue-61076.stderr b/src/test/ui/async-await/issue-61076.stderr index 88ea7251eaf1f..afba889f014fe 100644 --- a/src/test/ui/async-await/issue-61076.stderr +++ b/src/test/ui/async-await/issue-61076.stderr @@ -2,25 +2,27 @@ error[E0277]: the `?` operator can only be applied to values that implement `Try --> $DIR/issue-61076.rs:42:5 | LL | foo()?; - | ^^^^^^ - | | - | the `?` operator cannot be applied to type `impl Future` - | help: consider using `.await` here: `foo().await?` + | ^^^^^^ the `?` operator cannot be applied to type `impl Future` | = help: the trait `Try` is not implemented for `impl Future` = note: required by `into_result` +help: consider `await`ing on the `Future` + | +LL | foo().await?; + | ^^^^^^ error[E0277]: the `?` operator can only be applied to values that implement `Try` --> $DIR/issue-61076.rs:56:5 | LL | t?; - | ^^ - | | - | the `?` operator cannot be applied to type `T` - | help: consider using `.await` here: `t.await?` + | ^^ the `?` operator cannot be applied to type `T` | = help: the trait `Try` is not implemented for `T` = note: required by `into_result` +help: consider `await`ing on the `Future` + | +LL | t.await?; + | ^^^^^^ error[E0609]: no field `0` on type `impl Future` --> $DIR/issue-61076.rs:58:26 diff --git a/src/test/ui/async-await/suggest-missing-await-closure.fixed b/src/test/ui/async-await/suggest-missing-await-closure.fixed index 37b30ffe6800f..febcd02184261 100644 --- a/src/test/ui/async-await/suggest-missing-await-closure.fixed +++ b/src/test/ui/async-await/suggest-missing-await-closure.fixed @@ -15,8 +15,8 @@ async fn suggest_await_in_async_closure() { let x = make_u32(); take_u32(x.await) //~^ ERROR mismatched types [E0308] - //~| HELP consider using `.await` here - //~| SUGGESTION x.await + //~| HELP consider `await`ing on the `Future` + //~| SUGGESTION .await }; } diff --git a/src/test/ui/async-await/suggest-missing-await-closure.rs b/src/test/ui/async-await/suggest-missing-await-closure.rs index 18076a1516171..faabf6ee3f16f 100644 --- a/src/test/ui/async-await/suggest-missing-await-closure.rs +++ b/src/test/ui/async-await/suggest-missing-await-closure.rs @@ -15,8 +15,8 @@ async fn suggest_await_in_async_closure() { let x = make_u32(); take_u32(x) //~^ ERROR mismatched types [E0308] - //~| HELP consider using `.await` here - //~| SUGGESTION x.await + //~| HELP consider `await`ing on the `Future` + //~| SUGGESTION .await }; } diff --git a/src/test/ui/async-await/suggest-missing-await-closure.stderr b/src/test/ui/async-await/suggest-missing-await-closure.stderr index ed2c4cbfccc98..2151057aa7fc0 100644 --- a/src/test/ui/async-await/suggest-missing-await-closure.stderr +++ b/src/test/ui/async-await/suggest-missing-await-closure.stderr @@ -5,13 +5,14 @@ LL | async fn make_u32() -> u32 { | --- the `Output` of this `async fn`'s found opaque type ... LL | take_u32(x) - | ^ - | | - | expected `u32`, found opaque type - | help: consider using `.await` here: `x.await` + | ^ expected `u32`, found opaque type | = note: expected type `u32` found opaque type `impl Future` +help: consider `await`ing on the `Future` + | +LL | take_u32(x.await) + | ^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/async-await/suggest-missing-await.fixed b/src/test/ui/async-await/suggest-missing-await.fixed index 1ec59d906206e..e548fda7cb4f5 100644 --- a/src/test/ui/async-await/suggest-missing-await.fixed +++ b/src/test/ui/async-await/suggest-missing-await.fixed @@ -12,8 +12,8 @@ async fn suggest_await_in_async_fn() { let x = make_u32(); take_u32(x.await) //~^ ERROR mismatched types [E0308] - //~| HELP consider using `.await` here - //~| SUGGESTION x.await + //~| HELP consider `await`ing on the `Future` + //~| SUGGESTION .await } async fn dummy() {} @@ -23,8 +23,8 @@ async fn suggest_await_in_async_fn_return() { dummy().await; //~^ ERROR mismatched types [E0308] //~| HELP try adding a semicolon - //~| HELP consider using `.await` here - //~| SUGGESTION dummy().await + //~| HELP consider `await`ing on the `Future` + //~| SUGGESTION .await } fn main() {} diff --git a/src/test/ui/async-await/suggest-missing-await.rs b/src/test/ui/async-await/suggest-missing-await.rs index 70cc1f1d5a2c6..464a9602119e1 100644 --- a/src/test/ui/async-await/suggest-missing-await.rs +++ b/src/test/ui/async-await/suggest-missing-await.rs @@ -12,8 +12,8 @@ async fn suggest_await_in_async_fn() { let x = make_u32(); take_u32(x) //~^ ERROR mismatched types [E0308] - //~| HELP consider using `.await` here - //~| SUGGESTION x.await + //~| HELP consider `await`ing on the `Future` + //~| SUGGESTION .await } async fn dummy() {} @@ -23,8 +23,8 @@ async fn suggest_await_in_async_fn_return() { dummy() //~^ ERROR mismatched types [E0308] //~| HELP try adding a semicolon - //~| HELP consider using `.await` here - //~| SUGGESTION dummy().await + //~| HELP consider `await`ing on the `Future` + //~| SUGGESTION .await } fn main() {} diff --git a/src/test/ui/async-await/suggest-missing-await.stderr b/src/test/ui/async-await/suggest-missing-await.stderr index c6355680e253b..d729420e93019 100644 --- a/src/test/ui/async-await/suggest-missing-await.stderr +++ b/src/test/ui/async-await/suggest-missing-await.stderr @@ -5,13 +5,14 @@ LL | async fn make_u32() -> u32 { | --- the `Output` of this `async fn`'s found opaque type ... LL | take_u32(x) - | ^ - | | - | expected `u32`, found opaque type - | help: consider using `.await` here: `x.await` + | ^ expected `u32`, found opaque type | = note: expected type `u32` found opaque type `impl Future` +help: consider `await`ing on the `Future` + | +LL | take_u32(x.await) + | ^^^^^^ error[E0308]: mismatched types --> $DIR/suggest-missing-await.rs:23:5 @@ -28,10 +29,10 @@ help: try adding a semicolon | LL | dummy(); | ^ -help: consider using `.await` here +help: consider `await`ing on the `Future` | LL | dummy().await - | + | ^^^^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/suggestions/issue-72766.stderr b/src/test/ui/suggestions/issue-72766.stderr index a1a5949b19660..5c9c549fa0779 100644 --- a/src/test/ui/suggestions/issue-72766.stderr +++ b/src/test/ui/suggestions/issue-72766.stderr @@ -2,13 +2,14 @@ error[E0277]: the `?` operator can only be applied to values that implement `Try --> $DIR/issue-72766.rs:14:5 | LL | SadGirl {}.call()?; - | ^^^^^^^^^^^^^^^^^^ - | | - | the `?` operator cannot be applied to type `impl Future` - | help: consider using `.await` here: `SadGirl {}.call().await?` + | ^^^^^^^^^^^^^^^^^^ the `?` operator cannot be applied to type `impl Future` | = help: the trait `Try` is not implemented for `impl Future` = note: required by `into_result` +help: consider `await`ing on the `Future` + | +LL | SadGirl {}.call().await?; + | ^^^^^^ error: aborting due to previous error From a4ee3ca1e4f1177516139a4704456958c7d08c91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 21 Oct 2020 19:08:28 -0700 Subject: [PATCH 09/25] Suggest semicolon removal on prior match arm --- compiler/rustc_typeck/src/check/_match.rs | 9 +++- .../match-prev-arm-needing-semi.rs | 32 +++++++++++ .../match-prev-arm-needing-semi.stderr | 53 +++++++++++++++++++ 3 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/suggestions/match-prev-arm-needing-semi.rs create mode 100644 src/test/ui/suggestions/match-prev-arm-needing-semi.stderr diff --git a/compiler/rustc_typeck/src/check/_match.rs b/compiler/rustc_typeck/src/check/_match.rs index 398e013e62fb5..94e886be54dcf 100644 --- a/compiler/rustc_typeck/src/check/_match.rs +++ b/compiler/rustc_typeck/src/check/_match.rs @@ -188,11 +188,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } } else { - let (arm_span, semi_span) = if let hir::ExprKind::Block(blk, _) = &arm.body.kind { + let (arm_span, mut semi_span) = if let hir::ExprKind::Block(blk, _) = &arm.body.kind + { self.find_block_span(blk, prior_arm_ty) } else { (arm.body.span, None) }; + if semi_span.is_none() && i > 0 { + if let hir::ExprKind::Block(blk, _) = &arms[i - 1].body.kind { + let (_, semi_span_prev) = self.find_block_span(blk, Some(arm_ty)); + semi_span = semi_span_prev; + } + } let (span, code) = match i { // The reason for the first arm to fail is not that the match arms diverge, // but rather that there's a prior obligation that doesn't hold. diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.rs b/src/test/ui/suggestions/match-prev-arm-needing-semi.rs new file mode 100644 index 0000000000000..d8d6de4bf5591 --- /dev/null +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.rs @@ -0,0 +1,32 @@ +// edition:2018 + +fn dummy() -> i32 { 42 } + +fn extra_semicolon() { + let _ = match true { //~ NOTE `match` arms have incompatible types + true => { + dummy(); //~ NOTE this is found to be + //~^ HELP consider removing this semicolon + } + false => dummy(), //~ ERROR `match` arms have incompatible types + //~^ NOTE expected `()`, found `i32` + }; +} + +async fn async_dummy() {} //~ NOTE the `Output` of this `async fn`'s found opaque type + +async fn async_extra_semicolon_same() { + let _ = match true { //~ NOTE `match` arms have incompatible types + true => { + async_dummy(); //~ NOTE this is found to be + //~^ HELP consider removing this semicolon + } + false => async_dummy(), //~ ERROR `match` arms have incompatible types + //~^ NOTE expected `()`, found opaque type + //~| NOTE expected type `()` + //~| HELP consider `await`ing on the `Future` + }; +} + +fn main() {} + diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr new file mode 100644 index 0000000000000..e242a018843a2 --- /dev/null +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr @@ -0,0 +1,53 @@ +error[E0308]: `match` arms have incompatible types + --> $DIR/match-prev-arm-needing-semi.rs:24:18 + | +LL | async fn async_dummy() {} + | - the `Output` of this `async fn`'s found opaque type +... +LL | let _ = match true { + | _____________- +LL | | true => { +LL | | async_dummy(); + | | -------------- this is found to be of type `()` +LL | | +LL | | } +LL | | false => async_dummy(), + | | ^^^^^^^^^^^^^ expected `()`, found opaque type +... | +LL | | +LL | | }; + | |_____- `match` arms have incompatible types + | + = note: expected type `()` + found opaque type `impl Future` +help: consider removing this semicolon + | +LL | async_dummy() + | -- +help: consider `await`ing on the `Future` + | +LL | false => async_dummy().await, + | ^^^^^^ + +error[E0308]: `match` arms have incompatible types + --> $DIR/match-prev-arm-needing-semi.rs:11:18 + | +LL | let _ = match true { + | _____________- +LL | | true => { +LL | | dummy(); + | | -------- + | | | | + | | | help: consider removing this semicolon + | | this is found to be of type `()` +LL | | +LL | | } +LL | | false => dummy(), + | | ^^^^^^^ expected `()`, found `i32` +LL | | +LL | | }; + | |_____- `match` arms have incompatible types + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. From 671d7c4afb36a7dcedf9ec8a6f3ef00c19bfc260 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 21 Oct 2020 19:43:15 -0700 Subject: [PATCH 10/25] Account for possible boxable `impl Future` in semicolon removal suggestions --- .../src/infer/error_reporting/mod.rs | 46 ++++++++++----- compiler/rustc_middle/src/traits/mod.rs | 4 +- compiler/rustc_typeck/src/check/_match.rs | 2 +- .../rustc_typeck/src/check/fn_ctxt/_impl.rs | 57 +++++++++++++++++-- .../rustc_typeck/src/check/fn_ctxt/checks.rs | 23 +++++--- .../match-prev-arm-needing-semi.rs | 15 ++++- .../match-prev-arm-needing-semi.stderr | 37 +++++++++++- 7 files changed, 152 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index 3a0ec6327c186..f6aa3c3d5ba6b 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -688,13 +688,22 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { }; let msg = "`match` arms have incompatible types"; err.span_label(outer_error_span, msg); - if let Some(sp) = semi_span { - err.span_suggestion_short( - sp, - "consider removing this semicolon", - String::new(), - Applicability::MachineApplicable, - ); + if let Some((sp, boxed)) = semi_span { + if boxed { + err.span_suggestion_verbose( + sp, + "consider removing this semicolon and boxing the expression", + String::new(), + Applicability::HasPlaceholders, + ); + } else { + err.span_suggestion_short( + sp, + "consider removing this semicolon", + String::new(), + Applicability::MachineApplicable, + ); + } } if let Some(ret_sp) = opt_suggest_box_span { // Get return type span and point to it. @@ -717,13 +726,22 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { if let Some(sp) = outer { err.span_label(sp, "`if` and `else` have incompatible types"); } - if let Some(sp) = semicolon { - err.span_suggestion_short( - sp, - "consider removing this semicolon", - String::new(), - Applicability::MachineApplicable, - ); + if let Some((sp, boxed)) = semicolon { + if boxed { + err.span_suggestion_verbose( + sp, + "consider removing this semicolon and boxing the expression", + String::new(), + Applicability::HasPlaceholders, + ); + } else { + err.span_suggestion_short( + sp, + "consider removing this semicolon", + String::new(), + Applicability::MachineApplicable, + ); + } } if let Some(ret_sp) = opt_suggest_box_span { self.suggest_boxing_for_return_impl_trait( diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index bbc46b8d60835..daa3010abb544 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -344,7 +344,7 @@ static_assert_size!(ObligationCauseCode<'_>, 32); pub struct MatchExpressionArmCause<'tcx> { pub arm_span: Span, pub scrut_span: Span, - pub semi_span: Option, + pub semi_span: Option<(Span, bool)>, pub source: hir::MatchSource, pub prior_arms: Vec, pub last_ty: Ty<'tcx>, @@ -357,7 +357,7 @@ pub struct IfExpressionCause { pub then: Span, pub else_sp: Span, pub outer: Option, - pub semicolon: Option, + pub semicolon: Option<(Span, bool)>, pub opt_suggest_box_span: Option, } diff --git a/compiler/rustc_typeck/src/check/_match.rs b/compiler/rustc_typeck/src/check/_match.rs index 94e886be54dcf..27c85317e82f9 100644 --- a/compiler/rustc_typeck/src/check/_match.rs +++ b/compiler/rustc_typeck/src/check/_match.rs @@ -521,7 +521,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &self, block: &'tcx hir::Block<'tcx>, expected_ty: Option>, - ) -> (Span, Option) { + ) -> (Span, Option<(Span, bool)>) { if let Some(expr) = &block.expr { (expr.span, None) } else if let Some(stmt) = block.stmts.last() { diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs index 017b0abd1d607..f26a168bcb288 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs @@ -1061,7 +1061,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &self, blk: &'tcx hir::Block<'tcx>, expected_ty: Ty<'tcx>, - ) -> Option { + ) -> Option<(Span, bool)> { // Be helpful when the user wrote `{... expr;}` and // taking the `;` off is enough to fix the error. let last_stmt = blk.stmts.last()?; @@ -1070,13 +1070,62 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { _ => return None, }; let last_expr_ty = self.node_ty(last_expr.hir_id); - if matches!(last_expr_ty.kind(), ty::Error(_)) - || self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err() + let needs_box = match (last_expr_ty.kind(), expected_ty.kind()) { + (ty::Opaque(last_def_id, last_bounds), ty::Opaque(exp_def_id, exp_bounds)) => { + debug!( + "both opaque, likely future {:?} {:?} {:?} {:?}", + last_def_id, last_bounds, exp_def_id, exp_bounds + ); + let last_hir_id = self.tcx.hir().local_def_id_to_hir_id(last_def_id.expect_local()); + let exp_hir_id = self.tcx.hir().local_def_id_to_hir_id(exp_def_id.expect_local()); + if let ( + hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: last_bounds, .. }), + hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: exp_bounds, .. }), + ) = ( + &self.tcx.hir().expect_item(last_hir_id).kind, + &self.tcx.hir().expect_item(exp_hir_id).kind, + ) { + debug!("{:?} {:?}", last_bounds, exp_bounds); + last_bounds.iter().zip(exp_bounds.iter()).all(|(left, right)| { + match (left, right) { + ( + hir::GenericBound::Trait(tl, ml), + hir::GenericBound::Trait(tr, mr), + ) => { + tl.trait_ref.trait_def_id() == tr.trait_ref.trait_def_id() + && ml == mr + } + ( + hir::GenericBound::LangItemTrait(langl, _, _, argsl), + hir::GenericBound::LangItemTrait(langr, _, _, argsr), + ) => { + // FIXME: consider the bounds! + debug!("{:?} {:?}", argsl, argsr); + langl == langr + } + _ => false, + } + }) + } else { + false + } + } + _ => false, + }; + debug!( + "needs_box {:?} {:?} {:?}", + needs_box, + last_expr_ty.kind(), + self.can_sub(self.param_env, last_expr_ty, expected_ty) + ); + if (matches!(last_expr_ty.kind(), ty::Error(_)) + || self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err()) + && !needs_box { return None; } let original_span = original_sp(last_stmt.span, blk.span); - Some(original_span.with_lo(original_span.hi() - BytePos(1))) + Some((original_span.with_lo(original_span.hi() - BytePos(1)), needs_box)) } // Instantiates the given path, which must refer to an item with the given diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index fd2f5eb5018d4..a124ad16612b1 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -758,13 +758,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expected_ty: Ty<'tcx>, err: &mut DiagnosticBuilder<'_>, ) { - if let Some(span_semi) = self.could_remove_semicolon(blk, expected_ty) { - err.span_suggestion( - span_semi, - "consider removing this semicolon", - String::new(), - Applicability::MachineApplicable, - ); + if let Some((span_semi, boxed)) = self.could_remove_semicolon(blk, expected_ty) { + if boxed { + err.span_suggestion_verbose( + span_semi, + "consider removing this semicolon and boxing the expression", + String::new(), + Applicability::HasPlaceholders, + ); + } else { + err.span_suggestion_short( + span_semi, + "consider removing this semicolon", + String::new(), + Applicability::MachineApplicable, + ); + } } } diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.rs b/src/test/ui/suggestions/match-prev-arm-needing-semi.rs index d8d6de4bf5591..9704242e10541 100644 --- a/src/test/ui/suggestions/match-prev-arm-needing-semi.rs +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.rs @@ -14,6 +14,7 @@ fn extra_semicolon() { } async fn async_dummy() {} //~ NOTE the `Output` of this `async fn`'s found opaque type +async fn async_dummy2() {} //~ NOTE the `Output` of this `async fn`'s found opaque type async fn async_extra_semicolon_same() { let _ = match true { //~ NOTE `match` arms have incompatible types @@ -28,5 +29,17 @@ async fn async_extra_semicolon_same() { }; } -fn main() {} +async fn async_extra_semicolon_different() { + let _ = match true { //~ NOTE `match` arms have incompatible types + true => { + async_dummy(); //~ NOTE this is found to be + //~^ HELP consider removing this semicolon + } + false => async_dummy2(), //~ ERROR `match` arms have incompatible types + //~^ NOTE expected `()`, found opaque type + //~| NOTE expected type `()` + //~| HELP consider `await`ing on the `Future` + }; +} +fn main() {} diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr index e242a018843a2..00b02fbc0072f 100644 --- a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr @@ -1,5 +1,5 @@ error[E0308]: `match` arms have incompatible types - --> $DIR/match-prev-arm-needing-semi.rs:24:18 + --> $DIR/match-prev-arm-needing-semi.rs:25:18 | LL | async fn async_dummy() {} | - the `Output` of this `async fn`'s found opaque type @@ -20,7 +20,7 @@ LL | | }; | = note: expected type `()` found opaque type `impl Future` -help: consider removing this semicolon +help: consider removing this semicolon and boxing the expression | LL | async_dummy() | -- @@ -29,6 +29,37 @@ help: consider `await`ing on the `Future` LL | false => async_dummy().await, | ^^^^^^ +error[E0308]: `match` arms have incompatible types + --> $DIR/match-prev-arm-needing-semi.rs:38:18 + | +LL | async fn async_dummy2() {} + | - the `Output` of this `async fn`'s found opaque type +... +LL | let _ = match true { + | _____________- +LL | | true => { +LL | | async_dummy(); + | | -------------- this is found to be of type `()` +LL | | +LL | | } +LL | | false => async_dummy2(), + | | ^^^^^^^^^^^^^^ expected `()`, found opaque type +... | +LL | | +LL | | }; + | |_____- `match` arms have incompatible types + | + = note: expected type `()` + found opaque type `impl Future` +help: consider removing this semicolon and boxing the expression + | +LL | async_dummy() + | -- +help: consider `await`ing on the `Future` + | +LL | false => async_dummy2().await, + | ^^^^^^ + error[E0308]: `match` arms have incompatible types --> $DIR/match-prev-arm-needing-semi.rs:11:18 | @@ -48,6 +79,6 @@ LL | | LL | | }; | |_____- `match` arms have incompatible types -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0308`. From 62ba365195e37b0508dc35f73b55243cb1aef7f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 22 Oct 2020 10:27:40 -0700 Subject: [PATCH 11/25] Review comments: use newtype instead of `bool` --- .../src/infer/error_reporting/mod.rs | 5 +- compiler/rustc_middle/src/traits/mod.rs | 17 ++++++- compiler/rustc_typeck/src/check/_match.rs | 39 ++++++++++------ .../rustc_typeck/src/check/fn_ctxt/_impl.rs | 46 +++++++++---------- .../rustc_typeck/src/check/fn_ctxt/checks.rs | 4 +- 5 files changed, 68 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index f6aa3c3d5ba6b..9e93cfc27d0dd 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -54,6 +54,7 @@ use crate::infer::OriginalQueryValues; use crate::traits::error_reporting::report_object_safety_error; use crate::traits::{ IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode, + StatementAsExpression, }; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; @@ -689,7 +690,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let msg = "`match` arms have incompatible types"; err.span_label(outer_error_span, msg); if let Some((sp, boxed)) = semi_span { - if boxed { + if matches!(boxed, StatementAsExpression::NeedsBoxing) { err.span_suggestion_verbose( sp, "consider removing this semicolon and boxing the expression", @@ -727,7 +728,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { err.span_label(sp, "`if` and `else` have incompatible types"); } if let Some((sp, boxed)) = semicolon { - if boxed { + if matches!(boxed, StatementAsExpression::NeedsBoxing) { err.span_suggestion_verbose( sp, "consider removing this semicolon and boxing the expression", diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index daa3010abb544..4deb7225dcb61 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -340,11 +340,24 @@ impl ObligationCauseCode<'_> { #[cfg(target_arch = "x86_64")] static_assert_size!(ObligationCauseCode<'_>, 32); +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +pub enum StatementAsExpression { + CorrectType, + NeedsBoxing, +} + +impl<'tcx> ty::Lift<'tcx> for StatementAsExpression { + type Lifted = StatementAsExpression; + fn lift_to_tcx(self, _tcx: TyCtxt<'tcx>) -> Option { + Some(self) + } +} + #[derive(Clone, Debug, PartialEq, Eq, Hash, Lift)] pub struct MatchExpressionArmCause<'tcx> { pub arm_span: Span, pub scrut_span: Span, - pub semi_span: Option<(Span, bool)>, + pub semi_span: Option<(Span, StatementAsExpression)>, pub source: hir::MatchSource, pub prior_arms: Vec, pub last_ty: Ty<'tcx>, @@ -357,7 +370,7 @@ pub struct IfExpressionCause { pub then: Span, pub else_sp: Span, pub outer: Option, - pub semicolon: Option<(Span, bool)>, + pub semicolon: Option<(Span, StatementAsExpression)>, pub opt_suggest_box_span: Option, } diff --git a/compiler/rustc_typeck/src/check/_match.rs b/compiler/rustc_typeck/src/check/_match.rs index 27c85317e82f9..e8eea65137ff7 100644 --- a/compiler/rustc_typeck/src/check/_match.rs +++ b/compiler/rustc_typeck/src/check/_match.rs @@ -9,6 +9,7 @@ use rustc_trait_selection::opaque_types::InferCtxtExt as _; use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; use rustc_trait_selection::traits::{ IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode, + StatementAsExpression, }; impl<'a, 'tcx> FnCtxt<'a, 'tcx> { @@ -188,18 +189,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } } else { - let (arm_span, mut semi_span) = if let hir::ExprKind::Block(blk, _) = &arm.body.kind - { - self.find_block_span(blk, prior_arm_ty) - } else { - (arm.body.span, None) - }; - if semi_span.is_none() && i > 0 { - if let hir::ExprKind::Block(blk, _) = &arms[i - 1].body.kind { - let (_, semi_span_prev) = self.find_block_span(blk, Some(arm_ty)); - semi_span = semi_span_prev; - } - } + let (arm_span, semi_span) = + self.get_appropriate_arm_semicolon_removal_span(&arms, i, prior_arm_ty, arm_ty); let (span, code) = match i { // The reason for the first arm to fail is not that the match arms diverge, // but rather that there's a prior obligation that doesn't hold. @@ -249,6 +240,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { coercion.complete(self) } + fn get_appropriate_arm_semicolon_removal_span( + &self, + arms: &'tcx [hir::Arm<'tcx>], + i: usize, + prior_arm_ty: Option>, + arm_ty: Ty<'tcx>, + ) -> (Span, Option<(Span, StatementAsExpression)>) { + let arm = &arms[i]; + let (arm_span, mut semi_span) = if let hir::ExprKind::Block(blk, _) = &arm.body.kind { + self.find_block_span(blk, prior_arm_ty) + } else { + (arm.body.span, None) + }; + if semi_span.is_none() && i > 0 { + if let hir::ExprKind::Block(blk, _) = &arms[i - 1].body.kind { + let (_, semi_span_prev) = self.find_block_span(blk, Some(arm_ty)); + semi_span = semi_span_prev; + } + } + (arm_span, semi_span) + } + /// When the previously checked expression (the scrutinee) diverges, /// warn the user about the match arms being unreachable. fn warn_arms_when_scrutinee_diverges( @@ -521,7 +534,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &self, block: &'tcx hir::Block<'tcx>, expected_ty: Option>, - ) -> (Span, Option<(Span, bool)>) { + ) -> (Span, Option<(Span, StatementAsExpression)>) { if let Some(expr) = &block.expr { (expr.span, None) } else if let Some(stmt) = block.stmts.last() { diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs index f26a168bcb288..f87e6b607d46e 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs @@ -33,7 +33,9 @@ use rustc_span::{self, BytePos, MultiSpan, Span}; use rustc_trait_selection::infer::InferCtxtExt as _; use rustc_trait_selection::opaque_types::InferCtxtExt as _; use rustc_trait_selection::traits::error_reporting::InferCtxtExt as _; -use rustc_trait_selection::traits::{self, ObligationCauseCode, TraitEngine, TraitEngineExt}; +use rustc_trait_selection::traits::{ + self, ObligationCauseCode, StatementAsExpression, TraitEngine, TraitEngineExt, +}; use std::collections::hash_map::Entry; use std::slice; @@ -1061,7 +1063,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &self, blk: &'tcx hir::Block<'tcx>, expected_ty: Ty<'tcx>, - ) -> Option<(Span, bool)> { + ) -> Option<(Span, StatementAsExpression)> { // Be helpful when the user wrote `{... expr;}` and // taking the `;` off is enough to fix the error. let last_stmt = blk.stmts.last()?; @@ -1078,49 +1080,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); let last_hir_id = self.tcx.hir().local_def_id_to_hir_id(last_def_id.expect_local()); let exp_hir_id = self.tcx.hir().local_def_id_to_hir_id(exp_def_id.expect_local()); - if let ( - hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: last_bounds, .. }), - hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: exp_bounds, .. }), - ) = ( + match ( &self.tcx.hir().expect_item(last_hir_id).kind, &self.tcx.hir().expect_item(exp_hir_id).kind, ) { - debug!("{:?} {:?}", last_bounds, exp_bounds); - last_bounds.iter().zip(exp_bounds.iter()).all(|(left, right)| { + ( + hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: last_bounds, .. }), + hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: exp_bounds, .. }), + ) if last_bounds.iter().zip(exp_bounds.iter()).all(|(left, right)| { match (left, right) { ( hir::GenericBound::Trait(tl, ml), hir::GenericBound::Trait(tr, mr), - ) => { - tl.trait_ref.trait_def_id() == tr.trait_ref.trait_def_id() - && ml == mr + ) if tl.trait_ref.trait_def_id() == tr.trait_ref.trait_def_id() + && ml == mr => + { + true } ( hir::GenericBound::LangItemTrait(langl, _, _, argsl), hir::GenericBound::LangItemTrait(langr, _, _, argsr), - ) => { + ) if langl == langr => { // FIXME: consider the bounds! debug!("{:?} {:?}", argsl, argsr); - langl == langr + true } _ => false, } - }) - } else { - false + }) => + { + StatementAsExpression::NeedsBoxing + } + _ => StatementAsExpression::CorrectType, } } - _ => false, + _ => StatementAsExpression::CorrectType, }; - debug!( - "needs_box {:?} {:?} {:?}", - needs_box, - last_expr_ty.kind(), - self.can_sub(self.param_env, last_expr_ty, expected_ty) - ); if (matches!(last_expr_ty.kind(), ty::Error(_)) || self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err()) - && !needs_box + && matches!(needs_box, StatementAsExpression::CorrectType) { return None; } diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index a124ad16612b1..a820661d8432a 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -20,7 +20,7 @@ use rustc_middle::ty::{self, Ty}; use rustc_session::Session; use rustc_span::symbol::{sym, Ident}; use rustc_span::{self, MultiSpan, Span}; -use rustc_trait_selection::traits::{self, ObligationCauseCode}; +use rustc_trait_selection::traits::{self, ObligationCauseCode, StatementAsExpression}; use std::mem::replace; use std::slice; @@ -759,7 +759,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err: &mut DiagnosticBuilder<'_>, ) { if let Some((span_semi, boxed)) = self.could_remove_semicolon(blk, expected_ty) { - if boxed { + if let StatementAsExpression::NeedsBoxing = boxed { err.span_suggestion_verbose( span_semi, "consider removing this semicolon and boxing the expression", From 3a0227bc49397879b240373d84b2a80e05569724 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 22 Oct 2020 10:51:49 -0700 Subject: [PATCH 12/25] Silence unnecessary `await foo?` knock-down error --- .../rustc_parse/src/parser/diagnostics.rs | 8 +- .../incorrect-syntax-suggestions.rs | 5 +- .../incorrect-syntax-suggestions.stderr | 95 +++++++------------ 3 files changed, 40 insertions(+), 68 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 1ea01d95a134e..39e1256a57835 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -1207,7 +1207,13 @@ impl<'a> Parser<'a> { self.recover_await_prefix(await_sp)? }; let sp = self.error_on_incorrect_await(lo, hi, &expr, is_question); - let expr = self.mk_expr(lo.to(sp), ExprKind::Await(expr), attrs); + let kind = match expr.kind { + // Avoid knock-down errors as we don't know whether to interpret this as `foo().await?` + // or `foo()?.await` (the very reason we went with postfix syntax 😅). + ExprKind::Try(_) => ExprKind::Err, + _ => ExprKind::Await(expr), + }; + let expr = self.mk_expr(lo.to(sp), kind, attrs); self.maybe_recover_from_bad_qpath(expr, true) } diff --git a/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.rs b/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.rs index 337487fc80b0e..554ac673d5155 100644 --- a/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.rs +++ b/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.rs @@ -14,7 +14,6 @@ async fn foo2() -> Result<(), ()> { } async fn foo3() -> Result<(), ()> { let _ = await bar()?; //~ ERROR incorrect use of `await` - //~^ ERROR the `?` operator can only be applied to values that implement `Try` Ok(()) } async fn foo21() -> Result<(), ()> { @@ -60,9 +59,7 @@ fn foo10() -> Result<(), ()> { Ok(()) } fn foo11() -> Result<(), ()> { - let _ = await bar()?; //~ ERROR `await` is only allowed inside `async` functions and blocks - //~^ ERROR incorrect use of `await` - //~| ERROR the `?` operator can only be applied to values that implement `Try` + let _ = await bar()?; //~ ERROR incorrect use of `await` Ok(()) } fn foo12() -> Result<(), ()> { diff --git a/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr b/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr index d90ce0f1bb482..52615df6008ff 100644 --- a/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr +++ b/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr @@ -17,103 +17,103 @@ LL | let _ = await bar()?; | ^^^^^^^^^^^^ help: `await` is a postfix operation: `bar()?.await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:21:13 + --> $DIR/incorrect-syntax-suggestions.rs:20:13 | LL | let _ = await { bar() }; | ^^^^^^^^^^^^^^^ help: `await` is a postfix operation: `{ bar() }.await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:25:13 + --> $DIR/incorrect-syntax-suggestions.rs:24:13 | LL | let _ = await(bar()); | ^^^^^^^^^^^^ help: `await` is a postfix operation: `(bar()).await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:29:13 + --> $DIR/incorrect-syntax-suggestions.rs:28:13 | LL | let _ = await { bar() }?; | ^^^^^^^^^^^^^^^ help: `await` is a postfix operation: `{ bar() }.await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:33:14 + --> $DIR/incorrect-syntax-suggestions.rs:32:14 | LL | let _ = (await bar())?; | ^^^^^^^^^^^ help: `await` is a postfix operation: `bar().await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:37:24 + --> $DIR/incorrect-syntax-suggestions.rs:36:24 | LL | let _ = bar().await(); | ^^ help: `await` is not a method call, remove the parentheses error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:41:24 + --> $DIR/incorrect-syntax-suggestions.rs:40:24 | LL | let _ = bar().await()?; | ^^ help: `await` is not a method call, remove the parentheses error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:53:13 + --> $DIR/incorrect-syntax-suggestions.rs:52:13 | LL | let _ = await bar(); | ^^^^^^^^^^^ help: `await` is a postfix operation: `bar().await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:58:13 + --> $DIR/incorrect-syntax-suggestions.rs:57:13 | LL | let _ = await? bar(); | ^^^^^^^^^^^^ help: `await` is a postfix operation: `bar().await?` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:63:13 + --> $DIR/incorrect-syntax-suggestions.rs:62:13 | LL | let _ = await bar()?; | ^^^^^^^^^^^^ help: `await` is a postfix operation: `bar()?.await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:69:14 + --> $DIR/incorrect-syntax-suggestions.rs:66:14 | LL | let _ = (await bar())?; | ^^^^^^^^^^^ help: `await` is a postfix operation: `bar().await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:74:24 + --> $DIR/incorrect-syntax-suggestions.rs:71:24 | LL | let _ = bar().await(); | ^^ help: `await` is not a method call, remove the parentheses error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:79:24 + --> $DIR/incorrect-syntax-suggestions.rs:76:24 | LL | let _ = bar().await()?; | ^^ help: `await` is not a method call, remove the parentheses error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:107:13 + --> $DIR/incorrect-syntax-suggestions.rs:104:13 | LL | let _ = await!(bar()); | ^^^^^^^^^^^^^ help: `await` is a postfix operation: `bar().await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:111:13 + --> $DIR/incorrect-syntax-suggestions.rs:108:13 | LL | let _ = await!(bar())?; | ^^^^^^^^^^^^^ help: `await` is a postfix operation: `bar().await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:116:17 + --> $DIR/incorrect-syntax-suggestions.rs:113:17 | LL | let _ = await!(bar())?; | ^^^^^^^^^^^^^ help: `await` is a postfix operation: `bar().await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:124:17 + --> $DIR/incorrect-syntax-suggestions.rs:121:17 | LL | let _ = await!(bar())?; | ^^^^^^^^^^^^^ help: `await` is a postfix operation: `bar().await` error: expected expression, found `=>` - --> $DIR/incorrect-syntax-suggestions.rs:132:25 + --> $DIR/incorrect-syntax-suggestions.rs:129:25 | LL | match await { await => () } | ----- ^^ expected expression @@ -121,13 +121,13 @@ LL | match await { await => () } | while parsing this incorrect await expression error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:132:11 + --> $DIR/incorrect-syntax-suggestions.rs:129:11 | LL | match await { await => () } | ^^^^^^^^^^^^^^^^^^^^^ help: `await` is a postfix operation: `{ await => () }.await` error: expected one of `.`, `?`, `{`, or an operator, found `}` - --> $DIR/incorrect-syntax-suggestions.rs:135:1 + --> $DIR/incorrect-syntax-suggestions.rs:132:1 | LL | match await { await => () } | ----- - expected one of `.`, `?`, `{`, or an operator @@ -138,7 +138,7 @@ LL | } | ^ unexpected token error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:53:13 + --> $DIR/incorrect-syntax-suggestions.rs:52:13 | LL | fn foo9() -> Result<(), ()> { | ---- this is not `async` @@ -146,7 +146,7 @@ LL | let _ = await bar(); | ^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:58:13 + --> $DIR/incorrect-syntax-suggestions.rs:57:13 | LL | fn foo10() -> Result<(), ()> { | ----- this is not `async` @@ -154,15 +154,7 @@ LL | let _ = await? bar(); | ^^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:63:13 - | -LL | fn foo11() -> Result<(), ()> { - | ----- this is not `async` -LL | let _ = await bar()?; - | ^^^^^^^^^^^^ only allowed inside `async` functions and blocks - -error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:69:14 + --> $DIR/incorrect-syntax-suggestions.rs:66:14 | LL | fn foo12() -> Result<(), ()> { | ----- this is not `async` @@ -170,7 +162,7 @@ LL | let _ = (await bar())?; | ^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:74:13 + --> $DIR/incorrect-syntax-suggestions.rs:71:13 | LL | fn foo13() -> Result<(), ()> { | ----- this is not `async` @@ -178,7 +170,7 @@ LL | let _ = bar().await(); | ^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:79:13 + --> $DIR/incorrect-syntax-suggestions.rs:76:13 | LL | fn foo14() -> Result<(), ()> { | ----- this is not `async` @@ -186,7 +178,7 @@ LL | let _ = bar().await()?; | ^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:84:13 + --> $DIR/incorrect-syntax-suggestions.rs:81:13 | LL | fn foo15() -> Result<(), ()> { | ----- this is not `async` @@ -194,7 +186,7 @@ LL | let _ = bar().await; | ^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:88:13 + --> $DIR/incorrect-syntax-suggestions.rs:85:13 | LL | fn foo16() -> Result<(), ()> { | ----- this is not `async` @@ -202,7 +194,7 @@ LL | let _ = bar().await?; | ^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:93:17 + --> $DIR/incorrect-syntax-suggestions.rs:90:17 | LL | fn foo() -> Result<(), ()> { | --- this is not `async` @@ -210,7 +202,7 @@ LL | let _ = bar().await?; | ^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:100:17 + --> $DIR/incorrect-syntax-suggestions.rs:97:17 | LL | let foo = || { | -- this is not `async` @@ -218,7 +210,7 @@ LL | let _ = bar().await?; | ^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:116:17 + --> $DIR/incorrect-syntax-suggestions.rs:113:17 | LL | fn foo() -> Result<(), ()> { | --- this is not `async` @@ -226,36 +218,13 @@ LL | let _ = await!(bar())?; | ^^^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:124:17 + --> $DIR/incorrect-syntax-suggestions.rs:121:17 | LL | let foo = || { | -- this is not `async` LL | let _ = await!(bar())?; | ^^^^^^^^^^^^^ only allowed inside `async` functions and blocks -error[E0277]: the `?` operator can only be applied to values that implement `Try` - --> $DIR/incorrect-syntax-suggestions.rs:16:19 - | -LL | let _ = await bar()?; - | ^^^^^^ the `?` operator cannot be applied to type `impl Future` - | - = help: the trait `Try` is not implemented for `impl Future` - = note: required by `into_result` -help: consider `await`ing on the `Future` - | -LL | let _ = await bar().await?; - | ^^^^^^ - -error[E0277]: the `?` operator can only be applied to values that implement `Try` - --> $DIR/incorrect-syntax-suggestions.rs:63:19 - | -LL | let _ = await bar()?; - | ^^^^^^ the `?` operator cannot be applied to type `impl Future` - | - = help: the trait `Try` is not implemented for `impl Future` - = note: required by `into_result` - -error: aborting due to 36 previous errors +error: aborting due to 33 previous errors -Some errors have detailed explanations: E0277, E0728. -For more information about an error, try `rustc --explain E0277`. +For more information about this error, try `rustc --explain E0728`. From 1829b4a887553797d6fa018ae2518ca0d45b67ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 22 Oct 2020 11:34:46 -0700 Subject: [PATCH 13/25] Add test case for different `impl Future`s --- .../match-prev-arm-needing-semi.rs | 11 ++++++++ .../match-prev-arm-needing-semi.stderr | 28 +++++++++++++++++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.rs b/src/test/ui/suggestions/match-prev-arm-needing-semi.rs index 9704242e10541..81e6abf7d7ba4 100644 --- a/src/test/ui/suggestions/match-prev-arm-needing-semi.rs +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.rs @@ -15,6 +15,7 @@ fn extra_semicolon() { async fn async_dummy() {} //~ NOTE the `Output` of this `async fn`'s found opaque type async fn async_dummy2() {} //~ NOTE the `Output` of this `async fn`'s found opaque type +//~^ NOTE the `Output` of this `async fn`'s found opaque type async fn async_extra_semicolon_same() { let _ = match true { //~ NOTE `match` arms have incompatible types @@ -42,4 +43,14 @@ async fn async_extra_semicolon_different() { }; } +async fn async_different_futures() { + let _ = match true { //~ NOTE `match` arms have incompatible types + true => async_dummy(), //~ NOTE this is found to be + false => async_dummy2(), //~ ERROR `match` arms have incompatible types + //~^ NOTE expected opaque type, found a different opaque type + //~| NOTE expected type `impl Future` + //~| NOTE distinct uses of `impl Trait` result in different opaque types + }; +} + fn main() {} diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr index 00b02fbc0072f..8f4b860589efd 100644 --- a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr @@ -1,5 +1,5 @@ error[E0308]: `match` arms have incompatible types - --> $DIR/match-prev-arm-needing-semi.rs:25:18 + --> $DIR/match-prev-arm-needing-semi.rs:26:18 | LL | async fn async_dummy() {} | - the `Output` of this `async fn`'s found opaque type @@ -30,7 +30,7 @@ LL | false => async_dummy().await, | ^^^^^^ error[E0308]: `match` arms have incompatible types - --> $DIR/match-prev-arm-needing-semi.rs:38:18 + --> $DIR/match-prev-arm-needing-semi.rs:39:18 | LL | async fn async_dummy2() {} | - the `Output` of this `async fn`'s found opaque type @@ -60,6 +60,28 @@ help: consider `await`ing on the `Future` LL | false => async_dummy2().await, | ^^^^^^ +error[E0308]: `match` arms have incompatible types + --> $DIR/match-prev-arm-needing-semi.rs:49:18 + | +LL | async fn async_dummy2() {} + | - the `Output` of this `async fn`'s found opaque type +... +LL | let _ = match true { + | _____________- +LL | | true => async_dummy(), + | | ------------- this is found to be of type `impl Future` +LL | | false => async_dummy2(), + | | ^^^^^^^^^^^^^^ expected opaque type, found a different opaque type +LL | | +LL | | +LL | | +LL | | }; + | |_____- `match` arms have incompatible types + | + = note: expected type `impl Future` (opaque type at <$DIR/match-prev-arm-needing-semi.rs:16:24>) + found opaque type `impl Future` (opaque type at <$DIR/match-prev-arm-needing-semi.rs:17:25>) + = note: distinct uses of `impl Trait` result in different opaque types + error[E0308]: `match` arms have incompatible types --> $DIR/match-prev-arm-needing-semi.rs:11:18 | @@ -79,6 +101,6 @@ LL | | LL | | }; | |_____- `match` arms have incompatible types -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0308`. From c5485115dcbbb5a0837c2ac8cabd5ead8a3b8a66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 22 Oct 2020 19:03:36 -0700 Subject: [PATCH 14/25] Add more `.await` suggestions on E0308 --- .../src/infer/error_reporting/mod.rs | 157 +++++++++++++----- compiler/rustc_middle/src/ty/error.rs | 37 ++--- compiler/rustc_typeck/src/check/demand.rs | 1 - .../src/check/fn_ctxt/suggestions.rs | 79 --------- .../dont-suggest-missing-await.stderr | 4 + src/test/ui/async-await/issue-61076.stderr | 4 + .../async-await/suggest-missing-await.fixed | 30 ---- .../ui/async-await/suggest-missing-await.rs | 1 - .../async-await/suggest-missing-await.stderr | 12 +- .../match-prev-arm-needing-semi.rs | 1 + .../match-prev-arm-needing-semi.stderr | 23 ++- .../ui/suggestions/opaque-type-error.stderr | 6 + 12 files changed, 167 insertions(+), 188 deletions(-) delete mode 100644 src/test/ui/async-await/suggest-missing-await.fixed diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index 9e93cfc27d0dd..fcf21c61d8fd9 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -50,7 +50,6 @@ use super::region_constraints::GenericKind; use super::{InferCtxt, RegionVariableOrigin, SubregionOrigin, TypeTrace, ValuePairs}; use crate::infer; -use crate::infer::OriginalQueryValues; use crate::traits::error_reporting::report_object_safety_error; use crate::traits::{ IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode, @@ -65,7 +64,6 @@ use rustc_hir::def_id::DefId; use rustc_hir::lang_items::LangItem; use rustc_hir::{Item, ItemKind, Node}; use rustc_middle::ty::error::TypeError; -use rustc_middle::ty::ParamEnvAnd; use rustc_middle::ty::{ self, subst::{Subst, SubstsRef}, @@ -1621,6 +1619,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { Mismatch::Variable(exp_found) => Some(exp_found), Mismatch::Fixed(_) => None, }; + let exp_found = match terr { + // `terr` has more accurate type information than `exp_found` in match expressions. + ty::error::TypeError::Sorts(terr) + if exp_found.map_or(false, |ef| terr.found == ef.found) => + { + Some(*terr) + } + _ => exp_found, + }; + debug!("exp_found {:?} terr {:?}", exp_found, terr); if let Some(exp_found) = exp_found { self.suggest_as_ref_where_appropriate(span, &exp_found, diag); self.suggest_await_on_expect_found(cause, span, &exp_found, diag); @@ -1642,6 +1650,53 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { self.note_error_origin(diag, cause, exp_found); } + fn get_impl_future_output_ty(&self, ty: Ty<'tcx>) -> Option> { + if let ty::Opaque(def_id, substs) = ty.kind() { + let future_trait = self.tcx.require_lang_item(LangItem::Future, None); + // Future::Output + let item_def_id = self + .tcx + .associated_items(future_trait) + .in_definition_order() + .next() + .unwrap() + .def_id; + + let bounds = self.tcx.explicit_item_bounds(*def_id); + + for (predicate, _) in bounds { + let predicate = predicate.subst(self.tcx, substs); + if let ty::PredicateAtom::Projection(projection_predicate) = + predicate.skip_binders() + { + if projection_predicate.projection_ty.item_def_id == item_def_id { + // We don't account for multiple `Future::Output = Ty` contraints. + return Some(projection_predicate.ty); + } + } + } + } + None + } + + /// A possible error is to forget to add `.await` when using futures: + /// + /// ``` + /// async fn make_u32() -> u32 { + /// 22 + /// } + /// + /// fn take_u32(x: u32) {} + /// + /// async fn foo() { + /// let x = make_u32(); + /// take_u32(x); + /// } + /// ``` + /// + /// This routine checks if the found type `T` implements `Future` where `U` is the + /// expected type. If this is the case, and we are inside of an async body, it suggests adding + /// `.await` to the tail of the expression. fn suggest_await_on_expect_found( &self, cause: &ObligationCause<'tcx>, @@ -1651,50 +1706,76 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { ) { debug!( "suggest_await_on_expect_found: exp_span={:?}, expected_ty={:?}, found_ty={:?}", - exp_span, exp_found.expected, exp_found.found + exp_span, exp_found.expected, exp_found.found, ); - if let ty::Opaque(def_id, _) = *exp_found.expected.kind() { - let future_trait = self.tcx.require_lang_item(LangItem::Future, None); - // Future::Output - let item_def_id = self - .tcx - .associated_items(future_trait) - .in_definition_order() - .next() - .unwrap() - .def_id; + if let ObligationCauseCode::CompareImplMethodObligation { .. } = &cause.code { + return; + } - let projection_ty = self.tcx.projection_ty_from_predicates((def_id, item_def_id)); - if let Some(projection_ty) = projection_ty { - let projection_query = self.canonicalize_query( - &ParamEnvAnd { param_env: self.tcx.param_env(def_id), value: projection_ty }, - &mut OriginalQueryValues::default(), - ); - if let Ok(resp) = self.tcx.normalize_projection_ty(projection_query) { - let normalized_ty = resp.value.value.normalized_ty; - debug!("suggest_await_on_expect_found: normalized={:?}", normalized_ty); - if ty::TyS::same_type(normalized_ty, exp_found.found) { - let span = if let ObligationCauseCode::Pattern { - span, - origin_expr: _, - root_ty: _, - } = cause.code - { - // scrutinee's span - span.unwrap_or(exp_span) - } else { - exp_span - }; - diag.span_suggestion_verbose( - span.shrink_to_hi(), - "consider awaiting on the future", - ".await".to_string(), + match ( + self.get_impl_future_output_ty(exp_found.expected), + self.get_impl_future_output_ty(exp_found.found), + ) { + (Some(exp), Some(found)) if ty::TyS::same_type(exp, found) => match &cause.code { + ObligationCauseCode::IfExpression(box IfExpressionCause { then, .. }) => { + diag.multipart_suggestion( + "consider `await`ing on both `Future`s", + vec![ + (then.shrink_to_hi(), ".await".to_string()), + (exp_span.shrink_to_hi(), ".await".to_string()), + ], + Applicability::MaybeIncorrect, + ); + } + ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause { + prior_arms, + .. + }) => { + if let [.., arm_span] = &prior_arms[..] { + diag.multipart_suggestion( + "consider `await`ing on both `Future`s", + vec![ + (arm_span.shrink_to_hi(), ".await".to_string()), + (exp_span.shrink_to_hi(), ".await".to_string()), + ], Applicability::MaybeIncorrect, ); + } else { + diag.help("consider `await`ing on both `Future`s"); } } + _ => { + diag.help("consider `await`ing on both `Future`s"); + } + }, + (_, Some(ty)) if ty::TyS::same_type(exp_found.expected, ty) => { + let span = match cause.code { + // scrutinee's span + ObligationCauseCode::Pattern { span: Some(span), .. } => span, + _ => exp_span, + }; + diag.span_suggestion_verbose( + span.shrink_to_hi(), + "consider `await`ing on the `Future`", + ".await".to_string(), + Applicability::MaybeIncorrect, + ); + } + (Some(ty), _) if ty::TyS::same_type(ty, exp_found.found) => { + let span = match cause.code { + // scrutinee's span + ObligationCauseCode::Pattern { span: Some(span), .. } => span, + _ => exp_span, + }; + diag.span_suggestion_verbose( + span.shrink_to_hi(), + "consider `await`ing on the `Future`", + ".await".to_string(), + Applicability::MaybeIncorrect, + ); } + _ => {} } } diff --git a/compiler/rustc_middle/src/ty/error.rs b/compiler/rustc_middle/src/ty/error.rs index 235f8749cf917..5ec0ec0c56ad6 100644 --- a/compiler/rustc_middle/src/ty/error.rs +++ b/compiler/rustc_middle/src/ty/error.rs @@ -334,26 +334,15 @@ impl<'tcx> TyCtxt<'tcx> { debug!("note_and_explain_type_err err={:?} cause={:?}", err, cause); match err { Sorts(values) => { - let expected_str = values.expected.sort_string(self); - let found_str = values.found.sort_string(self); - if expected_str == found_str && expected_str == "closure" { - db.note("no two closures, even if identical, have the same type"); - db.help("consider boxing your closure and/or using it as a trait object"); - } - if expected_str == found_str && expected_str == "opaque type" { - // Issue #63167 - db.note("distinct uses of `impl Trait` result in different opaque types"); - let e_str = values.expected.to_string(); - let f_str = values.found.to_string(); - if e_str == f_str && &e_str == "impl std::future::Future" { - // FIXME: use non-string based check. - db.help( - "if both `Future`s have the same `Output` type, consider \ - `.await`ing on both of them", - ); - } - } match (values.expected.kind(), values.found.kind()) { + (ty::Closure(..), ty::Closure(..)) => { + db.note("no two closures, even if identical, have the same type"); + db.help("consider boxing your closure and/or using it as a trait object"); + } + (ty::Opaque(..), ty::Opaque(..)) => { + // Issue #63167 + db.note("distinct uses of `impl Trait` result in different opaque types"); + } (ty::Float(_), ty::Infer(ty::IntVar(_))) => { if let Ok( // Issue #53280 @@ -382,12 +371,12 @@ impl<'tcx> TyCtxt<'tcx> { } db.note( "a type parameter was expected, but a different one was found; \ - you might be missing a type parameter or trait bound", + you might be missing a type parameter or trait bound", ); db.note( "for more information, visit \ - https://doc.rust-lang.org/book/ch10-02-traits.html\ - #traits-as-parameters", + https://doc.rust-lang.org/book/ch10-02-traits.html\ + #traits-as-parameters", ); } (ty::Projection(_), ty::Projection(_)) => { @@ -471,8 +460,8 @@ impl Trait for X { } db.note( "for more information, visit \ - https://doc.rust-lang.org/book/ch10-02-traits.html\ - #traits-as-parameters", + https://doc.rust-lang.org/book/ch10-02-traits.html\ + #traits-as-parameters", ); } (ty::Param(p), ty::Closure(..) | ty::Generator(..)) => { diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index b8143787a2ddf..241803fab1e68 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -33,7 +33,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return; } self.suggest_boxing_when_appropriate(err, expr, expected, expr_ty); - self.suggest_missing_await(err, expr, expected, expr_ty); self.suggest_missing_parentheses(err, expr); self.note_need_for_fn_pointer(err, expected, expr_ty); self.note_internal_mutation_in_method(err, expr, expected, expr_ty); diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs index 18db8d63850b2..a8ad9f4fdf8af 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs @@ -3,7 +3,6 @@ use crate::astconv::AstConv; use rustc_ast::util::parser::ExprPrecedence; use rustc_span::{self, Span}; -use rustc_trait_selection::traits; use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_hir as hir; @@ -13,7 +12,6 @@ use rustc_hir::{ExprKind, ItemKind, Node}; use rustc_infer::infer; use rustc_middle::ty::{self, Ty}; use rustc_span::symbol::kw; -use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _; use std::iter; @@ -433,83 +431,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - /// A possible error is to forget to add `.await` when using futures: - /// - /// ``` - /// async fn make_u32() -> u32 { - /// 22 - /// } - /// - /// fn take_u32(x: u32) {} - /// - /// async fn foo() { - /// let x = make_u32(); - /// take_u32(x); - /// } - /// ``` - /// - /// This routine checks if the found type `T` implements `Future` where `U` is the - /// expected type. If this is the case, and we are inside of an async body, it suggests adding - /// `.await` to the tail of the expression. - pub(in super::super) fn suggest_missing_await( - &self, - err: &mut DiagnosticBuilder<'_>, - expr: &hir::Expr<'_>, - expected: Ty<'tcx>, - found: Ty<'tcx>, - ) { - debug!("suggest_missing_await: expr={:?} expected={:?}, found={:?}", expr, expected, found); - // `.await` is not permitted outside of `async` bodies, so don't bother to suggest if the - // body isn't `async`. - let item_id = self.tcx().hir().get_parent_node(self.body_id); - if let Some(body_id) = self.tcx().hir().maybe_body_owned_by(item_id) { - let body = self.tcx().hir().body(body_id); - if let Some(hir::GeneratorKind::Async(_)) = body.generator_kind { - let sp = expr.span; - // Check for `Future` implementations by constructing a predicate to - // prove: `::Output == U` - let future_trait = self.tcx.require_lang_item(LangItem::Future, Some(sp)); - let item_def_id = self - .tcx - .associated_items(future_trait) - .in_definition_order() - .next() - .unwrap() - .def_id; - // `::Output` - let projection_ty = ty::ProjectionTy { - // `T` - substs: self - .tcx - .mk_substs_trait(found, self.fresh_substs_for_item(sp, item_def_id)), - // `Future::Output` - item_def_id, - }; - - let predicate = ty::PredicateAtom::Projection(ty::ProjectionPredicate { - projection_ty, - ty: expected, - }) - .potentially_quantified(self.tcx, ty::PredicateKind::ForAll); - let obligation = traits::Obligation::new(self.misc(sp), self.param_env, predicate); - - debug!("suggest_missing_await: trying obligation {:?}", obligation); - - if self.infcx.predicate_may_hold(&obligation) { - debug!("suggest_missing_await: obligation held: {:?}", obligation); - err.span_suggestion_verbose( - sp.shrink_to_hi(), - "consider `await`ing on the `Future`", - ".await".to_string(), - Applicability::MaybeIncorrect, - ); - } else { - debug!("suggest_missing_await: obligation did not hold: {:?}", obligation) - } - } - } - } - pub(in super::super) fn suggest_missing_parentheses( &self, err: &mut DiagnosticBuilder<'_>, diff --git a/src/test/ui/async-await/dont-suggest-missing-await.stderr b/src/test/ui/async-await/dont-suggest-missing-await.stderr index e70ed9badbd33..14e72c2b1e7e2 100644 --- a/src/test/ui/async-await/dont-suggest-missing-await.stderr +++ b/src/test/ui/async-await/dont-suggest-missing-await.stderr @@ -9,6 +9,10 @@ LL | take_u32(x) | = note: expected type `u32` found opaque type `impl Future` +help: consider `await`ing on the `Future` + | +LL | take_u32(x.await) + | ^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/async-await/issue-61076.stderr b/src/test/ui/async-await/issue-61076.stderr index afba889f014fe..df54ac88acee1 100644 --- a/src/test/ui/async-await/issue-61076.stderr +++ b/src/test/ui/async-await/issue-61076.stderr @@ -53,6 +53,10 @@ LL | Tuple(_) => {} | = note: expected opaque type `impl Future` found struct `Tuple` +help: consider `await`ing on the `Future` + | +LL | match tuple().await { + | ^^^^^^ error: aborting due to 6 previous errors diff --git a/src/test/ui/async-await/suggest-missing-await.fixed b/src/test/ui/async-await/suggest-missing-await.fixed deleted file mode 100644 index e548fda7cb4f5..0000000000000 --- a/src/test/ui/async-await/suggest-missing-await.fixed +++ /dev/null @@ -1,30 +0,0 @@ -// edition:2018 -// run-rustfix - -fn take_u32(_x: u32) {} - -async fn make_u32() -> u32 { - 22 -} - -#[allow(unused)] -async fn suggest_await_in_async_fn() { - let x = make_u32(); - take_u32(x.await) - //~^ ERROR mismatched types [E0308] - //~| HELP consider `await`ing on the `Future` - //~| SUGGESTION .await -} - -async fn dummy() {} - -#[allow(unused)] -async fn suggest_await_in_async_fn_return() { - dummy().await; - //~^ ERROR mismatched types [E0308] - //~| HELP try adding a semicolon - //~| HELP consider `await`ing on the `Future` - //~| SUGGESTION .await -} - -fn main() {} diff --git a/src/test/ui/async-await/suggest-missing-await.rs b/src/test/ui/async-await/suggest-missing-await.rs index 464a9602119e1..d629054911dac 100644 --- a/src/test/ui/async-await/suggest-missing-await.rs +++ b/src/test/ui/async-await/suggest-missing-await.rs @@ -1,5 +1,4 @@ // edition:2018 -// run-rustfix fn take_u32(_x: u32) {} diff --git a/src/test/ui/async-await/suggest-missing-await.stderr b/src/test/ui/async-await/suggest-missing-await.stderr index d729420e93019..46615dae7e2ba 100644 --- a/src/test/ui/async-await/suggest-missing-await.stderr +++ b/src/test/ui/async-await/suggest-missing-await.stderr @@ -1,5 +1,5 @@ error[E0308]: mismatched types - --> $DIR/suggest-missing-await.rs:13:14 + --> $DIR/suggest-missing-await.rs:12:14 | LL | async fn make_u32() -> u32 { | --- the `Output` of this `async fn`'s found opaque type @@ -15,7 +15,7 @@ LL | take_u32(x.await) | ^^^^^^ error[E0308]: mismatched types - --> $DIR/suggest-missing-await.rs:23:5 + --> $DIR/suggest-missing-await.rs:22:5 | LL | async fn dummy() {} | - the `Output` of this `async fn`'s found opaque type @@ -25,14 +25,14 @@ LL | dummy() | = note: expected unit type `()` found opaque type `impl Future` -help: try adding a semicolon - | -LL | dummy(); - | ^ help: consider `await`ing on the `Future` | LL | dummy().await | ^^^^^^ +help: try adding a semicolon + | +LL | dummy(); + | ^ error: aborting due to 2 previous errors diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.rs b/src/test/ui/suggestions/match-prev-arm-needing-semi.rs index 81e6abf7d7ba4..b8ac030b0bbbe 100644 --- a/src/test/ui/suggestions/match-prev-arm-needing-semi.rs +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.rs @@ -46,6 +46,7 @@ async fn async_extra_semicolon_different() { async fn async_different_futures() { let _ = match true { //~ NOTE `match` arms have incompatible types true => async_dummy(), //~ NOTE this is found to be + //~| HELP consider `await`ing on both `Future`s false => async_dummy2(), //~ ERROR `match` arms have incompatible types //~^ NOTE expected opaque type, found a different opaque type //~| NOTE expected type `impl Future` diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr index 8f4b860589efd..7a4f74a1994ce 100644 --- a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr @@ -20,14 +20,14 @@ LL | | }; | = note: expected type `()` found opaque type `impl Future` -help: consider removing this semicolon and boxing the expression - | -LL | async_dummy() - | -- help: consider `await`ing on the `Future` | LL | false => async_dummy().await, | ^^^^^^ +help: consider removing this semicolon and boxing the expression + | +LL | async_dummy() + | -- error[E0308]: `match` arms have incompatible types --> $DIR/match-prev-arm-needing-semi.rs:39:18 @@ -51,17 +51,17 @@ LL | | }; | = note: expected type `()` found opaque type `impl Future` -help: consider removing this semicolon and boxing the expression - | -LL | async_dummy() - | -- help: consider `await`ing on the `Future` | LL | false => async_dummy2().await, | ^^^^^^ +help: consider removing this semicolon and boxing the expression + | +LL | async_dummy() + | -- error[E0308]: `match` arms have incompatible types - --> $DIR/match-prev-arm-needing-semi.rs:49:18 + --> $DIR/match-prev-arm-needing-semi.rs:50:18 | LL | async fn async_dummy2() {} | - the `Output` of this `async fn`'s found opaque type @@ -81,6 +81,11 @@ LL | | }; = note: expected type `impl Future` (opaque type at <$DIR/match-prev-arm-needing-semi.rs:16:24>) found opaque type `impl Future` (opaque type at <$DIR/match-prev-arm-needing-semi.rs:17:25>) = note: distinct uses of `impl Trait` result in different opaque types +help: consider `await`ing on both `Future`s + | +LL | true => async_dummy().await, +LL | false => async_dummy2().await, + | error[E0308]: `match` arms have incompatible types --> $DIR/match-prev-arm-needing-semi.rs:11:18 diff --git a/src/test/ui/suggestions/opaque-type-error.stderr b/src/test/ui/suggestions/opaque-type-error.stderr index a7c2b82942f05..d74076cbc9b8e 100644 --- a/src/test/ui/suggestions/opaque-type-error.stderr +++ b/src/test/ui/suggestions/opaque-type-error.stderr @@ -16,6 +16,12 @@ LL | | }.await = note: expected type `impl Future` (opaque type at <$DIR/opaque-type-error.rs:8:19>) found opaque type `impl Future` (opaque type at <$DIR/opaque-type-error.rs:12:19>) = note: distinct uses of `impl Trait` result in different opaque types +help: consider `await`ing on both `Future`s + | +LL | thing_one().await +LL | } else { +LL | thing_two().await + | error: aborting due to previous error From f5d7443a6bd90b78e61b9c47e5032b5e1be1e49f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 22 Oct 2020 20:02:55 -0700 Subject: [PATCH 15/25] Suggest semicolon removal and boxing when appropriate --- .../src/infer/error_reporting/mod.rs | 35 ++++++++++++++----- .../match-prev-arm-needing-semi.stderr | 23 +++++++----- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index fcf21c61d8fd9..f7e4ace8fc5fc 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -688,12 +688,26 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let msg = "`match` arms have incompatible types"; err.span_label(outer_error_span, msg); if let Some((sp, boxed)) = semi_span { - if matches!(boxed, StatementAsExpression::NeedsBoxing) { - err.span_suggestion_verbose( + if let (StatementAsExpression::NeedsBoxing, [.., prior_arm]) = + (boxed, &prior_arms[..]) + { + err.multipart_suggestion( + "consider removing this semicolon and boxing the expressions", + vec![ + (prior_arm.shrink_to_lo(), "Box::new(".to_string()), + (prior_arm.shrink_to_hi(), ")".to_string()), + (arm_span.shrink_to_lo(), "Box::new(".to_string()), + (arm_span.shrink_to_hi(), ")".to_string()), + (sp, String::new()), + ], + Applicability::HasPlaceholders, + ); + } else if matches!(boxed, StatementAsExpression::NeedsBoxing) { + err.span_suggestion_short( sp, - "consider removing this semicolon and boxing the expression", + "consider removing this semicolon and boxing the expressions", String::new(), - Applicability::HasPlaceholders, + Applicability::MachineApplicable, ); } else { err.span_suggestion_short( @@ -727,11 +741,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { } if let Some((sp, boxed)) = semicolon { if matches!(boxed, StatementAsExpression::NeedsBoxing) { - err.span_suggestion_verbose( - sp, + err.multipart_suggestion( "consider removing this semicolon and boxing the expression", - String::new(), - Applicability::HasPlaceholders, + vec![ + (then.shrink_to_lo(), "Box::new(".to_string()), + (then.shrink_to_hi(), ")".to_string()), + (else_sp.shrink_to_lo(), "Box::new(".to_string()), + (else_sp.shrink_to_hi(), ")".to_string()), + (sp, String::new()), + ], + Applicability::MachineApplicable, ); } else { err.span_suggestion_short( diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr index 7a4f74a1994ce..e9803a78f94b3 100644 --- a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr @@ -24,10 +24,13 @@ help: consider `await`ing on the `Future` | LL | false => async_dummy().await, | ^^^^^^ -help: consider removing this semicolon and boxing the expression +help: consider removing this semicolon and boxing the expressions + | +LL | Box::new(async_dummy()) +LL | +LL | } +LL | false => Box::new(async_dummy()), | -LL | async_dummy() - | -- error[E0308]: `match` arms have incompatible types --> $DIR/match-prev-arm-needing-semi.rs:39:18 @@ -55,10 +58,13 @@ help: consider `await`ing on the `Future` | LL | false => async_dummy2().await, | ^^^^^^ -help: consider removing this semicolon and boxing the expression +help: consider removing this semicolon and boxing the expressions + | +LL | Box::new(async_dummy()) +LL | +LL | } +LL | false => Box::new(async_dummy2()), | -LL | async_dummy() - | -- error[E0308]: `match` arms have incompatible types --> $DIR/match-prev-arm-needing-semi.rs:50:18 @@ -70,10 +76,10 @@ LL | let _ = match true { | _____________- LL | | true => async_dummy(), | | ------------- this is found to be of type `impl Future` +LL | | LL | | false => async_dummy2(), | | ^^^^^^^^^^^^^^ expected opaque type, found a different opaque type -LL | | -LL | | +... | LL | | LL | | }; | |_____- `match` arms have incompatible types @@ -84,6 +90,7 @@ LL | | }; help: consider `await`ing on both `Future`s | LL | true => async_dummy().await, +LL | LL | false => async_dummy2().await, | From d413bb6f5716261f2740eb67540df1da1469ce12 Mon Sep 17 00:00:00 2001 From: chansuke Date: Sat, 18 Jul 2020 21:59:53 +0900 Subject: [PATCH 16/25] `#[deny(unsafe_op_in_unsafe_fn)]` in sys/wasm --- library/std/src/sys/wasm/alloc.rs | 8 +++--- library/std/src/sys/wasm/condvar_atomics.rs | 11 +++++--- library/std/src/sys/wasm/mod.rs | 2 ++ library/std/src/sys/wasm/mutex_atomics.rs | 29 +++++++++++++-------- 4 files changed, 32 insertions(+), 18 deletions(-) diff --git a/library/std/src/sys/wasm/alloc.rs b/library/std/src/sys/wasm/alloc.rs index 32b8b5bdaea7a..e12af19718a7c 100644 --- a/library/std/src/sys/wasm/alloc.rs +++ b/library/std/src/sys/wasm/alloc.rs @@ -25,25 +25,25 @@ unsafe impl GlobalAlloc for System { #[inline] unsafe fn alloc(&self, layout: Layout) -> *mut u8 { let _lock = lock::lock(); - DLMALLOC.malloc(layout.size(), layout.align()) + unsafe { DLMALLOC.malloc(layout.size(), layout.align()) } } #[inline] unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { let _lock = lock::lock(); - DLMALLOC.calloc(layout.size(), layout.align()) + unsafe { DLMALLOC.calloc(layout.size(), layout.align()) } } #[inline] unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { let _lock = lock::lock(); - DLMALLOC.free(ptr, layout.size(), layout.align()) + unsafe { DLMALLOC.free(ptr, layout.size(), layout.align()) } } #[inline] unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { let _lock = lock::lock(); - DLMALLOC.realloc(ptr, layout.size(), layout.align(), new_size) + unsafe { DLMALLOC.realloc(ptr, layout.size(), layout.align(), new_size) } } } diff --git a/library/std/src/sys/wasm/condvar_atomics.rs b/library/std/src/sys/wasm/condvar_atomics.rs index a96bb18e6ef1a..b9133e9fb7dbc 100644 --- a/library/std/src/sys/wasm/condvar_atomics.rs +++ b/library/std/src/sys/wasm/condvar_atomics.rs @@ -44,13 +44,18 @@ impl Condvar { pub unsafe fn notify_one(&self) { self.cnt.fetch_add(1, SeqCst); - wasm32::memory_atomic_notify(self.ptr(), 1); + // SAFETY: ptr() is always valid + unsafe { + wasm32::memory_atomic_notify(self.ptr(), 1); + } } #[inline] pub unsafe fn notify_all(&self) { - self.cnt.fetch_add(1, SeqCst); - wasm32::memory_atomic_notify(self.ptr(), u32::MAX); // -1 == "wake everyone" + unsafe { + self.cnt.fetch_add(1, SeqCst); + wasm32::memory_atomic_notify(self.ptr(), u32::MAX); // -1 == "wake everyone" + } } pub unsafe fn wait(&self, mutex: &Mutex) { diff --git a/library/std/src/sys/wasm/mod.rs b/library/std/src/sys/wasm/mod.rs index 11c6896f050b2..82683c0f624cf 100644 --- a/library/std/src/sys/wasm/mod.rs +++ b/library/std/src/sys/wasm/mod.rs @@ -14,6 +14,8 @@ //! compiling for wasm. That way it's a compile time error for something that's //! guaranteed to be a runtime error! +#![deny(unsafe_op_in_unsafe_fn)] + pub mod alloc; pub mod args; #[path = "../unsupported/cmath.rs"] diff --git a/library/std/src/sys/wasm/mutex_atomics.rs b/library/std/src/sys/wasm/mutex_atomics.rs index 2970fcf806cbf..11d7f4c389dec 100644 --- a/library/std/src/sys/wasm/mutex_atomics.rs +++ b/library/std/src/sys/wasm/mutex_atomics.rs @@ -28,11 +28,14 @@ impl Mutex { pub unsafe fn lock(&self) { while !self.try_lock() { - let val = wasm32::memory_atomic_wait32( - self.ptr(), - 1, // we expect our mutex is locked - -1, // wait infinitely - ); + // SAFETY: the caller must uphold the safety contract for `memory_atomic_wait32`. + let val = unsafe { + wasm32::memory_atomic_wait32( + self.ptr(), + 1, // we expect our mutex is locked + -1, // wait infinitely + ) + }; // we should have either woke up (0) or got a not-equal due to a // race (1). We should never time out (2) debug_assert!(val == 0 || val == 1); @@ -47,7 +50,7 @@ impl Mutex { #[inline] pub unsafe fn try_lock(&self) -> bool { - self.locked.compare_exchange(0, 1, SeqCst, SeqCst).is_ok() + unsafe { self.locked.compare_exchange(0, 1, SeqCst, SeqCst).is_ok() } } #[inline] @@ -83,7 +86,7 @@ unsafe impl Sync for ReentrantMutex {} impl ReentrantMutex { pub const unsafe fn uninitialized() -> ReentrantMutex { - ReentrantMutex { owner: AtomicU32::new(0), recursions: UnsafeCell::new(0) } + unsafe { ReentrantMutex { owner: AtomicU32::new(0), recursions: UnsafeCell::new(0) } } } pub unsafe fn init(&self) { @@ -93,19 +96,20 @@ impl ReentrantMutex { pub unsafe fn lock(&self) { let me = thread::my_id(); while let Err(owner) = self._try_lock(me) { - let val = wasm32::memory_atomic_wait32(self.ptr(), owner as i32, -1); + // SAFETY: the caller must gurantee that `self.ptr()` and `owner` are valid i32. + let val = unsafe { wasm32::memory_atomic_wait32(self.ptr(), owner as i32, -1) }; debug_assert!(val == 0 || val == 1); } } #[inline] pub unsafe fn try_lock(&self) -> bool { - self._try_lock(thread::my_id()).is_ok() + unsafe { self._try_lock(thread::my_id()).is_ok() } } #[inline] unsafe fn _try_lock(&self, id: u32) -> Result<(), u32> { - let id = id.checked_add(1).unwrap(); // make sure `id` isn't 0 + let id = id.checked_add(1).unwrap(); match self.owner.compare_exchange(0, id, SeqCst, SeqCst) { // we transitioned from unlocked to locked Ok(_) => { @@ -132,7 +136,10 @@ impl ReentrantMutex { match *self.recursions.get() { 0 => { self.owner.swap(0, SeqCst); - wasm32::memory_atomic_notify(self.ptr() as *mut i32, 1); // wake up one waiter, if any + // SAFETY: the caller must gurantee that `self.ptr()` is valid i32. + unsafe { + wasm32::atomic_notify(self.ptr() as *mut i32, 1); + } // wake up one waiter, if any } ref mut n => *n -= 1, } From eed45107da8da6293d9cecc302ad3b9870848b51 Mon Sep 17 00:00:00 2001 From: chansuke Date: Wed, 21 Oct 2020 08:15:28 +0900 Subject: [PATCH 17/25] Add some description for (malloc/calloc/free/realloc) --- library/std/src/sys/wasm/alloc.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/std/src/sys/wasm/alloc.rs b/library/std/src/sys/wasm/alloc.rs index e12af19718a7c..b85f4d50021e3 100644 --- a/library/std/src/sys/wasm/alloc.rs +++ b/library/std/src/sys/wasm/alloc.rs @@ -24,24 +24,28 @@ static mut DLMALLOC: dlmalloc::Dlmalloc = dlmalloc::DLMALLOC_INIT; unsafe impl GlobalAlloc for System { #[inline] unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + // SAFETY: DLMALLOC.malloc() is guranteed to be safe since lock::lock() aqcuire a globl lock let _lock = lock::lock(); unsafe { DLMALLOC.malloc(layout.size(), layout.align()) } } #[inline] unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { + // SAFETY: DLMALLOC.calloc() is guranteed to be safe since lock::lock() aqcuire a globl lock let _lock = lock::lock(); unsafe { DLMALLOC.calloc(layout.size(), layout.align()) } } #[inline] unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + // SAFETY: DLMALLOC.free() is guranteed to be safe since lock::lock() aqcuire a globl lock let _lock = lock::lock(); unsafe { DLMALLOC.free(ptr, layout.size(), layout.align()) } } #[inline] unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { + // SAFETY: DLMALLOC.realloc() is guranteed to be safe since lock::lock() aqcuire a globl lock let _lock = lock::lock(); unsafe { DLMALLOC.realloc(ptr, layout.size(), layout.align(), new_size) } } From de87ae79610925502f45ec07cf24bac51e037ed1 Mon Sep 17 00:00:00 2001 From: chansuke Date: Sat, 24 Oct 2020 17:59:58 +0900 Subject: [PATCH 18/25] Add documents for DLMALLOC --- library/std/src/sys/wasm/alloc.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/library/std/src/sys/wasm/alloc.rs b/library/std/src/sys/wasm/alloc.rs index b85f4d50021e3..b61a7872265f3 100644 --- a/library/std/src/sys/wasm/alloc.rs +++ b/library/std/src/sys/wasm/alloc.rs @@ -24,28 +24,32 @@ static mut DLMALLOC: dlmalloc::Dlmalloc = dlmalloc::DLMALLOC_INIT; unsafe impl GlobalAlloc for System { #[inline] unsafe fn alloc(&self, layout: Layout) -> *mut u8 { - // SAFETY: DLMALLOC.malloc() is guranteed to be safe since lock::lock() aqcuire a globl lock + // SAFETY: DLMALLOC access is guranteed to be safe because the lock gives us unique and non-reentrant access. + // Calling malloc() is safe because preconditions on this function match the trait method preconditions. let _lock = lock::lock(); unsafe { DLMALLOC.malloc(layout.size(), layout.align()) } } #[inline] unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { - // SAFETY: DLMALLOC.calloc() is guranteed to be safe since lock::lock() aqcuire a globl lock + // SAFETY: DLMALLOC access is guranteed to be safe because the lock gives us unique and non-reentrant access. + // Calling calloc() is safe because preconditions on this function match the trait method preconditions. let _lock = lock::lock(); unsafe { DLMALLOC.calloc(layout.size(), layout.align()) } } #[inline] unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { - // SAFETY: DLMALLOC.free() is guranteed to be safe since lock::lock() aqcuire a globl lock + // SAFETY: DLMALLOC access is guranteed to be safe because the lock gives us unique and non-reentrant access. + // Calling free() is safe because preconditions on this function match the trait method preconditions. let _lock = lock::lock(); unsafe { DLMALLOC.free(ptr, layout.size(), layout.align()) } } #[inline] unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { - // SAFETY: DLMALLOC.realloc() is guranteed to be safe since lock::lock() aqcuire a globl lock + // SAFETY: DLMALLOC access is guranteed to be safe because the lock gives us unique and non-reentrant access. + // Calling realloc() is safe because preconditions on this function match the trait method preconditions. let _lock = lock::lock(); unsafe { DLMALLOC.realloc(ptr, layout.size(), layout.align(), new_size) } } From d147f78e367386bf63ccb03d453e151e37cfdd81 Mon Sep 17 00:00:00 2001 From: chansuke Date: Sat, 24 Oct 2020 18:14:17 +0900 Subject: [PATCH 19/25] Fix unsafe operation of wasm32::memory_atomic_notify --- library/std/src/sys/wasm/condvar_atomics.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/wasm/condvar_atomics.rs b/library/std/src/sys/wasm/condvar_atomics.rs index b9133e9fb7dbc..c2c47910582a0 100644 --- a/library/std/src/sys/wasm/condvar_atomics.rs +++ b/library/std/src/sys/wasm/condvar_atomics.rs @@ -52,8 +52,9 @@ impl Condvar { #[inline] pub unsafe fn notify_all(&self) { + self.cnt.fetch_add(1, SeqCst); + // SAFETY: memory_atomic_notify()is always valid unsafe { - self.cnt.fetch_add(1, SeqCst); wasm32::memory_atomic_notify(self.ptr(), u32::MAX); // -1 == "wake everyone" } } From d37b8cf729187e5dcabb3650031eb806d1f79770 Mon Sep 17 00:00:00 2001 From: chansuke Date: Sat, 24 Oct 2020 18:22:18 +0900 Subject: [PATCH 20/25] Remove unnecessary unsafe block from condvar_atomics & mutex_atomics --- library/std/src/sys/wasm/condvar_atomics.rs | 2 +- library/std/src/sys/wasm/mutex_atomics.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library/std/src/sys/wasm/condvar_atomics.rs b/library/std/src/sys/wasm/condvar_atomics.rs index c2c47910582a0..0c1c076cc9142 100644 --- a/library/std/src/sys/wasm/condvar_atomics.rs +++ b/library/std/src/sys/wasm/condvar_atomics.rs @@ -53,7 +53,7 @@ impl Condvar { #[inline] pub unsafe fn notify_all(&self) { self.cnt.fetch_add(1, SeqCst); - // SAFETY: memory_atomic_notify()is always valid + // SAFETY: ptr() is always valid unsafe { wasm32::memory_atomic_notify(self.ptr(), u32::MAX); // -1 == "wake everyone" } diff --git a/library/std/src/sys/wasm/mutex_atomics.rs b/library/std/src/sys/wasm/mutex_atomics.rs index 11d7f4c389dec..479182ffa44d5 100644 --- a/library/std/src/sys/wasm/mutex_atomics.rs +++ b/library/std/src/sys/wasm/mutex_atomics.rs @@ -50,7 +50,7 @@ impl Mutex { #[inline] pub unsafe fn try_lock(&self) -> bool { - unsafe { self.locked.compare_exchange(0, 1, SeqCst, SeqCst).is_ok() } + self.locked.compare_exchange(0, 1, SeqCst, SeqCst).is_ok() } #[inline] @@ -86,7 +86,7 @@ unsafe impl Sync for ReentrantMutex {} impl ReentrantMutex { pub const unsafe fn uninitialized() -> ReentrantMutex { - unsafe { ReentrantMutex { owner: AtomicU32::new(0), recursions: UnsafeCell::new(0) } } + ReentrantMutex { owner: AtomicU32::new(0), recursions: UnsafeCell::new(0) } } pub unsafe fn init(&self) { From 7b4c397b73912d3c604667933fb7e64f0c1a366a Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Fri, 23 Oct 2020 18:00:18 +0900 Subject: [PATCH 21/25] Do not try to report on closures to avoid ICE --- .../nice_region_error/static_impl_trait.rs | 8 ++++++++ src/test/ui/regions/issue-78262.rs | 9 +++++++++ src/test/ui/regions/issue-78262.stderr | 18 ++++++++++++++++++ 3 files changed, 35 insertions(+) create mode 100644 src/test/ui/regions/issue-78262.rs create mode 100644 src/test/ui/regions/issue-78262.stderr diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs index 441cfeea20a48..e9d5ebad7de03 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs @@ -39,6 +39,14 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { ) if **sub_r == RegionKind::ReStatic => { // This is for an implicit `'static` requirement coming from `impl dyn Trait {}`. if let ObligationCauseCode::UnifyReceiver(ctxt) = &cause.code { + // This may have a closure and it would cause ICE + // through `find_param_with_region` (#78262). + let anon_reg_sup = tcx.is_suitable_region(sup_r)?; + let fn_returns = tcx.return_type_impl_or_dyn_traits(anon_reg_sup.def_id); + if fn_returns.is_empty() { + return None; + } + let param = self.find_param_with_region(sup_r, sub_r)?; let lifetime = if sup_r.has_name() { format!("lifetime `{}`", sup_r) diff --git a/src/test/ui/regions/issue-78262.rs b/src/test/ui/regions/issue-78262.rs new file mode 100644 index 0000000000000..2324152d2c081 --- /dev/null +++ b/src/test/ui/regions/issue-78262.rs @@ -0,0 +1,9 @@ +trait TT {} + +impl dyn TT { + fn func(&self) {} +} + +fn main() { + let f = |x: &dyn TT| x.func(); //~ ERROR: mismatched types +} diff --git a/src/test/ui/regions/issue-78262.stderr b/src/test/ui/regions/issue-78262.stderr new file mode 100644 index 0000000000000..580cea00ecd4f --- /dev/null +++ b/src/test/ui/regions/issue-78262.stderr @@ -0,0 +1,18 @@ +error[E0308]: mismatched types + --> $DIR/issue-78262.rs:8:28 + | +LL | let f = |x: &dyn TT| x.func(); + | ^^^^ lifetime mismatch + | + = note: expected reference `&(dyn TT + 'static)` + found reference `&dyn TT` +note: the anonymous lifetime #1 defined on the body at 8:13... + --> $DIR/issue-78262.rs:8:13 + | +LL | let f = |x: &dyn TT| x.func(); + | ^^^^^^^^^^^^^^^^^^^^^ + = note: ...does not necessarily outlive the static lifetime + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. From 4ec396ea5dd7bddfaa667766ab6cd8824c8028da Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Sun, 25 Oct 2020 11:43:26 +0900 Subject: [PATCH 22/25] Test with NLL explicitly --- .../{issue-78262.stderr => issue-78262.default.stderr} | 6 +++--- src/test/ui/regions/issue-78262.nll.stderr | 10 ++++++++++ src/test/ui/regions/issue-78262.rs | 7 ++++++- 3 files changed, 19 insertions(+), 4 deletions(-) rename src/test/ui/regions/{issue-78262.stderr => issue-78262.default.stderr} (79%) create mode 100644 src/test/ui/regions/issue-78262.nll.stderr diff --git a/src/test/ui/regions/issue-78262.stderr b/src/test/ui/regions/issue-78262.default.stderr similarity index 79% rename from src/test/ui/regions/issue-78262.stderr rename to src/test/ui/regions/issue-78262.default.stderr index 580cea00ecd4f..e97b8eca94892 100644 --- a/src/test/ui/regions/issue-78262.stderr +++ b/src/test/ui/regions/issue-78262.default.stderr @@ -1,13 +1,13 @@ error[E0308]: mismatched types - --> $DIR/issue-78262.rs:8:28 + --> $DIR/issue-78262.rs:12:28 | LL | let f = |x: &dyn TT| x.func(); | ^^^^ lifetime mismatch | = note: expected reference `&(dyn TT + 'static)` found reference `&dyn TT` -note: the anonymous lifetime #1 defined on the body at 8:13... - --> $DIR/issue-78262.rs:8:13 +note: the anonymous lifetime #1 defined on the body at 12:13... + --> $DIR/issue-78262.rs:12:13 | LL | let f = |x: &dyn TT| x.func(); | ^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/test/ui/regions/issue-78262.nll.stderr b/src/test/ui/regions/issue-78262.nll.stderr new file mode 100644 index 0000000000000..4607dbad4220b --- /dev/null +++ b/src/test/ui/regions/issue-78262.nll.stderr @@ -0,0 +1,10 @@ +error[E0521]: borrowed data escapes outside of closure + --> $DIR/issue-78262.rs:12:26 + | +LL | let f = |x: &dyn TT| x.func(); + | - ^^^^^^^^ `x` escapes the closure body here + | | + | `x` is a reference that is only valid in the closure body + +error: aborting due to previous error + diff --git a/src/test/ui/regions/issue-78262.rs b/src/test/ui/regions/issue-78262.rs index 2324152d2c081..0bdb0abac307d 100644 --- a/src/test/ui/regions/issue-78262.rs +++ b/src/test/ui/regions/issue-78262.rs @@ -1,3 +1,7 @@ +// revisions: nll default +// ignore-compare-mode-nll +//[nll]compile-flags: -Z borrowck=mir + trait TT {} impl dyn TT { @@ -5,5 +9,6 @@ impl dyn TT { } fn main() { - let f = |x: &dyn TT| x.func(); //~ ERROR: mismatched types + let f = |x: &dyn TT| x.func(); //[default]~ ERROR: mismatched types + //[nll]~^ ERROR: borrowed data escapes outside of closure } From 17a4c43a9cd46da279ddfc77ca49ecd9308f4bf8 Mon Sep 17 00:00:00 2001 From: Stein Somers Date: Sun, 25 Oct 2020 16:10:10 +0100 Subject: [PATCH 23/25] BTreeMap: move generic support functions out of navigate.rs --- library/alloc/src/collections/btree/mem.rs | 34 ++++++++++++++ library/alloc/src/collections/btree/mod.rs | 1 + .../alloc/src/collections/btree/navigate.rs | 47 +++---------------- 3 files changed, 42 insertions(+), 40 deletions(-) create mode 100644 library/alloc/src/collections/btree/mem.rs diff --git a/library/alloc/src/collections/btree/mem.rs b/library/alloc/src/collections/btree/mem.rs new file mode 100644 index 0000000000000..5e7d9fa3f91ba --- /dev/null +++ b/library/alloc/src/collections/btree/mem.rs @@ -0,0 +1,34 @@ +use core::intrinsics; +use core::mem; +use core::ptr; + +/// This replaces the value behind the `v` unique reference by calling the +/// relevant function. +/// +/// If a panic occurs in the `change` closure, the entire process will be aborted. +#[inline] +pub fn take_mut(v: &mut T, change: impl FnOnce(T) -> T) { + replace(v, |value| (change(value), ())) +} + +/// This replaces the value behind the `v` unique reference by calling the +/// relevant function, and returns a result obtained along the way. +/// +/// If a panic occurs in the `change` closure, the entire process will be aborted. +#[inline] +pub fn replace(v: &mut T, change: impl FnOnce(T) -> (T, R)) -> R { + struct PanicGuard; + impl Drop for PanicGuard { + fn drop(&mut self) { + intrinsics::abort() + } + } + let guard = PanicGuard; + let value = unsafe { ptr::read(v) }; + let (new_value, ret) = change(value); + unsafe { + ptr::write(v, new_value); + } + mem::forget(guard); + ret +} diff --git a/library/alloc/src/collections/btree/mod.rs b/library/alloc/src/collections/btree/mod.rs index bcc50ed561587..8e1ba175fba03 100644 --- a/library/alloc/src/collections/btree/mod.rs +++ b/library/alloc/src/collections/btree/mod.rs @@ -1,5 +1,6 @@ mod borrow; pub mod map; +mod mem; mod navigate; mod node; mod remove; diff --git a/library/alloc/src/collections/btree/navigate.rs b/library/alloc/src/collections/btree/navigate.rs index 55ce7d275464e..de78148fc82be 100644 --- a/library/alloc/src/collections/btree/navigate.rs +++ b/library/alloc/src/collections/btree/navigate.rs @@ -1,7 +1,5 @@ use core::borrow::Borrow; use core::cmp::Ordering; -use core::intrinsics; -use core::mem; use core::ops::Bound::{Excluded, Included, Unbounded}; use core::ops::RangeBounds; use core::ptr; @@ -304,37 +302,6 @@ macro_rules! def_next_kv_uncheched_dealloc { def_next_kv_uncheched_dealloc! {unsafe fn next_kv_unchecked_dealloc: right_kv} def_next_kv_uncheched_dealloc! {unsafe fn next_back_kv_unchecked_dealloc: left_kv} -/// This replaces the value behind the `v` unique reference by calling the -/// relevant function. -/// -/// If a panic occurs in the `change` closure, the entire process will be aborted. -#[inline] -fn take_mut(v: &mut T, change: impl FnOnce(T) -> T) { - replace(v, |value| (change(value), ())) -} - -/// This replaces the value behind the `v` unique reference by calling the -/// relevant function, and returns a result obtained along the way. -/// -/// If a panic occurs in the `change` closure, the entire process will be aborted. -#[inline] -fn replace(v: &mut T, change: impl FnOnce(T) -> (T, R)) -> R { - struct PanicGuard; - impl Drop for PanicGuard { - fn drop(&mut self) { - intrinsics::abort() - } - } - let guard = PanicGuard; - let value = unsafe { ptr::read(v) }; - let (new_value, ret) = change(value); - unsafe { - ptr::write(v, new_value); - } - mem::forget(guard); - ret -} - impl<'a, K, V> Handle, K, V, marker::Leaf>, marker::Edge> { /// Moves the leaf edge handle to the next leaf edge and returns references to the /// key and value in between. @@ -342,7 +309,7 @@ impl<'a, K, V> Handle, K, V, marker::Leaf>, marker::Ed /// # Safety /// There must be another KV in the direction travelled. pub unsafe fn next_unchecked(&mut self) -> (&'a K, &'a V) { - replace(self, |leaf_edge| { + super::mem::replace(self, |leaf_edge| { let kv = leaf_edge.next_kv(); let kv = unsafe { unwrap_unchecked(kv.ok()) }; (kv.next_leaf_edge(), kv.into_kv()) @@ -355,7 +322,7 @@ impl<'a, K, V> Handle, K, V, marker::Leaf>, marker::Ed /// # Safety /// There must be another KV in the direction travelled. pub unsafe fn next_back_unchecked(&mut self) -> (&'a K, &'a V) { - replace(self, |leaf_edge| { + super::mem::replace(self, |leaf_edge| { let kv = leaf_edge.next_back_kv(); let kv = unsafe { unwrap_unchecked(kv.ok()) }; (kv.next_back_leaf_edge(), kv.into_kv()) @@ -370,7 +337,7 @@ impl<'a, K, V> Handle, K, V, marker::Leaf>, marker::E /// # Safety /// There must be another KV in the direction travelled. pub unsafe fn next_unchecked(&mut self) -> (&'a K, &'a mut V) { - let kv = replace(self, |leaf_edge| { + let kv = super::mem::replace(self, |leaf_edge| { let kv = leaf_edge.next_kv(); let kv = unsafe { unwrap_unchecked(kv.ok()) }; (unsafe { ptr::read(&kv) }.next_leaf_edge(), kv) @@ -385,7 +352,7 @@ impl<'a, K, V> Handle, K, V, marker::Leaf>, marker::E /// # Safety /// There must be another KV in the direction travelled. pub unsafe fn next_back_unchecked(&mut self) -> (&'a K, &'a mut V) { - let kv = replace(self, |leaf_edge| { + let kv = super::mem::replace(self, |leaf_edge| { let kv = leaf_edge.next_back_kv(); let kv = unsafe { unwrap_unchecked(kv.ok()) }; (unsafe { ptr::read(&kv) }.next_back_leaf_edge(), kv) @@ -401,7 +368,7 @@ impl<'a, K, V> Handle, K, V, marker::Leaf>, marker::Edge /// # Safety /// There must be another KV in the direction travelled. pub unsafe fn move_next_unchecked(&mut self) { - take_mut(self, |leaf_edge| { + super::mem::take_mut(self, |leaf_edge| { let kv = leaf_edge.next_kv(); let kv = unsafe { unwrap_unchecked(kv.ok()) }; kv.next_leaf_edge() @@ -423,7 +390,7 @@ impl Handle, marker::Edge> { /// call this method again subject to its safety conditions, or call counterpart /// `next_back_unchecked` subject to its safety conditions. pub unsafe fn next_unchecked(&mut self) -> (K, V) { - replace(self, |leaf_edge| { + super::mem::replace(self, |leaf_edge| { let kv = unsafe { next_kv_unchecked_dealloc(leaf_edge) }; let k = unsafe { ptr::read(kv.reborrow().into_kv().0) }; let v = unsafe { ptr::read(kv.reborrow().into_kv().1) }; @@ -444,7 +411,7 @@ impl Handle, marker::Edge> { /// call this method again subject to its safety conditions, or call counterpart /// `next_unchecked` subject to its safety conditions. pub unsafe fn next_back_unchecked(&mut self) -> (K, V) { - replace(self, |leaf_edge| { + super::mem::replace(self, |leaf_edge| { let kv = unsafe { next_back_kv_unchecked_dealloc(leaf_edge) }; let k = unsafe { ptr::read(kv.reborrow().into_kv().0) }; let v = unsafe { ptr::read(kv.reborrow().into_kv().1) }; From 2df927720516cc70e0dea521e1728d08383c39c9 Mon Sep 17 00:00:00 2001 From: Stein Somers Date: Mon, 19 Oct 2020 10:23:11 +0200 Subject: [PATCH 24/25] BTreeMap: reuse NodeRef as Root, keep BoxedNode confined to edges --- library/alloc/src/collections/btree/map.rs | 2 +- library/alloc/src/collections/btree/node.rs | 116 +++++++++--------- .../alloc/src/collections/btree/node/tests.rs | 4 +- library/alloc/src/collections/btree/split.rs | 4 +- src/etc/gdb_providers.py | 17 +-- src/test/debuginfo/pretty-std-collections.rs | 4 +- 6 files changed, 75 insertions(+), 72 deletions(-) diff --git a/library/alloc/src/collections/btree/map.rs b/library/alloc/src/collections/btree/map.rs index 4f9aa44b6b510..ffbba4a6bff07 100644 --- a/library/alloc/src/collections/btree/map.rs +++ b/library/alloc/src/collections/btree/map.rs @@ -1418,7 +1418,7 @@ impl IntoIterator for BTreeMap { fn into_iter(self) -> IntoIter { let mut me = ManuallyDrop::new(self); if let Some(root) = me.root.take() { - let (f, b) = root.into_ref().full_range(); + let (f, b) = root.full_range(); IntoIter { front: Some(f), back: Some(b), length: me.length } } else { diff --git a/library/alloc/src/collections/btree/node.rs b/library/alloc/src/collections/btree/node.rs index f5aff9bf494e9..47da1f0b337d6 100644 --- a/library/alloc/src/collections/btree/node.rs +++ b/library/alloc/src/collections/btree/node.rs @@ -119,15 +119,11 @@ struct BoxedNode { } impl BoxedNode { - fn from_leaf(node: Box>) -> Self { - BoxedNode { ptr: Box::into_unique(node) } + fn from_owned(mut node: NonNull>) -> Self { + BoxedNode { ptr: unsafe { Unique::new_unchecked(node.as_mut()) } } } - fn from_internal(node: Box>) -> Self { - BoxedNode { ptr: Unique::from(&mut Box::leak(node).data) } - } - - fn as_ptr(&self) -> NonNull> { + fn as_nonnull(&self) -> NonNull> { NonNull::from(self.ptr) } } @@ -135,58 +131,59 @@ impl BoxedNode { /// An owned tree. /// /// Note that this does not have a destructor, and must be cleaned up manually. -pub struct Root { - node: BoxedNode, - /// The number of levels below the root node. - height: usize, -} +pub type Root = NodeRef; unsafe impl Sync for Root {} unsafe impl Send for Root {} impl Root { - /// Returns the number of levels below the root. - pub fn height(&self) -> usize { - self.height + /// Returns a new root node that is initially empty. + pub fn new_leaf() -> Self { + Self::from_leaf(Box::new(unsafe { LeafNode::new() })) } - /// Returns a new owned tree, with its own root node that is initially empty. - pub fn new_leaf() -> Self { - Root { node: BoxedNode::from_leaf(Box::new(unsafe { LeafNode::new() })), height: 0 } + fn from_leaf(leaf: Box>) -> Self { + NodeRef { height: 0, node: NonNull::from(Box::leak(leaf)), _marker: PhantomData } + } + + fn from_internal(internal: Box>, height: usize) -> Self { + NodeRef { height, node: NonNull::from(&mut Box::leak(internal).data), _marker: PhantomData } } - /// Borrows and returns an immutable reference to the node owned by the root. + /// Reborrows the owned node as an immutable reference. pub fn node_as_ref(&self) -> NodeRef, K, V, marker::LeafOrInternal> { - NodeRef { height: self.height, node: self.node.as_ptr(), _marker: PhantomData } + NodeRef { height: self.height, node: self.node, _marker: PhantomData } } - /// Borrows and returns a mutable reference to the node owned by the root. + /// Reborrows the owned node as a mutable reference. pub fn node_as_mut(&mut self) -> NodeRef, K, V, marker::LeafOrInternal> { - NodeRef { height: self.height, node: self.node.as_ptr(), _marker: PhantomData } + NodeRef { height: self.height, node: self.node, _marker: PhantomData } } - /// Borrows and returns a mutable reference to the leaf node owned by the root. + /// Reborrows the owned leaf node as a mutable reference. /// # Safety - /// The root node is a leaf. + /// The owned node must be a leaf. unsafe fn leaf_node_as_mut(&mut self) -> NodeRef, K, V, marker::Leaf> { debug_assert!(self.height == 0); - NodeRef { height: self.height, node: self.node.as_ptr(), _marker: PhantomData } + NodeRef { height: self.height, node: self.node, _marker: PhantomData } } - /// Borrows and returns a mutable reference to the internal node owned by the root. + /// Reborrows the owned internal node as a mutable reference. /// # Safety - /// The root node is not a leaf. + /// The owned node must not be a leaf. unsafe fn internal_node_as_mut(&mut self) -> NodeRef, K, V, marker::Internal> { debug_assert!(self.height > 0); - NodeRef { height: self.height, node: self.node.as_ptr(), _marker: PhantomData } + NodeRef { height: self.height, node: self.node, _marker: PhantomData } } + /// Reborrows the owned internal node as a slightly mutable reference. pub fn node_as_valmut(&mut self) -> NodeRef, K, V, marker::LeafOrInternal> { - NodeRef { height: self.height, node: self.node.as_ptr(), _marker: PhantomData } + NodeRef { height: self.height, node: self.node, _marker: PhantomData } } - pub fn into_ref(self) -> NodeRef { - NodeRef { height: self.height, node: self.node.as_ptr(), _marker: PhantomData } + /// Narrows the type-specific node reference to a type-agnostic pointer. + fn into_boxed_node(self) -> BoxedNode { + BoxedNode::from_owned(self.node) } /// Adds a new internal node with a single edge pointing to the previous root node, @@ -194,10 +191,11 @@ impl Root { /// and is the opposite of `pop_internal_level`. pub fn push_internal_level(&mut self) -> NodeRef, K, V, marker::Internal> { let mut new_node = Box::new(unsafe { InternalNode::new() }); - new_node.edges[0].write(unsafe { ptr::read(&mut self.node) }); - - self.node = BoxedNode::from_internal(new_node); - self.height += 1; + let new_height = self.height + 1; + super::mem::take_mut(self, |root| { + new_node.edges[0].write(root.into_boxed_node()); + Root::from_internal(new_node, new_height) + }); unsafe { let mut ret = self.internal_node_as_mut(); @@ -218,15 +216,14 @@ impl Root { pub fn pop_internal_level(&mut self) { assert!(self.height > 0); - let top = self.node.ptr; + let top = self.node; - let mut internal_node = unsafe { self.internal_node_as_mut() }; - self.node = unsafe { internal_node.as_internal_mut().edges[0].assume_init_read() }; - self.height -= 1; + let internal_node = NodeRef { height: self.height, node: self.node, _marker: PhantomData }; + *self = internal_node.first_edge().descend(); self.node_as_mut().as_leaf_mut().parent = None; unsafe { - Global.dealloc(NonNull::from(top).cast(), Layout::new::>()); + Global.dealloc(top.cast(), Layout::new::>()); } } } @@ -368,6 +365,10 @@ impl NodeRef { } impl NodeRef { + fn from_boxed_node(edge: BoxedNode, height: usize) -> Self { + NodeRef { height, node: edge.as_nonnull(), _marker: PhantomData } + } + /// Finds the parent of the current node. Returns `Ok(handle)` if the current /// node actually has a parent, where `handle` points to the edge of the parent /// that points to the current node. Returns `Err(self)` if the current node has @@ -650,7 +651,7 @@ impl<'a, K: 'a, V: 'a> NodeRef, K, V, marker::Internal> { unsafe { ptr::write(self.key_mut_at(idx), key); ptr::write(self.val_mut_at(idx), val); - self.as_internal_mut().edges.get_unchecked_mut(idx + 1).write(edge.node); + self.as_internal_mut().edges.get_unchecked_mut(idx + 1).write(edge.into_boxed_node()); Handle::new_edge(self.reborrow_mut(), idx + 1).correct_parent_link(); } } @@ -664,7 +665,7 @@ impl<'a, K: 'a, V: 'a> NodeRef, K, V, marker::Internal> { unsafe { slice_insert(self.keys_mut(), 0, key); slice_insert(self.vals_mut(), 0, val); - slice_insert(self.edges_mut(), 0, edge.node); + slice_insert(self.edges_mut(), 0, edge.into_boxed_node()); } self.as_leaf_mut().len += 1; @@ -688,10 +689,10 @@ impl<'a, K: 'a, V: 'a> NodeRef, K, V, marker::LeafOrInternal> { let edge = match self.reborrow_mut().force() { ForceResult::Leaf(_) => None, ForceResult::Internal(internal) => { - let edge = ptr::read(internal.edge_at(idx + 1)); - let mut new_root = Root { node: edge, height: internal.height - 1 }; - new_root.node_as_mut().as_leaf_mut().parent = None; - Some(new_root) + let boxed_node = ptr::read(internal.edge_at(idx + 1)); + let mut edge = Root::from_boxed_node(boxed_node, internal.height - 1); + edge.node_as_mut().as_leaf_mut().parent = None; + Some(edge) } }; @@ -714,13 +715,13 @@ impl<'a, K: 'a, V: 'a> NodeRef, K, V, marker::LeafOrInternal> { let edge = match self.reborrow_mut().force() { ForceResult::Leaf(_) => None, ForceResult::Internal(mut internal) => { - let edge = slice_remove(internal.edges_mut(), 0); - let mut new_root = Root { node: edge, height: internal.height - 1 }; - new_root.node_as_mut().as_leaf_mut().parent = None; + let boxed_node = slice_remove(internal.edges_mut(), 0); + let mut edge = Root::from_boxed_node(boxed_node, internal.height - 1); + edge.node_as_mut().as_leaf_mut().parent = None; internal.correct_childrens_parent_links(0..old_len); - Some(new_root) + Some(edge) } }; @@ -984,7 +985,7 @@ impl<'a, K: 'a, V: 'a> Handle, K, V, marker::Internal>, unsafe { slice_insert(self.node.keys_mut(), self.idx, key); slice_insert(self.node.vals_mut(), self.idx, val); - slice_insert(self.node.edges_mut(), self.idx + 1, edge.node); + slice_insert(self.node.edges_mut(), self.idx + 1, edge.into_boxed_node()); self.node.as_leaf_mut().len += 1; self.node.correct_childrens_parent_links((self.idx + 1)..=self.node.len()); @@ -1074,11 +1075,10 @@ impl Handle, marke // reference (Rust issue #73987) and invalidate any other references // to or inside the array, should any be around. let internal_node = self.node.as_internal_ptr(); - NodeRef { - height: self.node.height - 1, - node: unsafe { (&*(*internal_node).edges.get_unchecked(self.idx).as_ptr()).as_ptr() }, - _marker: PhantomData, - } + NodeRef::from_boxed_node( + unsafe { (*internal_node).edges.get_unchecked(self.idx).assume_init_read() }, + self.node.height - 1, + ) } } @@ -1163,7 +1163,7 @@ impl<'a, K: 'a, V: 'a> Handle, K, V, marker::Leaf>, mark let (k, v) = self.split_leaf_data(&mut new_node); - let right = Root { node: BoxedNode::from_leaf(new_node), height: 0 }; + let right = Root::from_leaf(new_node); (self.node, k, v, right) } } @@ -1215,7 +1215,7 @@ impl<'a, K: 'a, V: 'a> Handle, K, V, marker::Internal>, let (k, v) = self.split_leaf_data(&mut new_node.data); let height = self.node.height; - let mut right = Root { node: BoxedNode::from_internal(new_node), height }; + let mut right = Root::from_internal(new_node, height); right.internal_node_as_mut().correct_childrens_parent_links(0..=new_len); diff --git a/library/alloc/src/collections/btree/node/tests.rs b/library/alloc/src/collections/btree/node/tests.rs index d6527057c5d77..aa2a30ee100d7 100644 --- a/library/alloc/src/collections/btree/node/tests.rs +++ b/library/alloc/src/collections/btree/node/tests.rs @@ -134,8 +134,8 @@ fn test_partial_cmp_eq() { assert_eq!(top_edge_1.partial_cmp(&top_edge_2), None); root1.pop_internal_level(); - unsafe { root1.into_ref().deallocate_and_ascend() }; - unsafe { root2.into_ref().deallocate_and_ascend() }; + unsafe { root1.deallocate_and_ascend() }; + unsafe { root2.deallocate_and_ascend() }; } #[test] diff --git a/library/alloc/src/collections/btree/split.rs b/library/alloc/src/collections/btree/split.rs index 5f00a5a25abad..5f85471c79801 100644 --- a/library/alloc/src/collections/btree/split.rs +++ b/library/alloc/src/collections/btree/split.rs @@ -9,7 +9,7 @@ impl Root { K: Borrow, { debug_assert!(right_root.height() == 0); - debug_assert!(right_root.node_as_ref().len() == 0); + debug_assert!(right_root.len() == 0); let left_root = self; for _ in 0..left_root.height() { @@ -48,7 +48,7 @@ impl Root { /// Removes empty levels on the top, but keeps an empty leaf if the entire tree is empty. fn fix_top(&mut self) { - while self.height() > 0 && self.node_as_ref().len() == 0 { + while self.height() > 0 && self.len() == 0 { self.pop_internal_level(); } } diff --git a/src/etc/gdb_providers.py b/src/etc/gdb_providers.py index eec3027085c91..fa21305424de7 100644 --- a/src/etc/gdb_providers.py +++ b/src/etc/gdb_providers.py @@ -207,25 +207,25 @@ def children(self): yield "borrow", self.borrow -# Yields children (in a provider's sense of the word) for a tree headed by a BoxedNode. +# Yields children (in a provider's sense of the word) for a tree headed by a node. # In particular, yields each key/value pair in the node and in any child nodes. -def children_of_node(boxed_node, height): +def children_of_node(node_ptr, height): def cast_to_internal(node): internal_type_name = node.type.target().name.replace("LeafNode", "InternalNode", 1) internal_type = lookup_type(internal_type_name) return node.cast(internal_type.pointer()) - node_ptr = unwrap_unique_or_non_null(boxed_node["ptr"]) leaf = node_ptr.dereference() keys = leaf["keys"] vals = leaf["vals"] edges = cast_to_internal(node_ptr)["edges"] if height > 0 else None - length = int(leaf["len"]) + length = leaf["len"] for i in xrange(0, length + 1): if height > 0: boxed_child_node = edges[i]["value"]["value"] - for child in children_of_node(boxed_child_node, height - 1): + child_node = unwrap_unique_or_non_null(boxed_child_node["ptr"]) + for child in children_of_node(child_node, height - 1): yield child if i < length: # Avoid "Cannot perform pointer math on incomplete type" on zero-sized arrays. @@ -240,9 +240,12 @@ def children_of_map(map): root = map["root"] if root.type.name.startswith("core::option::Option<"): root = root.cast(gdb.lookup_type(root.type.name[21:-1])) - boxed_root_node = root["node"] + node_ptr = root["node"] + if node_ptr.type.name.startswith("alloc::collections::btree::node::BoxedNode<"): + node_ptr = node_ptr["ptr"] + node_ptr = unwrap_unique_or_non_null(node_ptr) height = root["height"] - for child in children_of_node(boxed_root_node, height): + for child in children_of_node(node_ptr, height): yield child diff --git a/src/test/debuginfo/pretty-std-collections.rs b/src/test/debuginfo/pretty-std-collections.rs index cc2a3a345102a..b79f00a9d0438 100644 --- a/src/test/debuginfo/pretty-std-collections.rs +++ b/src/test/debuginfo/pretty-std-collections.rs @@ -101,7 +101,7 @@ fn main() { btree_set.insert(i); } - let mut empty_btree_set: BTreeSet = BTreeSet::new(); + let empty_btree_set: BTreeSet = BTreeSet::new(); // BTreeMap let mut btree_map = BTreeMap::new(); @@ -109,7 +109,7 @@ fn main() { btree_map.insert(i, i); } - let mut empty_btree_map: BTreeMap = BTreeMap::new(); + let empty_btree_map: BTreeMap = BTreeMap::new(); let mut option_btree_map: BTreeMap> = BTreeMap::new(); option_btree_map.insert(false, None); From cabf6d052329c8ba5f23c7fc4873e9db64874997 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 27 Sep 2020 18:58:56 -0700 Subject: [PATCH 25/25] Tweak `if let` suggestion to be more liberal with suggestion and to not ICE Fix #77218. Fix #77238. --- compiler/rustc_typeck/src/check/expr.rs | 50 ++++++++++++---------- src/test/ui/issues/issue-77218.rs | 7 +++ src/test/ui/issues/issue-77218.stderr | 14 ++++++ src/test/ui/suggestions/if-let-typo.rs | 1 - src/test/ui/suggestions/if-let-typo.stderr | 18 +++----- 5 files changed, 56 insertions(+), 34 deletions(-) create mode 100644 src/test/ui/issues/issue-77218.rs create mode 100644 src/test/ui/issues/issue-77218.stderr diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index 9990f86a36b13..03e448a00cc77 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -769,34 +769,40 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let mut err = self.demand_suptype_diag(expr.span, expected_ty, actual_ty).unwrap(); let lhs_ty = self.check_expr(&lhs); let rhs_ty = self.check_expr(&rhs); - if self.can_coerce(lhs_ty, rhs_ty) { - if !lhs.is_syntactic_place_expr() { - // Do not suggest `if let x = y` as `==` is way more likely to be the intention. - if let hir::Node::Expr(hir::Expr { - kind: ExprKind::Match(_, _, hir::MatchSource::IfDesugar { .. }), - .. - }) = self.tcx.hir().get( - self.tcx.hir().get_parent_node(self.tcx.hir().get_parent_node(expr.hir_id)), - ) { - // Likely `if let` intended. - err.span_suggestion_verbose( - expr.span.shrink_to_lo(), - "you might have meant to use pattern matching", - "let ".to_string(), - Applicability::MaybeIncorrect, - ); - } + let (applicability, eq) = if self.can_coerce(rhs_ty, lhs_ty) { + (Applicability::MachineApplicable, true) + } else { + (Applicability::MaybeIncorrect, false) + }; + if !lhs.is_syntactic_place_expr() { + // Do not suggest `if let x = y` as `==` is way more likely to be the intention. + if let hir::Node::Expr(hir::Expr { + kind: + ExprKind::Match( + _, + _, + hir::MatchSource::IfDesugar { .. } | hir::MatchSource::WhileDesugar, + ), + .. + }) = self.tcx.hir().get( + self.tcx.hir().get_parent_node(self.tcx.hir().get_parent_node(expr.hir_id)), + ) { + // Likely `if let` intended. + err.span_suggestion_verbose( + expr.span.shrink_to_lo(), + "you might have meant to use pattern matching", + "let ".to_string(), + applicability, + ); } + } + if eq { err.span_suggestion_verbose( *span, "you might have meant to compare for equality", "==".to_string(), - Applicability::MaybeIncorrect, + applicability, ); - } else { - // Do this to cause extra errors about the assignment. - let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace); - let _ = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs)); } if self.sess().if_let_suggestions.borrow().get(&expr.span).is_some() { diff --git a/src/test/ui/issues/issue-77218.rs b/src/test/ui/issues/issue-77218.rs new file mode 100644 index 0000000000000..bc992c21dca5c --- /dev/null +++ b/src/test/ui/issues/issue-77218.rs @@ -0,0 +1,7 @@ +fn main() { + let value = [7u8]; + while Some(0) = value.get(0) { //~ ERROR mismatched types + //~^ NOTE expected `bool`, found `()` + //~| HELP you might have meant to use pattern matching + } +} diff --git a/src/test/ui/issues/issue-77218.stderr b/src/test/ui/issues/issue-77218.stderr new file mode 100644 index 0000000000000..eca44725eb258 --- /dev/null +++ b/src/test/ui/issues/issue-77218.stderr @@ -0,0 +1,14 @@ +error[E0308]: mismatched types + --> $DIR/issue-77218.rs:3:11 + | +LL | while Some(0) = value.get(0) { + | ^^^^^^^^^^^^^^^^^^^^^^ expected `bool`, found `()` + | +help: you might have meant to use pattern matching + | +LL | while let Some(0) = value.get(0) { + | ^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/suggestions/if-let-typo.rs b/src/test/ui/suggestions/if-let-typo.rs index c1e417b97f619..87def13c476c7 100644 --- a/src/test/ui/suggestions/if-let-typo.rs +++ b/src/test/ui/suggestions/if-let-typo.rs @@ -4,6 +4,5 @@ fn main() { if Some(x) = foo {} //~ ERROR cannot find value `x` in this scope if Some(foo) = bar {} //~ ERROR mismatched types if 3 = foo {} //~ ERROR mismatched types - //~^ ERROR mismatched types if Some(3) = foo {} //~ ERROR mismatched types } diff --git a/src/test/ui/suggestions/if-let-typo.stderr b/src/test/ui/suggestions/if-let-typo.stderr index bb2ea8cb4778a..d8e50cae55ad1 100644 --- a/src/test/ui/suggestions/if-let-typo.stderr +++ b/src/test/ui/suggestions/if-let-typo.stderr @@ -24,23 +24,19 @@ help: you might have meant to compare for equality LL | if Some(foo) == bar {} | ^^ -error[E0308]: mismatched types - --> $DIR/if-let-typo.rs:6:12 - | -LL | if 3 = foo {} - | ^^^ expected integer, found enum `Option` - | - = note: expected type `{integer}` - found enum `Option<{integer}>` - error[E0308]: mismatched types --> $DIR/if-let-typo.rs:6:8 | LL | if 3 = foo {} | ^^^^^^^ expected `bool`, found `()` + | +help: you might have meant to use pattern matching + | +LL | if let 3 = foo {} + | ^^^ error[E0308]: mismatched types - --> $DIR/if-let-typo.rs:8:8 + --> $DIR/if-let-typo.rs:7:8 | LL | if Some(3) = foo {} | ^^^^^^^^^^^^^ expected `bool`, found `()` @@ -54,7 +50,7 @@ help: you might have meant to compare for equality LL | if Some(3) == foo {} | ^^ -error: aborting due to 5 previous errors +error: aborting due to 4 previous errors Some errors have detailed explanations: E0308, E0425. For more information about an error, try `rustc --explain E0308`.