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

I wonder if all the methods of the crate could be made const #24

Closed
felix91gr opened this issue Mar 9, 2023 · 7 comments
Closed

I wonder if all the methods of the crate could be made const #24

felix91gr opened this issue Mar 9, 2023 · 7 comments

Comments

@felix91gr
Copy link

Hellu. First of all, thanks for the crate. It's freaking awesome.

So I was doing some exercises at exercism.org, as practice, and there was an exercise that used this crate. I realized that most of the functions it requires don't (or almost don't) need any runtime information to be computed:

  1. value_to_color_string can be solved with a const Hashmap<u32, &str>
  2. colors is basically a const Vec<ResistorColor>

So long story short, I realized that I couldn't make these things const because, among other things, enum-iterator itself's methods aren't const. So I came and tried to add constness for the methods, thinking of submitting that as a PR, but I ran into a wall in the language that took me to rust-lang/rust#101900.

I think the methods in the crate can't currently be made const (since they are Trait methods after all?), but I could be wrong.

Anywho. I'm 99% sure you've already delved into this, and that it's in some todo-list of your own. But perhaps it wasn't, and this helps somewhat.

Also this allows me to say thanks for the crate. It's a really cool crate. Thanks :D

@stephaneyfx
Copy link
Owner

Hello and thank you for the kind words!

Indeed, I plan on making everything const when support becomes stable. const in trait methods is still unstable and in flux. In the meantime, an alternative is to define a static Vec<ResistorColor> using once_cell::sync::Lazy.

@stephaneyfx
Copy link
Owner

Closing as this requires some support that only exists in nightly and is unlikely to stabilize soon or even in its current form.

This is definitely a highly desirable feature which I will implement when support lands in stable.

@felix91gr
Copy link
Author

I plan on making everything const when support becomes stable.

Yay 🌈

In the meantime, an alternative is to define a static Vec<ResistorColor> using once_cell::sync::Lazy.

Ohh, I didn't know about that crate! Nor did I know about the RFC to include it in std... that's really cool!

My lazy kung-fu was last updated when I learned about the lazy_static! macro a good few years ago.

Given how the process is trending towards once_cell and the extra features that API packs (like lazy values for local variables of functions!) I'm sure as heck gonna use that from now on ✨

and thank you for the kind words!

Of course ^^ 🫶🏽

@pronebird
Copy link

On a related note. Just today I had to implement clap::ValueEnum for some of my enum types which requires to return a slice with all enum variants. Is it not possible to generate a method as a part of derive macro that simply puts all enum variants into a static slice, i.e fn all_variants() -> &'static [Self] { &[Self::A, Self::B, ...etc...] }?

@stephaneyfx
Copy link
Owner

Is it not possible to generate a method as a part of derive macro that simply puts all enum variants into a static slice [...]?

It is not possible without special-casing the macro as it supports nested types and relies on trait functions to instantiate values. The following solution seems simple enough (though it does pull in another crate):

use enum_iterator::Sequence;
use once_cell::sync::Lazy;

#[derive(Debug, PartialEq, Sequence)]
pub enum Foo {
    A,
    B(bool),
}

pub fn all_foos() -> &'static [Foo] {
    static FOOS: Lazy<Vec<Foo>> = Lazy::new(|| enum_iterator::all().collect());
    &**FOOS
}

@pronebird
Copy link

pronebird commented Feb 11, 2024

I see. Thanks for posting the example.

I have done similar today with std::sync::Once although it's unsafe but I assume Lazy uses some sort of internal mutex lock which is a bit of an overkill for that kind of task -- I mean in my case it's 10 enum variants inside of a slice without nested values.

Obviously hardcoding is not difficult, it's just that as the types evolve and grow it becomes harder to keep track of things. So eventually I decided to plug in a derive(clap::ValueEnum) for my enum and call it a day. I wanted to isolate my lib from clap but whatever... But I still use enum_iterator::Sequence for other things such as computing things at runtime by walking over enum variants etc..

@stephaneyfx
Copy link
Owner

You're welcome. Actually, I just remembered it's possible to achieve without an additional crate as the needed functionality was merged into std:

use enum_iterator::Sequence;
use std::sync::OnceLock;

#[derive(Debug, PartialEq, Sequence)]
pub enum Foo {
    A,
    B(bool),
}

pub fn all_foos() -> &'static [Foo] {
    static FOOS: OnceLock<Vec<Foo>> = OnceLock::new();
    &**FOOS.get_or_init(|| enum_iterator::all().collect())
}

If you only need this &'static [Foo] for clap, the locking that happens under the hood likely does not matter. If this were in a tight loop, that could be a different story.

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

No branches or pull requests

3 participants