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

Regression in match patterns in beta #57894

Closed
Razican opened this issue Jan 25, 2019 · 4 comments
Closed

Regression in match patterns in beta #57894

Razican opened this issue Jan 25, 2019 · 4 comments
Assignees
Labels
P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Razican
Copy link
Contributor

Razican commented Jan 25, 2019

Hello, the pattern matching in match arms seems to have different behavior in stable than in beta/nightly.

I tried this code:

use std::i8;

fn main() {
    let a = Some(-50_i8);
    match a {
        Some(c @ i8::MIN...-101) | Some(c @ 101...i8::MAX) => println!("out of bounds, {}", c),
        _ => println!("OK"),
    }
}

I expected to see this happen: In all chains, it should print "OK".

Instead, this happened: It prints "OK" in stable, but it prints "out of bounds, -50" in beta/nightly.

Example in the playground

I understand that this code is not very idiomatic, but still, I think that the behavior should be consistent between Rust versions.

@sfackler sfackler added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Jan 25, 2019
@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jan 25, 2019
@Centril
Copy link
Contributor

Centril commented Jan 25, 2019

Reduced:

fn main() {
    assert_eq!(false, match -50_i8 { -128i8...-101i8 => true, _ => false, });
}

// also with if-let:
fn main() {
    assert_eq!(false, if let -128i8...-101i8 = -50_i8 { true } else { false });
}

cc @varkor

@Centril Centril added P-high High priority and removed T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jan 25, 2019
@pietroalbini pietroalbini added I-nominated and removed P-high High priority labels Jan 26, 2019
@jonas-schievink
Copy link
Contributor

jonas-schievink commented Jan 28, 2019

Introduced between nightly-2018-12-14 and nightly-2018-12-15, resp.

rustc 1.32.0-nightly (f4a421ee3 2018-12-13)

rustc 1.33.0-nightly (96d1334e5 2018-12-14)

Commits between those nightlies - indeed looks like #56695 is suspicious

@varkor
Copy link
Member

varkor commented Jan 28, 2019

#56695 does look like the culprit. I'll try to fix this tomorrow. Thanks @jonas-schievink!

@pnkfelix
Copy link
Member

pnkfelix commented Jan 31, 2019

triage: P-high. Assigned to @varkor

@pnkfelix pnkfelix added P-high High priority and removed I-nominated labels Jan 31, 2019
bors added a commit that referenced this issue Feb 1, 2019
…oli-obk

Fix bug in integer range matching

Fixes #57894.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. 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

7 participants