Skip to content

Commit

Permalink
Immediately register new opaque types in the global list.
Browse files Browse the repository at this point in the history
Previously each opaque type instantiation would create new inference vars, even for the same opaque type/substs combination. Now there is a global map in InferCtxt that gets filled whenever we encounter an opaque type.
  • Loading branch information
oli-obk committed Aug 6, 2021
1 parent 816b9fc commit 20371b9
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 119 deletions.
126 changes: 58 additions & 68 deletions compiler/rustc_mir/src/borrow_check/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,54 +179,55 @@ pub(crate) fn type_check<'mir, 'tcx>(
liveness::generate(&mut cx, body, elements, flow_inits, move_data, location_table);

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

for (_, revealed_ty) in &mut opaque_type_values {
*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();
}
}
let opaque_type_values = mem::take(&mut infcx.inner.borrow_mut().opaque_types);

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
.into_iter()
.filter_map(|(opaque_type_key, decl)| {
let mut revealed_ty = infcx.resolve_vars_if_possible(decl.concrete_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();
}
let concrete_is_opaque = if let ty::Opaque(def_id, _) = revealed_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,
);
None
} else {
Some((opaque_type_key, revealed_ty))
}
})
.collect()
},
);

Expand Down Expand Up @@ -865,7 +866,6 @@ struct TypeChecker<'a, 'tcx> {
reported_errors: FxHashSet<(Ty<'tcx>, Span)>,
borrowck_context: &'a mut BorrowCheckContext<'a, 'tcx>,
universal_region_relations: &'a UniversalRegionRelations<'tcx>,
opaque_type_values: VecMap<OpaqueTypeKey<'tcx>, Ty<'tcx>>,
}

struct BorrowCheckContext<'a, 'tcx> {
Expand Down Expand Up @@ -1025,7 +1025,6 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
borrowck_context,
reported_errors: Default::default(),
universal_region_relations,
opaque_type_values: VecMap::default(),
};
checker.check_user_type_annotations();
checker
Expand Down Expand Up @@ -1289,10 +1288,8 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
let body = self.body;
let mir_def_id = body.source.def_id().expect_local();

let mut opaque_type_values = VecMap::new();

debug!("eq_opaque_type_and_type: mir_def_id={:?}", mir_def_id);
let opaque_type_map = self.fully_perform_op(
self.fully_perform_op(
locations,
category,
CustomTypeOp::new(
Expand All @@ -1307,20 +1304,18 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
// to `Box<?T>`, returning an `opaque_type_map` mapping `{Foo<T> -> ?T}`.
// (Note that the key of the map is both the def-id of `Foo` along with
// any generic parameters.)
let (output_ty, opaque_type_map) =
obligations.add(infcx.instantiate_opaque_types(
mir_def_id,
dummy_body_id,
param_env,
anon_ty,
locations.span(body),
));
let output_ty = obligations.add(infcx.instantiate_opaque_types(
mir_def_id,
dummy_body_id,
param_env,
anon_ty,
locations.span(body),
));
debug!(
"eq_opaque_type_and_type: \
instantiated output_ty={:?} \
opaque_type_map={:#?} \
revealed_ty={:?}",
output_ty, opaque_type_map, revealed_ty
output_ty, revealed_ty
);

// Make sure that the inferred types are well-formed. I'm
Expand All @@ -1338,26 +1333,21 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
.eq(output_ty, revealed_ty)?,
);

for &(opaque_type_key, opaque_decl) in &opaque_type_map {
opaque_type_values.insert(opaque_type_key, opaque_decl.concrete_ty);
}

debug!("eq_opaque_type_and_type: equated");

Ok(InferOk { value: opaque_type_map, obligations: obligations.into_vec() })
Ok(InferOk { value: (), obligations: obligations.into_vec() })
},
|| "input_output".to_string(),
),
)?;

self.opaque_type_values.extend(opaque_type_values);

let universal_region_relations = self.universal_region_relations;

// Finally, if we instantiated the anon types successfully, we
// have to solve any bounds (e.g., `-> impl Iterator` needs to
// prove that `T: Iterator` where `T` is the type we
// instantiated it with).
let opaque_type_map = self.infcx.inner.borrow().opaque_types.clone();
for (opaque_type_key, opaque_decl) in opaque_type_map {
self.fully_perform_op(
locations,
Expand Down
25 changes: 14 additions & 11 deletions compiler/rustc_trait_selection/src/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_infer::infer::error_reporting::unexpected_hidden_region_diagnostic;
use rustc_infer::infer::free_regions::FreeRegionRelations;
use rustc_infer::infer::opaque_types::{OpaqueTypeDecl, OpaqueTypeMap};
use rustc_infer::infer::opaque_types::OpaqueTypeDecl;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_infer::infer::{self, InferCtxt, InferOk};
use rustc_middle::ty::fold::{BottomUpFolder, TypeFoldable, TypeFolder, TypeVisitor};
Expand Down Expand Up @@ -37,7 +37,7 @@ pub trait InferCtxtExt<'tcx> {
param_env: ty::ParamEnv<'tcx>,
value: T,
value_span: Span,
) -> InferOk<'tcx, (T, OpaqueTypeMap<'tcx>)>;
) -> InferOk<'tcx, T>;

fn constrain_opaque_types<FRR: FreeRegionRelations<'tcx>>(&self, free_region_relations: &FRR);

Expand Down Expand Up @@ -99,7 +99,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
param_env: ty::ParamEnv<'tcx>,
value: T,
value_span: Span,
) -> InferOk<'tcx, (T, OpaqueTypeMap<'tcx>)> {
) -> InferOk<'tcx, T> {
debug!(
"instantiate_opaque_types(value={:?}, parent_def_id={:?}, body_id={:?}, \
param_env={:?}, value_span={:?})",
Expand All @@ -111,11 +111,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
body_id,
param_env,
value_span,
opaque_types: Default::default(),
obligations: vec![],
};
let value = instantiator.instantiate_opaque_types_in_map(value);
InferOk { value: (value, instantiator.opaque_types), obligations: instantiator.obligations }
InferOk { value, obligations: instantiator.obligations }
}

/// Given the map `opaque_types` containing the opaque
Expand Down Expand Up @@ -862,7 +861,6 @@ struct Instantiator<'a, 'tcx> {
body_id: hir::HirId,
param_env: ty::ParamEnv<'tcx>,
value_span: Span,
opaque_types: OpaqueTypeMap<'tcx>,
obligations: Vec<PredicateObligation<'tcx>>,
}

Expand Down Expand Up @@ -972,7 +970,7 @@ impl<'a, 'tcx> Instantiator<'a, 'tcx> {

// Use the same type variable if the exact same opaque type appears more
// than once in the return type (e.g., if it's passed to a type alias).
if let Some(opaque_defn) = self.opaque_types.get(&opaque_type_key) {
if let Some(opaque_defn) = infcx.inner.borrow().opaque_types.get(&opaque_type_key) {
debug!("instantiate_opaque_types: returning concrete ty {:?}", opaque_defn.concrete_ty);
return opaque_defn.concrete_ty;
}
Expand All @@ -994,10 +992,15 @@ impl<'a, 'tcx> Instantiator<'a, 'tcx> {
// Foo, impl Bar)`.
let definition_span = self.value_span;

self.opaque_types.insert(
OpaqueTypeKey { def_id, substs },
OpaqueTypeDecl { opaque_type: ty, definition_span, concrete_ty: ty_var, origin },
);
{
let mut infcx = self.infcx.inner.borrow_mut();
infcx.opaque_types.insert(
OpaqueTypeKey { def_id, substs },
OpaqueTypeDecl { opaque_type: ty, definition_span, concrete_ty: ty_var, origin },
);
infcx.opaque_types_vars.insert(ty_var, ty);
}

debug!("instantiate_opaque_types: ty_var={:?}", ty_var);
self.compute_opaque_type_obligations(opaque_type_key, span);

Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_typeck/src/check/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,8 +598,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
{
let impl_trait_ret_ty =
self.infcx.instantiate_opaque_types(id, self.body_id, self.param_env, ty, span);
let mut suggest_box = !impl_trait_ret_ty.obligations.is_empty();
for o in impl_trait_ret_ty.obligations {
assert!(
impl_trait_ret_ty.obligations.is_empty(),
"we should never get new obligations here"
);
let obligations = self.fulfillment_cx.borrow().pending_obligations();
let mut suggest_box = !obligations.is_empty();
for o in obligations {
match o.predicate.kind().skip_binder() {
ty::PredicateKind::Trait(t, constness) => {
let pred = ty::PredicateKind::Trait(
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_typeck/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,10 +650,11 @@ fn check_opaque_meets_bounds<'tcx>(

let misc_cause = traits::ObligationCause::misc(span, hir_id);

let (_, opaque_type_map) = inh.register_infer_ok_obligations(
let _ = inh.register_infer_ok_obligations(
infcx.instantiate_opaque_types(def_id, hir_id, param_env, opaque_ty, span),
);

let opaque_type_map = infcx.inner.borrow().opaque_types.clone();
for (OpaqueTypeKey { def_id, substs }, opaque_defn) in opaque_type_map {
match infcx
.at(&misc_cause, param_env)
Expand Down
24 changes: 7 additions & 17 deletions compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,23 +374,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
parent_def_id, value
);

let (value, opaque_type_map) =
self.register_infer_ok_obligations(self.instantiate_opaque_types(
parent_def_id,
self.body_id,
self.param_env,
value,
value_span,
));

let mut infcx = self.infcx.inner.borrow_mut();

for (ty, decl) in opaque_type_map {
let _ = infcx.opaque_types.insert(ty, decl);
let _ = infcx.opaque_types_vars.insert(decl.concrete_ty, decl.opaque_type);
}

value
self.register_infer_ok_obligations(self.instantiate_opaque_types(
parent_def_id,
self.body_id,
self.param_env,
value,
value_span,
))
}

/// Convenience method which tracks extra diagnostic information for normalization
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/type-alias-impl-trait/issue-63279.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

#![feature(type_alias_impl_trait)]

type Closure = impl FnOnce(); //~ ERROR: type mismatch resolving
type Closure = impl FnOnce();

fn c() -> Closure {
|| -> Closure { || () }
|| -> Closure { || () } //~ ERROR: mismatched types
}

fn main() {}
17 changes: 11 additions & 6 deletions src/test/ui/type-alias-impl-trait/issue-63279.stderr
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
error[E0271]: type mismatch resolving `<[closure@$DIR/issue-63279.rs:8:5: 8:28] as FnOnce<()>>::Output == ()`
--> $DIR/issue-63279.rs:5:16
error[E0308]: mismatched types
--> $DIR/issue-63279.rs:8:5
|
LL | type Closure = impl FnOnce();
| ^^^^^^^^^^^^^ expected `()`, found opaque type
| ------------- the found opaque type
...
LL | || -> Closure { || () }
| ^^^^^^^^^^^^^^^^^^^^^^^ expected closure, found a different closure
|
= note: expected unit type `()`
found opaque type `impl FnOnce<()>`
= note: expected type `[closure@$DIR/issue-63279.rs:8:21: 8:26]`
found closure `[closure@$DIR/issue-63279.rs:8:5: 8:28]`
= note: no two closures, even if identical, have the same type
= help: consider boxing your closure and/or using it as a trait object

error: aborting due to previous error

For more information about this error, try `rustc --explain E0271`.
For more information about this error, try `rustc --explain E0308`.
3 changes: 1 addition & 2 deletions src/test/ui/type-alias-impl-trait/issue-74280.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ type Test = impl Copy;

fn test() -> Test {
let y = || -> Test { () };
//~^ ERROR: concrete type differs from previous defining opaque type use
7
7 //~ ERROR mismatched types
}

fn main() {}
15 changes: 5 additions & 10 deletions src/test/ui/type-alias-impl-trait/issue-74280.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
error: concrete type differs from previous defining opaque type use
--> $DIR/issue-74280.rs:8:13
error[E0308]: mismatched types
--> $DIR/issue-74280.rs:9:5
|
LL | let y = || -> Test { () };
| ^^^^^^^^^^^^^^^^^ expected `i32`, got `()`
|
note: previous use here
--> $DIR/issue-74280.rs:7:1
|
LL | fn test() -> Test {
| ^^^^^^^^^^^^^^^^^
LL | 7
| ^ expected `()`, found integer

error: aborting due to previous error

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

0 comments on commit 20371b9

Please sign in to comment.