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

RFC: Add a special TryFrom and Into derive macro, specifically for C-Style enums #3604

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

agreyyy
Copy link

@agreyyy agreyyy commented Apr 1, 2024

I think this a good quality of life proposal similar to the proposal to add a derive Default for enums, not big in scope or magnitude.
TLDR:
I want to make a derive macro for TryFrom and Into, that generate an impl of TryFrom and Into for any C-Style Enum:

//using these two macros
#[derive(FromInt, IntoInt)] // names subject to change
enum CStyleEnum {
   Variant1 = 10,
   Variant2 = 20,
   ...
}

//convert from a number to the C-style enum easily
let from_num = CStyleEnum::try_from(10); // Ok(CStyleEnum::Variant1)
let failed_from = CStyleEnum::try_from(21); // Err(())
//convert into a number from a C-style enum more generically
let into_num = CStyleEnum::Variant2.into(); //20

Rendered

@agreyyy agreyyy changed the title Feature: Add a special TryFrom and Into derive macro, specifically for C-Style enums RFC: Add a special TryFrom and Into derive macro, specifically for C-Style enums Apr 1, 2024
- Why is this design the best in the space of possible designs?
- What other designs have been considered and what is the rationale for not choosing them?

I don\`t think any other designs have been introduced/considered since again, C-style enums are very niche, but this problem is not very logically hard, unlike `Futures`, `const evaluation`, etc.. The impact of not having these helper macros is that it makes a niche part of the language harder to use than it needs to be, and forces the developer to write more boilerplate to get their code to work as intended. This functionality could and is done by a library like [strum](https://docs.rs/strum/latest/strum/), however, since this is a very simple and niche feature, I still think its in the scope of being added to the language, also I think since everyone who has used rust extensively knows of the uses and implications of TryFrom and Into traits, instead of some other 3rd-party library defined traits (e.g. FromRepr in the case of strum).
Copy link
Member

Choose a reason for hiding this comment

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

What do you mean by "niche" here? Isn't being niche (i.e. very specialized), plus having a library alternative, exactly why this feature should not be standardized into the language?

@kennytm
Copy link
Member

kennytm commented Apr 1, 2024

Note that the RFC itself can already been done with the num_enum package (and tons of other alternatives).

---
[dependencies]
num_enum = "0.7.2"
---

use num_enum::{TryFromPrimitive, IntoPrimitive, TryFromPrimitiveError};

#[derive(TryFromPrimitive, IntoPrimitive, PartialEq, Debug)]
#[repr(i32)]
enum CStyleEnum {
    Variant1 = 10,
    Variant2 = 20,
}

fn main() {
    assert_eq!(CStyleEnum::try_from(10), Ok(CStyleEnum::Variant1));
    assert_eq!(CStyleEnum::try_from(21), Err(TryFromPrimitiveError { number: 21 }));
    assert_eq!(i32::from(CStyleEnum::Variant2), 20);
}

Unlike #3107 about #[derive(Default)], the standard library did not have #[derive(TryFrom, Into)] already.


For non-C-style enums, the derive_more package provided #[derive(From, TryInto)] which is based on shape rather than discriminant.

#[derive(From, Debug)]
#[repr(i32)]
enum NotReallyCStyleEnum {
    A = 10,
    B(i32) = 20,
}

fn main() {
    dbg!(NotReallyCStyleEnum::try_from(10_i32)); // B(10), not A
}

@clarfonthey
Copy link
Contributor

clarfonthey commented Apr 1, 2024

While I would like to see From and TryFrom derives supported by the standard library, I have to admit that so many crates exist to do this and it's going to take a very robust proposal to accomplish this.

This isn't saying that your proposal is bad, genuinely, just that the bar is extremely high for this IMHO. Since we can't really change what the shape of the standard library version would be once stabilised, we're going to need a very solid crate implementing exactly what form this should take, and see it widely used before any standard library version is likely to get added.

I like derive_more, but as mentioned, its functionality is split with stuff like num_enum, and it's not clear how From and TryFrom should be derived in all cases. Again, we need a very comprehensive proposal to make it in the standard library, and right now we just have several perfectly reasonable proposals who aren't as comprehensive, which is fine since they're in separate crates and people can use what they need.

@ehuss ehuss added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Apr 1, 2024
@ehuss
Copy link
Contributor

ehuss commented Apr 1, 2024

I'm tentatively marking this t-libs-api, but I don't actually know if that's the right team assignment.

It does seem like there is a lot of background and prior-art that isn't mentioned in the RFC. For example, this has been brought up in #2783 and https://internals.rust-lang.org/t/derive-tryfrom-for-c-like-enums/12940 among other places.

- No new named concepts
- Explaining the feature

We have a C-Style enum we derive TryFromInt and IntoInt for (name is subject to change). This allows us to save time writing boilerplate for constructing our enum from integers, and converting our enum to integers
Copy link
Member

Choose a reason for hiding this comment

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

It's very unusual for derive(XXX) to actually derive YYY. Why are you leaning towards having the derive TryFromInt emit an implementation of TryFrom? Why not call the derive TryFrom?

```
It worked! We can construct our C-style DNSOpCode enum **safely**, with **no boilerplate**, how cool

I think that this code all logically makes sense and there is really no other way I can think of to implement TryFrom<Integer> for a C-style enum, in terms of maintainability and readability I think the code isn\`t unambigous, and the reader can tell that there is one clear intent in the use of the try_form and into function, to try and get a `DNSOpCode` from an integer, or convert a `DNSOpCode` into an integer. Also, since this feature is completely additive, and it is opt-in, it won\`t break or deprecate any existing code. This feature isn\`t changing how anything new or old works, so I think it will be just as easy for beginners to learn as experienced rust programmers
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 that this code all logically makes sense and there is really no other way I can think of to implement TryFrom for a C-style enum

I can think of plausible alternative implementations. Consider that Rust draws a subtle distinction between an enum's discriminant (which is the logical value associated with a variant) and its tag (which is the actual in-memory value used to distinguish it from other variants. An enum can have a discriminant but no tag, or a discriminant that's a different value from its tag, or a discriminant that's a different type from its tag. For instance, the Variant below has a logical discriminant of -42isize, but no tag at all:

enum Example {
    Variant = -42
}

In any case where the tag is different from the discriminant, what should the behavior of this derive be? Your RFC leans towards following the value (but not necessarily the type) of the discriminant, but that's not the only plausible implementation.

This is the technical portion of the RFC. Explain the design in sufficient detail that:


I want to propose a **TryFrom<{Integer}>** and **Into<{Integer}>** derive macro becomes available for all C-style enums. C-style enums are a subset of all integers, which is why C-style Enums can be cast to integers using the ``as {usize/u8/isize, etc..}`` syntax. I propose that we create a derive macro, that automatically allows C-Style Enums to be cast to Integers using ``.into()``, not just ``as``. Since only _some_ Integers can be turned into a C-Style Enum, I propose that a TryFrom<{Integer}> derive macro can be made for them, to allow easy and more generic conversion between a C-Style Enum and Integers.
Copy link
Member

Choose a reason for hiding this comment

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

I want to propose a TryFrom<{Integer}> and Into<{Integer}> derive macro becomes available for all C-style enums.

The term "C-style enum" is ill-defined in Rust. We use either:

  • "fieldless enum" for an enum where no variant has fields
  • "unit-only enum" for an enum where all variants are unit-like

See the reference for more information: https://doc.rust-lang.org/reference/items/enumerations.html

Comment on lines +120 to +127
This one is really simple, its just using the `as` conversion inside the function, to convert the inputted enum to any number. But this allows it to be used inside generics where Into<{Integer}> is a trait bound, among others.
```
impl Into<{Integer}> for InputtedEnum {
fn into(self) -> {Integer} {
self as {Integer}
}
}
```
Copy link
Member

Choose a reason for hiding this comment

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

For field-less (but not unit-only) enums, this will produce incorrect results. Consider this:

enum Example {
  Foo
  Bar {}
  Baz()
}

Baz as {integer} will produce not the discriminant of Baz, but rather the address of its implicit constructor.

```

likewise, we can also use the Into trait to do the same thing
opcode_num and opcode_num1 are the same, however opcode_num2 can be used in generic contexts where `as` cannot
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 see many APIs that consume impl Into<u8>. Is there an instance where such an API would be useful for a type like DNSOpCode?

@matklad
Copy link
Member

matklad commented Apr 14, 2024

See also rust-lang/rust#86772

matklad referenced this pull request in matklad/matklad.github.io Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants