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

Nightly regression in Structual Eq check for match #117626

Closed
CeleritasCelery opened this issue Nov 6, 2023 · 3 comments · Fixed by #117636
Closed

Nightly regression in Structual Eq check for match #117626

CeleritasCelery opened this issue Nov 6, 2023 · 3 comments · Fixed by #117636
Labels
C-bug Category: This is a bug. P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@CeleritasCelery
Copy link
Contributor

Code

I tried this code:

#[derive(PartialEq)]
pub(crate) struct NonMatchable ();

impl Eq for NonMatchable {}

#[derive(PartialEq, Eq)]
pub(crate) enum Enum {
    A(NonMatchable),
    B(*const u8),
}

impl Enum {
    const CONST: Enum = Enum::B(std::ptr::null());
}

fn main() {
    let _ = match Enum::CONST {
        Enum::CONST => 0,
        _ => 1,
    };
}

The above code compiles fine on beta and stable, but fails on the latest nightly. It used to pass on nightly too (last I tested it). The errors is this:

error: to use a constant of type `NonMatchable` in a pattern, `NonMatchable` must be annotated with `#[derive(PartialEq, Eq)]`
  --> src/main.rs:18:9
   |
18 |         Enum::CONST => 0,
   |         ^^^^^^^^^^^
   |
   = note: the traits must be derived, manual `impl`s are not sufficient
   = note: see https://doc.rust-lang.org/stable/std/marker/trait.StructuralEq.html for details

The error is claiming that we are using NonMatchable in the pattern when we are not. Interestingly if we change the value of Enum::B to be something other then a pointer, this passes (though that should not make a difference). Also if we don't use a constant but instead match on std::ptr::null() directly, it passes as well. We only see this issue in the nightly compiler when we both using a constant and the variant in the constant is a pointer. Unless I am misunderstanding something here, this should not be an error.

Version it worked on

It most recently worked on:

binary: rustc
commit-hash: 489647f984b2b3a5ee6b2a0d46a527c8d926ceae
commit-date: 2023-10-21
host: aarch64-apple-darwin
release: 1.74.0-beta.4
LLVM version: 17.0.3

Version with regression

rustc --version --verbose:

binary: rustc
commit-hash: a2f5f9691b6ce64c1703feaf9363710dfd7a56cf
commit-date: 2023-11-02
host: aarch64-apple-darwin
release: 1.75.0-nightly
LLVM version: 17.0.4
@CeleritasCelery CeleritasCelery added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Nov 6, 2023
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Nov 6, 2023
@bvanjoi
Copy link
Contributor

bvanjoi commented Nov 6, 2023

I believe there has been a regression as per the pull request #116522, and I plan to investigate it further at a later time.

@apiraino
Copy link
Contributor

apiraino commented Nov 6, 2023

bisection seems to confirm PR #116522 thanks @bvanjoi for looking into that!

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high -needs-triage

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Nov 6, 2023
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 6, 2023
@lqd
Copy link
Member

lqd commented Nov 6, 2023

That does indeed bisect to #116522

@apiraino apiraino removed the regression-untriaged Untriaged performance or correctness regression. label Dec 28, 2023
bvanjoi added a commit to bvanjoi/rust that referenced this issue Dec 28, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 4, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 4, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#117636 (add test for rust-lang#117626)
 - rust-lang#118704 (Promote `riscv32{im|imafc}` targets to tier 2)
 - rust-lang#119184 (Switch from using `//~ERROR` annotations with `--error-format` to `error-pattern`)
 - rust-lang#119325 (custom mir: make it clear what the return block is)
 - rust-lang#119391 (Use Result::flatten in catch_with_exit_code)
 - rust-lang#119431 (Support reg_addr register class in s390x inline assembly)
 - rust-lang#119475 (Remove libtest's dylib)
 - rust-lang#119532 (Make offset_of field parsing use metavariable which handles any spacing)
 - rust-lang#119553 (stop feed vis when cant access for trait item)
 - rust-lang#119556 (Reland optimized-compiler-builtins config)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 4, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#117636 (add test for rust-lang#117626)
 - rust-lang#118704 (Promote `riscv32{im|imafc}` targets to tier 2)
 - rust-lang#119184 (Switch from using `//~ERROR` annotations with `--error-format` to `error-pattern`)
 - rust-lang#119325 (custom mir: make it clear what the return block is)
 - rust-lang#119391 (Use Result::flatten in catch_with_exit_code)
 - rust-lang#119431 (Support reg_addr register class in s390x inline assembly)
 - rust-lang#119475 (Remove libtest's dylib)
 - rust-lang#119532 (Make offset_of field parsing use metavariable which handles any spacing)
 - rust-lang#119553 (stop feed vis when cant access for trait item)
 - rust-lang#119574 (Miri subtree update)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors closed this as completed in 5c47d77 Jan 4, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 4, 2024
Rollup merge of rust-lang#117636 - bvanjoi:fix-117626, r=TaKO8Ki

add test for rust-lang#117626

Close rust-lang#117626
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 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