Skip to content
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

Make mir borrowck's use of opaque types independent of the typeck query's result #87287

Merged
merged 7 commits into from
Jul 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/rustc_data_structures/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#![feature(new_uninit)]
#![feature(once_cell)]
#![feature(maybe_uninit_uninit_array)]
#![feature(min_type_alias_impl_trait)]
#![allow(rustc::default_hash_types)]
#![deny(unaligned_references)]

Expand Down
14 changes: 9 additions & 5 deletions compiler/rustc_data_structures/src/vec_map.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::borrow::Borrow;
use std::iter::FromIterator;
use std::slice::{Iter, IterMut};
use std::slice::Iter;
use std::vec::IntoIter;

use crate::stable_hasher::{HashStable, StableHasher};
Expand Down Expand Up @@ -67,9 +67,13 @@ where
self.into_iter()
}

pub fn iter_mut(&mut self) -> IterMut<'_, (K, V)> {
pub fn iter_mut(&mut self) -> impl Iterator<Item = (&K, &mut V)> {
self.into_iter()
}

pub fn retain(&mut self, f: impl Fn(&(K, V)) -> bool) {
self.0.retain(f)
}
}

impl<K, V> Default for VecMap<K, V> {
Expand Down Expand Up @@ -108,12 +112,12 @@ impl<'a, K, V> IntoIterator for &'a VecMap<K, V> {
}

impl<'a, K, V> IntoIterator for &'a mut VecMap<K, V> {
type Item = &'a mut (K, V);
type IntoIter = IterMut<'a, (K, V)>;
type Item = (&'a K, &'a mut V);
type IntoIter = impl Iterator<Item = Self::Item>;

#[inline]
fn into_iter(self) -> Self::IntoIter {
self.0.iter_mut()
self.0.iter_mut().map(|(k, v)| (&*k, v))
}
}

Expand Down
140 changes: 51 additions & 89 deletions compiler/rustc_mir/src/borrow_check/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use rustc_middle::mir::*;
use rustc_middle::ty::adjustment::PointerCast;
use rustc_middle::ty::cast::CastTy;
use rustc_middle::ty::fold::TypeFoldable;
use rustc_middle::ty::subst::{GenericArgKind, Subst, SubstsRef, UserSubsts};
use rustc_middle::ty::subst::{GenericArgKind, SubstsRef, UserSubsts};
use rustc_middle::ty::{
self, CanonicalUserTypeAnnotation, CanonicalUserTypeAnnotations, OpaqueTypeKey, RegionVid,
ToPredicate, Ty, TyCtxt, UserType, UserTypeAnnotationIndex, WithConstness,
Expand Down Expand Up @@ -60,7 +60,6 @@ use crate::borrow_check::{
LivenessValues, PlaceholderIndex, PlaceholderIndices, RegionValueElements,
},
region_infer::{ClosureRegionRequirementsExt, TypeTest},
renumber,
type_check::free_region_relations::{CreateResult, UniversalRegionRelations},
universal_regions::{DefiningTy, UniversalRegions},
Upvar,
Expand Down Expand Up @@ -180,7 +179,54 @@ pub(crate) fn type_check<'mir, 'tcx>(
liveness::generate(&mut cx, body, elements, flow_inits, move_data, location_table);

translate_outlives_facts(&mut cx);
cx.opaque_type_values
let mut opaque_type_values = cx.opaque_type_values;

for (_, revealed_ty) in &mut opaque_type_values {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does opaque_type_values contain elements at the start of the loop? It seems to me that the only place where elements were inserted into this field was in the code you deleted below?

Copy link
Contributor Author

@oli-obk oli-obk Jul 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but the function where I deleted code is called multiple times. The opaque types are collected over time.

*revealed_ty = infcx.resolve_vars_if_possible(*revealed_ty);
if revealed_ty.has_infer_types_or_consts() {
infcx.tcx.sess.delay_span_bug(
body.span,
&format!("could not resolve {:#?}", revealed_ty.kind()),
);
*revealed_ty = infcx.tcx.ty_error();
}
}

opaque_type_values.retain(|(opaque_type_key, resolved_ty)| {
let concrete_is_opaque = if let ty::Opaque(def_id, _) = resolved_ty.kind() {
*def_id == opaque_type_key.def_id
} else {
false
};

if concrete_is_opaque {
// We're using an opaque `impl Trait` type without
// 'revealing' it. For example, code like this:
//
// type Foo = impl Debug;
// fn foo1() -> Foo { ... }
// fn foo2() -> Foo { foo1() }
//
// In `foo2`, we're not revealing the type of `Foo` - we're
// just treating it as the opaque type.
//
// When this occurs, we do *not* want to try to equate
// the concrete type with the underlying defining type
// of the opaque type - this will always fail, since
// the defining type of an opaque type is always
// some other type (e.g. not itself)
// Essentially, none of the normal obligations apply here -
// we're just passing around some unknown opaque type,
// without actually looking at the underlying type it
// gets 'revealed' into
debug!(
"eq_opaque_type_and_type: non-defining use of {:?}",
opaque_type_key.def_id,
);
}
!concrete_is_opaque
});
opaque_type_values
},
);

Expand Down Expand Up @@ -1239,14 +1285,10 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
return Ok(());
}

let infcx = self.infcx;
let tcx = infcx.tcx;
let param_env = self.param_env;
let body = self.body;
let mir_def_id = body.source.def_id().expect_local();

// the "concrete opaque types" maps
let concrete_opaque_types = &tcx.typeck(mir_def_id).concrete_opaque_types;
let mut opaque_type_values = VecMap::new();

debug!("eq_opaque_type_and_type: mir_def_id={:?}", mir_def_id);
Expand Down Expand Up @@ -1296,88 +1338,8 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
.eq(output_ty, revealed_ty)?,
);

// For each opaque type `Foo<T>` inferred by this value, we want to equate
// the inference variable `?T` with the revealed type that was computed
// earlier by type check.
for &(opaque_type_key, opaque_decl) in &opaque_type_map {
let resolved_ty = infcx.resolve_vars_if_possible(opaque_decl.concrete_ty);
let concrete_is_opaque = if let ty::Opaque(def_id, _) = resolved_ty.kind() {
*def_id == opaque_type_key.def_id
} else {
false
};

// The revealed type computed by the earlier phase of type check.
// In our example, this would be `(U, u32)`. Note that this references
// the type parameter `U` from the definition of `Foo`.
let concrete_ty = match concrete_opaque_types
.get_by(|(key, _)| key.def_id == opaque_type_key.def_id)
{
None => {
if !concrete_is_opaque {
tcx.sess.delay_span_bug(
body.span,
&format!(
"Non-defining use of {:?} with revealed type",
opaque_type_key.def_id,
),
);
}
continue;
}
Some(concrete_ty) => concrete_ty,
};
debug!("concrete_ty = {:?}", concrete_ty);

// Apply the substitution, in this case `[U -> T]`, so that the
// concrete type becomes `Foo<(T, u32)>`
let subst_opaque_defn_ty = concrete_ty.subst(tcx, opaque_type_key.substs);

// "Renumber" this, meaning that we replace all the regions
// with fresh inference variables. Not relevant to our example.
let renumbered_opaque_defn_ty =
renumber::renumber_regions(infcx, subst_opaque_defn_ty);

debug!(
"eq_opaque_type_and_type: concrete_ty={:?}={:?} opaque_defn_ty={:?}",
concrete_ty, resolved_ty, renumbered_opaque_defn_ty,
);

if !concrete_is_opaque {
// Equate the instantiated opaque type `opaque_decl.concrete_ty` (`?T`,
// in our example) with the renumbered version that we took from
// the type check results (`Foo<(T, u32)>`).
obligations.add(
infcx
.at(&ObligationCause::dummy(), param_env)
.eq(opaque_decl.concrete_ty, renumbered_opaque_defn_ty)?,
);
opaque_type_values.insert(opaque_type_key, renumbered_opaque_defn_ty);
} else {
// We're using an opaque `impl Trait` type without
// 'revealing' it. For example, code like this:
//
// type Foo = impl Debug;
// fn foo1() -> Foo { ... }
// fn foo2() -> Foo { foo1() }
//
// In `foo2`, we're not revealing the type of `Foo` - we're
// just treating it as the opaque type.
//
// When this occurs, we do *not* want to try to equate
// the concrete type with the underlying defining type
// of the opaque type - this will always fail, since
// the defining type of an opaque type is always
// some other type (e.g. not itself)
// Essentially, none of the normal obligations apply here -
// we're just passing around some unknown opaque type,
// without actually looking at the underlying type it
// gets 'revealed' into
debug!(
"eq_opaque_type_and_type: non-defining use of {:?}",
opaque_type_key.def_id,
);
}
opaque_type_values.insert(opaque_type_key, opaque_decl.concrete_ty);
}

debug!("eq_opaque_type_and_type: equated");
Expand Down Expand Up @@ -1405,7 +1367,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
locations,
ConstraintCategory::OpaqueType,
CustomTypeOp::new(
|_cx| {
|infcx| {
infcx.constrain_opaque_type(
opaque_type_key,
&opaque_decl,
Expand Down
22 changes: 9 additions & 13 deletions compiler/rustc_trait_selection/src/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ pub struct OpaqueTypeDecl<'tcx> {
}

/// Whether member constraints should be generated for all opaque types
#[derive(Debug)]
pub enum GenerateMemberConstraints {
/// The default, used by typeck
WhenRequired,
Expand Down Expand Up @@ -354,8 +355,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
opaque_types: &OpaqueTypeMap<'tcx>,
free_region_relations: &FRR,
) {
debug!("constrain_opaque_types()");

for &(opaque_type_key, opaque_defn) in opaque_types {
self.constrain_opaque_type(
opaque_type_key,
Expand All @@ -367,6 +366,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}

/// See `constrain_opaque_types` for documentation.
#[instrument(level = "debug", skip(self, free_region_relations))]
fn constrain_opaque_type<FRR: FreeRegionRelations<'tcx>>(
&self,
opaque_type_key: OpaqueTypeKey<'tcx>,
Expand All @@ -376,15 +376,11 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
) {
let def_id = opaque_type_key.def_id;

debug!("constrain_opaque_type()");
debug!("constrain_opaque_type: def_id={:?}", def_id);
debug!("constrain_opaque_type: opaque_defn={:#?}", opaque_defn);

let tcx = self.tcx;

let concrete_ty = self.resolve_vars_if_possible(opaque_defn.concrete_ty);

debug!("constrain_opaque_type: concrete_ty={:?}", concrete_ty);
debug!(?concrete_ty);

let first_own_region = match opaque_defn.origin {
hir::OpaqueTyOrigin::FnReturn | hir::OpaqueTyOrigin::AsyncFn => {
Expand All @@ -397,7 +393,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
// type foo::<'p0..'pn>::Foo<'q0..'qm>
// fn foo<l0..'ln>() -> foo::<'static..'static>::Foo<'l0..'lm>.
//
// For these types we onlt iterate over `'l0..lm` below.
// For these types we only iterate over `'l0..lm` below.
tcx.generics_of(def_id).parent_count
}
// These opaque type inherit all lifetime parameters from their
Expand All @@ -410,10 +406,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
// If there are required region bounds, we can use them.
if opaque_defn.has_required_region_bounds {
let bounds = tcx.explicit_item_bounds(def_id);
debug!("constrain_opaque_type: predicates: {:#?}", bounds);
debug!("{:#?}", bounds);
let bounds: Vec<_> =
bounds.iter().map(|(bound, _)| bound.subst(tcx, opaque_type_key.substs)).collect();
debug!("constrain_opaque_type: bounds={:#?}", bounds);
debug!("{:#?}", bounds);
let opaque_type = tcx.mk_opaque(def_id, opaque_type_key.substs);

let required_region_bounds =
Expand Down Expand Up @@ -452,8 +448,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
};

// Compute the least upper bound of it with the other regions.
debug!("constrain_opaque_types: least_region={:?}", least_region);
debug!("constrain_opaque_types: subst_region={:?}", subst_region);
debug!(?least_region);
debug!(?subst_region);
match least_region {
None => least_region = Some(subst_region),
Some(lr) => {
Expand Down Expand Up @@ -484,7 +480,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}

let least_region = least_region.unwrap_or(tcx.lifetimes.re_static);
debug!("constrain_opaque_types: least_region={:?}", least_region);
debug!(?least_region);

if let GenerateMemberConstraints::IfNoStaticBound = mode {
if least_region != tcx.lifetimes.re_static {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ impl Bar for AssocNoCopy {
impl Thing for AssocNoCopy {
type Out = Box<dyn Bar<Assoc: Copy>>;
//~^ ERROR the trait bound `String: Copy` is not satisfied
//~| ERROR the trait bound `String: Copy` is not satisfied

fn func() -> Self::Out {
Box::new(AssocNoCopy)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,6 @@ error[E0277]: the trait bound `String: Copy` is not satisfied
LL | type Out = Box<dyn Bar<Assoc: Copy>>;
| ^^^^^^^^^^^ the trait `Copy` is not implemented for `String`

error[E0277]: the trait bound `String: Copy` is not satisfied
--> $DIR/assoc-type-eq-with-dyn-atb-fail.rs:30:28
|
LL | type Out = Box<dyn Bar<Assoc: Copy>>;
| ^^^^^^^^^^^ the trait `Copy` is not implemented for `String`

error: aborting due to 2 previous errors
error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
error: lifetime may not live long enough
--> $DIR/ret-impl-trait-one.rs:10:80
--> $DIR/ret-impl-trait-one.rs:10:65
|
LL | async fn async_ret_impl_trait1<'a, 'b>(a: &'a u8, b: &'b u8) -> impl Trait<'a> {
| ________________________________--__--__________________________________________^
| | | |
| | | lifetime `'b` defined here
| | lifetime `'a` defined here
LL | |
LL | | (a, b)
LL | | }
| |_^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b`
LL | async fn async_ret_impl_trait1<'a, 'b>(a: &'a u8, b: &'b u8) -> impl Trait<'a> {
| -- -- ^^^^^^^^^^^^^^ opaque type requires that `'b` must outlive `'a`
| | |
| | lifetime `'b` defined here
| lifetime `'a` defined here
|
= help: consider adding the following bound: `'b: 'a`

Expand Down
1 change: 0 additions & 1 deletion src/test/ui/impl-trait/auto-trait-leak.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ fn main() {
fn cycle1() -> impl Clone {
//~^ ERROR cycle detected
send(cycle2().clone());
//~^ ERROR cannot be sent between threads safely

Rc::new(Cell::new(5))
}
Expand Down
Loading