-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
check_match: don't treat privately uninhabited types as uninhabited #39980
Conversation
0de44df
to
a84eb95
Compare
|
||
if missing_ctors.is_empty() { | ||
// | ||
// However, if our scrutinee is *privately* an empty enum, we |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@@ -754,7 +774,19 @@ fn constructor_sub_pattern_tys<'a, 'tcx: 'a>(cx: &MatchCheckCtxt<'a, 'tcx>, | |||
ty::TyRef(_, ref ty_and_mut) => vec![ty_and_mut.ty], | |||
ty::TyAdt(adt, substs) => { | |||
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do enum fields not have appropriate visibility settings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They don't.
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) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this what we agreed on at the sprint.
(The code seems good, in any case.) |
In beta. |
This is a [breaking-change] from 1.15, because this used to compile: ```Rust enum Void {} fn foo(x: &Void) { match x {} } ```
@bors r+ |
📌 Commit 87e544b has been approved by |
…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.
…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
[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.
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
Fixes #38972, which is a regression in 1.16 from @canndrew's patchset.
r? @nikomatsakis
beta-nominating because regression.