Skip to content

Commit

Permalink
Auto merge of #119218 - Nadrieril:nested-opaque-reveal, r=compiler-er…
Browse files Browse the repository at this point in the history
…rors

Exhaustiveness: Reveal empty opaques in depth

Follow-up to #116821. As noted [there](#116821 (comment)), the current implementation doesn't detect emptiness of opaques when the opaque is nested inside a type. This doesn't matter for stable behavior (which ignores nested empty types anyway) but does matter for the [`exhaustive_patterns`](https://github.com/rust-lang/rust/issues/51085)/[`min_exhaustive_patterns`](https://github.com/rust-lang/rust/pull/118803) features.

This PR fixes this behavior by adding `InhabitedPredicate::apply_reveal_opaque` that considers opaque types when determining inhabitedness.

r? `@compiler-errors`
  • Loading branch information
bors committed Dec 23, 2023
2 parents 2d7be73 + 34307ab commit cdd6374
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 36 deletions.
75 changes: 59 additions & 16 deletions compiler/rustc_middle/src/ty/inhabitedness/inhabited_predicate.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use smallvec::SmallVec;

use crate::ty::context::TyCtxt;
use crate::ty::{self, DefId, ParamEnv, Ty};
use crate::ty::{self, DefId, OpaqueTypeKey, ParamEnv, Ty};

/// Represents whether some type is inhabited in a given context.
/// Examples of uninhabited types are `!`, `enum Void {}`, or a struct
Expand All @@ -23,42 +23,62 @@ pub enum InhabitedPredicate<'tcx> {
/// Inhabited if some generic type is inhabited.
/// These are replaced by calling [`Self::instantiate`].
GenericType(Ty<'tcx>),
/// Inhabited if either we don't know the hidden type or we know it and it is inhabited.
OpaqueType(OpaqueTypeKey<'tcx>),
/// A AND B
And(&'tcx [InhabitedPredicate<'tcx>; 2]),
/// A OR B
Or(&'tcx [InhabitedPredicate<'tcx>; 2]),
}

impl<'tcx> InhabitedPredicate<'tcx> {
/// Returns true if the corresponding type is inhabited in the given
/// `ParamEnv` and module
/// Returns true if the corresponding type is inhabited in the given `ParamEnv` and module.
pub fn apply(self, tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, module_def_id: DefId) -> bool {
let Ok(result) = self.apply_inner::<!>(tcx, param_env, &mut Default::default(), &|id| {
Ok(tcx.is_descendant_of(module_def_id, id))
});
self.apply_revealing_opaque(tcx, param_env, module_def_id, &|_| None)
}

/// Returns true if the corresponding type is inhabited in the given `ParamEnv` and module,
/// revealing opaques when possible.
pub fn apply_revealing_opaque(
self,
tcx: TyCtxt<'tcx>,
param_env: ParamEnv<'tcx>,
module_def_id: DefId,
reveal_opaque: &impl Fn(OpaqueTypeKey<'tcx>) -> Option<Ty<'tcx>>,
) -> bool {
let Ok(result) = self.apply_inner::<!>(
tcx,
param_env,
&mut Default::default(),
&|id| Ok(tcx.is_descendant_of(module_def_id, id)),
reveal_opaque,
);
result
}

/// Same as `apply`, but returns `None` if self contains a module predicate
pub fn apply_any_module(self, tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>) -> Option<bool> {
self.apply_inner(tcx, param_env, &mut Default::default(), &|_| Err(())).ok()
self.apply_inner(tcx, param_env, &mut Default::default(), &|_| Err(()), &|_| None).ok()
}

/// Same as `apply`, but `NotInModule(_)` predicates yield `false`. That is,
/// privately uninhabited types are considered always uninhabited.
pub fn apply_ignore_module(self, tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>) -> bool {
let Ok(result) =
self.apply_inner::<!>(tcx, param_env, &mut Default::default(), &|_| Ok(true));
self.apply_inner::<!>(tcx, param_env, &mut Default::default(), &|_| Ok(true), &|_| {
None
});
result
}

#[instrument(level = "debug", skip(tcx, param_env, in_module), ret)]
#[instrument(level = "debug", skip(tcx, param_env, in_module, reveal_opaque), ret)]
fn apply_inner<E: std::fmt::Debug>(
self,
tcx: TyCtxt<'tcx>,
param_env: ParamEnv<'tcx>,
eval_stack: &mut SmallVec<[Ty<'tcx>; 1]>, // for cycle detection
in_module: &impl Fn(DefId) -> Result<bool, E>,
reveal_opaque: &impl Fn(OpaqueTypeKey<'tcx>) -> Option<Ty<'tcx>>,
) -> Result<bool, E> {
match self {
Self::False => Ok(false),
Expand All @@ -84,18 +104,41 @@ impl<'tcx> InhabitedPredicate<'tcx> {
return Ok(true); // Recover; this will error later.
}
eval_stack.push(t);
let ret = pred.apply_inner(tcx, param_env, eval_stack, in_module);
let ret =
pred.apply_inner(tcx, param_env, eval_stack, in_module, reveal_opaque);
eval_stack.pop();
ret
}
}
}
Self::And([a, b]) => {
try_and(a, b, |x| x.apply_inner(tcx, param_env, eval_stack, in_module))
}
Self::Or([a, b]) => {
try_or(a, b, |x| x.apply_inner(tcx, param_env, eval_stack, in_module))
}
Self::OpaqueType(key) => match reveal_opaque(key) {
// Unknown opaque is assumed inhabited.
None => Ok(true),
// Known opaque type is inspected recursively.
Some(t) => {
// A cyclic opaque type can happen in corner cases that would only error later.
// See e.g. `tests/ui/type-alias-impl-trait/recursive-tait-conflicting-defn.rs`.
if eval_stack.contains(&t) {
return Ok(true); // Recover; this will error later.
}
eval_stack.push(t);
let ret = t.inhabited_predicate(tcx).apply_inner(
tcx,
param_env,
eval_stack,
in_module,
reveal_opaque,
);
eval_stack.pop();
ret
}
},
Self::And([a, b]) => try_and(a, b, |x| {
x.apply_inner(tcx, param_env, eval_stack, in_module, reveal_opaque)
}),
Self::Or([a, b]) => try_or(a, b, |x| {
x.apply_inner(tcx, param_env, eval_stack, in_module, reveal_opaque)
}),
}
}

Expand Down
16 changes: 14 additions & 2 deletions compiler/rustc_middle/src/ty/inhabitedness/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@

use crate::query::Providers;
use crate::ty::context::TyCtxt;
use crate::ty::{self, DefId, Ty, VariantDef, Visibility};
use crate::ty::{self, DefId, Ty, TypeVisitableExt, VariantDef, Visibility};

use rustc_type_ir::TyKind::*;

Expand Down Expand Up @@ -105,6 +105,7 @@ impl<'tcx> VariantDef {
impl<'tcx> Ty<'tcx> {
#[instrument(level = "debug", skip(tcx), ret)]
pub fn inhabited_predicate(self, tcx: TyCtxt<'tcx>) -> InhabitedPredicate<'tcx> {
debug_assert!(!self.has_infer());
match self.kind() {
// For now, unions are always considered inhabited
Adt(adt, _) if adt.is_union() => InhabitedPredicate::True,
Expand All @@ -113,7 +114,18 @@ impl<'tcx> Ty<'tcx> {
InhabitedPredicate::True
}
Never => InhabitedPredicate::False,
Param(_) | Alias(ty::Projection, _) => InhabitedPredicate::GenericType(self),
Param(_) | Alias(ty::Projection | ty::Weak, _) => InhabitedPredicate::GenericType(self),
Alias(ty::Opaque, alias_ty) => {
match alias_ty.def_id.as_local() {
// Foreign opaque is considered inhabited.
None => InhabitedPredicate::True,
// Local opaque type may possibly be revealed.
Some(local_def_id) => {
let key = ty::OpaqueTypeKey { def_id: local_def_id, args: alias_ty.args };
InhabitedPredicate::OpaqueType(key)
}
}
}
// FIXME(inherent_associated_types): Most likely we can just map to `GenericType` like above.
// However it's unclear if the args passed to `InhabitedPredicate::instantiate` are of the correct
// format, i.e. don't contain parent args. If you hit this case, please verify this beforehand.
Expand Down
18 changes: 14 additions & 4 deletions compiler/rustc_pattern_analysis/src/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_middle::mir::interpret::Scalar;
use rustc_middle::mir::{self, Const};
use rustc_middle::thir::{FieldPat, Pat, PatKind, PatRange, PatRangeBoundary};
use rustc_middle::ty::layout::IntegerExt;
use rustc_middle::ty::{self, Ty, TyCtxt, VariantDef};
use rustc_middle::ty::{self, OpaqueTypeKey, Ty, TyCtxt, VariantDef};
use rustc_span::{Span, DUMMY_SP};
use rustc_target::abi::{FieldIdx, Integer, VariantIdx, FIRST_VARIANT};
use smallvec::SmallVec;
Expand Down Expand Up @@ -74,8 +74,16 @@ impl<'p, 'tcx> fmt::Debug for RustcMatchCheckCtxt<'p, 'tcx> {
}

impl<'p, 'tcx> RustcMatchCheckCtxt<'p, 'tcx> {
pub(crate) fn is_uninhabited(&self, ty: Ty<'tcx>) -> bool {
!ty.is_inhabited_from(self.tcx, self.module, self.param_env)
fn reveal_opaque(&self, key: OpaqueTypeKey<'tcx>) -> Option<Ty<'tcx>> {
self.typeck_results.concrete_opaque_types.get(&key).map(|x| x.ty)
}
pub fn is_uninhabited(&self, ty: Ty<'tcx>) -> bool {
!ty.inhabited_predicate(self.tcx).apply_revealing_opaque(
self.tcx,
self.param_env,
self.module,
&|key| self.reveal_opaque(key),
)
}

/// Returns whether the given type is an enum from another crate declared `#[non_exhaustive]`.
Expand Down Expand Up @@ -319,7 +327,9 @@ impl<'p, 'tcx> RustcMatchCheckCtxt<'p, 'tcx> {
let is_inhabited = v
.inhabited_predicate(cx.tcx, *def)
.instantiate(cx.tcx, args)
.apply(cx.tcx, cx.param_env, cx.module);
.apply_revealing_opaque(cx.tcx, cx.param_env, cx.module, &|key| {
cx.reveal_opaque(key)
});
// Variants that depend on a disabled unstable feature.
let is_unstable = matches!(
cx.tcx.eval_stability(variant_def_id, None, DUMMY_SP, None),
Expand Down
21 changes: 15 additions & 6 deletions tests/ui/pattern/usefulness/impl-trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ fn option_never(x: Void) -> Option<impl Copy> {
}
match option_never(x) {
None => {}
// FIXME: Unreachable not detected because `is_uninhabited` does not look into opaque
// types.
_ => {}
_ => {} //~ ERROR unreachable
}
}
Some(x)
Expand Down Expand Up @@ -137,10 +135,21 @@ fn nested_empty_opaque(x: Void) -> X {
let opaque_void = nested_empty_opaque(x);
let secretely_void = SecretelyVoid(opaque_void);
match secretely_void {
// FIXME: Unreachable not detected because `is_uninhabited` does not look into opaque
// types.
_ => {}
_ => {} //~ ERROR unreachable
}
}
x
}

type Y = (impl Copy, impl Copy);
struct SecretelyDoubleVoid(Y);
fn super_nested_empty_opaque(x: Void) -> Y {
if false {
let opaque_void = super_nested_empty_opaque(x);
let secretely_void = SecretelyDoubleVoid(opaque_void);
match secretely_void {
_ => {} //~ ERROR unreachable
}
}
(x, x)
}
34 changes: 26 additions & 8 deletions tests/ui/pattern/usefulness/impl-trait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -23,51 +23,69 @@ LL | Some(_) => {}
| ^^^^^^^

error: unreachable pattern
--> $DIR/impl-trait.rs:61:13
--> $DIR/impl-trait.rs:49:13
|
LL | _ => {}
| ^

error: unreachable pattern
--> $DIR/impl-trait.rs:59:13
|
LL | Some(_) => {}
| ^^^^^^^

error: unreachable pattern
--> $DIR/impl-trait.rs:65:13
--> $DIR/impl-trait.rs:63:13
|
LL | _ => {}
| ^

error: unreachable pattern
--> $DIR/impl-trait.rs:78:9
--> $DIR/impl-trait.rs:76:9
|
LL | _ => {}
| ^

error: unreachable pattern
--> $DIR/impl-trait.rs:88:9
--> $DIR/impl-trait.rs:86:9
|
LL | _ => {}
| - matches any value
LL | Some((a, b)) => {}
| ^^^^^^^^^^^^ unreachable pattern

error: unreachable pattern
--> $DIR/impl-trait.rs:96:13
--> $DIR/impl-trait.rs:94:13
|
LL | _ => {}
| ^

error: unreachable pattern
--> $DIR/impl-trait.rs:107:9
--> $DIR/impl-trait.rs:105:9
|
LL | Some((mut x, mut y)) => {
| ^^^^^^^^^^^^^^^^^^^^

error: unreachable pattern
--> $DIR/impl-trait.rs:126:13
--> $DIR/impl-trait.rs:124:13
|
LL | _ => {}
| - matches any value
LL | Rec { n: 0, w: Some(Rec { n: 0, w: _ }) } => {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unreachable pattern

error: unreachable pattern
--> $DIR/impl-trait.rs:138:13
|
LL | _ => {}
| ^

error: unreachable pattern
--> $DIR/impl-trait.rs:151:13
|
LL | _ => {}
| ^

error[E0004]: non-exhaustive patterns: type `impl Copy` is non-empty
--> $DIR/impl-trait.rs:23:11
|
Expand Down Expand Up @@ -96,6 +114,6 @@ LL + _ => todo!(),
LL + }
|

error: aborting due to 12 previous errors
error: aborting due to 15 previous errors

For more information about this error, try `rustc --explain E0004`.

0 comments on commit cdd6374

Please sign in to comment.