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

Matching strings in beta and nightly #35044

Closed
hank-der-hafenarbeiter opened this issue Jul 26, 2016 · 6 comments
Closed

Matching strings in beta and nightly #35044

hank-der-hafenarbeiter opened this issue Jul 26, 2016 · 6 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

@hank-der-hafenarbeiter
Copy link
Contributor

hank-der-hafenarbeiter commented Jul 26, 2016

when matching &str in nightly or beta the following code always returns the first match case:

#[derive(Debug, PartialEq)]
enum Reg {
    EAX,
    EBX,
    ECX,
    EDX,
    ESP,
    EBP,
    ISP,
}

fn string_to_reg(_s:&str) -> Reg {
    match _s.as_ref() {
        "EAX" => Reg::EAX,
        "EBX" => Reg::EBX,
        "ECX" => Reg::ECX,
        "EDX" => Reg::EDX,
        "EBP" => Reg::EBP,
        "ESP" => Reg::ESP,
        "ISP" => Reg::ISP,
        &_    => panic!("bla bla bla"),  //removing this "&" fixes it
    }
}

fn main() {
    let vec = vec!["EAX", "EBX", "ECX", "EDX", "ESP", "EBP", "ISP"];
    let mut iter = vec.iter();
    println!("{:?}", string_to_reg(""));
    println!("{:?}", string_to_reg(iter.next().unwrap()));
    println!("{:?}", string_to_reg(iter.next().unwrap()));
    println!("{:?}", string_to_reg(iter.next().unwrap()));
    println!("{:?}", string_to_reg(iter.next().unwrap()));
    println!("{:?}",  string_to_reg(iter.next().unwrap()));     
    println!("{:?}", string_to_reg(iter.next().unwrap()));
    println!("{:?}",  string_to_reg(iter.next().unwrap()));
}

Expected behaviour is returning the fitting case. This can be fixed by removing the "&" in front of the last case.

Meta

tested on my machine with: rustc 1.11.0-nightly (bb4a79b 2016-06-15)
tested on here with beta and nightly showing the bug and stable giving a runtime error.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 26, 2016

The MIR looks fine, and it works fine with -Zorbit enabled

@arielb1 arielb1 added I-wrong 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 Jul 27, 2016
@arielb1
Copy link
Contributor

arielb1 commented Jul 27, 2016

I think I broke old trans _match with my slice-patterns PR (#32202).

@DemiMarie
Copy link
Contributor

Can we enforce MIR trans whenever slice patterns are in use?

@nikomatsakis nikomatsakis added the P-high High priority label Aug 4, 2016
@nikomatsakis
Copy link
Contributor

Discussed in @rust-lang/compiler meeting -- @arielb1 is going to try and revert PR #32202 on the beta branch only to fix this or else find a fix. We don't have to do anything on nightly because it's correct in MIR -- though it means that old trans is kind of broken.

Also, @arielb1 says it's all @eddyb's fault. ;)

@nikomatsakis
Copy link
Contributor

Update from @rust-lang/compiler meeting. @eddyb took a stab at fixing it but did not succeed. The problem is that, in the old code, literals (like "foo") and &_ are considered incompatible, and hence the match code just kind of does the wrong thing.

@eddyb points out that, on stable, this code does not currently compile. So to fix this for beta we could add some custom check that denies using & pattern to match an &str type. Or just using the old check-match.

arielb1 pushed a commit to arielb1/rust that referenced this issue Aug 14, 2016
Before PR rust-lang#32202, check_match tended to report bogus errors or ICE
when encountering a pattern that split a literal, e.g.

```Rust
match foo {
    "bar" => {},
    &_ => {}
}
```

That PR fixed these issues, but trans::_match generates bad code
when it encounters these matches. MIR trans has that fixed, but
it is waiting for 6 weeks in beta. Report an error when encountering
these instead.

Fixes issue rust-lang#35044.
@nikomatsakis
Copy link
Contributor

This is actually fixed on beta.

@arielb1 arielb1 closed this as completed Aug 18, 2016
pmatos pushed a commit to LinkiTools/rust that referenced this issue Sep 27, 2016
Before PR rust-lang#32202, check_match tended to report bogus errors or ICE
when encountering a pattern that split a literal, e.g.

```Rust
match foo {
    "bar" => {},
    &_ => {}
}
```

That PR fixed these issues, but trans::_match generates bad code
when it encounters these matches. MIR trans has that fixed, but
it is waiting for 6 weeks in beta. Report an error when encountering
these instead.

Fixes issue rust-lang#35044.
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

5 participants