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

Inside Rust: const eval roadmap #719

Closed
wants to merge 6 commits into from
Closed

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Oct 30, 2020

@oli-obk oli-obk requested a review from a team as a code owner October 30, 2020 11:55
wesleywiser
wesleywiser previously approved these changes Oct 30, 2020
Copy link
Member

@wesleywiser wesleywiser left a comment

Choose a reason for hiding this comment

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

Exciting! 🎉

team: the const eval working group <https://github.com/rust-lang/const-eval/>
---

Y'all probably noticed that const eval finally got loops, mutable local variables and branches. With those features existing, we were able to make a lot of basic arithmetic functions `const fn `. Things like `checked_div` needs to be able to check that the divisor is not zero and return `None` in that case. These features have been asked for a lot, and while it may not appear so, these were the low hanging fruit.
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand the purpose of the 2nd sentence here ("Things like..."). It seems unrelated to the first and third sentence, and just cutting it doesn't leave a gap as far as I can see.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know where to put an example... Maybe I could just say

basic arithmetic functions (e.g. checked_div) const fn

?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that seems much better. Also checked_div seems rather niche, isn't there something "bigger"?

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 unwrap_or works now and didn't before, anything with Option or Result really.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I chose checked_div because it's pretty obvious what it does. the Option/Result methods may still have some blockers somewhere, not sure if we fixed/stabilized the drop fixes needed for that.

Copy link
Member

Choose a reason for hiding this comment

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

What about "(e.g. checked and saturating arithmetic)"? That makes it less specific to "div".
(I am looking at https://blog.rust-lang.org/2020/08/27/Rust-1.46.0.html to see what got stabilized there.)

@RalfJung
Copy link
Member

One question about the skill tree itself: So far, I think we removed (or planned to remove) nodes when they got stabilized. Would it make more sense to instead change their color or so? That would (a) keep the useful dependency information around, and (b) give a nice sense of progress as the entire graph slowly changes color. :D

@oli-obk
Copy link
Contributor Author

oli-obk commented Nov 1, 2020

That would (a) keep the useful dependency information around, and

Is it still useful? There's no dependency anymore. It's more historical information. Maybe I could extend the skill tree generator to output two graphs? One with all nodes (and outdated ones grayed out or something) and one with just the still active nodes.

(b) give a nice sense of progress as the entire graph slowly changes color. :D

I like progress being the graph having less parts ^^ Like the entire graph is just the things we still have to work at without keeping parts in it that are done.

@RalfJung
Copy link
Member

RalfJung commented Nov 1, 2020

I like progress being the graph having less parts ^^ Like the entire graph is just the things we still have to work at without keeping parts in it that are done.

I expect more nodes will be added as we discover more things we want to do during CTFE. So we might make progress without the graph getting smaller. Or is that not how you intended this to go?

@oli-obk
Copy link
Contributor Author

oli-obk commented Nov 2, 2020

So we might make progress without the graph getting smaller. Or is that not how you intended this to go?

No, that is indeed how I intended this to work.

If we keep around old nodes the graph will grow indefinitely, even if it contains things that have been stable for a long time and some ppl on the project may not even know that this didn't work some time in the past. If we remove things (maybe after a grace period), the graph will only be the (future?) roadmap, and not a general overview over the past and future features of const eval.

What do you think about going with both our preferred systems and just making two graphs out of the same skill tree file?

@RalfJung
Copy link
Member

RalfJung commented Nov 4, 2020

If you don't think keeping the old nodes around is useful, I am okay with dropping them. We have the git history for historic information. I just felt like it could be an interesting thing to present, but I might just be wrong about it being interesting. ;)

@rylev
Copy link
Member

rylev commented Jun 22, 2022

@oli-obk I'm guessing this has gone pretty stale. Shall we close this?

@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 22, 2022

Yea, I guess

@oli-obk oli-obk closed this Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants