Skip to content
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

feat: enable construct event constantly #140

Merged
merged 1 commit into from
Aug 22, 2023

Conversation

tisonkun
Copy link
Contributor

@tisonkun tisonkun commented Aug 21, 2023

Maybe #133 can prevent this from being possible; since it includes sys details for Event.

These sys things are one of the main complain on mio that I want to get rid of.

Compare with mio, it can obtain a const TOKEN: Token = Token(0), while in polling we cannot obtain a const interest Event like const INTEREST: Event = Event::readable(1). I don't want to know about the sys things.

cc @notgull

@tisonkun
Copy link
Contributor Author

Perhaps I can use a const KEY: usize = 1 and Event::readable(KEY) and that should be fine.

Keep this PR open for ideas on whether we can make the event constructors themselves const.

Copy link
Member

@notgull notgull left a comment

Choose a reason for hiding this comment

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

I think this would be a good addition to this crate.

Comment on lines 655 to 657
EventExtra {
flags: AfdPollMask::empty(),
}
Copy link
Member

Choose a reason for hiding this comment

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

Fix the compile error by also making AfdPollMask::empty() constant.

@@ -186,7 +186,7 @@ impl Event {
/// All kinds of events (readable and writable).
///
/// Equivalent to: `Event { key, readable: true, writable: true }`
pub fn all(key: usize) -> Event {
pub const fn all(key: usize) -> Event {
Copy link
Member

Choose a reason for hiding this comment

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

You forgot to make the poll.rs version of EventEmpty const.

@tisonkun
Copy link
Contributor Author

tisonkun commented Aug 22, 2023

Updated at here. Let's see if all the empty branches can be const.

@tisonkun
Copy link
Contributor Author

All tests passed. PTAL @notgull

Copy link
Member

@notgull notgull left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@notgull notgull merged commit d8595b5 into smol-rs:master Aug 22, 2023
24 checks passed
@tisonkun tisonkun deleted the const-event branch August 22, 2023 01:36
@notgull notgull mentioned this pull request Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants