From d9a4b22d3291913a8f2158a1b7c195bc30c9286e Mon Sep 17 00:00:00 2001 From: Matthias Einwag Date: Tue, 29 Jan 2019 19:02:42 -0800 Subject: [PATCH 01/25] Update the future/task API This change updates the future and task API as discussed in the stabilization RFC at https://github.com/rust-lang/rfcs/pull/2592. Changes: - Replacing UnsafeWake with RawWaker and RawWakerVtable - Removal of LocalWaker - Removal of Arc-based Wake trait --- src/liballoc/boxed.rs | 6 +- src/liballoc/lib.rs | 4 - src/liballoc/task.rs | 130 -------------- src/libcore/future/future.rs | 36 ++-- src/libcore/task/mod.rs | 2 +- src/libcore/task/wake.rs | 316 +++++++++-------------------------- src/libstd/future.rs | 20 +-- src/libstd/lib.rs | 2 - src/libstd/panic.rs | 6 +- 9 files changed, 111 insertions(+), 411 deletions(-) delete mode 100644 src/liballoc/task.rs diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 8e01e12e0b8de..51549f92d4dbf 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -71,7 +71,7 @@ use core::ops::{ CoerceUnsized, DispatchFromDyn, Deref, DerefMut, Receiver, Generator, GeneratorState }; use core::ptr::{self, NonNull, Unique}; -use core::task::{LocalWaker, Poll}; +use core::task::{Waker, Poll}; use crate::vec::Vec; use crate::raw_vec::RawVec; @@ -896,7 +896,7 @@ impl Generator for Pin> { impl Future for Box { type Output = F::Output; - fn poll(mut self: Pin<&mut Self>, lw: &LocalWaker) -> Poll { - F::poll(Pin::new(&mut *self), lw) + fn poll(mut self: Pin<&mut Self>, waker: &Waker) -> Poll { + F::poll(Pin::new(&mut *self), waker) } } diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index 80097a128a5f4..cc73e282b6f6f 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -133,10 +133,6 @@ mod macros; pub mod alloc; -#[unstable(feature = "futures_api", - reason = "futures in libcore are unstable", - issue = "50547")] -pub mod task; // Primitive types using the heaps above // Need to conditionally define the mod from `boxed.rs` to avoid diff --git a/src/liballoc/task.rs b/src/liballoc/task.rs deleted file mode 100644 index 2261dabe2779a..0000000000000 --- a/src/liballoc/task.rs +++ /dev/null @@ -1,130 +0,0 @@ -//! Types and Traits for working with asynchronous tasks. - -pub use core::task::*; - -#[cfg(all(target_has_atomic = "ptr", target_has_atomic = "cas"))] -pub use if_arc::*; - -#[cfg(all(target_has_atomic = "ptr", target_has_atomic = "cas"))] -mod if_arc { - use super::*; - use core::marker::PhantomData; - use core::mem; - use core::ptr::{self, NonNull}; - use crate::sync::Arc; - - /// A way of waking up a specific task. - /// - /// Any task executor must provide a way of signaling that a task it owns - /// is ready to be `poll`ed again. Executors do so by implementing this trait. - pub trait Wake: Send + Sync { - /// Indicates that the associated task is ready to make progress and should - /// be `poll`ed. - /// - /// Executors generally maintain a queue of "ready" tasks; `wake` should place - /// the associated task onto this queue. - fn wake(arc_self: &Arc); - - /// Indicates that the associated task is ready to make progress and should - /// be `poll`ed. This function is like `wake`, but can only be called from the - /// thread on which this `Wake` was created. - /// - /// Executors generally maintain a queue of "ready" tasks; `wake_local` should place - /// the associated task onto this queue. - #[inline] - unsafe fn wake_local(arc_self: &Arc) { - Self::wake(arc_self); - } - } - - #[cfg(all(target_has_atomic = "ptr", target_has_atomic = "cas"))] - struct ArcWrapped(PhantomData); - - unsafe impl UnsafeWake for ArcWrapped { - #[inline] - unsafe fn clone_raw(&self) -> Waker { - let me: *const ArcWrapped = self; - let arc = (*(&me as *const *const ArcWrapped as *const Arc)).clone(); - Waker::from(arc) - } - - #[inline] - unsafe fn drop_raw(&self) { - let mut me: *const ArcWrapped = self; - let me = &mut me as *mut *const ArcWrapped as *mut Arc; - ptr::drop_in_place(me); - } - - #[inline] - unsafe fn wake(&self) { - let me: *const ArcWrapped = self; - T::wake(&*(&me as *const *const ArcWrapped as *const Arc)) - } - - #[inline] - unsafe fn wake_local(&self) { - let me: *const ArcWrapped = self; - T::wake_local(&*(&me as *const *const ArcWrapped as *const Arc)) - } - } - - impl From> for Waker - where T: Wake + 'static, - { - fn from(rc: Arc) -> Self { - unsafe { - let ptr = mem::transmute::, NonNull>>(rc); - Waker::new(ptr) - } - } - } - - /// Creates a `LocalWaker` from a local `wake`. - /// - /// This function requires that `wake` is "local" (created on the current thread). - /// The resulting `LocalWaker` will call `wake.wake_local()` when awoken, and - /// will call `wake.wake()` if awoken after being converted to a `Waker`. - #[inline] - pub unsafe fn local_waker(wake: Arc) -> LocalWaker { - let ptr = mem::transmute::, NonNull>>(wake); - LocalWaker::new(ptr) - } - - struct NonLocalAsLocal(ArcWrapped); - - unsafe impl UnsafeWake for NonLocalAsLocal { - #[inline] - unsafe fn clone_raw(&self) -> Waker { - self.0.clone_raw() - } - - #[inline] - unsafe fn drop_raw(&self) { - self.0.drop_raw() - } - - #[inline] - unsafe fn wake(&self) { - self.0.wake() - } - - #[inline] - unsafe fn wake_local(&self) { - // Since we're nonlocal, we can't call wake_local - self.0.wake() - } - } - - /// Creates a `LocalWaker` from a non-local `wake`. - /// - /// This function is similar to `local_waker`, but does not require that `wake` - /// is local to the current thread. The resulting `LocalWaker` will call - /// `wake.wake()` when awoken. - #[inline] - pub fn local_waker_from_nonlocal(wake: Arc) -> LocalWaker { - unsafe { - let ptr = mem::transmute::, NonNull>>(wake); - LocalWaker::new(ptr) - } - } -} diff --git a/src/libcore/future/future.rs b/src/libcore/future/future.rs index 539b07fc21eea..c40045245425b 100644 --- a/src/libcore/future/future.rs +++ b/src/libcore/future/future.rs @@ -5,7 +5,7 @@ use marker::Unpin; use ops; use pin::Pin; -use task::{Poll, LocalWaker}; +use task::{Poll, Waker}; /// A future represents an asynchronous computation. /// @@ -25,7 +25,7 @@ use task::{Poll, LocalWaker}; /// `await!` the value. #[must_use = "futures do nothing unless polled"] pub trait Future { - /// The result of the `Future`. + /// The type of value produced on completion. type Output; /// Attempt to resolve the future to a final value, registering @@ -42,16 +42,16 @@ pub trait Future { /// Once a future has finished, clients should not `poll` it again. /// /// When a future is not ready yet, `poll` returns `Poll::Pending` and - /// stores a clone of the [`LocalWaker`] to be woken once the future can + /// stores a clone of the [`Waker`] to be woken once the future can /// make progress. For example, a future waiting for a socket to become - /// readable would call `.clone()` on the [`LocalWaker`] and store it. + /// readable would call `.clone()` on the [`Waker`] and store it. /// When a signal arrives elsewhere indicating that the socket is readable, - /// `[LocalWaker::wake]` is called and the socket future's task is awoken. + /// `[Waker::wake]` is called and the socket future's task is awoken. /// Once a task has been woken up, it should attempt to `poll` the future /// again, which may or may not produce a final value. /// /// Note that on multiple calls to `poll`, only the most recent - /// [`LocalWaker`] passed to `poll` should be scheduled to receive a + /// [`Waker`] passed to `poll` should be scheduled to receive a /// wakeup. /// /// # Runtime characteristics @@ -74,16 +74,6 @@ pub trait Future { /// thread pool (or something similar) to ensure that `poll` can return /// quickly. /// - /// # [`LocalWaker`], [`Waker`] and thread-safety - /// - /// The `poll` function takes a [`LocalWaker`], an object which knows how to - /// awaken the current task. [`LocalWaker`] is not `Send` nor `Sync`, so in - /// order to make thread-safe futures the [`LocalWaker::into_waker`] method - /// should be used to convert the [`LocalWaker`] into a thread-safe version. - /// [`LocalWaker::wake`] implementations have the ability to be more - /// efficient, however, so when thread safety is not necessary, - /// [`LocalWaker`] should be preferred. - /// /// # Panics /// /// Once a future has completed (returned `Ready` from `poll`), @@ -93,18 +83,16 @@ pub trait Future { /// /// [`Poll::Pending`]: ../task/enum.Poll.html#variant.Pending /// [`Poll::Ready(val)`]: ../task/enum.Poll.html#variant.Ready - /// [`LocalWaker`]: ../task/struct.LocalWaker.html - /// [`LocalWaker::into_waker`]: ../task/struct.LocalWaker.html#method.into_waker - /// [`LocalWaker::wake`]: ../task/struct.LocalWaker.html#method.wake /// [`Waker`]: ../task/struct.Waker.html - fn poll(self: Pin<&mut Self>, lw: &LocalWaker) -> Poll; + /// [`Waker::wake`]: ../task/struct.Waker.html#method.wake + fn poll(self: Pin<&mut Self>, waker: &Waker) -> Poll; } impl<'a, F: ?Sized + Future + Unpin> Future for &'a mut F { type Output = F::Output; - fn poll(mut self: Pin<&mut Self>, lw: &LocalWaker) -> Poll { - F::poll(Pin::new(&mut **self), lw) + fn poll(mut self: Pin<&mut Self>, waker: &Waker) -> Poll { + F::poll(Pin::new(&mut **self), waker) } } @@ -115,7 +103,7 @@ where { type Output = <

::Target as Future>::Output; - fn poll(self: Pin<&mut Self>, lw: &LocalWaker) -> Poll { - Pin::get_mut(self).as_mut().poll(lw) + fn poll(self: Pin<&mut Self>, waker: &Waker) -> Poll { + Pin::get_mut(self).as_mut().poll(waker) } } diff --git a/src/libcore/task/mod.rs b/src/libcore/task/mod.rs index 9552e53ebf849..9b8f598116200 100644 --- a/src/libcore/task/mod.rs +++ b/src/libcore/task/mod.rs @@ -8,4 +8,4 @@ mod poll; pub use self::poll::Poll; mod wake; -pub use self::wake::{Waker, LocalWaker, UnsafeWake}; +pub use self::wake::{Waker, RawWaker, RawWakerVTable}; diff --git a/src/libcore/task/wake.rs b/src/libcore/task/wake.rs index 3f7098f1ef934..bb91bf2ecfb44 100644 --- a/src/libcore/task/wake.rs +++ b/src/libcore/task/wake.rs @@ -4,16 +4,76 @@ use fmt; use marker::Unpin; -use ptr::NonNull; + +/// A `RawWaker` allows the implementor of a task executor to create a `Waker` +/// which provides customized wakeup behavior. +/// +/// It consists of a data pointer and a virtual function pointer table (vtable) that +/// customizes the behavior of the `RawWaker`. +#[derive(PartialEq)] +pub struct RawWaker { + /// A data pointer, which can be used to store arbitrary data as required + /// by the executor. This could be e.g. a type-erased pointer to an `Arc` + /// that is associated with the task. + /// The value of this field gets passed to all functions that are part of + /// the vtable as first parameter. + pub data: *const (), + /// Virtual function pointer table that customizes the behavior of this waker. + pub vtable: &'static RawWakerVTable, +} + +impl fmt::Debug for RawWaker { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("RawWaker") + .finish() + } +} + +/// A virtual function pointer table (vtable) that specifies the behavior +/// of a `RawWaker`. +/// +/// The pointer passed to all functions inside the vtable is the `data` pointer +/// from the enclosing `RawWaker` object. +#[derive(PartialEq, Copy, Clone)] +pub struct RawWakerVTable { + /// This function will be called when the `RawWaker` gets cloned, e.g. when + /// the `Waker` in which the `RawWaker` is stored gets cloned. + /// + /// The implementation of this function must retain all resources that are + /// required for this additional instance of a `RawWaker` and associated + /// task. Calling `wake` on the resulting `RawWaker` should result in a wakeup + /// of the same task that would have been awoken by the original `RawWaker`. + pub clone: unsafe fn(*const ()) -> RawWaker, + + /// This function will be called when `wake` is called on the `Waker`. + /// It must wake up the task associated with this `RawWaker`. + pub wake: unsafe fn(*const ()), + + /// This function gets called when a `RawWaker` gets dropped. + /// + /// The implementation of this function must make sure to release any + /// resources that are associated with this instance of a `RawWaker` and + /// associated task. + pub drop_fn: unsafe fn(*const ()), +} + +impl fmt::Debug for RawWakerVTable { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("RawWakerVTable") + .finish() + } +} /// A `Waker` is a handle for waking up a task by notifying its executor that it /// is ready to be run. /// -/// This handle contains a trait object pointing to an instance of the `UnsafeWake` -/// trait, allowing notifications to get routed through it. +/// This handle encapsulates a `RawWaker` instance, which defines the +/// executor-specific wakeup behavior. +/// +/// Implements `Clone`, `Send`, and `Sync`. #[repr(transparent)] pub struct Waker { - inner: NonNull, + waker: RawWaker, } impl Unpin for Waker {} @@ -21,264 +81,52 @@ unsafe impl Send for Waker {} unsafe impl Sync for Waker {} impl Waker { - /// Constructs a new `Waker` directly. - /// - /// Note that most code will not need to call this. Implementers of the - /// `UnsafeWake` trait will typically provide a wrapper that calls this - /// but you otherwise shouldn't call it directly. - /// - /// If you're working with the standard library then it's recommended to - /// use the `Waker::from` function instead which works with the safe - /// `Arc` type and the safe `Wake` trait. - #[inline] - pub unsafe fn new(inner: NonNull) -> Self { - Waker { inner } - } - /// Wake up the task associated with this `Waker`. - #[inline] pub fn wake(&self) { - unsafe { self.inner.as_ref().wake() } + // The actual wakeup call is delegated through a virtual function call + // to the implementation which is defined by the executor. + unsafe { (self.waker.vtable.wake)(self.waker.data) } } - /// Returns whether or not this `Waker` and `other` awaken the same task. + /// Returns whether or not this `Waker` and other `Waker` have awaken the same task. /// /// This function works on a best-effort basis, and may return false even /// when the `Waker`s would awaken the same task. However, if this function - /// returns true, it is guaranteed that the `Waker`s will awaken the same - /// task. + /// returns `true`, it is guaranteed that the `Waker`s will awaken the same task. /// /// This function is primarily used for optimization purposes. - #[inline] pub fn will_wake(&self, other: &Waker) -> bool { - self.inner == other.inner + self.waker == other.waker } - /// Returns whether or not this `Waker` and `other` `LocalWaker` awaken - /// the same task. - /// - /// This function works on a best-effort basis, and may return false even - /// when the `Waker`s would awaken the same task. However, if this function - /// returns true, it is guaranteed that the `Waker`s will awaken the same - /// task. + /// Creates a new `Waker` from `RawWaker`. /// - /// This function is primarily used for optimization purposes. - #[inline] - pub fn will_wake_local(&self, other: &LocalWaker) -> bool { - self.will_wake(&other.0) + /// The method cannot check whether `RawWaker` fulfills the required API + /// contract to make it usable for `Waker` and is therefore unsafe. + pub unsafe fn new_unchecked(waker: RawWaker) -> Waker { + Waker { + waker: waker, + } } } impl Clone for Waker { - #[inline] fn clone(&self) -> Self { - unsafe { - self.inner.as_ref().clone_raw() + Waker { + waker: unsafe { (self.waker.vtable.clone)(self.waker.data) }, } } } -impl fmt::Debug for Waker { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("Waker") - .finish() - } -} - impl Drop for Waker { - #[inline] fn drop(&mut self) { - unsafe { - self.inner.as_ref().drop_raw() - } + unsafe { (self.waker.vtable.drop_fn)(self.waker.data) } } } -/// A `LocalWaker` is a handle for waking up a task by notifying its executor that it -/// is ready to be run. -/// -/// This is similar to the `Waker` type, but cannot be sent across threads. -/// Task executors can use this type to implement more optimized single-threaded wakeup -/// behavior. -#[repr(transparent)] -#[derive(Clone)] -pub struct LocalWaker(Waker); - -impl Unpin for LocalWaker {} -impl !Send for LocalWaker {} -impl !Sync for LocalWaker {} - -impl LocalWaker { - /// Constructs a new `LocalWaker` directly. - /// - /// Note that most code will not need to call this. Implementers of the - /// `UnsafeWake` trait will typically provide a wrapper that calls this - /// but you otherwise shouldn't call it directly. - /// - /// If you're working with the standard library then it's recommended to - /// use the `local_waker_from_nonlocal` or `local_waker` to convert a `Waker` - /// into a `LocalWaker`. - /// - /// For this function to be used safely, it must be sound to call `inner.wake_local()` - /// on the current thread. - #[inline] - pub unsafe fn new(inner: NonNull) -> Self { - LocalWaker(Waker::new(inner)) - } - - /// Borrows this `LocalWaker` as a `Waker`. - /// - /// `Waker` is nearly identical to `LocalWaker`, but is threadsafe - /// (implements `Send` and `Sync`). - #[inline] - pub fn as_waker(&self) -> &Waker { - &self.0 - } - - /// Converts this `LocalWaker` into a `Waker`. - /// - /// `Waker` is nearly identical to `LocalWaker`, but is threadsafe - /// (implements `Send` and `Sync`). - #[inline] - pub fn into_waker(self) -> Waker { - self.0 - } - - /// Wake up the task associated with this `LocalWaker`. - #[inline] - pub fn wake(&self) { - unsafe { self.0.inner.as_ref().wake_local() } - } - - /// Returns whether or not this `LocalWaker` and `other` `LocalWaker` awaken the same task. - /// - /// This function works on a best-effort basis, and may return false even - /// when the `LocalWaker`s would awaken the same task. However, if this function - /// returns true, it is guaranteed that the `LocalWaker`s will awaken the same - /// task. - /// - /// This function is primarily used for optimization purposes. - #[inline] - pub fn will_wake(&self, other: &LocalWaker) -> bool { - self.0.will_wake(&other.0) - } - - /// Returns whether or not this `LocalWaker` and `other` `Waker` awaken the same task. - /// - /// This function works on a best-effort basis, and may return false even - /// when the `Waker`s would awaken the same task. However, if this function - /// returns true, it is guaranteed that the `LocalWaker`s will awaken the same - /// task. - /// - /// This function is primarily used for optimization purposes. - #[inline] - pub fn will_wake_nonlocal(&self, other: &Waker) -> bool { - self.0.will_wake(other) - } -} - -impl From for Waker { - /// Converts a `LocalWaker` into a `Waker`. - /// - /// This conversion turns a `!Sync` `LocalWaker` into a `Sync` `Waker`, allowing a wakeup - /// object to be sent to another thread, but giving up its ability to do specialized - /// thread-local wakeup behavior. - #[inline] - fn from(local_waker: LocalWaker) -> Self { - local_waker.0 - } -} - -impl fmt::Debug for LocalWaker { +impl fmt::Debug for Waker { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("LocalWaker") + f.debug_struct("Waker") .finish() } } - -/// An unsafe trait for implementing custom memory management for a `Waker` or `LocalWaker`. -/// -/// A `Waker` conceptually is a cloneable trait object for `Wake`, and is -/// most often essentially just `Arc`. However, in some contexts -/// (particularly `no_std`), it's desirable to avoid `Arc` in favor of some -/// custom memory management strategy. This trait is designed to allow for such -/// customization. -/// -/// When using `std`, a default implementation of the `UnsafeWake` trait is provided for -/// `Arc` where `T: Wake`. -pub unsafe trait UnsafeWake: Send + Sync { - /// Creates a clone of this `UnsafeWake` and stores it behind a `Waker`. - /// - /// This function will create a new uniquely owned handle that under the - /// hood references the same notification instance. In other words calls - /// to `wake` on the returned handle should be equivalent to calls to - /// `wake` on this handle. - /// - /// # Unsafety - /// - /// This function is unsafe to call because it's asserting the `UnsafeWake` - /// value is in a consistent state, i.e., hasn't been dropped. - unsafe fn clone_raw(&self) -> Waker; - - /// Drops this instance of `UnsafeWake`, deallocating resources - /// associated with it. - /// - /// FIXME(cramertj) - /// This method is intended to have a signature such as: - /// - /// ```ignore (not-a-doctest) - /// fn drop_raw(self: *mut Self); - /// ``` - /// - /// Unfortunately in Rust today that signature is not object safe. - /// Nevertheless it's recommended to implement this function *as if* that - /// were its signature. As such it is not safe to call on an invalid - /// pointer, nor is the validity of the pointer guaranteed after this - /// function returns. - /// - /// # Unsafety - /// - /// This function is unsafe to call because it's asserting the `UnsafeWake` - /// value is in a consistent state, i.e., hasn't been dropped. - unsafe fn drop_raw(&self); - - /// Indicates that the associated task is ready to make progress and should - /// be `poll`ed. - /// - /// Executors generally maintain a queue of "ready" tasks; `wake` should place - /// the associated task onto this queue. - /// - /// # Panics - /// - /// Implementations should avoid panicking, but clients should also be prepared - /// for panics. - /// - /// # Unsafety - /// - /// This function is unsafe to call because it's asserting the `UnsafeWake` - /// value is in a consistent state, i.e., hasn't been dropped. - unsafe fn wake(&self); - - /// Indicates that the associated task is ready to make progress and should - /// be `poll`ed. This function is the same as `wake`, but can only be called - /// from the thread that this `UnsafeWake` is "local" to. This allows for - /// implementors to provide specialized wakeup behavior specific to the current - /// thread. This function is called by `LocalWaker::wake`. - /// - /// Executors generally maintain a queue of "ready" tasks; `wake_local` should place - /// the associated task onto this queue. - /// - /// # Panics - /// - /// Implementations should avoid panicking, but clients should also be prepared - /// for panics. - /// - /// # Unsafety - /// - /// This function is unsafe to call because it's asserting the `UnsafeWake` - /// value is in a consistent state, i.e., hasn't been dropped, and that the - /// `UnsafeWake` hasn't moved from the thread on which it was created. - unsafe fn wake_local(&self) { - self.wake() - } -} diff --git a/src/libstd/future.rs b/src/libstd/future.rs index d1203be3cf011..aa784746122db 100644 --- a/src/libstd/future.rs +++ b/src/libstd/future.rs @@ -5,7 +5,7 @@ use core::marker::Unpin; use core::pin::Pin; use core::option::Option; use core::ptr::NonNull; -use core::task::{LocalWaker, Poll}; +use core::task::{Waker, Poll}; use core::ops::{Drop, Generator, GeneratorState}; #[doc(inline)] @@ -32,10 +32,10 @@ impl> !Unpin for GenFuture {} #[unstable(feature = "gen_future", issue = "50547")] impl> Future for GenFuture { type Output = T::Return; - fn poll(self: Pin<&mut Self>, lw: &LocalWaker) -> Poll { + fn poll(self: Pin<&mut Self>, waker: &Waker) -> Poll { // Safe because we're !Unpin + !Drop mapping to a ?Unpin value let gen = unsafe { Pin::map_unchecked_mut(self, |s| &mut s.0) }; - set_task_waker(lw, || match gen.resume() { + set_task_waker(waker, || match gen.resume() { GeneratorState::Yielded(()) => Poll::Pending, GeneratorState::Complete(x) => Poll::Ready(x), }) @@ -43,10 +43,10 @@ impl> Future for GenFuture { } thread_local! { - static TLS_WAKER: Cell>> = Cell::new(None); + static TLS_WAKER: Cell>> = Cell::new(None); } -struct SetOnDrop(Option>); +struct SetOnDrop(Option>); impl Drop for SetOnDrop { fn drop(&mut self) { @@ -58,12 +58,12 @@ impl Drop for SetOnDrop { #[unstable(feature = "gen_future", issue = "50547")] /// Sets the thread-local task context used by async/await futures. -pub fn set_task_waker(lw: &LocalWaker, f: F) -> R +pub fn set_task_waker(waker: &Waker, f: F) -> R where F: FnOnce() -> R { let old_waker = TLS_WAKER.with(|tls_waker| { - tls_waker.replace(Some(NonNull::from(lw))) + tls_waker.replace(Some(NonNull::from(waker))) }); let _reset_waker = SetOnDrop(old_waker); f() @@ -78,7 +78,7 @@ where /// retrieved by a surrounding call to get_task_waker. pub fn get_task_waker(f: F) -> R where - F: FnOnce(&LocalWaker) -> R + F: FnOnce(&Waker) -> R { let waker_ptr = TLS_WAKER.with(|tls_waker| { // Clear the entry so that nested `get_task_waker` calls @@ -88,7 +88,7 @@ where let _reset_waker = SetOnDrop(waker_ptr); let waker_ptr = waker_ptr.expect( - "TLS LocalWaker not set. This is a rustc bug. \ + "TLS Waker not set. This is a rustc bug. \ Please file an issue on https://github.com/rust-lang/rust."); unsafe { f(waker_ptr.as_ref()) } } @@ -99,5 +99,5 @@ pub fn poll_with_tls_waker(f: Pin<&mut F>) -> Poll where F: Future { - get_task_waker(|lw| F::poll(f, lw)) + get_task_waker(|waker| F::poll(f, waker)) } diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index 244caf28ec7cd..207ba9ae02f05 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -468,8 +468,6 @@ pub mod task { //! Types and Traits for working with asynchronous tasks. #[doc(inline)] pub use core::task::*; - #[doc(inline)] - pub use alloc_crate::task::*; } #[unstable(feature = "futures_api", diff --git a/src/libstd/panic.rs b/src/libstd/panic.rs index d27f6ca88c2e9..862fdf051ccd1 100644 --- a/src/libstd/panic.rs +++ b/src/libstd/panic.rs @@ -12,7 +12,7 @@ use panicking; use ptr::{Unique, NonNull}; use rc::Rc; use sync::{Arc, Mutex, RwLock, atomic}; -use task::{LocalWaker, Poll}; +use task::{Waker, Poll}; use thread::Result; #[stable(feature = "panic_hooks", since = "1.10.0")] @@ -323,9 +323,9 @@ impl fmt::Debug for AssertUnwindSafe { impl<'a, F: Future> Future for AssertUnwindSafe { type Output = F::Output; - fn poll(self: Pin<&mut Self>, lw: &LocalWaker) -> Poll { + fn poll(self: Pin<&mut Self>, waker: &Waker) -> Poll { let pinned_field = unsafe { Pin::map_unchecked_mut(self, |x| &mut x.0) }; - F::poll(pinned_field, lw) + F::poll(pinned_field, waker) } } From 01a704cf3650710a3e3db221759207539de61613 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 3 Feb 2019 12:30:34 -0800 Subject: [PATCH 02/25] Apply suggestions from code review Co-Authored-By: Matthias247 --- src/libcore/task/wake.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libcore/task/wake.rs b/src/libcore/task/wake.rs index bb91bf2ecfb44..adaca50434557 100644 --- a/src/libcore/task/wake.rs +++ b/src/libcore/task/wake.rs @@ -8,7 +8,9 @@ use marker::Unpin; /// A `RawWaker` allows the implementor of a task executor to create a `Waker` /// which provides customized wakeup behavior. /// -/// It consists of a data pointer and a virtual function pointer table (vtable) that +/// [vtable]: https://en.wikipedia.org/wiki/Virtual_method_table +/// +/// It consists of a data pointer and a [virtual function pointer table (vtable)][vtable] that /// customizes the behavior of the `RawWaker`. #[derive(PartialEq)] pub struct RawWaker { @@ -16,7 +18,7 @@ pub struct RawWaker { /// by the executor. This could be e.g. a type-erased pointer to an `Arc` /// that is associated with the task. /// The value of this field gets passed to all functions that are part of - /// the vtable as first parameter. + /// the vtable as the first parameter. pub data: *const (), /// Virtual function pointer table that customizes the behavior of this waker. pub vtable: &'static RawWakerVTable, @@ -30,7 +32,7 @@ impl fmt::Debug for RawWaker { } /// A virtual function pointer table (vtable) that specifies the behavior -/// of a `RawWaker`. +/// of a [`RawWaker`]. /// /// The pointer passed to all functions inside the vtable is the `data` pointer /// from the enclosing `RawWaker` object. @@ -105,7 +107,7 @@ impl Waker { /// contract to make it usable for `Waker` and is therefore unsafe. pub unsafe fn new_unchecked(waker: RawWaker) -> Waker { Waker { - waker: waker, + waker, } } } From 9e6bc3c4386bf5f7f1885fdaab4ef01fdc93007e Mon Sep 17 00:00:00 2001 From: Matthias Einwag Date: Sun, 3 Feb 2019 12:59:51 -0800 Subject: [PATCH 03/25] Apply review suggestions and fix tests --- src/libcore/task/wake.rs | 65 ++++++----- .../compile-fail/must_use-in-stdlib-traits.rs | 4 +- src/test/run-pass/async-await.rs | 69 ++++++++++-- src/test/run-pass/futures-api.rs | 103 ++++++++++++------ 4 files changed, 163 insertions(+), 78 deletions(-) diff --git a/src/libcore/task/wake.rs b/src/libcore/task/wake.rs index adaca50434557..fe2de61c59446 100644 --- a/src/libcore/task/wake.rs +++ b/src/libcore/task/wake.rs @@ -5,14 +5,14 @@ use fmt; use marker::Unpin; -/// A `RawWaker` allows the implementor of a task executor to create a `Waker` +/// A `RawWaker` allows the implementor of a task executor to create a [`Waker`] /// which provides customized wakeup behavior. /// /// [vtable]: https://en.wikipedia.org/wiki/Virtual_method_table /// /// It consists of a data pointer and a [virtual function pointer table (vtable)][vtable] that /// customizes the behavior of the `RawWaker`. -#[derive(PartialEq)] +#[derive(PartialEq, Debug)] pub struct RawWaker { /// A data pointer, which can be used to store arbitrary data as required /// by the executor. This could be e.g. a type-erased pointer to an `Arc` @@ -24,55 +24,41 @@ pub struct RawWaker { pub vtable: &'static RawWakerVTable, } -impl fmt::Debug for RawWaker { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("RawWaker") - .finish() - } -} - /// A virtual function pointer table (vtable) that specifies the behavior /// of a [`RawWaker`]. /// /// The pointer passed to all functions inside the vtable is the `data` pointer -/// from the enclosing `RawWaker` object. -#[derive(PartialEq, Copy, Clone)] +/// from the enclosing [`RawWaker`] object. +#[derive(PartialEq, Copy, Clone, Debug)] pub struct RawWakerVTable { - /// This function will be called when the `RawWaker` gets cloned, e.g. when - /// the `Waker` in which the `RawWaker` is stored gets cloned. + /// This function will be called when the [`RawWaker`] gets cloned, e.g. when + /// the [`Waker`] in which the [`RawWaker`] is stored gets cloned. /// /// The implementation of this function must retain all resources that are - /// required for this additional instance of a `RawWaker` and associated - /// task. Calling `wake` on the resulting `RawWaker` should result in a wakeup - /// of the same task that would have been awoken by the original `RawWaker`. + /// required for this additional instance of a [`RawWaker`] and associated + /// task. Calling `wake` on the resulting [`RawWaker`] should result in a wakeup + /// of the same task that would have been awoken by the original [`RawWaker`]. pub clone: unsafe fn(*const ()) -> RawWaker, - /// This function will be called when `wake` is called on the `Waker`. - /// It must wake up the task associated with this `RawWaker`. + /// This function will be called when `wake` is called on the [`Waker`]. + /// It must wake up the task associated with this [`RawWaker`]. pub wake: unsafe fn(*const ()), - /// This function gets called when a `RawWaker` gets dropped. + /// This function gets called when a [`RawWaker`] gets dropped. /// /// The implementation of this function must make sure to release any - /// resources that are associated with this instance of a `RawWaker` and + /// resources that are associated with this instance of a [`RawWaker`] and /// associated task. - pub drop_fn: unsafe fn(*const ()), -} - -impl fmt::Debug for RawWakerVTable { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("RawWakerVTable") - .finish() - } + pub drop: unsafe fn(*const ()), } /// A `Waker` is a handle for waking up a task by notifying its executor that it /// is ready to be run. /// -/// This handle encapsulates a `RawWaker` instance, which defines the +/// This handle encapsulates a [`RawWaker`] instance, which defines the /// executor-specific wakeup behavior. /// -/// Implements `Clone`, `Send`, and `Sync`. +/// Implements [`Clone`], [`Send`], and [`Sync`]. #[repr(transparent)] pub struct Waker { waker: RawWaker, @@ -87,6 +73,10 @@ impl Waker { pub fn wake(&self) { // The actual wakeup call is delegated through a virtual function call // to the implementation which is defined by the executor. + + // SAFETY: This is safe because `Waker::new_unchecked` is the only way + // to initialize `wake` and `data` requiring the user to acknowledge + // that the contract of `RawWaker` is upheld. unsafe { (self.waker.vtable.wake)(self.waker.data) } } @@ -101,10 +91,11 @@ impl Waker { self.waker == other.waker } - /// Creates a new `Waker` from `RawWaker`. + /// Creates a new `Waker` from [`RawWaker`]. /// - /// The method cannot check whether `RawWaker` fulfills the required API - /// contract to make it usable for `Waker` and is therefore unsafe. + /// The behavior of the returned `Waker` is undefined if the contract defined + /// in [RawWaker]'s documentation is not upheld. Therefore this method is + /// unsafe. pub unsafe fn new_unchecked(waker: RawWaker) -> Waker { Waker { waker, @@ -115,6 +106,9 @@ impl Waker { impl Clone for Waker { fn clone(&self) -> Self { Waker { + // SAFETY: This is safe because `Waker::new_unchecked` is the only way + // to initialize `clone` and `data` requiring the user to acknowledge + // that the contract of [`RawWaker`] is upheld. waker: unsafe { (self.waker.vtable.clone)(self.waker.data) }, } } @@ -122,7 +116,10 @@ impl Clone for Waker { impl Drop for Waker { fn drop(&mut self) { - unsafe { (self.waker.vtable.drop_fn)(self.waker.data) } + // SAFETY: This is safe because `Waker::new_unchecked` is the only way + // to initialize `drop` and `data` requiring the user to acknowledge + // that the contract of `RawWaker` is upheld. + unsafe { (self.waker.vtable.drop)(self.waker.data) } } } diff --git a/src/test/compile-fail/must_use-in-stdlib-traits.rs b/src/test/compile-fail/must_use-in-stdlib-traits.rs index 7e446fdaeaf41..b4f07ab33214c 100644 --- a/src/test/compile-fail/must_use-in-stdlib-traits.rs +++ b/src/test/compile-fail/must_use-in-stdlib-traits.rs @@ -4,7 +4,7 @@ use std::iter::Iterator; use std::future::Future; -use std::task::{Poll, LocalWaker}; +use std::task::{Poll, Waker}; use std::pin::Pin; use std::unimplemented; @@ -13,7 +13,7 @@ struct MyFuture; impl Future for MyFuture { type Output = u32; - fn poll(self: Pin<&mut Self>, lw: &LocalWaker) -> Poll { + fn poll(self: Pin<&mut Self>, waker: &Waker) -> Poll { Poll::Pending } } diff --git a/src/test/run-pass/async-await.rs b/src/test/run-pass/async-await.rs index 552f38215738b..2def62a6ba60c 100644 --- a/src/test/run-pass/async-await.rs +++ b/src/test/run-pass/async-await.rs @@ -9,17 +9,70 @@ use std::sync::{ atomic::{self, AtomicUsize}, }; use std::task::{ - LocalWaker, Poll, Wake, - local_waker_from_nonlocal, + Poll, Waker, RawWaker, RawWakerVTable, }; +macro_rules! waker_vtable { + ($ty:ident) => { + &RawWakerVTable { + clone: clone_arc_raw::<$ty>, + drop: drop_arc_raw::<$ty>, + wake: wake_arc_raw::<$ty>, + } + }; +} + +pub trait ArcWake { + fn wake(arc_self: &Arc); + + fn into_waker(wake: Arc) -> Waker where Self: Sized + { + let ptr = Arc::into_raw(wake) as *const(); + + unsafe { + Waker::new_unchecked(RawWaker{ + data: ptr, + vtable: waker_vtable!(Self), + }) + } + } +} + +unsafe fn increase_refcount(data: *const()) { + // Retain Arc by creating a copy + let arc: Arc = Arc::from_raw(data as *const T); + let arc_clone = arc.clone(); + // Forget the Arcs again, so that the refcount isn't decrased + let _ = Arc::into_raw(arc); + let _ = Arc::into_raw(arc_clone); +} + +unsafe fn clone_arc_raw(data: *const()) -> RawWaker { + increase_refcount::(data); + RawWaker { + data: data, + vtable: waker_vtable!(T), + } +} + +unsafe fn drop_arc_raw(data: *const()) { + // Drop Arc + let _: Arc = Arc::from_raw(data as *const T); +} + +unsafe fn wake_arc_raw(data: *const()) { + let arc: Arc = Arc::from_raw(data as *const T); + ArcWake::wake(&arc); + let _ = Arc::into_raw(arc); +} + struct Counter { wakes: AtomicUsize, } -impl Wake for Counter { - fn wake(this: &Arc) { - this.wakes.fetch_add(1, atomic::Ordering::SeqCst); +impl ArcWake for Counter { + fn wake(arc_self: &Arc) { + arc_self.wakes.fetch_add(1, atomic::Ordering::SeqCst); } } @@ -29,11 +82,11 @@ fn wake_and_yield_once() -> WakeOnceThenComplete { WakeOnceThenComplete(false) } impl Future for WakeOnceThenComplete { type Output = (); - fn poll(mut self: Pin<&mut Self>, lw: &LocalWaker) -> Poll<()> { + fn poll(mut self: Pin<&mut Self>, waker: &Waker) -> Poll<()> { if self.0 { Poll::Ready(()) } else { - lw.wake(); + waker.wake(); self.0 = true; Poll::Pending } @@ -130,7 +183,7 @@ where { let mut fut = Box::pin(f(9)); let counter = Arc::new(Counter { wakes: AtomicUsize::new(0) }); - let waker = local_waker_from_nonlocal(counter.clone()); + let waker = ArcWake::into_waker(counter.clone()); assert_eq!(0, counter.wakes.load(atomic::Ordering::SeqCst)); assert_eq!(Poll::Pending, fut.as_mut().poll(&waker)); assert_eq!(1, counter.wakes.load(atomic::Ordering::SeqCst)); diff --git a/src/test/run-pass/futures-api.rs b/src/test/run-pass/futures-api.rs index e3023521100c4..058d09e2420ae 100644 --- a/src/test/run-pass/futures-api.rs +++ b/src/test/run-pass/futures-api.rs @@ -3,28 +3,75 @@ use std::future::Future; use std::pin::Pin; -use std::rc::Rc; use std::sync::{ Arc, atomic::{self, AtomicUsize}, }; use std::task::{ - Poll, Wake, Waker, LocalWaker, - local_waker, local_waker_from_nonlocal, + Poll, Waker, RawWaker, RawWakerVTable, }; -struct Counter { - local_wakes: AtomicUsize, - nonlocal_wakes: AtomicUsize, +macro_rules! waker_vtable { + ($ty:ident) => { + &RawWakerVTable { + clone: clone_arc_raw::<$ty>, + drop: drop_arc_raw::<$ty>, + wake: wake_arc_raw::<$ty>, + } + }; } -impl Wake for Counter { - fn wake(this: &Arc) { - this.nonlocal_wakes.fetch_add(1, atomic::Ordering::SeqCst); +pub trait ArcWake { + fn wake(arc_self: &Arc); + + fn into_waker(wake: Arc) -> Waker where Self: Sized + { + let ptr = Arc::into_raw(wake) as *const(); + + unsafe { + Waker::new_unchecked(RawWaker{ + data: ptr, + vtable: waker_vtable!(Self), + }) + } } +} + +unsafe fn increase_refcount(data: *const()) { + // Retain Arc by creating a copy + let arc: Arc = Arc::from_raw(data as *const T); + let arc_clone = arc.clone(); + // Forget the Arcs again, so that the refcount isn't decrased + let _ = Arc::into_raw(arc); + let _ = Arc::into_raw(arc_clone); +} - unsafe fn wake_local(this: &Arc) { - this.local_wakes.fetch_add(1, atomic::Ordering::SeqCst); +unsafe fn clone_arc_raw(data: *const()) -> RawWaker { + increase_refcount::(data); + RawWaker { + data: data, + vtable: waker_vtable!(T), + } +} + +unsafe fn drop_arc_raw(data: *const()) { + // Drop Arc + let _: Arc = Arc::from_raw(data as *const T); +} + +unsafe fn wake_arc_raw(data: *const()) { + let arc: Arc = Arc::from_raw(data as *const T); + ArcWake::wake(&arc); + let _ = Arc::into_raw(arc); +} + +struct Counter { + wakes: AtomicUsize, +} + +impl Wake for Counter { + fn wake(arc_self: &Arc) { + arc_self.wakes.fetch_add(1, atomic::Ordering::SeqCst); } } @@ -32,40 +79,28 @@ struct MyFuture; impl Future for MyFuture { type Output = (); - fn poll(self: Pin<&mut Self>, lw: &LocalWaker) -> Poll { - // Wake once locally - lw.wake(); - // Wake twice non-locally - let waker = lw.clone().into_waker(); + fn poll(self: Pin<&mut Self>, waker: &Waker) -> Poll { + // Wake twice waker.wake(); waker.wake(); Poll::Ready(()) } } -fn test_local_waker() { +fn test_waker() { let counter = Arc::new(Counter { - local_wakes: AtomicUsize::new(0), - nonlocal_wakes: AtomicUsize::new(0), + wakes: AtomicUsize::new(0), }); - let waker = unsafe { local_waker(counter.clone()) }; - assert_eq!(Poll::Ready(()), Pin::new(&mut MyFuture).poll(&waker)); - assert_eq!(1, counter.local_wakes.load(atomic::Ordering::SeqCst)); - assert_eq!(2, counter.nonlocal_wakes.load(atomic::Ordering::SeqCst)); -} + let waker = ArcWake::into_waker(counter.clone()); + assert_eq!(2, Arc::strong_count(&counter)); -fn test_local_as_nonlocal_waker() { - let counter = Arc::new(Counter { - local_wakes: AtomicUsize::new(0), - nonlocal_wakes: AtomicUsize::new(0), - }); - let waker: LocalWaker = local_waker_from_nonlocal(counter.clone()); assert_eq!(Poll::Ready(()), Pin::new(&mut MyFuture).poll(&waker)); - assert_eq!(0, counter.local_wakes.load(atomic::Ordering::SeqCst)); - assert_eq!(3, counter.nonlocal_wakes.load(atomic::Ordering::SeqCst)); + assert_eq!(2, counter.wakes.load(atomic::Ordering::SeqCst)); + + drop(waker); + assert_eq!(1, Arc::strong_count(&counter)); } fn main() { - test_local_waker(); - test_local_as_nonlocal_waker(); + test_waker(); } From f005e1c5d72c775bbb4370e9d8031fbc74efe4eb Mon Sep 17 00:00:00 2001 From: Matthias Einwag Date: Sun, 3 Feb 2019 14:59:22 -0800 Subject: [PATCH 04/25] Fix test --- src/test/run-pass/futures-api.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/run-pass/futures-api.rs b/src/test/run-pass/futures-api.rs index 058d09e2420ae..8ada7d4fa7416 100644 --- a/src/test/run-pass/futures-api.rs +++ b/src/test/run-pass/futures-api.rs @@ -69,7 +69,7 @@ struct Counter { wakes: AtomicUsize, } -impl Wake for Counter { +impl ArcWake for Counter { fn wake(arc_self: &Arc) { arc_self.wakes.fetch_add(1, atomic::Ordering::SeqCst); } From e1ec81459da4ba8e0633d90ddf440522a1587f35 Mon Sep 17 00:00:00 2001 From: Matthias Einwag Date: Tue, 5 Feb 2019 01:14:09 -0800 Subject: [PATCH 05/25] Apply more review suggestions --- src/libcore/future/future.rs | 8 +++++--- src/libcore/task/wake.rs | 6 ++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/libcore/future/future.rs b/src/libcore/future/future.rs index c40045245425b..470143d797afc 100644 --- a/src/libcore/future/future.rs +++ b/src/libcore/future/future.rs @@ -19,7 +19,8 @@ use task::{Poll, Waker}; /// final value. This method does not block if the value is not ready. Instead, /// the current task is scheduled to be woken up when it's possible to make /// further progress by `poll`ing again. The wake up is performed using -/// `cx.waker()`, a handle for waking up the current task. +/// the `waker` argument of the `poll()` method, which is a handle for waking +/// up the current task. /// /// When using a future, you generally won't call `poll` directly, but instead /// `await!` the value. @@ -78,8 +79,9 @@ pub trait Future { /// /// Once a future has completed (returned `Ready` from `poll`), /// then any future calls to `poll` may panic, block forever, or otherwise - /// cause bad behavior. The `Future` trait itself provides no guarantees - /// about the behavior of `poll` after a future has completed. + /// cause any kind of bad behavior expect causing memory unsafety. + /// The `Future` trait itself provides no guarantees about the behavior + /// of `poll` after a future has completed. /// /// [`Poll::Pending`]: ../task/enum.Poll.html#variant.Pending /// [`Poll::Ready(val)`]: ../task/enum.Poll.html#variant.Ready diff --git a/src/libcore/task/wake.rs b/src/libcore/task/wake.rs index fe2de61c59446..1f42d3e2690f6 100644 --- a/src/libcore/task/wake.rs +++ b/src/libcore/task/wake.rs @@ -42,6 +42,9 @@ pub struct RawWakerVTable { /// This function will be called when `wake` is called on the [`Waker`]. /// It must wake up the task associated with this [`RawWaker`]. + /// + /// The implemention of this function must not consume the provided data + /// pointer. pub wake: unsafe fn(*const ()), /// This function gets called when a [`RawWaker`] gets dropped. @@ -125,7 +128,10 @@ impl Drop for Waker { impl fmt::Debug for Waker { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let vtable_ptr = self.waker.vtable as *const RawWakerVTable; f.debug_struct("Waker") + .field("data", &self.waker.data) + .field("vtable", &vtable_ptr) .finish() } } From 363e992b9885e2ce55b389ab274f9cd88598104d Mon Sep 17 00:00:00 2001 From: Matthias Einwag Date: Tue, 5 Feb 2019 01:30:00 -0800 Subject: [PATCH 06/25] review suggestions --- src/libcore/future/future.rs | 6 ++++-- src/libcore/task/wake.rs | 5 +++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/libcore/future/future.rs b/src/libcore/future/future.rs index 470143d797afc..459e8a927e757 100644 --- a/src/libcore/future/future.rs +++ b/src/libcore/future/future.rs @@ -68,13 +68,15 @@ pub trait Future { /// typically do *not* suffer the same problems of "all wakeups must poll /// all events"; they are more like `epoll(4)`. /// - /// An implementation of `poll` should strive to return quickly, and must - /// *never* block. Returning quickly prevents unnecessarily clogging up + /// An implementation of `poll` should strive to return quickly, and should + /// not block. Returning quickly prevents unnecessarily clogging up /// threads or event loops. If it is known ahead of time that a call to /// `poll` may end up taking awhile, the work should be offloaded to a /// thread pool (or something similar) to ensure that `poll` can return /// quickly. /// + /// An implementation of `poll` may also never cause memory unsafety. + /// /// # Panics /// /// Once a future has completed (returned `Ready` from `poll`), diff --git a/src/libcore/task/wake.rs b/src/libcore/task/wake.rs index 1f42d3e2690f6..a877e033bc63b 100644 --- a/src/libcore/task/wake.rs +++ b/src/libcore/task/wake.rs @@ -29,6 +29,11 @@ pub struct RawWaker { /// /// The pointer passed to all functions inside the vtable is the `data` pointer /// from the enclosing [`RawWaker`] object. +/// +/// The functions inside this struct are only intended be called on the `data` +/// pointer of a properly constructed [`RawWaker`] object from inside the +/// [`RawWaker`] implementation. Calling one of the contained functions using +/// any other `data` pointer will cause undefined behavior. #[derive(PartialEq, Copy, Clone, Debug)] pub struct RawWakerVTable { /// This function will be called when the [`RawWaker`] gets cloned, e.g. when From 8e7ef03141c40e34bd740bfe521b656387af9d56 Mon Sep 17 00:00:00 2001 From: Matthias Einwag Date: Tue, 5 Feb 2019 20:32:35 -0800 Subject: [PATCH 07/25] Move ArcWake in common test file. --- src/test/run-pass/async-await.rs | 60 ++---------------------- src/test/run-pass/auxiliary/arc_wake.rs | 62 +++++++++++++++++++++++++ src/test/run-pass/futures-api.rs | 61 +++--------------------- 3 files changed, 73 insertions(+), 110 deletions(-) create mode 100644 src/test/run-pass/auxiliary/arc_wake.rs diff --git a/src/test/run-pass/async-await.rs b/src/test/run-pass/async-await.rs index 2def62a6ba60c..1843feed927a2 100644 --- a/src/test/run-pass/async-await.rs +++ b/src/test/run-pass/async-await.rs @@ -1,7 +1,10 @@ // edition:2018 +// aux-build:arc_wake.rs #![feature(arbitrary_self_types, async_await, await_macro, futures_api)] +extern crate arc_wake; + use std::pin::Pin; use std::future::Future; use std::sync::{ @@ -9,62 +12,9 @@ use std::sync::{ atomic::{self, AtomicUsize}, }; use std::task::{ - Poll, Waker, RawWaker, RawWakerVTable, + Poll, Waker, }; - -macro_rules! waker_vtable { - ($ty:ident) => { - &RawWakerVTable { - clone: clone_arc_raw::<$ty>, - drop: drop_arc_raw::<$ty>, - wake: wake_arc_raw::<$ty>, - } - }; -} - -pub trait ArcWake { - fn wake(arc_self: &Arc); - - fn into_waker(wake: Arc) -> Waker where Self: Sized - { - let ptr = Arc::into_raw(wake) as *const(); - - unsafe { - Waker::new_unchecked(RawWaker{ - data: ptr, - vtable: waker_vtable!(Self), - }) - } - } -} - -unsafe fn increase_refcount(data: *const()) { - // Retain Arc by creating a copy - let arc: Arc = Arc::from_raw(data as *const T); - let arc_clone = arc.clone(); - // Forget the Arcs again, so that the refcount isn't decrased - let _ = Arc::into_raw(arc); - let _ = Arc::into_raw(arc_clone); -} - -unsafe fn clone_arc_raw(data: *const()) -> RawWaker { - increase_refcount::(data); - RawWaker { - data: data, - vtable: waker_vtable!(T), - } -} - -unsafe fn drop_arc_raw(data: *const()) { - // Drop Arc - let _: Arc = Arc::from_raw(data as *const T); -} - -unsafe fn wake_arc_raw(data: *const()) { - let arc: Arc = Arc::from_raw(data as *const T); - ArcWake::wake(&arc); - let _ = Arc::into_raw(arc); -} +use arc_wake::ArcWake; struct Counter { wakes: AtomicUsize, diff --git a/src/test/run-pass/auxiliary/arc_wake.rs b/src/test/run-pass/auxiliary/arc_wake.rs new file mode 100644 index 0000000000000..0baaa378046f3 --- /dev/null +++ b/src/test/run-pass/auxiliary/arc_wake.rs @@ -0,0 +1,62 @@ +// edition:2018 + +#![feature(arbitrary_self_types, futures_api)] + +use std::sync::Arc; +use std::task::{ + Poll, Waker, RawWaker, RawWakerVTable, +}; + +macro_rules! waker_vtable { + ($ty:ident) => { + &RawWakerVTable { + clone: clone_arc_raw::<$ty>, + drop: drop_arc_raw::<$ty>, + wake: wake_arc_raw::<$ty>, + } + }; +} + +pub trait ArcWake { + fn wake(arc_self: &Arc); + + fn into_waker(wake: Arc) -> Waker where Self: Sized + { + let ptr = Arc::into_raw(wake) as *const(); + + unsafe { + Waker::new_unchecked(RawWaker{ + data: ptr, + vtable: waker_vtable!(Self), + }) + } + } +} + +unsafe fn increase_refcount(data: *const()) { + // Retain Arc by creating a copy + let arc: Arc = Arc::from_raw(data as *const T); + let arc_clone = arc.clone(); + // Forget the Arcs again, so that the refcount isn't decrased + let _ = Arc::into_raw(arc); + let _ = Arc::into_raw(arc_clone); +} + +unsafe fn clone_arc_raw(data: *const()) -> RawWaker { + increase_refcount::(data); + RawWaker { + data: data, + vtable: waker_vtable!(T), + } +} + +unsafe fn drop_arc_raw(data: *const()) { + // Drop Arc + let _: Arc = Arc::from_raw(data as *const T); +} + +unsafe fn wake_arc_raw(data: *const()) { + let arc: Arc = Arc::from_raw(data as *const T); + ArcWake::wake(&arc); + let _ = Arc::into_raw(arc); +} diff --git a/src/test/run-pass/futures-api.rs b/src/test/run-pass/futures-api.rs index 8ada7d4fa7416..fd4b585d34572 100644 --- a/src/test/run-pass/futures-api.rs +++ b/src/test/run-pass/futures-api.rs @@ -1,6 +1,10 @@ +// aux-build:arc_wake.rs + #![feature(arbitrary_self_types, futures_api)] #![allow(unused)] +extern crate arc_wake; + use std::future::Future; use std::pin::Pin; use std::sync::{ @@ -8,62 +12,9 @@ use std::sync::{ atomic::{self, AtomicUsize}, }; use std::task::{ - Poll, Waker, RawWaker, RawWakerVTable, + Poll, Waker, }; - -macro_rules! waker_vtable { - ($ty:ident) => { - &RawWakerVTable { - clone: clone_arc_raw::<$ty>, - drop: drop_arc_raw::<$ty>, - wake: wake_arc_raw::<$ty>, - } - }; -} - -pub trait ArcWake { - fn wake(arc_self: &Arc); - - fn into_waker(wake: Arc) -> Waker where Self: Sized - { - let ptr = Arc::into_raw(wake) as *const(); - - unsafe { - Waker::new_unchecked(RawWaker{ - data: ptr, - vtable: waker_vtable!(Self), - }) - } - } -} - -unsafe fn increase_refcount(data: *const()) { - // Retain Arc by creating a copy - let arc: Arc = Arc::from_raw(data as *const T); - let arc_clone = arc.clone(); - // Forget the Arcs again, so that the refcount isn't decrased - let _ = Arc::into_raw(arc); - let _ = Arc::into_raw(arc_clone); -} - -unsafe fn clone_arc_raw(data: *const()) -> RawWaker { - increase_refcount::(data); - RawWaker { - data: data, - vtable: waker_vtable!(T), - } -} - -unsafe fn drop_arc_raw(data: *const()) { - // Drop Arc - let _: Arc = Arc::from_raw(data as *const T); -} - -unsafe fn wake_arc_raw(data: *const()) { - let arc: Arc = Arc::from_raw(data as *const T); - ArcWake::wake(&arc); - let _ = Arc::into_raw(arc); -} +use arc_wake::ArcWake; struct Counter { wakes: AtomicUsize, From a1c4cf6889f69dc335a3f0b42b29e4d9a081005d Mon Sep 17 00:00:00 2001 From: Matthias Einwag Date: Wed, 6 Feb 2019 22:56:33 -0800 Subject: [PATCH 08/25] Change RawWaker constructor to const fn --- src/libcore/task/wake.rs | 28 +++++++++++++++++++++---- src/test/run-pass/auxiliary/arc_wake.rs | 10 ++------- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/libcore/task/wake.rs b/src/libcore/task/wake.rs index a877e033bc63b..21f0a8cea4168 100644 --- a/src/libcore/task/wake.rs +++ b/src/libcore/task/wake.rs @@ -19,9 +19,29 @@ pub struct RawWaker { /// that is associated with the task. /// The value of this field gets passed to all functions that are part of /// the vtable as the first parameter. - pub data: *const (), + data: *const (), /// Virtual function pointer table that customizes the behavior of this waker. - pub vtable: &'static RawWakerVTable, + vtable: &'static RawWakerVTable, +} + +impl RawWaker { + /// Creates a new `RawWaker` from the provided `data` pointer and `vtable`. + /// + /// The `data` pointer can be used to store arbitrary data as required + /// by the executor. This could be e.g. a type-erased pointer to an `Arc` + /// that is associated with the task. + /// The value of this poiner will get passed to all functions that are part + /// of the `vtable` as the first parameter. + /// + /// The `vtable` customizes the behavior of a `Waker` which gets created + /// from a `RawWaker`. For each operation on the `Waker`, the associated + /// function in the `vtable` of the underlying `RawWaker` will be called. + pub const fn new(data: *const (), vtable: &'static RawWakerVTable) -> RawWaker { + RawWaker { + data, + vtable, + } + } } /// A virtual function pointer table (vtable) that specifies the behavior @@ -102,8 +122,8 @@ impl Waker { /// Creates a new `Waker` from [`RawWaker`]. /// /// The behavior of the returned `Waker` is undefined if the contract defined - /// in [RawWaker]'s documentation is not upheld. Therefore this method is - /// unsafe. + /// in [`RawWaker`]'s and [`RawWakerVTable`]'s documentation is not upheld. + /// Therefore this method is unsafe. pub unsafe fn new_unchecked(waker: RawWaker) -> Waker { Waker { waker, diff --git a/src/test/run-pass/auxiliary/arc_wake.rs b/src/test/run-pass/auxiliary/arc_wake.rs index 0baaa378046f3..034e378af7f19 100644 --- a/src/test/run-pass/auxiliary/arc_wake.rs +++ b/src/test/run-pass/auxiliary/arc_wake.rs @@ -25,10 +25,7 @@ pub trait ArcWake { let ptr = Arc::into_raw(wake) as *const(); unsafe { - Waker::new_unchecked(RawWaker{ - data: ptr, - vtable: waker_vtable!(Self), - }) + Waker::new_unchecked(RawWaker::new(ptr, waker_vtable!(Self))) } } } @@ -44,10 +41,7 @@ unsafe fn increase_refcount(data: *const()) { unsafe fn clone_arc_raw(data: *const()) -> RawWaker { increase_refcount::(data); - RawWaker { - data: data, - vtable: waker_vtable!(T), - } + RawWaker::new(data, waker_vtable!(T)) } unsafe fn drop_arc_raw(data: *const()) { From 9e934e22151d5589cbe3cb1193250d8ac0b366fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 7 Feb 2019 05:39:54 -0800 Subject: [PATCH 09/25] Reweork incompatible match arms error - Point at the body expression of the match arm with the type error. - Point at the prior match arms explicitely stating the evaluated type. - Point at the entire match expr in a secondary span, instead of primary. - For type errors in the first match arm, the cause is outside of the match, treat as implicit block error to give a more appropriate error. --- src/librustc/infer/error_reporting/mod.rs | 29 +++++++++++++---------- src/librustc/traits/mod.rs | 2 ++ src/librustc/traits/structural_impls.rs | 12 +++++++--- src/librustc_typeck/check/_match.rs | 23 ++++++++++++++---- src/test/ui/if/if-let-arm-types.rs | 9 +++---- src/test/ui/if/if-let-arm-types.stderr | 22 ++++++----------- src/test/ui/issues/issue-11319.rs | 10 ++++---- src/test/ui/issues/issue-11319.stderr | 18 ++++++++------ src/test/ui/issues/issue-17728.rs | 3 ++- src/test/ui/issues/issue-17728.stderr | 10 ++++---- src/test/ui/issues/issue-24036.rs | 2 +- src/test/ui/issues/issue-24036.stderr | 14 +++++------ 12 files changed, 92 insertions(+), 62 deletions(-) diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs index 66e4cd49c807f..361aee4405b40 100644 --- a/src/librustc/infer/error_reporting/mod.rs +++ b/src/librustc/infer/error_reporting/mod.rs @@ -509,22 +509,27 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } } } - ObligationCauseCode::MatchExpressionArm { arm_span, source } => match source { + ObligationCauseCode::MatchExpressionArm { + source, + ref prior_arms, + .. + } => match source { hir::MatchSource::IfLetDesugar { .. } => { - let msg = "`if let` arm with an incompatible type"; - if self.tcx.sess.source_map().is_multiline(arm_span) { - err.span_note(arm_span, msg); - } else { - err.span_label(arm_span, msg); - } + let msg = "`if let` arms have incompatible types"; + err.span_label(cause.span, msg); } hir::MatchSource::TryDesugar => {} _ => { - let msg = "match arm with an incompatible type"; - if self.tcx.sess.source_map().is_multiline(arm_span) { - err.span_note(arm_span, msg); - } else { - err.span_label(arm_span, msg); + let msg = "`match` arms have incompatible types"; + err.span_label(cause.span, msg); + if prior_arms.len() < 4 { + for (sp, ty) in prior_arms { + err.span_label(*sp, format!("this is found to be of type `{}`", ty)); + } + } else if let Some((sp, ty)) = prior_arms.last() { + err.span_label(*sp, format!( + "this and all prior arms are found to be of type `{}`", ty, + )); } } }, diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 68383bef37a6a..9824ff8f397e1 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -145,6 +145,7 @@ impl<'tcx> ObligationCause<'tcx> { ObligationCauseCode::StartFunctionType => { tcx.sess.source_map().def_span(self.span) } + ObligationCauseCode::MatchExpressionArm { arm_span, .. } => arm_span, _ => self.span, } } @@ -223,6 +224,7 @@ pub enum ObligationCauseCode<'tcx> { MatchExpressionArm { arm_span: Span, source: hir::MatchSource, + prior_arms: Vec<(Span, Ty<'tcx>)>, }, /// Computing common supertype in the pattern guard for the arms of a match expression diff --git a/src/librustc/traits/structural_impls.rs b/src/librustc/traits/structural_impls.rs index 2f5df022218fe..c0e00cf291bac 100644 --- a/src/librustc/traits/structural_impls.rs +++ b/src/librustc/traits/structural_impls.rs @@ -513,10 +513,16 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> { trait_item_def_id, }), super::ExprAssignable => Some(super::ExprAssignable), - super::MatchExpressionArm { arm_span, source } => Some(super::MatchExpressionArm { + super::MatchExpressionArm { arm_span, - source: source, - }), + source, + ref prior_arms, + } => { + let prior_arms = prior_arms.iter().filter_map(|(sp, ty)| { + tcx.lift(ty).map(|ty| (*sp, ty)) + }).collect(); + Some(super::MatchExpressionArm { arm_span, source, prior_arms }) + } super::MatchExpressionArmPattern { span, ty } => { tcx.lift(&ty).map(|ty| super::MatchExpressionArmPattern { span, ty }) } diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index a90d83f3f8be0..c662a49abf9ea 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -689,6 +689,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); CoerceMany::with_coercion_sites(coerce_first, arms) }; + let mut other_arms = vec![]; // used only for diagnostics for (i, (arm, pats_diverge)) in arms.iter().zip(all_arm_pats_diverge).enumerate() { if let Some(ref g) = arm.guard { self.diverges.set(pats_diverge); @@ -709,17 +710,31 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); _ => false }; + let arm_span = if let hir::ExprKind::Block(ref blk, _) = arm.body.node { + // Point at the block expr instead of the entire block + blk.expr.as_ref().map(|e| e.span).unwrap_or(arm.body.span) + } else { + arm.body.span + }; if is_if_let_fallback { let cause = self.cause(expr.span, ObligationCauseCode::IfExpressionWithNoElse); assert!(arm_ty.is_unit()); coercion.coerce_forced_unit(self, &cause, &mut |_| (), true); } else { - let cause = self.cause(expr.span, ObligationCauseCode::MatchExpressionArm { - arm_span: arm.body.span, - source: match_src - }); + let cause = if i == 0 { + // 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. + self.cause(arm_span, ObligationCauseCode::BlockTailExpression(arm.body.id)) + } else { + self.cause(expr.span, ObligationCauseCode::MatchExpressionArm { + arm_span, + source: match_src, + prior_arms: other_arms.clone(), + }) + }; coercion.coerce(self, &cause, &arm.body, arm_ty); } + other_arms.push((arm_span, arm_ty)); } // We won't diverge unless the discriminant or all arms diverge. diff --git a/src/test/ui/if/if-let-arm-types.rs b/src/test/ui/if/if-let-arm-types.rs index 749c089ae9752..819f5dd1cfc35 100644 --- a/src/test/ui/if/if-let-arm-types.rs +++ b/src/test/ui/if/if-let-arm-types.rs @@ -1,10 +1,11 @@ fn main() { - if let Some(b) = None { //~ ERROR: `if let` arms have incompatible types - //~^ expected (), found integer - //~| expected type `()` - //~| found type `{integer}` + if let Some(b) = None { + //~^ NOTE if let` arms have incompatible types () } else { 1 }; + //~^^ ERROR: `if let` arms have incompatible types + //~| NOTE expected (), found integer + //~| NOTE expected type `()` } diff --git a/src/test/ui/if/if-let-arm-types.stderr b/src/test/ui/if/if-let-arm-types.stderr index fcf9e4695f675..6401a62c06ba2 100644 --- a/src/test/ui/if/if-let-arm-types.stderr +++ b/src/test/ui/if/if-let-arm-types.stderr @@ -1,25 +1,17 @@ error[E0308]: `if let` arms have incompatible types - --> $DIR/if-let-arm-types.rs:2:5 + --> $DIR/if-let-arm-types.rs:6:9 | -LL | / if let Some(b) = None { //~ ERROR: `if let` arms have incompatible types -LL | | //~^ expected (), found integer -LL | | //~| expected type `()` -LL | | //~| found type `{integer}` -... | +LL | / if let Some(b) = None { +LL | | //~^ NOTE if let` arms have incompatible types +LL | | () +LL | | } else { LL | | 1 + | | ^ expected (), found integer LL | | }; - | |_____^ expected (), found integer + | |_____- `if let` arms have incompatible types | = note: expected type `()` found type `{integer}` -note: `if let` arm with an incompatible type - --> $DIR/if-let-arm-types.rs:7:12 - | -LL | } else { - | ____________^ -LL | | 1 -LL | | }; - | |_____^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-11319.rs b/src/test/ui/issues/issue-11319.rs index ea901205544b6..726c437355e53 100644 --- a/src/test/ui/issues/issue-11319.rs +++ b/src/test/ui/issues/issue-11319.rs @@ -1,12 +1,14 @@ fn main() { match Some(10) { - //~^ ERROR match arms have incompatible types - //~| expected type `bool` - //~| found type `()` - //~| expected bool, found () + //~^ NOTE `match` arms have incompatible types Some(5) => false, + //~^ NOTE this is found to be of type `bool` Some(2) => true, + //~^ NOTE this is found to be of type `bool` None => (), + //~^ ERROR match arms have incompatible types + //~| NOTE expected bool, found () + //~| NOTE expected type `bool` _ => true } } diff --git a/src/test/ui/issues/issue-11319.stderr b/src/test/ui/issues/issue-11319.stderr index 44d63ba3e6879..10db477b8ca7b 100644 --- a/src/test/ui/issues/issue-11319.stderr +++ b/src/test/ui/issues/issue-11319.stderr @@ -1,16 +1,20 @@ error[E0308]: match arms have incompatible types - --> $DIR/issue-11319.rs:2:5 + --> $DIR/issue-11319.rs:8:20 | LL | / match Some(10) { -LL | | //~^ ERROR match arms have incompatible types -LL | | //~| expected type `bool` -LL | | //~| found type `()` -... | +LL | | //~^ NOTE `match` arms have incompatible types +LL | | Some(5) => false, + | | ----- this is found to be of type `bool` +LL | | //~^ NOTE this is found to be of type `bool` +LL | | Some(2) => true, + | | ---- this is found to be of type `bool` +LL | | //~^ NOTE this is found to be of type `bool` LL | | None => (), - | | -- match arm with an incompatible type + | | ^^ expected bool, found () +... | LL | | _ => true LL | | } - | |_____^ expected bool, found () + | |_____- `match` arms have incompatible types | = note: expected type `bool` found type `()` diff --git a/src/test/ui/issues/issue-17728.rs b/src/test/ui/issues/issue-17728.rs index 0f13ae3304d6c..15cea1d609d53 100644 --- a/src/test/ui/issues/issue-17728.rs +++ b/src/test/ui/issues/issue-17728.rs @@ -97,7 +97,7 @@ impl Debug for Player { } fn str_to_direction(to_parse: &str) -> RoomDirection { - match to_parse { //~ ERROR match arms have incompatible types + match to_parse { "w" | "west" => RoomDirection::West, "e" | "east" => RoomDirection::East, "n" | "north" => RoomDirection::North, @@ -108,6 +108,7 @@ fn str_to_direction(to_parse: &str) -> RoomDirection { "down" => RoomDirection::Down, _ => None } + //~^^ ERROR match arms have incompatible types } fn main() { diff --git a/src/test/ui/issues/issue-17728.stderr b/src/test/ui/issues/issue-17728.stderr index 355868f05690f..2c2efad19f569 100644 --- a/src/test/ui/issues/issue-17728.stderr +++ b/src/test/ui/issues/issue-17728.stderr @@ -10,17 +10,19 @@ LL | Some(entry) => Ok(entry), | ^^^^^^^^^ ...but data from `room` is returned here error[E0308]: match arms have incompatible types - --> $DIR/issue-17728.rs:100:5 + --> $DIR/issue-17728.rs:109:14 | -LL | / match to_parse { //~ ERROR match arms have incompatible types +LL | / match to_parse { LL | | "w" | "west" => RoomDirection::West, LL | | "e" | "east" => RoomDirection::East, LL | | "n" | "north" => RoomDirection::North, ... | +LL | | "down" => RoomDirection::Down, + | | ------------------- this and all prior arms are found to be of type `RoomDirection` LL | | _ => None - | | ---- match arm with an incompatible type + | | ^^^^ expected enum `RoomDirection`, found enum `std::option::Option` LL | | } - | |_____^ expected enum `RoomDirection`, found enum `std::option::Option` + | |_____- `match` arms have incompatible types | = note: expected type `RoomDirection` found type `std::option::Option<_>` diff --git a/src/test/ui/issues/issue-24036.rs b/src/test/ui/issues/issue-24036.rs index 3642085934abe..2f501b941b5a6 100644 --- a/src/test/ui/issues/issue-24036.rs +++ b/src/test/ui/issues/issue-24036.rs @@ -6,11 +6,11 @@ fn closure_to_loc() { fn closure_from_match() { let x = match 1usize { - //~^ ERROR match arms have incompatible types 1 => |c| c + 1, 2 => |c| c - 1, _ => |c| c - 1 }; + //~^^^ ERROR match arms have incompatible types } fn main() { } diff --git a/src/test/ui/issues/issue-24036.stderr b/src/test/ui/issues/issue-24036.stderr index 9f799c9b45069..fa9935fcf619d 100644 --- a/src/test/ui/issues/issue-24036.stderr +++ b/src/test/ui/issues/issue-24036.stderr @@ -10,20 +10,20 @@ LL | x = |c| c + 1; = help: consider boxing your closure and/or using it as a trait object error[E0308]: match arms have incompatible types - --> $DIR/issue-24036.rs:8:13 + --> $DIR/issue-24036.rs:10:14 | LL | let x = match 1usize { - | _____________^ -LL | | //~^ ERROR match arms have incompatible types + | _____________- LL | | 1 => |c| c + 1, + | | --------- this is found to be of type `[closure@$DIR/issue-24036.rs:9:14: 9:23]` LL | | 2 => |c| c - 1, - | | --------- match arm with an incompatible type + | | ^^^^^^^^^ expected closure, found a different closure LL | | _ => |c| c - 1 LL | | }; - | |_____^ expected closure, found a different closure + | |_____- `match` arms have incompatible types | - = note: expected type `[closure@$DIR/issue-24036.rs:10:14: 10:23]` - found type `[closure@$DIR/issue-24036.rs:11:14: 11:23]` + = note: expected type `[closure@$DIR/issue-24036.rs:9:14: 9:23]` + found type `[closure@$DIR/issue-24036.rs:10:14: 10:23]` = note: no two closures, even if identical, have the same type = help: consider boxing your closure and/or using it as a trait object From 7eb6a2a9729dd0f948fc30e391e32c0a51d3d0a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 7 Feb 2019 16:12:12 -0800 Subject: [PATCH 10/25] Add test for type mismatch on first match arm --- src/test/ui/match/match-type-err-first-arm.rs | 27 ++++++++++++++++ .../ui/match/match-type-err-first-arm.stderr | 31 +++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 src/test/ui/match/match-type-err-first-arm.rs create mode 100644 src/test/ui/match/match-type-err-first-arm.stderr diff --git a/src/test/ui/match/match-type-err-first-arm.rs b/src/test/ui/match/match-type-err-first-arm.rs new file mode 100644 index 0000000000000..31fa50d2b452b --- /dev/null +++ b/src/test/ui/match/match-type-err-first-arm.rs @@ -0,0 +1,27 @@ +fn main() { + let _ = test_func1(1); + let _ = test_func2(1); +} + +fn test_func1(n: i32) -> i32 { + //~^ NOTE expected `i32` because of return type + match n { + 12 => 'b', + //~^ ERROR mismatched types + //~| NOTE expected i32, found char + _ => 42, + } +} + +fn test_func2(n: i32) -> i32 { + let x = match n { + //~^ NOTE `match` arms have incompatible types + 12 => 'b', + //~^ NOTE this is found to be of type `char` + _ => 42, + //~^ ERROR match arms have incompatible types + //~| NOTE expected char, found integer + //~| NOTE expected type `char` + }; + x +} diff --git a/src/test/ui/match/match-type-err-first-arm.stderr b/src/test/ui/match/match-type-err-first-arm.stderr new file mode 100644 index 0000000000000..8f722343a5875 --- /dev/null +++ b/src/test/ui/match/match-type-err-first-arm.stderr @@ -0,0 +1,31 @@ +error[E0308]: mismatched types + --> $DIR/match-type-err-first-arm.rs:9:15 + | +LL | fn test_func1(n: i32) -> i32 { + | --- expected `i32` because of return type +... +LL | 12 => 'b', + | ^^^ expected i32, found char + +error[E0308]: match arms have incompatible types + --> $DIR/match-type-err-first-arm.rs:21:14 + | +LL | let x = match n { + | _____________- +LL | | //~^ NOTE `match` arms have incompatible types +LL | | 12 => 'b', + | | --- this is found to be of type `char` +LL | | //~^ NOTE this is found to be of type `char` +LL | | _ => 42, + | | ^^ expected char, found integer +... | +LL | | //~| NOTE expected type `char` +LL | | }; + | |_____- `match` arms have incompatible types + | + = note: expected type `char` + found type `{integer}` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. From 802c897eb3c95e6643bd4e2cd85052786458e3d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 8 Feb 2019 05:44:15 -0800 Subject: [PATCH 11/25] review comments: (marginally) reduce memory consumtion --- src/librustc/infer/error_reporting/mod.rs | 14 +++++++---- src/librustc/traits/mod.rs | 3 ++- src/librustc/traits/structural_impls.rs | 13 ++++++---- src/librustc_typeck/check/_match.rs | 8 ++++++- src/test/ui/match/match-type-err-first-arm.rs | 18 ++++++++++++++ .../ui/match/match-type-err-first-arm.stderr | 24 ++++++++++++++++++- 6 files changed, 68 insertions(+), 12 deletions(-) diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs index 361aee4405b40..82e9a1e3c895b 100644 --- a/src/librustc/infer/error_reporting/mod.rs +++ b/src/librustc/infer/error_reporting/mod.rs @@ -512,6 +512,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { ObligationCauseCode::MatchExpressionArm { source, ref prior_arms, + last_ty, .. } => match source { hir::MatchSource::IfLetDesugar { .. } => { @@ -522,13 +523,16 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { _ => { let msg = "`match` arms have incompatible types"; err.span_label(cause.span, msg); - if prior_arms.len() < 4 { - for (sp, ty) in prior_arms { - err.span_label(*sp, format!("this is found to be of type `{}`", ty)); + if prior_arms.len() <= 4 { + for sp in prior_arms { + err.span_label(*sp, format!( + "this is found to be of type `{}`", + last_ty, + )); } - } else if let Some((sp, ty)) = prior_arms.last() { + } else if let Some(sp) = prior_arms.last() { err.span_label(*sp, format!( - "this and all prior arms are found to be of type `{}`", ty, + "this and all prior arms are found to be of type `{}`", last_ty, )); } } diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 9824ff8f397e1..7fc27ba735e47 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -224,7 +224,8 @@ pub enum ObligationCauseCode<'tcx> { MatchExpressionArm { arm_span: Span, source: hir::MatchSource, - prior_arms: Vec<(Span, Ty<'tcx>)>, + prior_arms: Vec, + last_ty: Ty<'tcx>, }, /// Computing common supertype in the pattern guard for the arms of a match expression diff --git a/src/librustc/traits/structural_impls.rs b/src/librustc/traits/structural_impls.rs index c0e00cf291bac..8bbeec0b8ac2b 100644 --- a/src/librustc/traits/structural_impls.rs +++ b/src/librustc/traits/structural_impls.rs @@ -517,11 +517,16 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> { arm_span, source, ref prior_arms, + last_ty, } => { - let prior_arms = prior_arms.iter().filter_map(|(sp, ty)| { - tcx.lift(ty).map(|ty| (*sp, ty)) - }).collect(); - Some(super::MatchExpressionArm { arm_span, source, prior_arms }) + tcx.lift(&last_ty).map(|last_ty| { + super::MatchExpressionArm { + arm_span, + source, + prior_arms: prior_arms.clone(), + last_ty, + } + }) } super::MatchExpressionArmPattern { span, ty } => { tcx.lift(&ty).map(|ty| super::MatchExpressionArmPattern { span, ty }) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index c662a49abf9ea..cfc7cedc5e3e5 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -690,6 +690,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); }; let mut other_arms = vec![]; // used only for diagnostics + let mut prior_arm_ty = None; for (i, (arm, pats_diverge)) in arms.iter().zip(all_arm_pats_diverge).enumerate() { if let Some(ref g) = arm.guard { self.diverges.set(pats_diverge); @@ -730,11 +731,16 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); arm_span, source: match_src, prior_arms: other_arms.clone(), + last_ty: prior_arm_ty.unwrap(), }) }; coercion.coerce(self, &cause, &arm.body, arm_ty); } - other_arms.push((arm_span, arm_ty)); + other_arms.push(arm_span); + if other_arms.len() > 5 { + other_arms.remove(0); + } + prior_arm_ty = Some(arm_ty); } // We won't diverge unless the discriminant or all arms diverge. diff --git a/src/test/ui/match/match-type-err-first-arm.rs b/src/test/ui/match/match-type-err-first-arm.rs index 31fa50d2b452b..b4b84ef8f1cec 100644 --- a/src/test/ui/match/match-type-err-first-arm.rs +++ b/src/test/ui/match/match-type-err-first-arm.rs @@ -25,3 +25,21 @@ fn test_func2(n: i32) -> i32 { }; x } + +fn test_func3(n: i32) -> i32 { + let x = match n { + //~^ NOTE `match` arms have incompatible types + 1 => 'b', + 2 => 'b', + 3 => 'b', + 4 => 'b', + 5 => 'b', + 6 => 'b', + //~^ NOTE this and all prior arms are found to be of type `char` + _ => 42, + //~^ ERROR match arms have incompatible types + //~| NOTE expected char, found integer + //~| NOTE expected type `char` + }; + x +} diff --git a/src/test/ui/match/match-type-err-first-arm.stderr b/src/test/ui/match/match-type-err-first-arm.stderr index 8f722343a5875..db8bef8dc7755 100644 --- a/src/test/ui/match/match-type-err-first-arm.stderr +++ b/src/test/ui/match/match-type-err-first-arm.stderr @@ -26,6 +26,28 @@ LL | | }; = note: expected type `char` found type `{integer}` -error: aborting due to 2 previous errors +error[E0308]: match arms have incompatible types + --> $DIR/match-type-err-first-arm.rs:39:14 + | +LL | let x = match n { + | _____________- +LL | | //~^ NOTE `match` arms have incompatible types +LL | | 1 => 'b', +LL | | 2 => 'b', +... | +LL | | 6 => 'b', + | | --- this and all prior arms are found to be of type `char` +LL | | //~^ NOTE this and all prior arms are found to be of type `char` +LL | | _ => 42, + | | ^^ expected char, found integer +... | +LL | | //~| NOTE expected type `char` +LL | | }; + | |_____- `match` arms have incompatible types + | + = note: expected type `char` + found type `{integer}` + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0308`. From 7d6bfc53c85d3eb9470fd30621a1b54a61bbbdb6 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Fri, 8 Feb 2019 16:41:37 +0100 Subject: [PATCH 12/25] Extract block to insert an intrinsic into its own function --- src/librustc_codegen_llvm/context.rs | 29 ++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/librustc_codegen_llvm/context.rs b/src/librustc_codegen_llvm/context.rs index f679558844198..03de20861a9e5 100644 --- a/src/librustc_codegen_llvm/context.rs +++ b/src/librustc_codegen_llvm/context.rs @@ -465,6 +465,20 @@ impl CodegenCx<'b, 'tcx> { self.declare_intrinsic(key).unwrap_or_else(|| bug!("unknown intrinsic '{}'", key)) } + fn insert_intrinsic( + &self, name: &'static str, args: Option<&[&'b llvm::Type]>, ret: &'b llvm::Type + ) -> &'b llvm::Value { + let fn_ty = if let Some(args) = args { + self.type_func(args, ret) + } else { + self.type_variadic_func(&[], ret) + }; + let f = self.declare_cfn(name, fn_ty); + llvm::SetUnnamedAddr(f, false); + self.intrinsics.borrow_mut().insert(name, f.clone()); + f + } + fn declare_intrinsic( &self, key: &str @@ -472,26 +486,17 @@ impl CodegenCx<'b, 'tcx> { macro_rules! ifn { ($name:expr, fn() -> $ret:expr) => ( if key == $name { - let f = self.declare_cfn($name, self.type_func(&[], $ret)); - llvm::SetUnnamedAddr(f, false); - self.intrinsics.borrow_mut().insert($name, f.clone()); - return Some(f); + return Some(self.insert_intrinsic($name, Some(&[]), $ret)); } ); ($name:expr, fn(...) -> $ret:expr) => ( if key == $name { - let f = self.declare_cfn($name, self.type_variadic_func(&[], $ret)); - llvm::SetUnnamedAddr(f, false); - self.intrinsics.borrow_mut().insert($name, f.clone()); - return Some(f); + return Some(self.insert_intrinsic($name, None, $ret)); } ); ($name:expr, fn($($arg:expr),*) -> $ret:expr) => ( if key == $name { - let f = self.declare_cfn($name, self.type_func(&[$($arg),*], $ret)); - llvm::SetUnnamedAddr(f, false); - self.intrinsics.borrow_mut().insert($name, f.clone()); - return Some(f); + return Some(self.insert_intrinsic($name, Some(&[$($arg),*]), $ret)); } ); } From 1ef34a5a39641846e824b6450a705d6031002beb Mon Sep 17 00:00:00 2001 From: Matthias Einwag Date: Fri, 8 Feb 2019 18:47:19 -0800 Subject: [PATCH 13/25] Remove rustdoc test which referenced unstable API --- src/test/rustdoc-js/substring.js | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 src/test/rustdoc-js/substring.js diff --git a/src/test/rustdoc-js/substring.js b/src/test/rustdoc-js/substring.js deleted file mode 100644 index 3a6750151f7d8..0000000000000 --- a/src/test/rustdoc-js/substring.js +++ /dev/null @@ -1,10 +0,0 @@ -// exact-check - -const QUERY = 'waker_from'; - -const EXPECTED = { - 'others': [ - { 'path': 'std::task', 'name': 'local_waker_from_nonlocal' }, - { 'path': 'alloc::task', 'name': 'local_waker_from_nonlocal' }, - ], -}; From 05b4e7c8a9efa9593907008e16b8f65242c72594 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 8 Feb 2019 02:45:53 -0800 Subject: [PATCH 14/25] Add way to hide suggestion snippet window from cli output --- src/librustc_errors/diagnostic.rs | 9 ++-- src/librustc_errors/emitter.rs | 87 ++++++++++++++++++++----------- src/librustc_errors/lib.rs | 24 ++++++++- 3 files changed, 84 insertions(+), 36 deletions(-) diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs index aefe296ad0fa7..484cfd045a69f 100644 --- a/src/librustc_errors/diagnostic.rs +++ b/src/librustc_errors/diagnostic.rs @@ -1,4 +1,5 @@ use crate::CodeSuggestion; +use crate::SuggestionStyle; use crate::SubstitutionPart; use crate::Substitution; use crate::Applicability; @@ -243,7 +244,7 @@ impl Diagnostic { .collect(), }], msg: msg.to_owned(), - show_code_when_inline: true, + style: SuggestionStyle::ShowCode, applicability, }); self @@ -277,7 +278,7 @@ impl Diagnostic { }], }], msg: msg.to_owned(), - show_code_when_inline: true, + style: SuggestionStyle::ShowCode, applicability, }); self @@ -295,7 +296,7 @@ impl Diagnostic { }], }).collect(), msg: msg.to_owned(), - show_code_when_inline: true, + style: SuggestionStyle::ShowCode, applicability, }); self @@ -316,7 +317,7 @@ impl Diagnostic { }], }], msg: msg.to_owned(), - show_code_when_inline: false, + style: SuggestionStyle::HideCodeInline, applicability: applicability, }); self diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 2821201173ea0..5696b2eee8956 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -2,7 +2,10 @@ use Destination::*; use syntax_pos::{SourceFile, Span, MultiSpan}; -use crate::{Level, CodeSuggestion, DiagnosticBuilder, SubDiagnostic, SourceMapperDyn, DiagnosticId}; +use crate::{ + Level, CodeSuggestion, DiagnosticBuilder, SubDiagnostic, + SuggestionStyle, SourceMapperDyn, DiagnosticId, +}; use crate::snippet::{Annotation, AnnotationType, Line, MultilineAnnotation, StyledString, Style}; use crate::styled_buffer::StyledBuffer; @@ -45,7 +48,7 @@ impl Emitter for EmitterWriter { // don't display multiline suggestions as labels !sugg.substitutions[0].parts[0].snippet.contains('\n') { let substitution = &sugg.substitutions[0].parts[0].snippet.trim(); - let msg = if substitution.len() == 0 || !sugg.show_code_when_inline { + let msg = if substitution.len() == 0 || sugg.style.hide_inline() { // This substitution is only removal or we explicitly don't want to show the // code inline, don't show it format!("help: {}", sugg.msg) @@ -942,14 +945,15 @@ impl EmitterWriter { } } - fn emit_message_default(&mut self, - msp: &MultiSpan, - msg: &[(String, Style)], - code: &Option, - level: &Level, - max_line_num_len: usize, - is_secondary: bool) - -> io::Result<()> { + fn emit_message_default( + &mut self, + msp: &MultiSpan, + msg: &[(String, Style)], + code: &Option, + level: &Level, + max_line_num_len: usize, + is_secondary: bool, + ) -> io::Result<()> { let mut buffer = StyledBuffer::new(); let header_style = if is_secondary { Style::HeaderMsg @@ -1184,11 +1188,12 @@ impl EmitterWriter { } - fn emit_suggestion_default(&mut self, - suggestion: &CodeSuggestion, - level: &Level, - max_line_num_len: usize) - -> io::Result<()> { + fn emit_suggestion_default( + &mut self, + suggestion: &CodeSuggestion, + level: &Level, + max_line_num_len: usize, + ) -> io::Result<()> { if let Some(ref sm) = self.sm { let mut buffer = StyledBuffer::new(); @@ -1198,11 +1203,13 @@ impl EmitterWriter { buffer.append(0, &level_str, Style::Level(level.clone())); buffer.append(0, ": ", Style::HeaderMsg); } - self.msg_to_buffer(&mut buffer, - &[(suggestion.msg.to_owned(), Style::NoStyle)], - max_line_num_len, - "suggestion", - Some(Style::HeaderMsg)); + self.msg_to_buffer( + &mut buffer, + &[(suggestion.msg.to_owned(), Style::NoStyle)], + max_line_num_len, + "suggestion", + Some(Style::HeaderMsg), + ); // Render the replacements for each suggestion let suggestions = suggestion.splice_lines(&**sm); @@ -1340,22 +1347,40 @@ impl EmitterWriter { if !self.short_message { for child in children { let span = child.render_span.as_ref().unwrap_or(&child.span); - match self.emit_message_default(&span, - &child.styled_message(), - &None, - &child.level, - max_line_num_len, - true) { + match self.emit_message_default( + &span, + &child.styled_message(), + &None, + &child.level, + max_line_num_len, + true, + ) { Err(e) => panic!("failed to emit error: {}", e), _ => () } } for sugg in suggestions { - match self.emit_suggestion_default(sugg, - &Level::Help, - max_line_num_len) { - Err(e) => panic!("failed to emit error: {}", e), - _ => () + if sugg.style == SuggestionStyle::HideCodeAlways { + match self.emit_message_default( + &MultiSpan::new(), + &[(sugg.msg.to_owned(), Style::HeaderMsg)], + &None, + &Level::Help, + max_line_num_len, + true, + ) { + Err(e) => panic!("failed to emit error: {}", e), + _ => () + } + } else { + match self.emit_suggestion_default( + sugg, + &Level::Help, + max_line_num_len, + ) { + Err(e) => panic!("failed to emit error: {}", e), + _ => () + } } } } diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index ea530fa1bfb73..be959a29a5577 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -68,6 +68,27 @@ pub enum Applicability { Unspecified, } +#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash, RustcEncodable, RustcDecodable)] +pub enum SuggestionStyle { + /// Hide the suggested code when displaying this suggestion inline. + HideCodeInline, + /// Always hide the suggested code. + HideCodeAlways, + /// Always show the suggested code. + /// This will *not* show the code if the suggestion is inline *and* the suggested code is + /// empty. + ShowCode, +} + +impl SuggestionStyle { + fn hide_inline(&self) -> bool { + match *self { + SuggestionStyle::HideCodeAlways | SuggestionStyle::HideCodeInline => true, + SuggestionStyle::ShowCode => false, + } + } +} + #[derive(Clone, Debug, PartialEq, Hash, RustcEncodable, RustcDecodable)] pub struct CodeSuggestion { /// Each substitute can have multiple variants due to multiple @@ -93,7 +114,8 @@ pub struct CodeSuggestion { /// ``` pub substitutions: Vec, pub msg: String, - pub show_code_when_inline: bool, + /// Visual representation of this suggestion. + pub style: SuggestionStyle, /// Whether or not the suggestion is approximate /// /// Sometimes we may show suggestions with placeholders, From 6ea159ea7e4d7fb4c4527a60cb80ba5347bcff66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 8 Feb 2019 02:50:53 -0800 Subject: [PATCH 15/25] Expose hidden snippet suggestions --- src/librustc_errors/diagnostic.rs | 23 +++++++++++++++++++++++ src/librustc_errors/diagnostic_builder.rs | 20 ++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs index 484cfd045a69f..8394e66850e8c 100644 --- a/src/librustc_errors/diagnostic.rs +++ b/src/librustc_errors/diagnostic.rs @@ -323,6 +323,29 @@ impl Diagnostic { self } + /// Prints out a message with for a suggestion without showing the suggested code. + /// + /// This is intended to be used for suggestions that are obvious in what the changes need to + /// be from the message, showing the span label inline would be visually unpleasant + /// (marginally overlapping spans or multiline spans) and showing the snippet window wouldn't + /// improve understandability. + pub fn span_suggestion_hidden( + &mut self, sp: Span, msg: &str, suggestion: String, applicability: Applicability + ) -> &mut Self { + self.suggestions.push(CodeSuggestion { + substitutions: vec![Substitution { + parts: vec![SubstitutionPart { + snippet: suggestion, + span: sp, + }], + }], + msg: msg.to_owned(), + style: SuggestionStyle::HideCodeInline, + applicability: applicability, + }); + self + } + pub fn set_span>(&mut self, sp: S) -> &mut Self { self.span = sp.into(); self diff --git a/src/librustc_errors/diagnostic_builder.rs b/src/librustc_errors/diagnostic_builder.rs index fd4ea7f2d823f..5b37a7bb3467d 100644 --- a/src/librustc_errors/diagnostic_builder.rs +++ b/src/librustc_errors/diagnostic_builder.rs @@ -261,6 +261,26 @@ impl<'a> DiagnosticBuilder<'a> { ); self } + + pub fn span_suggestion_hidden( + &mut self, + sp: Span, + msg: &str, + suggestion: String, + applicability: Applicability, + ) -> &mut Self { + if !self.allow_suggestions { + return self + } + self.diagnostic.span_suggestion_hidden( + sp, + msg, + suggestion, + applicability, + ); + self + } + forward!(pub fn set_span>(&mut self, sp: S) -> &mut Self); forward!(pub fn code(&mut self, s: DiagnosticId) -> &mut Self); From 7cfba1c5e807821095c7b874c92bf4a11674be36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 8 Feb 2019 03:02:42 -0800 Subject: [PATCH 16/25] Never inline HideCodeAlways suggestions --- src/librustc_errors/emitter.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 5696b2eee8956..6e41196099887 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -46,7 +46,10 @@ impl Emitter for EmitterWriter { // don't display long messages as labels sugg.msg.split_whitespace().count() < 10 && // don't display multiline suggestions as labels - !sugg.substitutions[0].parts[0].snippet.contains('\n') { + !sugg.substitutions[0].parts[0].snippet.contains('\n') && + // when this style is set we want the suggestion to be a message, not inline + sugg.style != SuggestionStyle::HideCodeAlways + { let substitution = &sugg.substitutions[0].parts[0].snippet.trim(); let msg = if substitution.len() == 0 || sugg.style.hide_inline() { // This substitution is only removal or we explicitly don't want to show the From 235523c7d4acdbd38a6b31c53b7969475d460e97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 9 Feb 2019 03:39:08 -0800 Subject: [PATCH 17/25] Add way to completely hide suggestion from cli output --- src/librustc_errors/diagnostic.rs | 21 +++++++++++++++++++++ src/librustc_errors/diagnostic_builder.rs | 19 +++++++++++++++++++ src/librustc_errors/emitter.rs | 4 +++- src/librustc_errors/lib.rs | 6 ++++-- 4 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs index 8394e66850e8c..588cdfcb1afff 100644 --- a/src/librustc_errors/diagnostic.rs +++ b/src/librustc_errors/diagnostic.rs @@ -346,6 +346,27 @@ impl Diagnostic { self } + /// Adds a suggestion to the json output, but otherwise remains silent/undisplayed in the cli. + /// + /// This is intended to be used for suggestions that are *very* obvious in what the changes + /// need to be from the message, but we still want other tools to be able to apply them. + pub fn tool_only_span_suggestion( + &mut self, sp: Span, msg: &str, suggestion: String, applicability: Applicability + ) -> &mut Self { + self.suggestions.push(CodeSuggestion { + substitutions: vec![Substitution { + parts: vec![SubstitutionPart { + snippet: suggestion, + span: sp, + }], + }], + msg: msg.to_owned(), + style: SuggestionStyle::CompletelyHidden, + applicability: applicability, + }); + self + } + pub fn set_span>(&mut self, sp: S) -> &mut Self { self.span = sp.into(); self diff --git a/src/librustc_errors/diagnostic_builder.rs b/src/librustc_errors/diagnostic_builder.rs index 5b37a7bb3467d..9f838987f0c19 100644 --- a/src/librustc_errors/diagnostic_builder.rs +++ b/src/librustc_errors/diagnostic_builder.rs @@ -281,6 +281,25 @@ impl<'a> DiagnosticBuilder<'a> { self } + pub fn tool_only_span_suggestion( + &mut self, + sp: Span, + msg: &str, + suggestion: String, + applicability: Applicability, + ) -> &mut Self { + if !self.allow_suggestions { + return self + } + self.diagnostic.tool_only_span_suggestion( + sp, + msg, + suggestion, + applicability, + ); + self + } + forward!(pub fn set_span>(&mut self, sp: S) -> &mut Self); forward!(pub fn code(&mut self, s: DiagnosticId) -> &mut Self); diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 6e41196099887..5e7c5315d6876 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -1363,7 +1363,9 @@ impl EmitterWriter { } } for sugg in suggestions { - if sugg.style == SuggestionStyle::HideCodeAlways { + if sugg.style == SuggestionStyle::CompletelyHidden { + // do not display this suggestion, it is meant only for tools + } else if sugg.style == SuggestionStyle::HideCodeAlways { match self.emit_message_default( &MultiSpan::new(), &[(sugg.msg.to_owned(), Style::HeaderMsg)], diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index be959a29a5577..7805bf697c64a 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -72,8 +72,10 @@ pub enum Applicability { pub enum SuggestionStyle { /// Hide the suggested code when displaying this suggestion inline. HideCodeInline, - /// Always hide the suggested code. + /// Always hide the suggested code but display the message. HideCodeAlways, + /// Do not display this suggestion in the cli output, it is only meant for tools. + CompletelyHidden, /// Always show the suggested code. /// This will *not* show the code if the suggestion is inline *and* the suggested code is /// empty. @@ -83,8 +85,8 @@ pub enum SuggestionStyle { impl SuggestionStyle { fn hide_inline(&self) -> bool { match *self { - SuggestionStyle::HideCodeAlways | SuggestionStyle::HideCodeInline => true, SuggestionStyle::ShowCode => false, + _ => true, } } } From 87dd2e1df95f96dbf08a0f3ae77a2dcbd6d384e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 11 Feb 2019 11:16:22 -0800 Subject: [PATCH 18/25] Use hidden suggestions for unused imports lint --- src/librustc/lint/builtin.rs | 2 +- src/librustc_errors/diagnostic.rs | 30 +++++++++++++++++-- src/librustc_errors/diagnostic_builder.rs | 18 +++++++++++ src/librustc_errors/emitter.rs | 4 ++- src/test/ui/bad/bad-lint-cap2.stderr | 2 +- src/test/ui/bad/bad-lint-cap3.stderr | 2 +- src/test/ui/imports/unused.stderr | 2 +- src/test/ui/issues/issue-30730.stderr | 2 +- ...directives-on-use-items-issue-10534.stderr | 4 +-- src/test/ui/lint/lint-unused-imports.stderr | 16 +++++----- .../ui/lint/lints-in-foreign-macros.stderr | 6 ++-- .../rfc-2166-underscore-imports/basic.stderr | 4 +-- .../unused-2018.stderr | 4 +-- src/test/ui/span/multispan-import-lint.stderr | 4 --- .../use-nested-groups-unused-imports.stderr | 8 ++--- 15 files changed, 73 insertions(+), 35 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index cb31441ca47e1..98b9d637b7203 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -557,7 +557,7 @@ impl BuiltinLintDiagnostics { } BuiltinLintDiagnostics::UnusedImports(message, replaces) => { if !replaces.is_empty() { - db.multipart_suggestion( + db.tool_only_multipart_suggestion( &message, replaces, Applicability::MachineApplicable, diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs index 588cdfcb1afff..f0083949396b4 100644 --- a/src/librustc_errors/diagnostic.rs +++ b/src/librustc_errors/diagnostic.rs @@ -250,6 +250,32 @@ impl Diagnostic { self } + /// Prints out a message with for a multipart suggestion without showing the suggested code. + /// + /// This is intended to be used for suggestions that are obvious in what the changes need to + /// be from the message, showing the span label inline would be visually unpleasant + /// (marginally overlapping spans or multiline spans) and showing the snippet window wouldn't + /// improve understandability. + pub fn tool_only_multipart_suggestion( + &mut self, + msg: &str, + suggestion: Vec<(Span, String)>, + applicability: Applicability, + ) -> &mut Self { + self.suggestions.push(CodeSuggestion { + substitutions: vec![Substitution { + parts: suggestion + .into_iter() + .map(|(span, snippet)| SubstitutionPart { snippet, span }) + .collect(), + }], + msg: msg.to_owned(), + style: SuggestionStyle::CompletelyHidden, + applicability, + }); + self + } + /// Prints out a message with a suggested edit of the code. /// /// In case of short messages and a simple suggestion, rustc displays it as a label: @@ -318,7 +344,7 @@ impl Diagnostic { }], msg: msg.to_owned(), style: SuggestionStyle::HideCodeInline, - applicability: applicability, + applicability, }); self } @@ -341,7 +367,7 @@ impl Diagnostic { }], msg: msg.to_owned(), style: SuggestionStyle::HideCodeInline, - applicability: applicability, + applicability, }); self } diff --git a/src/librustc_errors/diagnostic_builder.rs b/src/librustc_errors/diagnostic_builder.rs index 9f838987f0c19..4ed7b0a2456e7 100644 --- a/src/librustc_errors/diagnostic_builder.rs +++ b/src/librustc_errors/diagnostic_builder.rs @@ -205,6 +205,24 @@ impl<'a> DiagnosticBuilder<'a> { self } + pub fn tool_only_multipart_suggestion( + &mut self, + msg: &str, + suggestion: Vec<(Span, String)>, + applicability: Applicability, + ) -> &mut Self { + if !self.allow_suggestions { + return self + } + self.diagnostic.tool_only_multipart_suggestion( + msg, + suggestion, + applicability, + ); + self + } + + pub fn span_suggestion( &mut self, sp: Span, diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 5e7c5315d6876..eaae1e9e84840 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -48,7 +48,9 @@ impl Emitter for EmitterWriter { // don't display multiline suggestions as labels !sugg.substitutions[0].parts[0].snippet.contains('\n') && // when this style is set we want the suggestion to be a message, not inline - sugg.style != SuggestionStyle::HideCodeAlways + sugg.style != SuggestionStyle::HideCodeAlways && + // trivial suggestion for tooling's sake, never shown + sugg.style != SuggestionStyle::CompletelyHidden { let substitution = &sugg.substitutions[0].parts[0].snippet.trim(); let msg = if substitution.len() == 0 || sugg.style.hide_inline() { diff --git a/src/test/ui/bad/bad-lint-cap2.stderr b/src/test/ui/bad/bad-lint-cap2.stderr index b9638722778ee..d7ec41489d156 100644 --- a/src/test/ui/bad/bad-lint-cap2.stderr +++ b/src/test/ui/bad/bad-lint-cap2.stderr @@ -2,7 +2,7 @@ error: unused import: `std::option` --> $DIR/bad-lint-cap2.rs:6:5 | LL | use std::option; //~ ERROR - | ----^^^^^^^^^^^- help: remove the whole `use` item + | ^^^^^^^^^^^ | note: lint level defined here --> $DIR/bad-lint-cap2.rs:4:9 diff --git a/src/test/ui/bad/bad-lint-cap3.stderr b/src/test/ui/bad/bad-lint-cap3.stderr index 21ed50b550afc..5bf0b089afa20 100644 --- a/src/test/ui/bad/bad-lint-cap3.stderr +++ b/src/test/ui/bad/bad-lint-cap3.stderr @@ -2,7 +2,7 @@ warning: unused import: `std::option` --> $DIR/bad-lint-cap3.rs:7:5 | LL | use std::option; //~ WARN - | ----^^^^^^^^^^^- help: remove the whole `use` item + | ^^^^^^^^^^^ | note: lint level defined here --> $DIR/bad-lint-cap3.rs:4:9 diff --git a/src/test/ui/imports/unused.stderr b/src/test/ui/imports/unused.stderr index fa82e974e1e29..b56e930158cc1 100644 --- a/src/test/ui/imports/unused.stderr +++ b/src/test/ui/imports/unused.stderr @@ -2,7 +2,7 @@ error: unused import: `super::f` --> $DIR/unused.rs:7:24 | LL | pub(super) use super::f; //~ ERROR unused - | ---------------^^^^^^^^- help: remove the whole `use` item + | ^^^^^^^^ | note: lint level defined here --> $DIR/unused.rs:1:9 diff --git a/src/test/ui/issues/issue-30730.stderr b/src/test/ui/issues/issue-30730.stderr index 3cfadd33b8fec..0a901076f467a 100644 --- a/src/test/ui/issues/issue-30730.stderr +++ b/src/test/ui/issues/issue-30730.stderr @@ -2,7 +2,7 @@ error: unused import: `std::thread` --> $DIR/issue-30730.rs:3:5 | LL | use std::thread; - | ----^^^^^^^^^^^- help: remove the whole `use` item + | ^^^^^^^^^^^ | note: lint level defined here --> $DIR/issue-30730.rs:2:9 diff --git a/src/test/ui/lint/lint-directives-on-use-items-issue-10534.stderr b/src/test/ui/lint/lint-directives-on-use-items-issue-10534.stderr index e588d24517c8c..170b98a12a848 100644 --- a/src/test/ui/lint/lint-directives-on-use-items-issue-10534.stderr +++ b/src/test/ui/lint/lint-directives-on-use-items-issue-10534.stderr @@ -2,7 +2,7 @@ error: unused import: `a::x` --> $DIR/lint-directives-on-use-items-issue-10534.rs:12:9 | LL | use a::x; //~ ERROR: unused import - | ----^^^^- help: remove the whole `use` item + | ^^^^ | note: lint level defined here --> $DIR/lint-directives-on-use-items-issue-10534.rs:1:9 @@ -14,7 +14,7 @@ error: unused import: `a::y` --> $DIR/lint-directives-on-use-items-issue-10534.rs:21:9 | LL | use a::y; //~ ERROR: unused import - | ----^^^^- help: remove the whole `use` item + | ^^^^ | note: lint level defined here --> $DIR/lint-directives-on-use-items-issue-10534.rs:20:12 diff --git a/src/test/ui/lint/lint-unused-imports.stderr b/src/test/ui/lint/lint-unused-imports.stderr index 7970b0201db70..18f2fae0067eb 100644 --- a/src/test/ui/lint/lint-unused-imports.stderr +++ b/src/test/ui/lint/lint-unused-imports.stderr @@ -2,7 +2,7 @@ error: unused import: `std::fmt::{}` --> $DIR/lint-unused-imports.rs:8:5 | LL | use std::fmt::{}; - | ----^^^^^^^^^^^^- help: remove the whole `use` item + | ^^^^^^^^^^^^ | note: lint level defined here --> $DIR/lint-unused-imports.rs:1:9 @@ -14,39 +14,37 @@ error: unused imports: `None`, `Some` --> $DIR/lint-unused-imports.rs:12:27 | LL | use std::option::Option::{Some, None}; - | --------------------------^^^^--^^^^-- help: remove the whole `use` item + | ^^^^ ^^^^ error: unused import: `test::A` --> $DIR/lint-unused-imports.rs:15:5 | LL | use test::A; //~ ERROR unused import: `test::A` - | ----^^^^^^^- help: remove the whole `use` item + | ^^^^^^^ error: unused import: `bar` --> $DIR/lint-unused-imports.rs:24:18 | LL | use test2::{foo, bar}; //~ ERROR unused import: `bar` - | --^^^ - | | - | help: remove the unused import + | ^^^ error: unused import: `foo::Square` --> $DIR/lint-unused-imports.rs:52:13 | LL | use foo::Square; //~ ERROR unused import: `foo::Square` - | ----^^^^^^^^^^^- help: remove the whole `use` item + | ^^^^^^^^^^^ error: unused import: `self::g` --> $DIR/lint-unused-imports.rs:68:9 | LL | use self::g; //~ ERROR unused import: `self::g` - | ----^^^^^^^- help: remove the whole `use` item + | ^^^^^^^ error: unused import: `test2::foo` --> $DIR/lint-unused-imports.rs:77:9 | LL | use test2::foo; //~ ERROR unused import: `test2::foo` - | ----^^^^^^^^^^- help: remove the whole `use` item + | ^^^^^^^^^^ error: unused import: `test::B2` --> $DIR/lint-unused-imports.rs:20:5 diff --git a/src/test/ui/lint/lints-in-foreign-macros.stderr b/src/test/ui/lint/lints-in-foreign-macros.stderr index b808ca708a311..8287ca5692bd9 100644 --- a/src/test/ui/lint/lints-in-foreign-macros.stderr +++ b/src/test/ui/lint/lints-in-foreign-macros.stderr @@ -2,7 +2,7 @@ warning: unused import: `std::string::ToString` --> $DIR/lints-in-foreign-macros.rs:11:16 | LL | () => {use std::string::ToString;} //~ WARN: unused import - | ----^^^^^^^^^^^^^^^^^^^^^- help: remove the whole `use` item + | ^^^^^^^^^^^^^^^^^^^^^ ... LL | mod a { foo!(); } | ------- in this macro invocation @@ -17,13 +17,13 @@ warning: unused import: `std::string::ToString` --> $DIR/lints-in-foreign-macros.rs:16:18 | LL | mod c { baz!(use std::string::ToString;); } //~ WARN: unused import - | ----^^^^^^^^^^^^^^^^^^^^^- help: remove the whole `use` item + | ^^^^^^^^^^^^^^^^^^^^^ warning: unused import: `std::string::ToString` --> $DIR/lints-in-foreign-macros.rs:17:19 | LL | mod d { baz2!(use std::string::ToString;); } //~ WARN: unused import - | ----^^^^^^^^^^^^^^^^^^^^^- help: remove the whole `use` item + | ^^^^^^^^^^^^^^^^^^^^^ warning: missing documentation for crate --> $DIR/lints-in-foreign-macros.rs:4:1 diff --git a/src/test/ui/rfc-2166-underscore-imports/basic.stderr b/src/test/ui/rfc-2166-underscore-imports/basic.stderr index c7b36eaf2e76b..3080359192603 100644 --- a/src/test/ui/rfc-2166-underscore-imports/basic.stderr +++ b/src/test/ui/rfc-2166-underscore-imports/basic.stderr @@ -2,7 +2,7 @@ warning: unused import: `m::Tr1 as _` --> $DIR/basic.rs:26:9 | LL | use m::Tr1 as _; //~ WARN unused import - | ----^^^^^^^^^^^- help: remove the whole `use` item + | ^^^^^^^^^^^ | note: lint level defined here --> $DIR/basic.rs:4:9 @@ -14,5 +14,5 @@ warning: unused import: `S as _` --> $DIR/basic.rs:27:9 | LL | use S as _; //~ WARN unused import - | ----^^^^^^- help: remove the whole `use` item + | ^^^^^^ diff --git a/src/test/ui/rfc-2166-underscore-imports/unused-2018.stderr b/src/test/ui/rfc-2166-underscore-imports/unused-2018.stderr index 0bbc17276d98b..4163c2876074b 100644 --- a/src/test/ui/rfc-2166-underscore-imports/unused-2018.stderr +++ b/src/test/ui/rfc-2166-underscore-imports/unused-2018.stderr @@ -2,7 +2,7 @@ error: unused import: `core::any` --> $DIR/unused-2018.rs:6:9 | LL | use core::any; //~ ERROR unused import: `core::any` - | ----^^^^^^^^^- help: remove the whole `use` item + | ^^^^^^^^^ | note: lint level defined here --> $DIR/unused-2018.rs:3:9 @@ -14,7 +14,7 @@ error: unused import: `core` --> $DIR/unused-2018.rs:10:9 | LL | use core; //~ ERROR unused import: `core` - | ----^^^^- help: remove the whole `use` item + | ^^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/span/multispan-import-lint.stderr b/src/test/ui/span/multispan-import-lint.stderr index 6bd0e9be81f5e..a730d081b7c03 100644 --- a/src/test/ui/span/multispan-import-lint.stderr +++ b/src/test/ui/span/multispan-import-lint.stderr @@ -10,8 +10,4 @@ note: lint level defined here LL | #![warn(unused)] | ^^^^^^ = note: #[warn(unused_imports)] implied by #[warn(unused)] -help: remove the unused imports - | -LL | use std::cmp::{min}; - | -- -- diff --git a/src/test/ui/use/use-nested-groups-unused-imports.stderr b/src/test/ui/use/use-nested-groups-unused-imports.stderr index 6af6f449de5e6..c8df6cbc57dca 100644 --- a/src/test/ui/use/use-nested-groups-unused-imports.stderr +++ b/src/test/ui/use/use-nested-groups-unused-imports.stderr @@ -2,7 +2,7 @@ error: unused imports: `*`, `Foo`, `baz::{}`, `foobar::*` --> $DIR/use-nested-groups-unused-imports.rs:16:11 | LL | use foo::{Foo, bar::{baz::{}, foobar::*}, *}; - | ----------^^^--------^^^^^^^--^^^^^^^^^---^-- help: remove the whole `use` item + | ^^^ ^^^^^^^ ^^^^^^^^^ ^ | note: lint level defined here --> $DIR/use-nested-groups-unused-imports.rs:3:9 @@ -14,15 +14,13 @@ error: unused import: `*` --> $DIR/use-nested-groups-unused-imports.rs:18:24 | LL | use foo::bar::baz::{*, *}; - | --^ - | | - | help: remove the unused import + | ^ error: unused import: `foo::{}` --> $DIR/use-nested-groups-unused-imports.rs:20:5 | LL | use foo::{}; - | ----^^^^^^^- help: remove the whole `use` item + | ^^^^^^^ error: aborting due to 3 previous errors From e73f96abe7cc3a76223e5d0b50592c287c517750 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 8 Feb 2019 12:20:55 +0100 Subject: [PATCH 19/25] make OpTy.op private, and ImmTy.imm public instead --- src/librustc_mir/const_eval.rs | 15 ++----- src/librustc_mir/interpret/cast.rs | 4 +- src/librustc_mir/interpret/operand.rs | 57 +++++++++++++++++++----- src/librustc_mir/interpret/place.rs | 26 +++-------- src/librustc_mir/interpret/terminator.rs | 14 +++--- src/librustc_mir/interpret/traits.rs | 4 ++ src/librustc_mir/transform/const_prop.rs | 25 +++++------ 7 files changed, 80 insertions(+), 65 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index fb0c19f764c13..5d6e9d64aeb58 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -77,7 +77,7 @@ pub fn op_to_const<'tcx>( let normalized_op = if normalize { ecx.try_read_immediate(op)? } else { - match op.op { + match *op { Operand::Indirect(mplace) => Err(mplace), Operand::Immediate(val) => Ok(val) } @@ -105,15 +105,6 @@ pub fn op_to_const<'tcx>( Ok(ty::Const { val, ty: op.layout.ty }) } -pub fn lazy_const_to_op<'tcx>( - ecx: &CompileTimeEvalContext<'_, '_, 'tcx>, - cnst: ty::LazyConst<'tcx>, - ty: ty::Ty<'tcx>, -) -> EvalResult<'tcx, OpTy<'tcx>> { - let op = ecx.const_value_to_op(cnst)?; - Ok(OpTy { op, layout: ecx.layout_of(ty)? }) -} - fn eval_body_and_ecx<'a, 'mir, 'tcx>( tcx: TyCtxt<'a, 'tcx, 'tcx>, cid: GlobalId<'tcx>, @@ -486,7 +477,7 @@ pub fn const_field<'a, 'tcx>( let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env); let result = (|| { // get the operand again - let op = lazy_const_to_op(&ecx, ty::LazyConst::Evaluated(value), value.ty)?; + let op = ecx.lazy_const_to_op(ty::LazyConst::Evaluated(value), value.ty)?; // downcast let down = match variant { None => op, @@ -512,7 +503,7 @@ pub fn const_variant_index<'a, 'tcx>( ) -> EvalResult<'tcx, VariantIdx> { trace!("const_variant_index: {:?}", val); let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env); - let op = lazy_const_to_op(&ecx, ty::LazyConst::Evaluated(val), val.ty)?; + let op = ecx.lazy_const_to_op(ty::LazyConst::Evaluated(val), val.ty)?; Ok(ecx.read_discriminant(op)?.1) } diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs index c3b71be8354da..ce62d79e585a8 100644 --- a/src/librustc_mir/interpret/cast.rs +++ b/src/librustc_mir/interpret/cast.rs @@ -9,7 +9,7 @@ use rustc::mir::interpret::{ use rustc::mir::CastKind; use rustc_apfloat::Float; -use super::{EvalContext, Machine, PlaceTy, OpTy, Immediate}; +use super::{EvalContext, Machine, PlaceTy, OpTy, ImmTy, Immediate}; impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { fn type_is_fat_ptr(&self, ty: Ty<'tcx>) -> bool { @@ -372,7 +372,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> assert_eq!(src.layout.fields.offset(i).bytes(), 0); assert_eq!(src_field_layout.size, src.layout.size); // just sawp out the layout - OpTy { op: src.op, layout: src_field_layout } + OpTy::from(ImmTy { imm: src.to_immediate(), layout: src_field_layout }) } }; if src_field.layout.ty == dst_field.layout.ty { diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index c0b26442dd918..4d0da5e6ecefb 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -11,7 +11,10 @@ use rustc::mir::interpret::{ ConstValue, Pointer, Scalar, EvalResult, EvalErrorKind, }; -use super::{EvalContext, Machine, MemPlace, MPlaceTy, MemoryKind}; +use super::{ + EvalContext, Machine, AllocMap, Allocation, AllocationExtra, + MemPlace, MPlaceTy, PlaceTy, Place, MemoryKind, +}; pub use rustc::mir::interpret::ScalarMaybeUndef; /// A `Value` represents a single immediate self-contained Rust value. @@ -112,7 +115,7 @@ impl<'tcx, Tag> Immediate { // as input for binary and cast operations. #[derive(Copy, Clone, Debug)] pub struct ImmTy<'tcx, Tag=()> { - immediate: Immediate, + crate imm: Immediate, // ideally we'd make this private, but const_prop needs this pub layout: TyLayout<'tcx>, } @@ -120,7 +123,7 @@ impl<'tcx, Tag> ::std::ops::Deref for ImmTy<'tcx, Tag> { type Target = Immediate; #[inline(always)] fn deref(&self) -> &Immediate { - &self.immediate + &self.imm } } @@ -180,7 +183,7 @@ impl Operand { #[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] pub struct OpTy<'tcx, Tag=()> { - crate op: Operand, // ideally we'd make this private, but const_prop needs this + op: Operand, pub layout: TyLayout<'tcx>, } @@ -206,7 +209,7 @@ impl<'tcx, Tag> From> for OpTy<'tcx, Tag> { #[inline(always)] fn from(val: ImmTy<'tcx, Tag>) -> Self { OpTy { - op: Operand::Immediate(val.immediate), + op: Operand::Immediate(val.imm), layout: val.layout } } @@ -324,8 +327,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> &self, op: OpTy<'tcx, M::PointerTag> ) -> EvalResult<'tcx, ImmTy<'tcx, M::PointerTag>> { - if let Ok(immediate) = self.try_read_immediate(op)? { - Ok(ImmTy { immediate, layout: op.layout }) + if let Ok(imm) = self.try_read_immediate(op)? { + Ok(ImmTy { imm, layout: op.layout }) } else { bug!("primitive read failed for type: {:?}", op.layout.ty); } @@ -469,6 +472,22 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> Ok(OpTy { op, layout }) } + /// Every place can be read from, so we can turm them into an operand + #[inline(always)] + pub fn place_to_op( + &self, + place: PlaceTy<'tcx, M::PointerTag> + ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> { + let op = match *place { + Place::Ptr(mplace) => { + Operand::Indirect(mplace) + } + Place::Local { frame, local } => + *self.stack[frame].locals[local].access()? + }; + Ok(OpTy { op, layout: place.layout }) + } + // Evaluate a place with the goal of reading from it. This lets us sometimes // avoid allocations. fn eval_place_to_op( @@ -531,10 +550,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> .collect() } - // Used when miri runs into a constant, and by CTFE. - // FIXME: CTFE should use allocations, then we can make this private (embed it into - // `eval_operand`, ideally). - pub(crate) fn const_value_to_op( + // Used when Miri runs into a constant, and (indirectly through lazy_const_to_op) by CTFE. + fn const_value_to_op( &self, val: ty::LazyConst<'tcx>, ) -> EvalResult<'tcx, Operand> { @@ -666,3 +683,21 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } } + +impl<'a, 'mir, 'tcx, M> EvalContext<'a, 'mir, 'tcx, M> +where + M: Machine<'a, 'mir, 'tcx, PointerTag=()>, + // FIXME: Working around https://github.com/rust-lang/rust/issues/24159 + M::MemoryMap: AllocMap, Allocation<(), M::AllocExtra>)>, + M::AllocExtra: AllocationExtra<(), M::MemoryExtra>, +{ + // FIXME: CTFE should use allocations, then we can remove this. + pub(crate) fn lazy_const_to_op( + &self, + cnst: ty::LazyConst<'tcx>, + ty: ty::Ty<'tcx>, + ) -> EvalResult<'tcx, OpTy<'tcx>> { + let op = self.const_value_to_op(cnst)?; + Ok(OpTy { op, layout: self.layout_of(ty)? }) + } +} diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 004a11e34d6e1..c1d0f0a8bd15b 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -244,10 +244,10 @@ impl<'tcx, Tag> MPlaceTy<'tcx, Tag> { } } -impl<'tcx, Tag: ::std::fmt::Debug> OpTy<'tcx, Tag> { +impl<'tcx, Tag: ::std::fmt::Debug + Copy> OpTy<'tcx, Tag> { #[inline(always)] pub fn try_as_mplace(self) -> Result, Immediate> { - match self.op { + match *self { Operand::Indirect(mplace) => Ok(MPlaceTy { mplace, layout: self.layout }), Operand::Immediate(imm) => Err(imm), } @@ -487,9 +487,9 @@ where Deref => self.deref_operand(base.into())?, Index(local) => { - let n = *self.frame().locals[local].access()?; - let n_layout = self.layout_of(self.tcx.types.usize)?; - let n = self.read_scalar(OpTy { op: n, layout: n_layout })?; + let layout = self.layout_of(self.tcx.types.usize)?; + let n = self.access_local(self.frame(), local, Some(layout))?; + let n = self.read_scalar(n)?; let n = n.to_bits(self.tcx.data_layout.pointer_size)?; self.mplace_field(base, u64::try_from(n).unwrap())? } @@ -991,22 +991,6 @@ where Ok(()) } - /// Every place can be read from, so we can turm them into an operand - #[inline(always)] - pub fn place_to_op( - &self, - place: PlaceTy<'tcx, M::PointerTag> - ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> { - let op = match place.place { - Place::Ptr(mplace) => { - Operand::Indirect(mplace) - } - Place::Local { frame, local } => - *self.stack[frame].locals[local].access()? - }; - Ok(OpTy { op, layout: place.layout }) - } - pub fn raw_const_to_mplace( &self, raw: RawConst<'tcx>, diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index be50daa17092f..72d60f259e727 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -7,7 +7,7 @@ use rustc_target::spec::abi::Abi; use rustc::mir::interpret::{EvalResult, PointerArithmetic, EvalErrorKind, Scalar}; use super::{ - EvalContext, Machine, Immediate, OpTy, PlaceTy, MPlaceTy, Operand, StackPopCleanup + EvalContext, Machine, Immediate, OpTy, ImmTy, PlaceTy, MPlaceTy, StackPopCleanup }; impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { @@ -418,8 +418,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let mut args = args.to_vec(); let pointee = args[0].layout.ty.builtin_deref(true).unwrap().ty; let fake_fat_ptr_ty = self.tcx.mk_mut_ptr(pointee); - args[0].layout = self.layout_of(fake_fat_ptr_ty)?.field(self, 0)?; - args[0].op = Operand::Immediate(Immediate::Scalar(ptr.ptr.into())); // strip vtable + args[0] = OpTy::from(ImmTy { // strip vtable + layout: self.layout_of(fake_fat_ptr_ty)?.field(self, 0)?, + imm: Immediate::Scalar(ptr.ptr.into()) + }); trace!("Patched self operand to {:#?}", args[0]); // recurse with concrete function self.eval_fn_call(instance, span, caller_abi, &args, dest, ret) @@ -448,8 +450,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> _ => (instance, place), }; - let arg = OpTy { - op: Operand::Immediate(place.to_ref()), + let arg = ImmTy { + imm: place.to_ref(), layout: self.layout_of(self.tcx.mk_mut_ptr(place.layout.ty))?, }; @@ -460,7 +462,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> instance, span, Abi::Rust, - &[arg], + &[arg.into()], Some(dest.into()), Some(target), ) diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index 65d7060b544d6..1b0a9b17d3686 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -22,6 +22,10 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let (ty, poly_trait_ref) = self.tcx.erase_regions(&(ty, poly_trait_ref)); if let Some(&vtable) = self.vtables.get(&(ty, poly_trait_ref)) { + // This means we guarantee that there are no duplicate vtables, we will + // always use the same vtable for the same (Type, Trait) combination. + // That's not what happens in rustc, but emulating per-crate deduplication + // does not sound like it actually makes anything any better. return Ok(Pointer::from(vtable).with_default_tag()); } diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 018f71c39e513..d8d24c06f5a8b 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -18,10 +18,9 @@ use rustc::ty::layout::{ HasTyCtxt, TargetDataLayout, HasDataLayout, }; -use crate::interpret::{self, EvalContext, ScalarMaybeUndef, Immediate, OpTy, MemoryKind}; +use crate::interpret::{EvalContext, ScalarMaybeUndef, Immediate, OpTy, ImmTy, MemoryKind}; use crate::const_eval::{ CompileTimeInterpreter, error_to_const_error, eval_promoted, mk_eval_cx, - lazy_const_to_op, }; use crate::transform::{MirPass, MirSource}; @@ -254,7 +253,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { source_info: SourceInfo, ) -> Option> { self.ecx.tcx.span = source_info.span; - match lazy_const_to_op(&self.ecx, *c.literal, c.ty) { + match self.ecx.lazy_const_to_op(*c.literal, c.ty) { Ok(op) => { Some((op, c.span)) }, @@ -345,15 +344,15 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { Rvalue::Len(_) => None, Rvalue::NullaryOp(NullOp::SizeOf, ty) => { type_size_of(self.tcx, self.param_env, ty).and_then(|n| Some(( - OpTy { - op: interpret::Operand::Immediate(Immediate::Scalar( + ImmTy { + imm: Immediate::Scalar( Scalar::Bits { bits: n as u128, size: self.tcx.data_layout.pointer_size.bytes() as u8, }.into() - )), + ), layout: self.tcx.layout_of(self.param_env.and(self.tcx.types.usize)).ok()?, - }, + }.into(), span, ))) } @@ -388,11 +387,11 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { // Now run the actual operation. this.ecx.unary_op(op, prim, arg.layout) })?; - let res = OpTy { - op: interpret::Operand::Immediate(Immediate::Scalar(val.into())), + let res = ImmTy { + imm: Immediate::Scalar(val.into()), layout: place_layout, }; - Some((res, span)) + Some((res.into(), span)) } Rvalue::CheckedBinaryOp(op, ref left, ref right) | Rvalue::BinaryOp(op, ref left, ref right) => { @@ -462,11 +461,11 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { } Immediate::Scalar(val.into()) }; - let res = OpTy { - op: interpret::Operand::Immediate(val), + let res = ImmTy { + imm: val, layout: place_layout, }; - Some((res, span)) + Some((res.into(), span)) }, } } From b376ae6671a20914d63691fdd4b543b2e6341d5b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 8 Feb 2019 14:00:52 +0100 Subject: [PATCH 20/25] make bin_op and unary_op APIs consistently work on ImmTy --- src/librustc_mir/const_eval.rs | 11 ++-- src/librustc_mir/interpret/intrinsics.rs | 2 +- src/librustc_mir/interpret/machine.rs | 10 ++-- src/librustc_mir/interpret/operand.rs | 20 ++++++- src/librustc_mir/interpret/operator.rs | 73 ++++++++++-------------- src/librustc_mir/interpret/step.rs | 2 +- src/librustc_mir/interpret/terminator.rs | 4 +- src/librustc_mir/transform/const_prop.rs | 9 ++- 8 files changed, 66 insertions(+), 65 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 5d6e9d64aeb58..7be7f4b439289 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -11,7 +11,7 @@ use rustc::hir::def::Def; use rustc::mir::interpret::{ConstEvalErr, ErrorHandled}; use rustc::mir; use rustc::ty::{self, TyCtxt, query::TyCtxtAt}; -use rustc::ty::layout::{self, LayoutOf, TyLayout, VariantIdx}; +use rustc::ty::layout::{self, LayoutOf, VariantIdx}; use rustc::ty::subst::Subst; use rustc::traits::Reveal; use rustc_data_structures::fx::FxHashMap; @@ -21,7 +21,8 @@ use syntax::ast::Mutability; use syntax::source_map::{Span, DUMMY_SP}; use crate::interpret::{self, - PlaceTy, MPlaceTy, MemPlace, OpTy, Operand, Immediate, Scalar, RawConst, ConstValue, Pointer, + PlaceTy, MPlaceTy, MemPlace, OpTy, ImmTy, Operand, Immediate, Scalar, Pointer, + RawConst, ConstValue, EvalResult, EvalError, EvalErrorKind, GlobalId, EvalContext, StackPopCleanup, Allocation, AllocId, MemoryKind, snapshot, RefTracking, @@ -379,10 +380,8 @@ impl<'a, 'mir, 'tcx> interpret::Machine<'a, 'mir, 'tcx> fn ptr_op( _ecx: &EvalContext<'a, 'mir, 'tcx, Self>, _bin_op: mir::BinOp, - _left: Scalar, - _left_layout: TyLayout<'tcx>, - _right: Scalar, - _right_layout: TyLayout<'tcx>, + _left: ImmTy<'tcx>, + _right: ImmTy<'tcx>, ) -> EvalResult<'tcx, (Scalar, bool)> { Err( ConstEvalError::NeedsRfc("pointer arithmetic or comparison".to_string()).into(), diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 78c5c0a6d751c..6466dd4098acd 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -173,7 +173,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> "unchecked_shr" => BinOp::Shr, _ => bug!("Already checked for int ops") }; - let (val, overflowed) = self.binary_op_imm(bin_op, l, r)?; + let (val, overflowed) = self.binary_op(bin_op, l, r)?; if overflowed { let layout = self.layout_of(substs.type_at(0))?; let r_val = r.to_scalar()?.to_bits(layout.size)?; diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 8f34b832f0b41..7fb4c47d92acb 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -7,11 +7,11 @@ use std::hash::Hash; use rustc::hir::{self, def_id::DefId}; use rustc::mir; -use rustc::ty::{self, layout::TyLayout, query::TyCtxtAt}; +use rustc::ty::{self, query::TyCtxtAt}; use super::{ Allocation, AllocId, EvalResult, Scalar, AllocationExtra, - EvalContext, PlaceTy, MPlaceTy, OpTy, Pointer, MemoryKind, + EvalContext, PlaceTy, MPlaceTy, OpTy, ImmTy, Pointer, MemoryKind, }; /// Whether this kind of memory is allowed to leak @@ -158,10 +158,8 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized { fn ptr_op( ecx: &EvalContext<'a, 'mir, 'tcx, Self>, bin_op: mir::BinOp, - left: Scalar, - left_layout: TyLayout<'tcx>, - right: Scalar, - right_layout: TyLayout<'tcx>, + left: ImmTy<'tcx, Self::PointerTag>, + right: ImmTy<'tcx, Self::PointerTag>, ) -> EvalResult<'tcx, (Scalar, bool)>; /// Heap allocations via the `box` keyword. diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 4d0da5e6ecefb..7da907028eebf 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -44,6 +44,11 @@ impl Immediate { } impl<'tcx, Tag> Immediate { + #[inline] + pub fn from_scalar(val: Scalar) -> Self { + Immediate::Scalar(ScalarMaybeUndef::Scalar(val)) + } + #[inline] pub fn erase_tag(self) -> Immediate { @@ -115,7 +120,7 @@ impl<'tcx, Tag> Immediate { // as input for binary and cast operations. #[derive(Copy, Clone, Debug)] pub struct ImmTy<'tcx, Tag=()> { - crate imm: Immediate, // ideally we'd make this private, but const_prop needs this + pub imm: Immediate, pub layout: TyLayout<'tcx>, } @@ -215,6 +220,19 @@ impl<'tcx, Tag> From> for OpTy<'tcx, Tag> { } } +impl<'tcx, Tag: Copy> ImmTy<'tcx, Tag> +{ + #[inline] + pub fn from_scalar(val: Scalar, layout: TyLayout<'tcx>) -> Self { + ImmTy { imm: Immediate::from_scalar(val), layout } + } + + #[inline] + pub fn to_bits(self) -> EvalResult<'tcx, u128> { + self.to_scalar()?.to_bits(self.layout.size) + } +} + impl<'tcx, Tag> OpTy<'tcx, Tag> { #[inline] diff --git a/src/librustc_mir/interpret/operator.rs b/src/librustc_mir/interpret/operator.rs index 5e3335f4c7219..b3b9c742d6c28 100644 --- a/src/librustc_mir/interpret/operator.rs +++ b/src/librustc_mir/interpret/operator.rs @@ -18,7 +18,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> right: ImmTy<'tcx, M::PointerTag>, dest: PlaceTy<'tcx, M::PointerTag>, ) -> EvalResult<'tcx> { - let (val, overflowed) = self.binary_op_imm(op, left, right)?; + let (val, overflowed) = self.binary_op(op, left, right)?; let val = Immediate::ScalarPair(val.into(), Scalar::from_bool(overflowed).into()); self.write_immediate(val, dest) } @@ -32,7 +32,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> right: ImmTy<'tcx, M::PointerTag>, dest: PlaceTy<'tcx, M::PointerTag>, ) -> EvalResult<'tcx> { - let (val, _overflowed) = self.binary_op_imm(op, left, right)?; + let (val, _overflowed) = self.binary_op(op, left, right)?; self.write_scalar(val, dest) } } @@ -272,69 +272,55 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> Ok((val, false)) } - /// Convenience wrapper that's useful when keeping the layout together with the - /// immediate value. + /// Returns the result of the specified operation and whether it overflowed. #[inline] - pub fn binary_op_imm( + pub fn binary_op( &self, bin_op: mir::BinOp, left: ImmTy<'tcx, M::PointerTag>, right: ImmTy<'tcx, M::PointerTag>, - ) -> EvalResult<'tcx, (Scalar, bool)> { - self.binary_op( - bin_op, - left.to_scalar()?, left.layout, - right.to_scalar()?, right.layout, - ) - } - - /// Returns the result of the specified operation and whether it overflowed. - pub fn binary_op( - &self, - bin_op: mir::BinOp, - left: Scalar, - left_layout: TyLayout<'tcx>, - right: Scalar, - right_layout: TyLayout<'tcx>, ) -> EvalResult<'tcx, (Scalar, bool)> { trace!("Running binary op {:?}: {:?} ({:?}), {:?} ({:?})", - bin_op, left, left_layout.ty, right, right_layout.ty); + bin_op, *left, left.layout.ty, *right, right.layout.ty); - match left_layout.ty.sty { + match left.layout.ty.sty { ty::Char => { - assert_eq!(left_layout.ty, right_layout.ty); - let left = left.to_char()?; - let right = right.to_char()?; + assert_eq!(left.layout.ty, right.layout.ty); + let left = left.to_scalar()?.to_char()?; + let right = right.to_scalar()?.to_char()?; self.binary_char_op(bin_op, left, right) } ty::Bool => { - assert_eq!(left_layout.ty, right_layout.ty); - let left = left.to_bool()?; - let right = right.to_bool()?; + assert_eq!(left.layout.ty, right.layout.ty); + let left = left.to_scalar()?.to_bool()?; + let right = right.to_scalar()?.to_bool()?; self.binary_bool_op(bin_op, left, right) } ty::Float(fty) => { - assert_eq!(left_layout.ty, right_layout.ty); - let left = left.to_bits(left_layout.size)?; - let right = right.to_bits(right_layout.size)?; + assert_eq!(left.layout.ty, right.layout.ty); + let left = left.to_bits()?; + let right = right.to_bits()?; self.binary_float_op(bin_op, fty, left, right) } _ => { // Must be integer(-like) types. Don't forget about == on fn pointers. - assert!(left_layout.ty.is_integral() || left_layout.ty.is_unsafe_ptr() || - left_layout.ty.is_fn()); - assert!(right_layout.ty.is_integral() || right_layout.ty.is_unsafe_ptr() || - right_layout.ty.is_fn()); + assert!(left.layout.ty.is_integral() || left.layout.ty.is_unsafe_ptr() || + left.layout.ty.is_fn()); + assert!(right.layout.ty.is_integral() || right.layout.ty.is_unsafe_ptr() || + right.layout.ty.is_fn()); // Handle operations that support pointer values - if left.is_ptr() || right.is_ptr() || bin_op == mir::BinOp::Offset { - return M::ptr_op(self, bin_op, left, left_layout, right, right_layout); + if left.to_scalar_ptr()?.is_ptr() || + right.to_scalar_ptr()?.is_ptr() || + bin_op == mir::BinOp::Offset + { + return M::ptr_op(self, bin_op, left, right); } // Everything else only works with "proper" bits - let left = left.to_bits(left_layout.size).expect("we checked is_ptr"); - let right = right.to_bits(right_layout.size).expect("we checked is_ptr"); - self.binary_int_op(bin_op, left, left_layout, right, right_layout) + let l = left.to_bits().expect("we checked is_ptr"); + let r = right.to_bits().expect("we checked is_ptr"); + self.binary_int_op(bin_op, l, left.layout, r, right.layout) } } } @@ -342,13 +328,14 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> pub fn unary_op( &self, un_op: mir::UnOp, - val: Scalar, - layout: TyLayout<'tcx>, + val: ImmTy<'tcx, M::PointerTag>, ) -> EvalResult<'tcx, Scalar> { use rustc::mir::UnOp::*; use rustc_apfloat::ieee::{Single, Double}; use rustc_apfloat::Float; + let layout = val.layout; + let val = val.to_scalar()?; trace!("Running unary op {:?}: {:?} ({:?})", un_op, val, layout.ty.sty); match layout.ty.sty { diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index 0c988eb681083..97ef2b5fa3485 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -176,7 +176,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> UnaryOp(un_op, ref operand) => { // The operand always has the same type as the result. let val = self.read_immediate(self.eval_operand(operand, Some(dest.layout))?)?; - let val = self.unary_op(un_op, val.to_scalar()?, dest.layout)?; + let val = self.unary_op(un_op, val)?; self.write_scalar(val, dest)?; } diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 72d60f259e727..c2ee3f5715bd3 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -51,8 +51,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // Compare using binary_op, to also support pointer values let const_int = Scalar::from_uint(const_int, discr.layout.size); let (res, _) = self.binary_op(mir::BinOp::Eq, - discr.to_scalar()?, discr.layout, - const_int, discr.layout, + discr, + ImmTy::from_scalar(const_int, discr.layout), )?; if res.to_bool()? { target_block = targets[index]; diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index d8d24c06f5a8b..7da00c4ea0c36 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -370,13 +370,12 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { let (arg, _) = self.eval_operand(arg, source_info)?; let val = self.use_ecx(source_info, |this| { - let prim = this.ecx.read_scalar(arg)?.not_undef()?; + let prim = this.ecx.read_immediate(arg)?; match op { UnOp::Neg => { // Need to do overflow check here: For actual CTFE, MIR // generation emits code that does this before calling the op. - let size = arg.layout.size; - if prim.to_bits(size)? == (1 << (size.bits() - 1)) { + if prim.to_bits()? == (1 << (prim.layout.size.bits() - 1)) { return err!(OverflowNeg); } } @@ -385,7 +384,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { } } // Now run the actual operation. - this.ecx.unary_op(op, prim, arg.layout) + this.ecx.unary_op(op, prim) })?; let res = ImmTy { imm: Immediate::Scalar(val.into()), @@ -446,7 +445,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { })?; trace!("const evaluating {:?} for {:?} and {:?}", op, left, right); let (val, overflow) = self.use_ecx(source_info, |this| { - this.ecx.binary_op_imm(op, l, r) + this.ecx.binary_op(op, l, r) })?; let val = if let Rvalue::CheckedBinaryOp(..) = *rvalue { Immediate::ScalarPair( From 1a5304ae93d79c0e1fe59cfca6266af970cda061 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 8 Feb 2019 15:26:45 +0100 Subject: [PATCH 21/25] fix whitespace --- src/librustc_mir/interpret/place.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index c1d0f0a8bd15b..b29e09900f6b1 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -487,7 +487,7 @@ where Deref => self.deref_operand(base.into())?, Index(local) => { - let layout = self.layout_of(self.tcx.types.usize)?; + let layout = self.layout_of(self.tcx.types.usize)?; let n = self.access_local(self.frame(), local, Some(layout))?; let n = self.read_scalar(n)?; let n = n.to_bits(self.tcx.data_layout.pointer_size)?; From 22d5e6a37a75db6b59931ee30c1de630f2b5016b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 13 Feb 2019 11:14:40 +0100 Subject: [PATCH 22/25] fix rebase fallout --- src/librustc_mir/interpret/intrinsics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 6466dd4098acd..e002c3fd511d6 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -126,7 +126,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> let l = self.read_immediate(args[0])?; let r = self.read_immediate(args[1])?; let is_add = intrinsic_name == "saturating_add"; - let (val, overflowed) = self.binary_op_imm(if is_add { + let (val, overflowed) = self.binary_op(if is_add { BinOp::Add } else { BinOp::Sub From e7f8e63ed425cef77a9ad43e463b39009c1a495a Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Wed, 23 Jan 2019 03:55:37 +0000 Subject: [PATCH 23/25] Convert old doc links to current edition Use footnote style to bypass the tidy check --- src/doc/guide-error-handling.md | 2 +- src/doc/guide-ownership.md | 2 +- src/doc/guide-pointers.md | 2 +- src/doc/guide-testing.md | 2 +- src/libcore/convert.rs | 5 +-- src/libcore/marker.rs | 4 +- src/libcore/mem.rs | 2 +- src/libproc_macro/lib.rs | 4 +- src/librustc/diagnostics.rs | 9 ++-- src/librustc_metadata/diagnostics.rs | 2 +- src/librustc_mir/diagnostics.rs | 14 +++--- src/librustc_typeck/diagnostics.rs | 55 +++++++++++++++--------- src/librustc_typeck/structured_errors.rs | 2 +- src/libstd/prelude/mod.rs | 8 ++-- src/libtest/lib.rs | 2 +- 15 files changed, 65 insertions(+), 50 deletions(-) diff --git a/src/doc/guide-error-handling.md b/src/doc/guide-error-handling.md index 54fa529f3aa8e..fd71d3e3c8e79 100644 --- a/src/doc/guide-error-handling.md +++ b/src/doc/guide-error-handling.md @@ -1,4 +1,4 @@ % Error Handling in Rust This content has moved into -[the Rust Programming Language book](book/error-handling.html). +[the Rust Programming Language book](book/ch09-00-error-handling.html). diff --git a/src/doc/guide-ownership.md b/src/doc/guide-ownership.md index 884f14726ca87..767dafc5baf92 100644 --- a/src/doc/guide-ownership.md +++ b/src/doc/guide-ownership.md @@ -1,4 +1,4 @@ % The (old) Rust Ownership Guide This content has moved into -[the Rust Programming Language book](book/ownership.html). +[the Rust Programming Language book](book/ch04-00-understanding-ownership.html). diff --git a/src/doc/guide-pointers.md b/src/doc/guide-pointers.md index dc80ec4399131..bafdb2fe0bbc3 100644 --- a/src/doc/guide-pointers.md +++ b/src/doc/guide-pointers.md @@ -2,6 +2,6 @@ This content has been removed, with no direct replacement. Rust only has two built-in pointer types now, -[references](book/references-and-borrowing.html) and [raw +[references](book/ch04-02-references-and-borrowing.html) and [raw pointers](book/raw-pointers.html). Older Rusts had many more pointer types, they’re gone now. diff --git a/src/doc/guide-testing.md b/src/doc/guide-testing.md index 67bcb0a5e546a..28d9fb48b73e7 100644 --- a/src/doc/guide-testing.md +++ b/src/doc/guide-testing.md @@ -1,4 +1,4 @@ % The (old) Rust Testing Guide This content has moved into -[the Rust Programming Language book](book/testing.html). +[the Rust Programming Language book](book/ch11-00-testing.html). diff --git a/src/libcore/convert.rs b/src/libcore/convert.rs index 203be541e492f..b24a442131e2b 100644 --- a/src/libcore/convert.rs +++ b/src/libcore/convert.rs @@ -113,9 +113,6 @@ pub const fn identity(x: T) -> T { x } /// - Use `Borrow` when the goal is related to writing code that is agnostic to /// the type of borrow and whether it is a reference or value /// -/// See [the book][book] for a more detailed comparison. -/// -/// [book]: ../../book/first-edition/borrow-and-asref.html /// [`Borrow`]: ../../std/borrow/trait.Borrow.html /// /// **Note: this trait must not fail**. If the conversion can fail, use a @@ -348,7 +345,7 @@ pub trait Into: Sized { /// [`String`]: ../../std/string/struct.String.html /// [`Into`]: trait.Into.html /// [`from`]: trait.From.html#tymethod.from -/// [book]: ../../book/first-edition/error-handling.html +/// [book]: ../../book/ch09-00-error-handling.html #[stable(feature = "rust1", since = "1.0.0")] pub trait From: Sized { /// Performs the conversion. diff --git a/src/libcore/marker.rs b/src/libcore/marker.rs index 65752ba032104..a92131ce84fff 100644 --- a/src/libcore/marker.rs +++ b/src/libcore/marker.rs @@ -78,7 +78,7 @@ impl !Send for *mut T { } /// // be made into an object /// ``` /// -/// [trait object]: ../../book/first-edition/trait-objects.html +/// [trait object]: ../../book/ch17-02-trait-objects.html #[stable(feature = "rust1", since = "1.0.0")] #[lang = "sized"] #[rustc_on_unimplemented( @@ -518,7 +518,7 @@ macro_rules! impls{ /// types. We track the Rust type using a phantom type parameter on /// the struct `ExternalResource` which wraps a handle. /// -/// [FFI]: ../../book/first-edition/ffi.html +/// [FFI]: ../../book/ch19-01-unsafe-rust.html#using-extern-functions-to-call-external-code /// /// ``` /// # #![allow(dead_code)] diff --git a/src/libcore/mem.rs b/src/libcore/mem.rs index 8fcbb73d9ce46..810a1f2fb4a14 100644 --- a/src/libcore/mem.rs +++ b/src/libcore/mem.rs @@ -298,7 +298,7 @@ pub const fn size_of() -> usize { /// then `size_of_val` can be used to get the dynamically-known size. /// /// [slice]: ../../std/primitive.slice.html -/// [trait object]: ../../book/first-edition/trait-objects.html +/// [trait object]: ../../book/ch17-02-trait-objects.html /// /// # Examples /// diff --git a/src/libproc_macro/lib.rs b/src/libproc_macro/lib.rs index 868190d01057d..5533dc2c4b100 100644 --- a/src/libproc_macro/lib.rs +++ b/src/libproc_macro/lib.rs @@ -5,7 +5,9 @@ //! function-like macros `#[proc_macro]`, macro attributes `#[proc_macro_attribute]` and //! custom derive attributes`#[proc_macro_derive]`. //! -//! See [the book](../book/first-edition/procedural-macros.html) for more. +//! See [the book] for more. +//! +//! [the book]: ../book/ch19-06-macros.html#procedural-macros-for-generating-code-from-attributes #![stable(feature = "proc_macro_lib", since = "1.15.0")] #![deny(missing_docs)] diff --git a/src/librustc/diagnostics.rs b/src/librustc/diagnostics.rs index 287a45a21eadf..de647771f9e75 100644 --- a/src/librustc/diagnostics.rs +++ b/src/librustc/diagnostics.rs @@ -1,3 +1,4 @@ +// ignore-tidy-linelength #![allow(non_snake_case)] // Error messages for EXXXX errors. @@ -406,7 +407,7 @@ fn baz<'a>(x: &'a str, y: &str) -> &str { } Lifetime elision in implementation headers was part of the lifetime elision RFC. It is, however, [currently unimplemented][iss15872]. -[book-le]: https://doc.rust-lang.org/nightly/book/first-edition/lifetimes.html#lifetime-elision +[book-le]: https://doc.rust-lang.org/book/ch10-03-lifetime-syntax.html#lifetime-elision [iss15872]: https://github.com/rust-lang/rust/issues/15872 "##, @@ -642,7 +643,9 @@ attributes: #![no_std] ``` -See also https://doc.rust-lang.org/book/first-edition/no-stdlib.html +See also the [unstable book][1]. + +[1]: https://doc.rust-lang.org/unstable-book/language-features/lang-items.html#writing-an-executable-without-stdlib "##, E0214: r##" @@ -1680,7 +1683,7 @@ fn main() { ``` To understand better how closures work in Rust, read: -https://doc.rust-lang.org/book/first-edition/closures.html +https://doc.rust-lang.org/book/ch13-01-closures.html "##, E0580: r##" diff --git a/src/librustc_metadata/diagnostics.rs b/src/librustc_metadata/diagnostics.rs index 1b1852434740c..a38601d11f1c1 100644 --- a/src/librustc_metadata/diagnostics.rs +++ b/src/librustc_metadata/diagnostics.rs @@ -35,7 +35,7 @@ extern {} ``` See more: -https://doc.rust-lang.org/book/first-edition/conditional-compilation.html +https://doc.rust-lang.org/reference/attributes.html#conditional-compilation "##, E0458: r##" diff --git a/src/librustc_mir/diagnostics.rs b/src/librustc_mir/diagnostics.rs index ea9e19c75c215..7f793134a9258 100644 --- a/src/librustc_mir/diagnostics.rs +++ b/src/librustc_mir/diagnostics.rs @@ -690,7 +690,7 @@ fn main() { } ``` -See also https://doc.rust-lang.org/book/first-edition/unsafe.html +See also https://doc.rust-lang.org/book/ch19-01-unsafe-rust.html "##, E0373: r##" @@ -873,7 +873,7 @@ that at most one writer or multiple readers can access the data at any one time. If you wish to learn more about ownership in Rust, start with the chapter in the Book: -https://doc.rust-lang.org/book/first-edition/ownership.html +https://doc.rust-lang.org/book/ch04-00-understanding-ownership.html "##, E0383: r##" @@ -1207,7 +1207,7 @@ let mut a = &mut i; Please note that in rust, you can either have many immutable references, or one mutable reference. Take a look at -https://doc.rust-lang.org/stable/book/references-and-borrowing.html for more +https://doc.rust-lang.org/book/ch04-02-references-and-borrowing.html for more information. Example: @@ -1374,7 +1374,7 @@ fn foo(a: &mut i32) { ``` For more information on the rust ownership system, take a look at -https://doc.rust-lang.org/stable/book/references-and-borrowing.html. +https://doc.rust-lang.org/book/ch04-02-references-and-borrowing.html. "##, E0503: r##" @@ -1430,7 +1430,7 @@ fn main() { ``` You can find more information about borrowing in the rust-book: -http://doc.rust-lang.org/stable/book/references-and-borrowing.html +http://doc.rust-lang.org/book/ch04-02-references-and-borrowing.html "##, E0504: r##" @@ -1614,7 +1614,7 @@ fn main() { ``` You can find more information about borrowing in the rust-book: -http://doc.rust-lang.org/stable/book/references-and-borrowing.html +http://doc.rust-lang.org/book/ch04-02-references-and-borrowing.html "##, E0506: r##" @@ -1825,7 +1825,7 @@ mem::replace(&mut borrowed.knight, TheDarkKnight).nothing_is_true(); // ok! ``` You can find more information about borrowing in the rust-book: -http://doc.rust-lang.org/book/first-edition/references-and-borrowing.html +http://doc.rust-lang.org/book/ch04-02-references-and-borrowing.html "##, E0508: r##" diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index c0a8dd87ee2d5..0a7a523ca9e15 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -1,3 +1,4 @@ +// ignore-tidy-linelength #![allow(non_snake_case)] register_long_diagnostics! { @@ -1543,7 +1544,9 @@ fn f() {} It is not possible to declare type parameters on a function that has the `start` attribute. Such a function must have the following type signature (for more -information: http://doc.rust-lang.org/stable/book/first-edition/no-stdlib.html): +information, view [the unstable book][1]): + +[1]: https://doc.rust-lang.org/unstable-book/language-features/lang-items.html#writing-an-executable-without-stdlib ``` # let _: @@ -2917,10 +2920,11 @@ impl Baz for Bar { } // Note: This is OK E0374: r##" A struct without a field containing an unsized type cannot implement -`CoerceUnsized`. An -[unsized type](https://doc.rust-lang.org/book/first-edition/unsized-types.html) -is any type that the compiler doesn't know the length or alignment of at -compile time. Any struct containing an unsized type is also unsized. +`CoerceUnsized`. An [unsized type][1] is any type that the compiler +doesn't know the length or alignment of at compile time. Any struct +containing an unsized type is also unsized. + +[1]: https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait Example of erroneous code: @@ -2977,9 +2981,9 @@ A struct with more than one field containing an unsized type cannot implement `CoerceUnsized`. This only occurs when you are trying to coerce one of the types in your struct to another type in the struct. In this case we try to impl `CoerceUnsized` from `T` to `U` which are both types that the struct -takes. An [unsized type] is any type that the compiler doesn't know the length -or alignment of at compile time. Any struct containing an unsized type is also -unsized. +takes. An [unsized type][1] is any type that the compiler doesn't know the +length or alignment of at compile time. Any struct containing an unsized type +is also unsized. Example of erroneous code: @@ -3024,7 +3028,7 @@ fn coerce_foo, U>(t: T) -> Foo { } ``` -[unsized type]: https://doc.rust-lang.org/book/first-edition/unsized-types.html +[1]: https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait "##, E0376: r##" @@ -3032,11 +3036,12 @@ The type you are trying to impl `CoerceUnsized` for is not a struct. `CoerceUnsized` can only be implemented for a struct. Unsized types are already able to be coerced without an implementation of `CoerceUnsized` whereas a struct containing an unsized type needs to know the unsized type -field it's containing is able to be coerced. An -[unsized type](https://doc.rust-lang.org/book/first-edition/unsized-types.html) +field it's containing is able to be coerced. An [unsized type][1] is any type that the compiler doesn't know the length or alignment of at compile time. Any struct containing an unsized type is also unsized. +[1]: https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait + Example of erroneous code: ```compile_fail,E0376 @@ -3882,8 +3887,10 @@ let c = 86u8 as char; // ok! assert_eq!(c, 'V'); ``` -For more information about casts, take a look at The Book: -https://doc.rust-lang.org/book/first-edition/casting-between-types.html +For more information about casts, take a look at the Type cast section in +[The Reference Book][1]. + +[1]: https://doc.rust-lang.org/reference/expressions/operator-expr.html#type-cast-expressions "##, E0605: r##" @@ -3911,8 +3918,10 @@ let v = 0 as *const u8; v as *const i8; // ok! ``` -For more information about casts, take a look at The Book: -https://doc.rust-lang.org/book/first-edition/casting-between-types.html +For more information about casts, take a look at the Type cast section in +[The Reference Book][1]. + +[1]: https://doc.rust-lang.org/reference/expressions/operator-expr.html#type-cast-expressions "##, E0606: r##" @@ -3933,8 +3942,10 @@ let x = &0u8; let y: u32 = *x as u32; // We dereference it first and then cast it. ``` -For more information about casts, take a look at The Book: -https://doc.rust-lang.org/book/first-edition/casting-between-types.html +For more information about casts, take a look at the Type cast section in +[The Reference Book][1]. + +[1]: https://doc.rust-lang.org/reference/expressions/operator-expr.html#type-cast-expressions "##, E0607: r##" @@ -3960,8 +3971,10 @@ pointer holds is their size. To fix this error, don't try to cast directly between thin and fat pointers. -For more information about casts, take a look at The Book: -https://doc.rust-lang.org/book/first-edition/casting-between-types.html +For more information about casts, take a look at the Type cast section in +[The Reference Book][1]. + +[1]: https://doc.rust-lang.org/reference/expressions/operator-expr.html#type-cast-expressions "##, E0609: r##" @@ -4019,8 +4032,8 @@ println!("x: {}, y: {}", variable.x, variable.y); ``` For more information about primitives and structs, take a look at The Book: -https://doc.rust-lang.org/book/first-edition/primitive-types.html -https://doc.rust-lang.org/book/first-edition/structs.html +https://doc.rust-lang.org/book/ch03-02-data-types.html +https://doc.rust-lang.org/book/ch05-00-structs.html "##, E0614: r##" diff --git a/src/librustc_typeck/structured_errors.rs b/src/librustc_typeck/structured_errors.rs index 7e1ef8e021350..dea13c0e83611 100644 --- a/src/librustc_typeck/structured_errors.rs +++ b/src/librustc_typeck/structured_errors.rs @@ -137,7 +137,7 @@ To fix this error, don't try to cast directly between thin and fat pointers. For more information about casts, take a look at The Book: -https://doc.rust-lang.org/book/first-edition/casting-between-types.html"); +https://doc.rust-lang.org/reference/expressions/operator-expr.html#type-cast-expressions"); err } } diff --git a/src/libstd/prelude/mod.rs b/src/libstd/prelude/mod.rs index bf689bad559d3..551e982a3c685 100644 --- a/src/libstd/prelude/mod.rs +++ b/src/libstd/prelude/mod.rs @@ -129,10 +129,10 @@ //! [`std::string`]: ../string/index.html //! [`std::vec`]: ../vec/index.html //! [`to_owned`]: ../borrow/trait.ToOwned.html#tymethod.to_owned -//! [book-closures]: ../../book/first-edition/closures.html -//! [book-dtor]: ../../book/first-edition/drop.html -//! [book-enums]: ../../book/first-edition/enums.html -//! [book-iter]: ../../book/first-edition/iterators.html +//! [book-closures]: ../../book/ch13-01-closures.html +//! [book-dtor]: ../../book/ch15-03-drop.html +//! [book-enums]: ../../book/ch06-01-defining-an-enum.html +//! [book-iter]: ../../book/ch13-02-iterators.html #![stable(feature = "rust1", since = "1.0.0")] diff --git a/src/libtest/lib.rs b/src/libtest/lib.rs index 2cc80ddea2df4..d93f8b495d565 100644 --- a/src/libtest/lib.rs +++ b/src/libtest/lib.rs @@ -6,7 +6,7 @@ //! benchmarks themselves) should be done via the `#[test]` and //! `#[bench]` attributes. //! -//! See the [Testing Chapter](../book/first-edition/testing.html) of the book for more details. +//! See the [Testing Chapter](../book/ch11-00-testing.html) of the book for more details. // Currently, not much of this is meant for users. It is intended to // support the simplest interface possible for representing and From 285d4a7eb67cdf3ebba4b8fe60a5c5a6abf3edfc Mon Sep 17 00:00:00 2001 From: Dan Robertson Date: Wed, 13 Feb 2019 17:52:22 +0000 Subject: [PATCH 24/25] suggestion-diagnostics: as_ref improve snippet Improve the code snippet suggested in suggestion-diagnostics when suggesting the use of as_ref. --- src/librustc_typeck/check/demand.rs | 15 ++++++++++----- src/test/ui/suggestions/as-ref.stderr | 16 ++++++++-------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 33e93b582e540..bb9fc872b85d2 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -210,7 +210,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { /// ``` /// opt.map(|arg| { takes_ref(arg) }); /// ``` - fn can_use_as_ref(&self, expr: &hir::Expr) -> Option<(Span, &'static str, String)> { + fn can_use_as_ref( + &self, + expr: &hir::Expr, + ) -> Option<(Span, &'static str, String)> { if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = expr.node { if let hir::def::Def::Local(id) = path.def { let parent = self.tcx.hir().get_parent_node(id); @@ -233,10 +236,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self_ty.starts_with("std::option::Option") || self_ty.starts_with("std::result::Result") ) && (name == "map" || name == "and_then"); - if is_as_ref_able { - return Some((span.shrink_to_lo(), - "consider using `as_ref` instead", - "as_ref().".into())); + match (is_as_ref_able, self.sess().source_map().span_to_snippet(*span)) { + (true, Ok(src)) => { + return Some((*span, "consider using `as_ref` instead", + format!("as_ref().{}", src))); + }, + _ => () } } } diff --git a/src/test/ui/suggestions/as-ref.stderr b/src/test/ui/suggestions/as-ref.stderr index 7273496a7ce06..8143acc957b4c 100644 --- a/src/test/ui/suggestions/as-ref.stderr +++ b/src/test/ui/suggestions/as-ref.stderr @@ -2,9 +2,9 @@ error[E0308]: mismatched types --> $DIR/as-ref.rs:6:27 | LL | opt.map(|arg| takes_ref(arg)); - | - ^^^ expected &Foo, found struct `Foo` + | --- ^^^ expected &Foo, found struct `Foo` | | - | help: consider using `as_ref` instead: `as_ref().` + | help: consider using `as_ref` instead: `as_ref().map` | = note: expected type `&Foo` found type `Foo` @@ -13,9 +13,9 @@ error[E0308]: mismatched types --> $DIR/as-ref.rs:8:37 | LL | opt.and_then(|arg| Some(takes_ref(arg))); - | - ^^^ expected &Foo, found struct `Foo` + | -------- ^^^ expected &Foo, found struct `Foo` | | - | help: consider using `as_ref` instead: `as_ref().` + | help: consider using `as_ref` instead: `as_ref().and_then` | = note: expected type `&Foo` found type `Foo` @@ -24,9 +24,9 @@ error[E0308]: mismatched types --> $DIR/as-ref.rs:11:27 | LL | opt.map(|arg| takes_ref(arg)); - | - ^^^ expected &Foo, found struct `Foo` + | --- ^^^ expected &Foo, found struct `Foo` | | - | help: consider using `as_ref` instead: `as_ref().` + | help: consider using `as_ref` instead: `as_ref().map` | = note: expected type `&Foo` found type `Foo` @@ -35,9 +35,9 @@ error[E0308]: mismatched types --> $DIR/as-ref.rs:13:35 | LL | opt.and_then(|arg| Ok(takes_ref(arg))); - | - ^^^ expected &Foo, found struct `Foo` + | -------- ^^^ expected &Foo, found struct `Foo` | | - | help: consider using `as_ref` instead: `as_ref().` + | help: consider using `as_ref` instead: `as_ref().and_then` | = note: expected type `&Foo` found type `Foo` From 5d65e8cc7aec3cdcd99367c58367a16c99a8ae22 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 7 Feb 2019 16:03:57 +1100 Subject: [PATCH 25/25] Reduce the size of `hir::Expr`. From 104 bytes to 72 bytes on x86-64. This slightly reduces instruction counts. Also add an assertion about the size. --- src/librustc/hir/lowering.rs | 10 +++++----- src/librustc/hir/mod.rs | 8 ++++++-- src/librustc_save_analysis/lib.rs | 9 +++++++-- src/librustc_typeck/check/demand.rs | 6 +++++- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index cc5b105bad0d4..84487c40f8745 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -3859,7 +3859,7 @@ impl<'a> LoweringContext<'a> { hir::ExprKind::Call(f, args.iter().map(|x| self.lower_expr(x)).collect()) } ExprKind::MethodCall(ref seg, ref args) => { - let hir_seg = self.lower_path_segment( + let hir_seg = P(self.lower_path_segment( e.span, seg, ParamMode::Optional, @@ -3867,7 +3867,7 @@ impl<'a> LoweringContext<'a> { ParenthesizedGenericArgs::Err, ImplTraitContext::disallowed(), None, - ); + )); let args = args.iter().map(|x| self.lower_expr(x)).collect(); hir::ExprKind::MethodCall(hir_seg, seg.ident.span, args) } @@ -4148,7 +4148,7 @@ impl<'a> LoweringContext<'a> { node: if is_unit { hir::ExprKind::Path(struct_path) } else { - hir::ExprKind::Struct(struct_path, fields, None) + hir::ExprKind::Struct(P(struct_path), fields, None) }, span: e.span, attrs: e.attrs.clone(), @@ -4220,13 +4220,13 @@ impl<'a> LoweringContext<'a> { hir::ExprKind::InlineAsm(P(hir_asm), outputs, inputs) } ExprKind::Struct(ref path, ref fields, ref maybe_expr) => hir::ExprKind::Struct( - self.lower_qpath( + P(self.lower_qpath( e.id, &None, path, ParamMode::Optional, ImplTraitContext::disallowed(), - ), + )), fields.iter().map(|x| self.lower_field(x)).collect(), maybe_expr.as_ref().map(|x| P(self.lower_expr(x))), ), diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index bf16ec0be83e7..e389b918d3c35 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1319,6 +1319,10 @@ pub struct Expr { pub hir_id: HirId, } +// `Expr` is used a lot. Make sure it doesn't unintentionally get bigger. +#[cfg(target_arch = "x86_64")] +static_assert!(MEM_SIZE_OF_EXPR: std::mem::size_of::() == 72); + impl Expr { pub fn precedence(&self) -> ExprPrecedence { match self.node { @@ -1438,7 +1442,7 @@ pub enum ExprKind { /// and the remaining elements are the rest of the arguments. /// Thus, `x.foo::(a, b, c, d)` is represented as /// `ExprKind::MethodCall(PathSegment { foo, [Bar, Baz] }, [x, a, b, c, d])`. - MethodCall(PathSegment, Span, HirVec), + MethodCall(P, Span, HirVec), /// A tuple (e.g., `(a, b, c ,d)`). Tup(HirVec), /// A binary operation (e.g., `a + b`, `a * b`). @@ -1506,7 +1510,7 @@ pub enum ExprKind { /// /// For example, `Foo {x: 1, y: 2}`, or /// `Foo {x: 1, .. base}`, where `base` is the `Option`. - Struct(QPath, HirVec, Option>), + Struct(P, HirVec, Option>), /// An array literal constructed from one repeated element. /// diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs index 8d20c44a5a4ef..8ab9a8e8dda86 100644 --- a/src/librustc_save_analysis/lib.rs +++ b/src/librustc_save_analysis/lib.rs @@ -614,11 +614,16 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> { Some(def) if def != HirDef::Err => def, _ => self.get_path_def(self.tcx.hir().get_parent_node(id)), } - }, + } + Node::Expr(&hir::Expr { node: hir::ExprKind::Struct(ref qpath, ..), .. - }) | + }) => { + let hir_id = self.tcx.hir().node_to_hir_id(id); + self.tables.qpath_def(qpath, hir_id) + } + Node::Expr(&hir::Expr { node: hir::ExprKind::Path(ref qpath), .. diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 33e93b582e540..f6a0fd5caccdb 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -430,7 +430,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { match expr.node { // All built-in range literals but `..=` and `..` desugar to Structs - ExprKind::Struct(QPath::Resolved(None, ref path), _, _) | + ExprKind::Struct(ref qpath, _, _) => { + if let QPath::Resolved(None, ref path) = **qpath { + return is_range_path(&path) && span_is_range_literal(&expr.span); + } + } // `..` desugars to its struct path ExprKind::Path(QPath::Resolved(None, ref path)) => { return is_range_path(&path) && span_is_range_literal(&expr.span);