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

Tracking issue for RFC 1872: `exhaustive_patterns` feature #51085

Open
kennytm opened this Issue May 26, 2018 · 3 comments

Comments

Projects
None yet
4 participants
@kennytm
Member

kennytm commented May 26, 2018

This tracks the exhaustive_patterns feature which allows uninhabited variant to be omitted
(bug report: #12609; relevant RFC: rust-lang/rfcs#1872).

fn safe_unwrap<T>(x: Result<T, !>) -> T {
    match x {
        Ok(y) => y,
    }
}
  • Implementation (separated out from never_type in #47630)
  • Stabilization

bors added a commit that referenced this issue May 26, 2018

Auto merge of #51090 - kennytm:tidy-check-missing-tracking-issue, r=a…
…lexcrichton

Ensure every unstable language feature has a tracking issue.

Filled in the missing numbers:

* `abi_ptx` → #38788
* `generators` → #43122
* `global_allocator` → #27389

Reused existing tracking issues because they were decomposed from a larger feature

* `*_target_feature` → #44839 (reusing the old `target_feature` number)
* `proc_macros_*` → #38356 (reusing the to-be-stabilized `proc_macros` number)

Filed new issues

* `exhaustive_patterns` → #51085
* `pattern_parentheses` → #51087
* `wasm_custom_section` and `wasm_import_module` → #51088

bors added a commit that referenced this issue May 27, 2018

Auto merge of #51090 - kennytm:tidy-check-missing-tracking-issue, r=a…
…lexcrichton

Ensure every unstable language feature has a tracking issue.

Filled in the missing numbers:

* `abi_ptx` → #38788
* `generators` → #43122
* `global_allocator` → #27389

Reused existing tracking issues because they were decomposed from a larger feature

* `*_target_feature` → #44839 (reusing the old `target_feature` number)
* `proc_macros_*` → #38356 (reusing the to-be-stabilized `proc_macros` number)

Filed new issues

* `exhaustive_patterns` → #51085
* `pattern_parentheses` → #51087
* `wasm_custom_section` and `wasm_import_module` → #51088
@varkor

This comment has been minimized.

Member

varkor commented Oct 16, 2018

The lack of this feature has unintuitive consequences (e.g. #55123). I haven't seen any problems arising from it either.

cc @rust-lang/lang: could this be put forward for stabilisation?

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Oct 16, 2018

@varkor I am mildly reluctant until we have agreed upon the story around "never patterns" -- but I suppose given a sufficiently conservative definition of "uninhabited pattern" I'd be all right with it. Do you think you could write-up a "stabilization report" that includes examples of the tests we have and also documents the limits of what sorts of things would be considered uninhabited in stable code?

(That said, I think "never patterns" gives us a path forward that addresses my concerns around aggressive uninhabited definitions, by just turning the "ambiguous" cases into lints in the presence of unsafe code. I'd be happier though if we had an RFC on the topic.)

@nikic

This comment has been minimized.

Contributor

nikic commented Oct 26, 2018

The exhaustive_patterns feature currently seems to have a pretty big performance issue. Having perf top running while compiling librustc, I noticed that it spends an unreasonable amount of time inside ty::inhabitedness. Disabling exhaustive_patterns (it's not actually used in librustc/librustc_mir) shaved off ~30s (of 12min total) from stage1 compiler artifact compile time (in a totally unscientific benchmark, so take with a grain of salt).

I would expect that this issue will resolve itself if there is a consensus to go with references never being uninhabited (ref #54125 (comment)), as the reference handling is what I believe makes this expensive (not looking through references both reduces the amount of types we need to look at, and allows us to drop the expensive hashmap and nested hashsets used to avoid unbounded recursion). If the consensus goes in the other direction, then this code should probably be optimized a bit prior to stabilization of the feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment