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

as-casts for enums with tuple-only variants are not prohibited #113904

Open
nalepamarcin opened this issue Jul 20, 2023 · 4 comments
Open

as-casts for enums with tuple-only variants are not prohibited #113904

nalepamarcin opened this issue Jul 20, 2023 · 4 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nalepamarcin
Copy link

Code

#[repr(u8)]
pub enum Enum {
    V1(u8)  = 0,
    V2(u16),
    V3(u32) = 5,
}

pub fn main() -> () {
    assert_eq!(Enum::V1 as u8, 0);
    assert_eq!(Enum::V2 as u8, 1);
    assert_eq!(Enum::V3 as u8, 5);
}

Current output

-
compiles without any warnings or errors

Desired output

No response

Rationale and extra context

Hello,
According to https://doc.rust-lang.org/reference/items/enumerations.html#casting as-casting enum variants to their discriminants should be possible if they do not have explicit discriminants, or where only unit variants are explicit (there's also case for unit-only enums but it's irrelevant to the case).

Despite all that I can write code as seen in the example that clearly brakes only unit variants are explicit rule.
Moreover, it compiles without any warnings or errors and produces counter-intuitive results in the runtime, e.g.:

thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `208`,
 right: `0`', prog.rs:9:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

I've confirmed (using recommended pointer casting method) that created enum variant holds the correct value i.e. Enum::V1(0).discriminant() is indeed 0.

If I correctly understand the intentions of the authors, the expressions like Enum::V1 as u8 should not be allowed at this time.

As for the future, I think that it would be nice if there was a way to fetch the discriminant value of the variant without creating it in the first place, i.e. discussed constructions would be allowed and returned discriminant of that variant.

Few related discussions I've came about while researching this:
#60553 (comment)
#88621

Other cases

No response

Anything else?

No response

@nalepamarcin nalepamarcin added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 20, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 20, 2023
@clubby789
Copy link
Contributor

clubby789 commented Jul 21, 2023

Enum::V1 here is actually a function pointer, something like fn V1(val: u8) { Enum::V1(val) }. You're taking the address of the function and casting it to u8, i.e. truncating it to the least significant byte. This is more noticeable from the disassembly:

playground::main:
	sub	rsp, 56
	lea	rax, [rip + playground::Enum::V1] // load address of function
	mov	byte ptr [rsp + 7], al                 // take lowest byte

Enum::V1(0) as u8 (constructing then casting the enum) is indeed forbidden.

@workingjubilee
Copy link
Member

I thought we added a lint against that...

@clubby789
Copy link
Contributor

It's a Clippy one I believe

 --> src/main.rs:9:16
  |
9 |     assert_eq!(Enum::V1 as u8, 0);
  |                ^^^^^^^^^^^^^^ help: try: `Enum::V1 as usize`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast_with_truncation
  = note: `#[warn(clippy::fn_to_numeric_cast_with_truncation)]` on by default

@nalepamarcin
Copy link
Author

You're taking the address of the function and casting it to u8

Oh, thanks for explaining that. It makes more sense now :)

TBH, I'm not sure how to proceed with that being the case. Taking a function pointer seems like a legitimate use case and it just happens to be very similar to getting discriminant.

@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants