Skip to content

Commit

Permalink
Auto merge of #65989 - Aaron1011:fix/normalize-param-env, r=<try>
Browse files Browse the repository at this point in the history
Normalize all opaque types when converting ParamEnv to Reveal::All

When we normalize a type using a ParamEnv with a reveal mode of
RevealMode::All, we will normalize opaque types to their underlying
types (e.g. `type MyOpaque = impl Foo` -> `StructThatImplsFoo`).
However, the ParamEnv may still have predicates referring to the
un-normalized opaque type (e.g. `<T as MyTrait<MyOpaque>>`). This can
cause trait projection to fail, since a type containing normalized
opaque types will not match up with the un-normalized type in the
`ParamEnv`.

To fix this, we now explicitly normalize all opaque types in
caller_bounds of a `ParamEnv` when changing its mode to
`RevealMode::All`. This ensures that all predicatse will refer to the
underlying types of any opaque types involved, allowing them to be
matched up properly during projection. To reflect the fact that
normalization is occuring, `ParamEnv::with_reveal_all` is renamed to
`ParamEnv::with_reveal_all_normalized`

Fixes #65918
  • Loading branch information
bors committed Dec 7, 2019
2 parents ae1b871 + 10a5c93 commit ff0aa14
Show file tree
Hide file tree
Showing 9 changed files with 158 additions and 76 deletions.
2 changes: 1 addition & 1 deletion src/librustc/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ pub fn find_associated_item<'tcx>(
match ancestors.leaf_def(tcx, item.ident, item.kind) {
Some(node_item) => {
let substs = tcx.infer_ctxt().enter(|infcx| {
let param_env = param_env.with_reveal_all();
let param_env = param_env.with_reveal_all_normalized(tcx);
let substs = substs.rebase_onto(tcx, trait_def_id, impl_data.substs);
let substs = translate_substs(&infcx, param_env, impl_data.impl_def_id,
substs, node_item.node);
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1949,7 +1949,7 @@ impl<'tcx> LayoutOf for LayoutCx<'tcx, TyCtxt<'tcx>> {
/// Computes the layout of a type. Note that this implicitly
/// executes in "reveal all" mode.
fn layout_of(&self, ty: Ty<'tcx>) -> Self::TyLayout {
let param_env = self.param_env.with_reveal_all();
let param_env = self.param_env.with_reveal_all_normalized(self.tcx);
let ty = self.tcx.normalize_erasing_regions(param_env, ty);
let details = self.tcx.layout_raw(param_env.and(ty))?;
let layout = TyLayout {
Expand All @@ -1976,7 +1976,7 @@ impl LayoutOf for LayoutCx<'tcx, ty::query::TyCtxtAt<'tcx>> {
/// Computes the layout of a type. Note that this implicitly
/// executes in "reveal all" mode.
fn layout_of(&self, ty: Ty<'tcx>) -> Self::TyLayout {
let param_env = self.param_env.with_reveal_all();
let param_env = self.param_env.with_reveal_all_normalized(*self.tcx);
let ty = self.tcx.normalize_erasing_regions(param_env, ty);
let details = self.tcx.layout_raw(param_env.and(ty))?;
let layout = TyLayout {
Expand Down
12 changes: 9 additions & 3 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1666,7 +1666,7 @@ impl<'tcx> ParamEnv<'tcx> {
/// environments like codegen or doing optimizations.
///
/// N.B., if you want to have predicates in scope, use `ParamEnv::new`,
/// or invoke `param_env.with_reveal_all()`.
/// or invoke `param_env.with_reveal_all_normalized()`.
#[inline]
pub fn reveal_all() -> Self {
Self::new(List::empty(), Reveal::All, None)
Expand All @@ -1688,8 +1688,14 @@ impl<'tcx> ParamEnv<'tcx> {
/// the desired behavior during codegen and certain other special
/// contexts; normally though we want to use `Reveal::UserFacing`,
/// which is the default.
pub fn with_reveal_all(self) -> Self {
ty::ParamEnv { reveal: Reveal::All, ..self }
///
/// All opaque types in the caller_bounds of the `ParamEnv`
/// will be normalized to their underlying types.
pub fn with_reveal_all_normalized(self, tcx: TyCtxt<'tcx>) -> Self {
let caller_bounds = tcx.normalize_impl_trait_types(
&self.caller_bounds
);
ty::ParamEnv { reveal: Reveal::All, caller_bounds, ..self }
}

/// Returns this same environment but with no caller bounds.
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2319,7 +2319,7 @@ impl<'tcx> Const<'tcx> {
ty: Ty<'tcx>,
) -> Option<u128> {
assert_eq!(self.ty, ty);
let size = tcx.layout_of(param_env.with_reveal_all().and(ty)).ok()?.size;
let size = tcx.layout_of(param_env.with_reveal_all_normalized(tcx).and(ty)).ok()?.size;
// if `ty` does not depend on generic parameters, use an empty param_env
self.eval(tcx, param_env).val.try_to_bits(size)
}
Expand All @@ -2331,7 +2331,7 @@ impl<'tcx> Const<'tcx> {
param_env: ParamEnv<'tcx>,
) -> &Const<'tcx> {
let try_const_eval = |did, param_env: ParamEnv<'tcx>, substs| {
let param_env_and_substs = param_env.with_reveal_all().and(substs);
let param_env_and_substs = param_env.with_reveal_all_normalized(tcx).and(substs);

// Avoid querying `tcx.const_eval(...)` with any e.g. inference vars.
if param_env_and_substs.has_local_value() {
Expand Down
153 changes: 88 additions & 65 deletions src/librustc/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::mir::interpret::{sign_extend, truncate};
use crate::ich::NodeIdHashingMode;
use crate::traits::{self, ObligationCause};
use crate::ty::{self, DefIdTree, Ty, TyCtxt, GenericParamDefKind, TypeFoldable};
use crate::ty::fold::TypeFolder;
use crate::ty::subst::{Subst, InternalSubsts, SubstsRef, GenericArgKind};
use crate::ty::query::TyCtxtAt;
use crate::ty::TyKind::*;
Expand Down Expand Up @@ -699,81 +700,32 @@ impl<'tcx> TyCtxt<'tcx> {
}
}

/// Normalizes all opaque types in the given value, replacing them
/// with their underlying types.
pub fn normalize_impl_trait_types<T: TypeFoldable<'tcx>>(self, val: &T) -> T {
let mut visitor = OpaqueTypeExpander {
seen_opaque_tys: FxHashSet::default(),
expanded_cache: FxHashMap::default(),
primary_def_id: None,
found_recursion: false,
check_recursion: false,
tcx: self,
};
val.fold_with(&mut visitor)
}

/// Expands the given impl trait type, stopping if the type is recursive.
pub fn try_expand_impl_trait_type(
self,
def_id: DefId,
substs: SubstsRef<'tcx>,
) -> Result<Ty<'tcx>, Ty<'tcx>> {
use crate::ty::fold::TypeFolder;

struct OpaqueTypeExpander<'tcx> {
// Contains the DefIds of the opaque types that are currently being
// expanded. When we expand an opaque type we insert the DefId of
// that type, and when we finish expanding that type we remove the
// its DefId.
seen_opaque_tys: FxHashSet<DefId>,
// Cache of all expansions we've seen so far. This is a critical
// optimization for some large types produced by async fn trees.
expanded_cache: FxHashMap<(DefId, SubstsRef<'tcx>), Ty<'tcx>>,
primary_def_id: DefId,
found_recursion: bool,
tcx: TyCtxt<'tcx>,
}

impl<'tcx> OpaqueTypeExpander<'tcx> {
fn expand_opaque_ty(
&mut self,
def_id: DefId,
substs: SubstsRef<'tcx>,
) -> Option<Ty<'tcx>> {
if self.found_recursion {
return None;
}
let substs = substs.fold_with(self);
if self.seen_opaque_tys.insert(def_id) {
let expanded_ty = match self.expanded_cache.get(&(def_id, substs)) {
Some(expanded_ty) => expanded_ty,
None => {
let generic_ty = self.tcx.type_of(def_id);
let concrete_ty = generic_ty.subst(self.tcx, substs);
let expanded_ty = self.fold_ty(concrete_ty);
self.expanded_cache.insert((def_id, substs), expanded_ty);
expanded_ty
}
};
self.seen_opaque_tys.remove(&def_id);
Some(expanded_ty)
} else {
// If another opaque type that we contain is recursive, then it
// will report the error, so we don't have to.
self.found_recursion = def_id == self.primary_def_id;
None
}
}
}

impl<'tcx> TypeFolder<'tcx> for OpaqueTypeExpander<'tcx> {
fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx
}

fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> {
if let ty::Opaque(def_id, substs) = t.kind {
self.expand_opaque_ty(def_id, substs).unwrap_or(t)
} else if t.has_projections() {
t.super_fold_with(self)
} else {
t
}
}
}

let mut visitor = OpaqueTypeExpander {
seen_opaque_tys: FxHashSet::default(),
expanded_cache: FxHashMap::default(),
primary_def_id: def_id,
primary_def_id: Some(def_id),
found_recursion: false,
check_recursion: true,
tcx: self,
};
let expanded_type = visitor.expand_opaque_ty(def_id, substs).unwrap();
Expand All @@ -785,6 +737,77 @@ impl<'tcx> TyCtxt<'tcx> {
}
}

/// Expands any nested opaque types, caching the expansion
/// of each (DefId, SubstsRef) pair
struct OpaqueTypeExpander<'tcx> {
/// Contains the `DefId`s of the opaque types that are currently being
/// expanded. When we expand an opaque type we insert the `DefId` of
/// that type, and when we finish expanding that type we remove the
/// its `DefId`.
seen_opaque_tys: FxHashSet<DefId>,
/// Cache of all expansions we've seen so far. This is a critical
/// optimization for some large types produced by `async fn` trees.
expanded_cache: FxHashMap<(DefId, SubstsRef<'tcx>), Ty<'tcx>>,
primary_def_id: Option<DefId>,
found_recursion: bool,
/// Whether or not to check for recursive opaque types.
/// This is `true` when we're explicitly checking for opaque type
/// recursion, and 'false' otherwise to avoid unecessary work.
check_recursion: bool,
tcx: TyCtxt<'tcx>,
}

impl<'tcx> OpaqueTypeExpander<'tcx> {
fn expand_opaque_ty(
&mut self,
def_id: DefId,
substs: SubstsRef<'tcx>,
) -> Option<Ty<'tcx>> {
if self.found_recursion {
return None;
}
let substs = substs.fold_with(self);
if !self.check_recursion || self.seen_opaque_tys.insert(def_id) {
let expanded_ty = match self.expanded_cache.get(&(def_id, substs)) {
Some(expanded_ty) => expanded_ty,
None => {
let generic_ty = self.tcx.type_of(def_id);
let concrete_ty = generic_ty.subst(self.tcx, substs);
let expanded_ty = self.fold_ty(concrete_ty);
self.expanded_cache.insert((def_id, substs), expanded_ty);
expanded_ty
}
};
if self.check_recursion {
self.seen_opaque_tys.remove(&def_id);
}
Some(expanded_ty)
} else {
// If another opaque type that we contain is recursive, then it
// will report the error, so we don't have to.
self.found_recursion = def_id == *self.primary_def_id.as_ref().unwrap();
None
}
}
}

impl<'tcx> TypeFolder<'tcx> for OpaqueTypeExpander<'tcx> {
fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx
}

fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> {
if let ty::Opaque(def_id, substs) = t.kind {
self.expand_opaque_ty(def_id, substs).unwrap_or(t)
} else if t.has_projections() {
t.super_fold_with(self)
} else {
t
}
}
}


impl<'tcx> ty::TyS<'tcx> {
/// Checks whether values of this type `T` are *moved* or *copied*
/// when referenced -- this amounts to a check for whether `T:
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_utils/symbol_names/v0.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ impl Printer<'tcx> for SymbolMangler<'tcx> {
let parent_def_id = DefId { index: key.parent.unwrap(), ..impl_def_id };

let mut param_env = self.tcx.param_env(impl_def_id)
.with_reveal_all();
.with_reveal_all_normalized(self.tcx);
if !substs.is_empty() {
param_env = param_env.subst(self.tcx, substs);
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ fn build_drop_shim<'tcx>(
});
}
let patch = {
let param_env = tcx.param_env(def_id).with_reveal_all();
let param_env = tcx.param_env(def_id).with_reveal_all_normalized(tcx);
let mut elaborator = DropShimElaborator {
body: &body,
patch: MirPatch::new(&body),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/transform/elaborate_drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops {
debug!("elaborate_drops({:?} @ {:?})", src, body.span);

let def_id = src.def_id();
let param_env = tcx.param_env(src.def_id()).with_reveal_all();
let param_env = tcx.param_env(src.def_id()).with_reveal_all_normalized(tcx);
let move_data = match MoveData::gather_moves(body, tcx) {
Ok(move_data) => move_data,
Err(_) => bug!("No `move_errors` should be allowed in MIR borrowck"),
Expand Down
53 changes: 53 additions & 0 deletions src/test/ui/type-alias-impl-trait/opaque-type-in-predicate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// run-pass

// Regression test for issue #65918 - checks that we do not
// ICE when attempting to normalize a predicate containting
// opaque types

#![feature(type_alias_impl_trait)]

use std::marker::PhantomData;

/* copied Index and TryFrom for convinience (and simplicity) */
trait MyIndex<T> {
type O;
fn my_index(self) -> Self::O;
}
trait MyFrom<T>: Sized {
type Error;
fn my_from(value: T) -> Result<Self, Self::Error>;
}

/* MCVE starts here */
trait F {}
impl F for () {}
type DummyT<T> = impl F;
fn _dummy_t<T>() -> DummyT<T> {}

struct Phantom1<T>(PhantomData<T>);
struct Phantom2<T>(PhantomData<T>);
struct Scope<T>(Phantom2<DummyT<T>>);

impl<T> Scope<T> {
fn new() -> Self {
Scope(Phantom2(PhantomData))
}
}

impl<T> MyFrom<Phantom2<T>> for Phantom1<T> {
type Error = ();
fn my_from(_: Phantom2<T>) -> Result<Self, Self::Error> {
Ok(Phantom1(PhantomData))
}
}

impl<T: MyFrom<Phantom2<DummyT<U>>>, U> MyIndex<Phantom1<T>> for Scope<U> {
type O = T;
fn my_index(self) -> Self::O {
MyFrom::my_from(self.0).ok().unwrap()
}
}

fn main() {
let _pos: Phantom1<DummyT<()>> = Scope::new().my_index();
}

0 comments on commit ff0aa14

Please sign in to comment.