Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 4 additions & 2 deletions library/core/src/task/poll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,17 @@ impl<T> Poll<T> {

/// Returns `true` if this is `Poll::Ready`
#[inline]
#[rustc_const_stable(feature = "const_poll", since = "1.48.0")]
#[stable(feature = "futures_api", since = "1.36.0")]
pub fn is_ready(&self) -> bool {
pub const fn is_ready(&self) -> bool {
matches!(*self, Poll::Ready(_))
}

/// Returns `true` if this is `Poll::Pending`
#[inline]
#[rustc_const_stable(feature = "const_poll", since = "1.48.0")]
#[stable(feature = "futures_api", since = "1.36.0")]
pub fn is_pending(&self) -> bool {
pub const fn is_pending(&self) -> bool {
!self.is_ready()
}
}
Expand Down
13 changes: 13 additions & 0 deletions src/test/ui/consts/std/poll.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// run-pass

use std::task::Poll;

fn main() {
const POLL : Poll<usize> = Poll::Pending;

const IS_READY : bool = POLL.is_ready();
assert!(!IS_READY);

const IS_PENDING : bool = POLL.is_pending();
assert!(IS_PENDING);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, why are we using a UI test for this, especially a run-pass one? I would think that these integrated tests are much costlier than standard #[test] tests, as well as much harder to debug. Linking a whole separate executable to check two const methods of stdlib is an overkill.

I would suggest testing similar things by unit or integrated #[test] tests in core or std itself.

It might also be a good idea to sift through existing test suite for stdlib tests which can be moved to unit tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, yeah, we already have a bunch of tests like this...

I've opened https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/UI.20tests.20for.20stdlib for discussion, it might be that I am missing some context here of course.

Copy link
Contributor

Choose a reason for hiding this comment

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

The conclusion is that we want to move such tests to library/: #76268

@CDirkx if you could move some of the already existing const tests in a similar way, that would be awesome :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I'll have a look later today

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tests are moved 👍