Skip to content

Stabilize futures_api #59739

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/liballoc/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,7 @@ impl<G: ?Sized + Generator> Generator for Pin<Box<G>> {
}
}

#[unstable(feature = "futures_api", issue = "50547")]
#[stable(feature = "futures_api", since = "1.36.0")]
impl<F: ?Sized + Future + Unpin> Future for Box<F> {
type Output = F::Output;

Expand Down
1 change: 0 additions & 1 deletion src/liballoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@
#![feature(fmt_internals)]
#![feature(fn_traits)]
#![feature(fundamental)]
#![feature(futures_api)]
#![feature(lang_items)]
#![feature(libc)]
#![feature(needs_allocator)]
Expand Down
9 changes: 6 additions & 3 deletions src/libcore/future/future.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
#![unstable(feature = "futures_api",
reason = "futures in libcore are unstable",
issue = "50547")]
#![stable(feature = "futures_api", since = "1.36.0")]

use crate::marker::Unpin;
use crate::ops;
Expand All @@ -26,8 +24,10 @@ use crate::task::{Context, Poll};
/// `await!` the value.
#[doc(spotlight)]
#[must_use = "futures do nothing unless polled"]
#[stable(feature = "futures_api", since = "1.36.0")]
pub trait Future {
/// The type of value produced on completion.
#[stable(feature = "futures_api", since = "1.36.0")]
type Output;

/// Attempt to resolve the future to a final value, registering
Expand Down Expand Up @@ -92,9 +92,11 @@ pub trait Future {
/// [`Context`]: ../task/struct.Context.html
/// [`Waker`]: ../task/struct.Waker.html
/// [`Waker::wake`]: ../task/struct.Waker.html#method.wake
#[stable(feature = "futures_api", since = "1.36.0")]
fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output>;
}

#[stable(feature = "futures_api", since = "1.36.0")]
impl<F: ?Sized + Future + Unpin> Future for &mut F {
type Output = F::Output;

Expand All @@ -103,6 +105,7 @@ impl<F: ?Sized + Future + Unpin> Future for &mut F {
}
}

#[stable(feature = "futures_api", since = "1.36.0")]
impl<P> Future for Pin<P>
where
P: Unpin + ops::DerefMut,
Expand Down
5 changes: 2 additions & 3 deletions src/libcore/future/mod.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
#![unstable(feature = "futures_api",
reason = "futures in libcore are unstable",
issue = "50547")]
#![stable(feature = "futures_api", since = "1.36.0")]

//! Asynchronous values.

mod future;
#[stable(feature = "futures_api", since = "1.36.0")]
pub use self::future::Future;
6 changes: 3 additions & 3 deletions src/libcore/task/mod.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
#![unstable(feature = "futures_api",
reason = "futures in libcore are unstable",
issue = "50547")]
#![stable(feature = "futures_api", since = "1.36.0")]

//! Types and Traits for working with asynchronous tasks.

mod poll;
#[stable(feature = "futures_api", since = "1.36.0")]
pub use self::poll::Poll;

mod wake;
#[stable(feature = "futures_api", since = "1.36.0")]
pub use self::wake::{Context, Waker, RawWaker, RawWakerVTable};
20 changes: 16 additions & 4 deletions src/libcore/task/poll.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
#![unstable(feature = "futures_api",
reason = "futures in libcore are unstable",
issue = "50547")]
#![stable(feature = "futures_api", since = "1.36.0")]

use crate::ops::Try;
use crate::result::Result;
Expand All @@ -9,20 +7,27 @@ use crate::result::Result;
/// scheduled to receive a wakeup instead.
#[must_use = "this `Poll` may be a `Pending` variant, which should be handled"]
#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
#[stable(feature = "futures_api", since = "1.36.0")]
pub enum Poll<T> {
/// Represents that a value is immediately ready.
Ready(T),
#[stable(feature = "futures_api", since = "1.36.0")]
Ready(
#[stable(feature = "futures_api", since = "1.36.0")]
T
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder why we stabilize this field. Is this intended?

Copy link
Member

Choose a reason for hiding this comment

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

@lzutao other std enums with exposed fields have stabilization attributes as well e.g.

Some(#[stable(feature = "rust1", since = "1.0.0")] T),

The main way to use Poll is to match on it and access the field directly, it's basically a context specific version of Option.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good to know. Thanks @Nemo157 .

),

/// Represents that a value is not ready yet.
///
/// When a function returns `Pending`, the function *must* also
/// ensure that the current task is scheduled to be awoken when
/// progress can be made.
#[stable(feature = "futures_api", since = "1.36.0")]
Pending,
}

impl<T> Poll<T> {
/// Changes the ready value of this `Poll` with the closure provided.
#[stable(feature = "futures_api", since = "1.36.0")]
pub fn map<U, F>(self, f: F) -> Poll<U>
where F: FnOnce(T) -> U
{
Expand All @@ -34,6 +39,7 @@ impl<T> Poll<T> {

/// Returns `true` if this is `Poll::Ready`
#[inline]
#[stable(feature = "futures_api", since = "1.36.0")]
pub fn is_ready(&self) -> bool {
match *self {
Poll::Ready(_) => true,
Expand All @@ -43,13 +49,15 @@ impl<T> Poll<T> {

/// Returns `true` if this is `Poll::Pending`
#[inline]
#[stable(feature = "futures_api", since = "1.36.0")]
pub fn is_pending(&self) -> bool {
!self.is_ready()
}
}

impl<T, E> Poll<Result<T, E>> {
/// Changes the success value of this `Poll` with the closure provided.
#[stable(feature = "futures_api", since = "1.36.0")]
pub fn map_ok<U, F>(self, f: F) -> Poll<Result<U, E>>
where F: FnOnce(T) -> U
{
Expand All @@ -61,6 +69,7 @@ impl<T, E> Poll<Result<T, E>> {
}

/// Changes the error value of this `Poll` with the closure provided.
#[stable(feature = "futures_api", since = "1.36.0")]
pub fn map_err<U, F>(self, f: F) -> Poll<Result<T, U>>
where F: FnOnce(E) -> U
{
Expand All @@ -72,12 +81,14 @@ impl<T, E> Poll<Result<T, E>> {
}
}

#[stable(feature = "futures_api", since = "1.36.0")]
impl<T> From<T> for Poll<T> {
fn from(t: T) -> Poll<T> {
Poll::Ready(t)
}
}

#[stable(feature = "futures_api", since = "1.36.0")]
impl<T, E> Try for Poll<Result<T, E>> {
type Ok = Poll<T>;
type Error = E;
Expand All @@ -102,6 +113,7 @@ impl<T, E> Try for Poll<Result<T, E>> {
}
}

#[stable(feature = "futures_api", since = "1.36.0")]
impl<T, E> Try for Poll<Option<Result<T, E>>> {
type Ok = Poll<Option<T>>;
type Error = E;
Expand Down
36 changes: 27 additions & 9 deletions src/libcore/task/wake.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
#![unstable(feature = "futures_api",
reason = "futures in libcore are unstable",
issue = "50547")]
#![stable(feature = "futures_api", since = "1.36.0")]

use crate::fmt;
use crate::marker::{PhantomData, Unpin};
Expand All @@ -13,6 +11,7 @@ use crate::marker::{PhantomData, Unpin};
/// It consists of a data pointer and a [virtual function pointer table (vtable)][vtable] that
/// customizes the behavior of the `RawWaker`.
#[derive(PartialEq, Debug)]
#[stable(feature = "futures_api", since = "1.36.0")]
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`
Expand All @@ -37,9 +36,7 @@ impl RawWaker {
/// from a `RawWaker`. For each operation on the `Waker`, the associated
/// function in the `vtable` of the underlying `RawWaker` will be called.
#[rustc_promotable]
#[unstable(feature = "futures_api",
reason = "futures in libcore are unstable",
issue = "50547")]
#[stable(feature = "futures_api", since = "1.36.0")]
pub const fn new(data: *const (), vtable: &'static RawWakerVTable) -> RawWaker {
RawWaker {
data,
Expand All @@ -58,6 +55,7 @@ impl RawWaker {
/// 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.
#[stable(feature = "futures_api", since = "1.36.0")]
#[derive(PartialEq, Copy, Clone, Debug)]
pub struct RawWakerVTable {
/// This function will be called when the [`RawWaker`] gets cloned, e.g. when
Expand Down Expand Up @@ -131,9 +129,14 @@ impl RawWakerVTable {
/// resources that are associated with this instance of a [`RawWaker`] and
/// associated task.
#[rustc_promotable]
#[unstable(feature = "futures_api",
reason = "futures in libcore are unstable",
issue = "50547")]
#[cfg_attr(stage0, unstable(feature = "futures_api_const_fn_ptr", issue = "50547"))]
#[cfg_attr(not(stage0), stable(feature = "futures_api", since = "1.36.0"))]
// `rustc_allow_const_fn_ptr` is a hack that should not be used anywhere else
// without first consulting with T-Lang.
//
// FIXME: remove whenever we have a stable way to accept fn pointers from const fn
// (see https://github.com/rust-rfcs/const-eval/issues/19#issuecomment-472799062)
#[cfg_attr(not(stage0), rustc_allow_const_fn_ptr)]
pub const fn new(
clone: unsafe fn(*const ()) -> RawWaker,
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit surprised we're stabilizing this implementation detail, I would've expected an actual trait to be involved, and these fn pointers to be obtained internally (which I think is allowed? oh but const fn can't have trait bounds, hmpf).

Also, *const () in a public API? Seems a bit off, I thought we were using extern type pointers nowadays?


Anyway, regarding the matter at hand, which is constructors that happen to have banned types, I think this is how we can allow them without allowing the same types in any non-constructor const fn: rust-lang/const-eval#19 (comment) (by making them pattern aliases)

It also means calls would be promotable, which is good, right?

Copy link
Member

Choose a reason for hiding this comment

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

Actually, there is no reason for this to this unsafe.

While the fields of this struct have to be unsafe fn(*const ()), the arguments could be fn(&mut T) where the constructor is generic over T.
Sadly, this can't be directly coupled with RawWaker's constructor, but that's only because RawWakerVTable isn't itself generic... RawWaker could store a RawWakerVTable<SomePrivateExternType>, and it would all be much safer.

I'm sorry I haven't remarked this before, I kept thinking this "raw vtable" stuff was kept internal so therefore no need to make it more contrived for safety, I guess I was wrong...

Copy link
Member

@eddyb eddyb Apr 6, 2019

Choose a reason for hiding this comment

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

Bonus: you could pass a closure where a fn(&mut T) is expected, and it will just work!

So you can add extra behavior or w/e, pretty easily.

Copy link
Member

Choose a reason for hiding this comment

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

I think making the arguments &mut would make this implementation unsound since it uses the same data pointer for all instances.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was already discussed on the RFC (why it isn't a trait and why futures-rs will provide a safe trait for it).

Copy link
Member

Choose a reason for hiding this comment

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

@cramertj oops, I should've made separate comments, your resolving meant people wouldn't see my approval of rustc_allow_const_fn_ptr (#59739 (comment))

wake: unsafe fn(*const ()),
Expand All @@ -153,6 +156,7 @@ impl RawWakerVTable {
///
/// Currently, `Context` only serves to provide access to a `&Waker`
/// which can be used to wake the current task.
#[stable(feature = "futures_api", since = "1.36.0")]
pub struct Context<'a> {
waker: &'a Waker,
// Ensure we future-proof against variance changes by forcing
Expand All @@ -164,6 +168,7 @@ pub struct Context<'a> {

impl<'a> Context<'a> {
/// Create a new `Context` from a `&Waker`.
#[stable(feature = "futures_api", since = "1.36.0")]
#[inline]
pub fn from_waker(waker: &'a Waker) -> Self {
Context {
Expand All @@ -173,12 +178,14 @@ impl<'a> Context<'a> {
}

/// Returns a reference to the `Waker` for the current task.
#[stable(feature = "futures_api", since = "1.36.0")]
#[inline]
pub fn waker(&self) -> &'a Waker {
&self.waker
}
}

#[stable(feature = "futures_api", since = "1.36.0")]
impl fmt::Debug for Context<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("Context")
Expand All @@ -195,17 +202,22 @@ impl fmt::Debug for Context<'_> {
///
/// Implements [`Clone`], [`Send`], and [`Sync`].
#[repr(transparent)]
#[stable(feature = "futures_api", since = "1.36.0")]
pub struct Waker {
waker: RawWaker,
}

#[stable(feature = "futures_api", since = "1.36.0")]
impl Unpin for Waker {}
#[stable(feature = "futures_api", since = "1.36.0")]
unsafe impl Send for Waker {}
#[stable(feature = "futures_api", since = "1.36.0")]
unsafe impl Sync for Waker {}

impl Waker {
/// Wake up the task associated with this `Waker`.
#[inline]
#[stable(feature = "futures_api", since = "1.36.0")]
pub fn wake(self) {
// The actual wakeup call is delegated through a virtual function call
// to the implementation which is defined by the executor.
Expand All @@ -227,6 +239,7 @@ impl Waker {
/// where an owned `Waker` is available. This method should be preferred to
/// calling `waker.clone().wake()`.
#[inline]
#[stable(feature = "futures_api", since = "1.36.0")]
pub fn wake_by_ref(&self) {
// The actual wakeup call is delegated through a virtual function call
// to the implementation which is defined by the executor.
Expand All @@ -243,6 +256,7 @@ impl Waker {
///
/// This function is primarily used for optimization purposes.
#[inline]
#[stable(feature = "futures_api", since = "1.36.0")]
pub fn will_wake(&self, other: &Waker) -> bool {
self.waker == other.waker
}
Expand All @@ -253,13 +267,15 @@ impl Waker {
/// in [`RawWaker`]'s and [`RawWakerVTable`]'s documentation is not upheld.
/// Therefore this method is unsafe.
#[inline]
#[stable(feature = "futures_api", since = "1.36.0")]
pub unsafe fn from_raw(waker: RawWaker) -> Waker {
Waker {
waker,
}
}
}

#[stable(feature = "futures_api", since = "1.36.0")]
impl Clone for Waker {
#[inline]
fn clone(&self) -> Self {
Expand All @@ -272,6 +288,7 @@ impl Clone for Waker {
}
}

#[stable(feature = "futures_api", since = "1.36.0")]
impl Drop for Waker {
#[inline]
fn drop(&mut self) {
Expand All @@ -282,6 +299,7 @@ impl Drop for Waker {
}
}

#[stable(feature = "futures_api", since = "1.36.0")]
impl fmt::Debug for Waker {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let vtable_ptr = self.waker.vtable as *const RawWakerVTable;
Expand Down
1 change: 1 addition & 0 deletions src/librustc/ich/impls_syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ impl_stable_hash_for!(struct ::syntax::attr::Stability {
feature,
rustc_depr,
promotable,
allow_const_fn_ptr,
const_stability
});

Expand Down
1 change: 1 addition & 0 deletions src/librustc/middle/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,7 @@ impl<'a, 'tcx> Index<'tcx> {
rustc_depr: None,
const_stability: None,
promotable: false,
allow_const_fn_ptr: false,
});
annotator.parent_stab = Some(stability);
}
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,8 @@ rustc_queries! {
/// constructor function).
query is_promotable_const_fn(_: DefId) -> bool {}

query const_fn_is_allowed_fn_ptr(_: DefId) -> bool {}

/// True if this is a foreign item (i.e., linked via `extern { ... }`).
query is_foreign_item(_: DefId) -> bool {}

Expand Down
7 changes: 7 additions & 0 deletions src/librustc/ty/constness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,16 @@ pub fn provide<'tcx>(providers: &mut Providers<'tcx>) {
}
}

fn const_fn_is_allowed_fn_ptr<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> bool {
tcx.is_const_fn(def_id) &&
tcx.lookup_stability(def_id)
.map(|stab| stab.allow_const_fn_ptr).unwrap_or(false)
}

*providers = Providers {
is_const_fn_raw,
is_promotable_const_fn,
const_fn_is_allowed_fn_ptr,
..*providers
};
}
Loading