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

Possible codegen regression when matching against nested enums #104519

Closed
anp opened this issue Nov 17, 2022 · 4 comments · Fixed by #104535
Closed

Possible codegen regression when matching against nested enums #104519

anp opened this issue Nov 17, 2022 · 4 comments · Fixed by #104535
Assignees
Labels
C-bug Category: This is a bug. P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@anp
Copy link
Member

anp commented Nov 17, 2022

Code

I tried this code (reduced from a much larger test):

fn main() {
    let output = OpenResult::Ok(());
    match output {
        OpenResult::Ok(()) => (),
        _ => unreachable!("this won't be reached"),
    }
    match output {
        OpenResult::Ok(()) => (),
        _ => unreachable!("somehow matching twice is the trick"),
    }
}

enum OpenResult {
    Ok(()),

    // we don't produce either of these variants but both are necessary to trigger the issue
    #[allow(dead_code)]
    Err(()),
    #[allow(dead_code)]
    TransportErr(TransportErr),
}

#[allow(dead_code)] // don't need to produce this to repro
#[repr(i32)]
enum TransportErr {
    UnknownMethod = -2,
}

(playground)

I expected to see this happen: runs with 0 exit code and no output

Instead, this happened:

thread 'main' panicked at 'internal error: entered unreachable code: somehow matching twice is the trick', src/main.rs:9:14

Version it worked on

This code works on stable and beta, and as recently as a3c0a02.

Version with regression

This reproduces with the 2022-11-16 nightly on the playground. cargo-bisect-rustc suggests this was introduced by #102872.

@rustbot modify labels: +regression-from-stable-to-nightly -regression-untriaged

@anp anp added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Nov 17, 2022
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed regression-untriaged Untriaged performance or correctness regression. labels Nov 17, 2022
@Noratrieb
Copy link
Member

That looks like a real miscompilation :(
cc @mikebenfield

@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Nov 17, 2022
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 17, 2022
@mikebenfield
Copy link
Contributor

I've found the problem; it is indeed an issue with my code in codegen_get_discr. I'll have a PR with a fix some time today.

@apiraino
Copy link
Contributor

Since there is some work in progress to fix this:

@rustbot assign @mikebenfield

bors added a commit to rust-lang-ci/rust that referenced this issue Dec 4, 2022
rustc_codegen_ssa: Fix for codegen_get_discr

When doing the optimized implementation of getting the discriminant, the arithmetic needs to be done in the tag type so wrapping behavior works correctly.

Fixes rust-lang#104519
@bors bors closed this as completed in 31c0645 Dec 4, 2022
Aaron1011 pushed a commit to Aaron1011/rust that referenced this issue Jan 6, 2023
rustc_codegen_ssa: Fix for codegen_get_discr

When doing the optimized implementation of getting the discriminant, the arithmetic needs to be done in the tag type so wrapping behavior works correctly.

Fixes rust-lang#104519
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants