enumset icon indicating copy to clipboard operation
enumset copied to clipboard

Add const macro version of `try_from_repr`.

Open ijackson opened this issue 1 month ago • 2 comments

For Reasons, we want a const version of from_repr.

(We want to have generic types that are parameterised over enumset constants. But Rust only supports integers as const generic parameters. So we're using the repr as the const parameter. We would like to be able to convert that repr back to an EnumSet<T> constly so that any wrong bit values are a compile time rather than runtime panic.)

There's from_repr but it's not const. It can't simply be made const because .expect() isn't available in const. try_from_repr relies on .and_not which would need to be replaced with macro calls (which may be much slower). So just making try_from_repr const is not straightforward. (#27 relates.)

Would you be open to an enum_set_try_from_repr macro? (The version returning Option is sufficient for all use cases, even if the downstream crate has to manually unwrap it, so I propose providing only that fallible version.)

(In our code, it happens that we don't actually need this const from_repr constructor, because the only reason this is const is an error check, and we can do that with ::all().repr() and ad-hoc masking in the packed integer repr.. But it would be nicer to do things in a more principled way.)

ijackson avatar Nov 18 '25 17:11 ijackson

So, the feature is fine, as right now I'm doing what I can to support some const time operations without Rust having all the features I need to do it properly. I'll look at implementing it later.

However, your use case is pretty unusual. Using an EnumSet as a type parameter requires casting it to an integer and back outside a use case like serialization, somewhat defeats the benefit of using enumset over one of the simpler bit flags crates. I looked at the code in arti, and would suggest you use a "settings" trait design instead (not sure what the proper name would be):

#[derive(Debug, Clone)]
pub struct RelayFlagsParser<'s, Options: RelayFlagsParserOptions> {
    flags: DocRelayFlags,
    prev: Option<&'s str>,
    _phantom: PhantomData<Options>,
}

pub trait RelayFlagsParserOptions {
    const PARSE_IMPLICIT: EnumSet<RelayFlag>;
    const ENCODE_OMIT: EnumSet<RelayFlag>;
}

pub enum Consensus {}
impl RelayFlagsParserOptions for Consensus {
    const PARSE_IMPLICIT: EnumSet<RelayFlag> = RELAY_FLAGS_CONSENSUS_PARSE_IMPLICIT;
    const ENCODE_OMIT: EnumSet<RelayFlag> = RELAY_FLAGS_CONSENSUS_ENCODE_OMIT;
}

pub enum Vote {}
impl RelayFlagsParserOptions for Consensus {
    const PARSE_IMPLICIT: EnumSet<RelayFlag> = EnumSet::empty();
    const ENCODE_OMIT: EnumSet<RelayFlag> = EnumSet::empty();
}

pub type ConsensusRelayFlagsParser<'s> = RelayFlagsParser<'s, Consensus>;
pub type VoteRelayFlagsParser<'s> = RelayFlagsParser<'s, Vote>;

This has the advantage of not bypassing the type system, and leaving the responsibility of correctness up to the existing enumset const operations rather than needing an explicit check.

Lymia avatar Nov 21 '25 04:11 Lymia

I looked at the code in arti, and would suggest you use a "settings" trait design instead

Thanks! ❤️ I don't know why this didn't occur to me! I will do that instead.

Nevertheless, if you like, I could still do the const macro here, even though I won't be using it.

ijackson avatar Nov 24 '25 17:11 ijackson