check_match: don't treat privately uninhabited types as uninhabited#39980

arielb1 · 2017-02-20T17:23:16Z

Fixes #38972, which is a regression in 1.16 from @canndrew's patchset.

beta-nominating because regression.

nikomatsakis · 2017-02-21T19:30:28Z

src/librustc_const_eval/_match.rs

-
-        if missing_ctors.is_empty() {
+        //
+        // However, if our scrutinee is *privately* an empty enum, we


Might be worth defining what 'privately empty' means -- I presume it means that the uninhabited values are not visible from this module? I feel like there was logic to this effect before.

nikomatsakis · 2017-02-21T19:32:15Z

src/librustc_const_eval/_match.rs

            adt.variants[ctor.variant_index_for_adt(adt)].fields.iter().map(|field| {
-                field.ty(cx.tcx, substs)
+                let is_visible = adt.is_enum()
+                    || field.vis.is_accessible_from(cx.module, cx.tcx);


do enum fields not have appropriate visibility settings?

They don't.

nikomatsakis · 2017-02-21T19:33:39Z

src/librustc_const_eval/check_match.rs

+            let pat_ty = self.tables.node_id_to_type(scrut.id);
+            let module = self.tcx.hir.local_def_id(self.tcx.hir.get_module_parent(scrut.id));
+            if inlined_arms.is_empty() {
+                if !pat_ty.is_uninhabited_from(module, self.tcx) {


I think eventually this wants to be a stricter definition of uninhabited (i.e., one that says "you could only write dead-code arms"), but I guess that can come later. Probably so long as never-type feature flag is not in use this one will match what we want.

Isn't this what we agreed on at the sprint.

nikomatsakis · 2017-02-21T19:34:56Z

@arielb1 how far have the inhabitedness changes from @canndrew gone in the release pipeline? are they still in beta only, or have they made it to release?

If the latter, do we have to worry about breaking things with this PR? I am imagining code where we accepted matches with fewer arms.

nikomatsakis · 2017-02-21T19:35:20Z

(The code seems good, in any case.)

arielb1 · 2017-02-23T11:05:47Z

In beta.

This is a [breaking-change] from 1.15, because this used to compile: ```Rust enum Void {} fn foo(x: &Void) { match x {} } ```

nikomatsakis · 2017-02-24T01:34:47Z

@bors r+

bors · 2017-02-24T01:34:48Z

📌 Commit 87e544b has been approved by nikomatsakis

@canndrew

…komatsakis check_match: don't treat privately uninhabited types as uninhabited Fixes rust-lang#38972, which is a regression in 1.16 from @canndrew's patchset. r? @nikomatsakis beta-nominating because regression.

@canndrew

…komatsakis check_match: don't treat privately uninhabited types as uninhabited Fixes rust-lang#38972, which is a regression in 1.16 from @canndrew's patchset. r? @nikomatsakis beta-nominating because regression.

Rollup of 28 pull requests - Successful merges: #39859, #39864, #39888, #39903, #39905, #39914, #39945, #39950, #39953, #39961, #39980, #39988, #39993, #39995, #40019, #40020, #40022, #40024, #40025, #40026, #40027, #40031, #40035, #40037, #40038, #40064, #40069, #40086 - Failed merges: #39927, #40008, #40047

@nikomatsakis

[beta] next - #39913 - #39730 - #39674 - #39602 - #39586 - #39471 - #39980 - #40020 - #40135 @nikomatsakis [this commit](3787d33) did not pick cleanly. You might peek at it. I took the liberty of accepting all the nominations myself, but the [packed struct alignment](#39586) PR is quite large. It did pick fine though and there's a comment there suggesting it works on beta cc @rust-lang/compiler. cc @alexcrichton

the match-checking code used to use TyErr for signaling "unknown, inhabited" types for a long time. It had been switched to using the exact type in rust-lang#38069, to handle uninhabited types. However, in rust-lang#39980, we discovered that we still needed the "unknown inhabited" logic, but I used `()` instead of `TyErr` to handle that. Revert to using `TyErr` to fix that problem.

@nikomatsakis

check_match: fix handling of privately uninhabited types the match-checking code used to use TyErr for signaling "unknown, inhabited" types for a long time. It had been switched to using the exact type in #38069, to handle uninhabited types. However, in #39980, we discovered that we still needed the "unknown inhabited" logic, but I used `()` instead of `TyErr` to handle that. Revert to using `TyErr` to fix that problem. Fixes #46964. r? @nikomatsakis

arielb1 added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Feb 20, 2017

rust-highfive assigned nikomatsakis Feb 20, 2017

check_match: don't treat privately uninhabited types as uninhabited

a84eb95

Fixes rust-lang#38972.

arielb1 force-pushed the privately-uninhabited branch from 0de44df to a84eb95 Compare February 20, 2017 18:36

nikomatsakis reviewed Feb 21, 2017

View reviewed changes

use a more conservative inhabitableness rule

87e544b

This is a [breaking-change] from 1.15, because this used to compile: ```Rust enum Void {} fn foo(x: &Void) { match x {} } ```

nikomatsakis added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Feb 24, 2017

eddyb mentioned this pull request Feb 25, 2017

Rollup of 29 pull requests #40089

Closed

eddyb mentioned this pull request Feb 25, 2017

Rollup of 28 pull requests #40091

Merged

bors merged commit 87e544b into rust-lang:master Feb 25, 2017

brson mentioned this pull request Feb 25, 2017

[beta] next #40046

Merged

alexcrichton removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Feb 25, 2017

arielb1 mentioned this pull request Dec 25, 2017

check_match: fix handling of privately uninhabited types #47001

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

check_match: don't treat privately uninhabited types as uninhabited#39980