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

Unreachable expression in void-1.0.2, Rust 1.16 #38975

Closed
brson opened this issue Jan 10, 2017 · 9 comments
Closed

Unreachable expression in void-1.0.2, Rust 1.16 #38975

brson opened this issue Jan 10, 2017 · 9 comments
Labels
A-type-system Area: Type system P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@brson
Copy link
Contributor

brson commented Jan 10, 2017

https://github.com/reem/rust-void.git a6e061227f47ba8798b7e828ed0ac4e25382eb15

Not on stable/beta.

128 brian@ip-10-145-43-250:~⟫ rustc +nightly -Vv
rustc 1.16.0-nightly (47c8d9fdc 2017-01-08)
binary: rustc
commit-hash: 47c8d9fdcf2e6502cf4ca7d7f059fdc1a2810afa
commit-date: 2017-01-08
host: x86_64-unknown-linux-gnu
release: 1.16.0-nightly
LLVM version: 3.9
brian@ip-10-145-43-250:~/dev/rust-void⟫ cargo +nightly test
   Compiling void v1.0.2 (file:///mnt2/dev/rust-void)
warning: unreachable pattern, #[warn(unreachable_patterns)] on by default
  --> src/lib.rs:98:13
   |
98 |             Err(e) => unreachable(e)
   |             ^^^^^^

warning: unreachable pattern, #[warn(unreachable_patterns)] on by default
   --> src/lib.rs:116:13
    |
116 |             Ok(v) => unreachable(v),
    |             ^^^^^

error: unreachable pattern
  --> src/lib.rs:98:13
   |
98 |             Err(e) => unreachable(e)
   |             ^^^^^^
   |
note: lint level defined here
  --> src/lib.rs:1:24
   |
1  | #![cfg_attr(test, deny(warnings))]
   |                        ^^^^^^^^

error: unreachable pattern
   --> src/lib.rs:116:13
    |
116 |             Ok(v) => unreachable(v),
    |             ^^^^^

error: aborting due to 2 previous errors

Build failed, waiting for other jobs to finish...
error: Could not compile `void`.

To learn more, run the command again with --verbose.

The code here is interesting. I don't quite see what the compiler does.

cc @reem

@brson brson added A-type-system Area: Type system 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. labels Jan 10, 2017
@durka
Copy link
Contributor

durka commented Jan 11, 2017

I think this is the same as #38977?

@arielb1
Copy link
Contributor

arielb1 commented Jan 11, 2017

That's intentionally caused by #38069 + #[deny(unreachable_patterns)]. Can be fixed by removing the arm or allowing the lint.

@brson
Copy link
Contributor Author

brson commented Jan 11, 2017

Woah!

So I can now write:

    #[inline]
    fn void_unwrap(self) -> T {
        match self {
            Ok(val) => val,
        }
    }

That is, the match is not exhaustive over its variants! That seems very very surprising to me. It is tying this reachability analysis to the semantics of match in a new way that I would not expect.

This type of match is an error on stable today.

@brson
Copy link
Contributor Author

brson commented Jan 11, 2017

I guess, in some sense the new semantics only accept more code, so it's backwards compatible. As long as the lint is off you can write it with the empty pattern...

@nikomatsakis
Copy link
Contributor

@brson yes, that was the idea. And yes, the example is surprising at first, but makes sense when you consider that a Void value can never exist in practice (or else you've done something wrong)

@durka
Copy link
Contributor

durka commented Jan 11, 2017 via email

@brson
Copy link
Contributor Author

brson commented Jan 12, 2017

Expected behavior. Please add this as a test case.

@brson brson added E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. P-low Low priority and removed regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. labels Jan 12, 2017
@brson
Copy link
Contributor Author

brson commented Jan 12, 2017

@eddyb says we don't need this test.

@brson brson closed this as completed Jan 12, 2017
@canndrew
Copy link
Contributor

@brson

That seems very very surprising to me. It is tying this reachability analysis to the semantics of match in a new way that I would not expect.

It's consistent with how all other enums are handled:

enum ThreeVariants {
    V0,
    V1,
    V2,
}

enum TwoVariants {
    V0,
    V1,
}

enum OneVariant {
    V0,
}

enum ZeroVariants {
}


fn unwrap_three(x: Result<u32, ThreeVariants>) -> u32 {
    match x {
        Ok(y)   => y,
        Err(ThreeVariants::V0) => 0,
        Err(ThreeVariants::V1) => 1,
        Err(ThreeVariants::V2) => 2,
    }
}

fn unwrap_two(x: Result<u32, TwoVariants>) -> u32 {
    match x {
        Ok(y)   => y,
        Err(TwoVariants::V0) => 0,
        Err(TwoVariants::V1) => 1,
    }
}

fn unwrap_one(x: Result<u32, OneVariant>) -> u32 {
    match x {
        Ok(y)   => y,
        Err(OneVariant::V0) => 0,
    }
}

fn unwrap_zero(x: Result<u32, ZeroVariants>) -> u32 {
    match x {
        Ok(y)   => y,
    }
}

In other words, the fact that you can nest patterns and expand an n-variant enum in-place implies that you can elide empty enums like this (assuming we're being consistent).

So you can still write this:

fn unwrap_zero(x: Result<u32, ZeroVariants>) -> u32 {
    match x {
        Ok(y) => y,
        Err(z) => match z {},
    }
}

But if you instead write it by expanding Err(z) into the constituent variants for z you get

fn unwrap_zero(x: Result<u32, ZeroVariants>) -> u32 {
    match x {
        Ok(y) => y,
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system P-low Low priority 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