Skip to content

Commit

Permalink
Rollup merge of #99383 - ouz-a:issue_57961, r=oli-obk
Browse files Browse the repository at this point in the history 
Formalize defining_use_anchor

This tackles issue #57961

Introduces new enum called `DefiningAnchor` that replaces `Option<LocalDefId>` of `defining_use_anchor`. Now every use of it is explicit and exhaustively matched, catching errors like one in the linked issue. This is not a perfect fix but it's a step in the right direction.

r? `@oli-obk`
  • Loading branch information
@Dylan-DPC
Dylan-DPC authored Jul 20, 2022
2 parents 3c3c5da + 64dc377 commit eecfdfb
Show file tree
Hide file tree
Showing 18 changed files with 292 additions and 184 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/consumers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use rustc_hir::def_id::LocalDefId;
use rustc_index::vec::IndexVec;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_infer::infer::{DefiningAnchor, TyCtxtInferExt};
use rustc_middle::mir::Body;
use rustc_middle::ty::{self, TyCtxt};

Expand Down Expand Up @@ -31,7 +31,7 @@ pub fn get_body_with_borrowck_facts<'tcx>(
def: ty::WithOptConstParam<LocalDefId>,
) -> BodyWithBorrowckFacts<'tcx> {
let (input_body, promoted) = tcx.mir_promoted(def);
tcx.infer_ctxt().with_opaque_type_inference(def.did).enter(|infcx| {
tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bind(def.did)).enter(|infcx| {
let input_body: &Body<'_> = &input_body.borrow();
let promoted: &IndexVec<_, _> = &promoted.borrow();
*super::do_mir_borrowck(&infcx, input_body, promoted, true).1.unwrap()
Expand Down
15 changes: 9 additions & 6 deletions compiler/rustc_borrowck/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use rustc_hir as hir;
use rustc_hir::def_id::LocalDefId;
use rustc_index::bit_set::ChunkedBitSet;
use rustc_index::vec::IndexVec;
use rustc_infer::infer::{InferCtxt, TyCtxtInferExt};
use rustc_infer::infer::{DefiningAnchor, InferCtxt, TyCtxtInferExt};
use rustc_middle::mir::{
traversal, Body, ClearCrossCrate, Local, Location, Mutability, Operand, Place, PlaceElem,
PlaceRef, VarDebugInfoContents,
Expand Down Expand Up @@ -130,11 +130,14 @@ fn mir_borrowck<'tcx>(
debug!("run query mir_borrowck: {}", tcx.def_path_str(def.did.to_def_id()));
let hir_owner = tcx.hir().local_def_id_to_hir_id(def.did).owner;

let opt_closure_req = tcx.infer_ctxt().with_opaque_type_inference(hir_owner).enter(|infcx| {
let input_body: &Body<'_> = &input_body.borrow();
let promoted: &IndexVec<_, _> = &promoted.borrow();
do_mir_borrowck(&infcx, input_body, promoted, false).0
});
let opt_closure_req = tcx
.infer_ctxt()
.with_opaque_type_inference(DefiningAnchor::Bind(hir_owner))
.enter(|infcx| {
let input_body: &Body<'_> = &input_body.borrow();
let promoted: &IndexVec<_, _> = &promoted.borrow();
do_mir_borrowck(&infcx, input_body, promoted, false).0
});
debug!("mir_borrowck done");

tcx.arena.alloc(opt_closure_req)
Expand Down
102 changes: 54 additions & 48 deletions compiler/rustc_borrowck/src/region_infer/opaque_types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use rustc_data_structures::vec_map::VecMap;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::OpaqueTyOrigin;
use rustc_infer::infer::error_reporting::unexpected_hidden_region_diagnostic;
use rustc_infer::infer::InferCtxt;
use rustc_infer::infer::TyCtxtInferExt as _;
use rustc_infer::infer::{DefiningAnchor, InferCtxt};
use rustc_infer::traits::{Obligation, ObligationCause, TraitEngine};
use rustc_middle::ty::fold::{TypeFolder, TypeSuperFoldable};
use rustc_middle::ty::subst::{GenericArg, GenericArgKind, InternalSubsts};
Expand Down Expand Up @@ -269,59 +269,65 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
// FIXME(oli-obk): Also do region checks here and then consider removing `check_opaque_meets_bounds` entirely.
let param_env = self.tcx.param_env(def_id);
let body_id = self.tcx.local_def_id_to_hir_id(def_id);
self.tcx.infer_ctxt().enter(move |infcx| {
// Require the hidden type to be well-formed with only the generics of the opaque type.
// Defining use functions may have more bounds than the opaque type, which is ok, as long as the
// hidden type is well formed even without those bounds.
let predicate =
ty::Binder::dummy(ty::PredicateKind::WellFormed(definition_ty.into()))
.to_predicate(infcx.tcx);
let mut fulfillment_cx = <dyn TraitEngine<'tcx>>::new(infcx.tcx);

// Require that the hidden type actually fulfills all the bounds of the opaque type, even without
// the bounds that the function supplies.
match infcx.register_hidden_type(
OpaqueTypeKey { def_id, substs: id_substs },
ObligationCause::misc(instantiated_ty.span, body_id),
param_env,
definition_ty,
origin,
) {
Ok(infer_ok) => {
for obligation in infer_ok.obligations {
fulfillment_cx.register_predicate_obligation(&infcx, obligation);
// HACK This bubble is required for this tests to pass:
// type-alias-impl-trait/issue-67844-nested-opaque.rs
self.tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bubble).enter(
move |infcx| {
// Require the hidden type to be well-formed with only the generics of the opaque type.
// Defining use functions may have more bounds than the opaque type, which is ok, as long as the
// hidden type is well formed even without those bounds.
let predicate =
ty::Binder::dummy(ty::PredicateKind::WellFormed(definition_ty.into()))
.to_predicate(infcx.tcx);
let mut fulfillment_cx = <dyn TraitEngine<'tcx>>::new(infcx.tcx);

// Require that the hidden type actually fulfills all the bounds of the opaque type, even without
// the bounds that the function supplies.
match infcx.register_hidden_type(
OpaqueTypeKey { def_id, substs: id_substs },
ObligationCause::misc(instantiated_ty.span, body_id),
param_env,
definition_ty,
origin,
) {
Ok(infer_ok) => {
for obligation in infer_ok.obligations {
fulfillment_cx.register_predicate_obligation(&infcx, obligation);
}
}
Err(err) => {
infcx
.report_mismatched_types(
&ObligationCause::misc(instantiated_ty.span, body_id),
self.tcx.mk_opaque(def_id.to_def_id(), id_substs),
definition_ty,
err,
)
.emit();
}
}
Err(err) => {
infcx
.report_mismatched_types(
&ObligationCause::misc(instantiated_ty.span, body_id),
self.tcx.mk_opaque(def_id.to_def_id(), id_substs),
definition_ty,
err,
)
.emit();
}
}

fulfillment_cx.register_predicate_obligation(
&infcx,
Obligation::misc(instantiated_ty.span, body_id, param_env, predicate),
);
fulfillment_cx.register_predicate_obligation(
&infcx,
Obligation::misc(instantiated_ty.span, body_id, param_env, predicate),
);

// Check that all obligations are satisfied by the implementation's
// version.
let errors = fulfillment_cx.select_all_or_error(&infcx);
// Check that all obligations are satisfied by the implementation's
// version.
let errors = fulfillment_cx.select_all_or_error(&infcx);

let _ = infcx.inner.borrow_mut().opaque_type_storage.take_opaque_types();
// This is still required for many(half of the tests in ui/type-alias-impl-trait)
// tests to pass
let _ = infcx.inner.borrow_mut().opaque_type_storage.take_opaque_types();

if errors.is_empty() {
definition_ty
} else {
infcx.report_fulfillment_errors(&errors, None, false);
self.tcx.ty_error()
}
})
if errors.is_empty() {
definition_ty
} else {
infcx.report_fulfillment_errors(&errors, None, false);
self.tcx.ty_error()
}
},
)
} else {
definition_ty
}
Expand Down
32 changes: 25 additions & 7 deletions compiler/rustc_infer/src/infer/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,17 +239,31 @@ impl<'tcx> InferCtxtInner<'tcx> {
}
}

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum DefiningAnchor {
/// `DefId` of the item.
Bind(LocalDefId),
/// When opaque types are not resolved, we `Bubble` up, meaning
/// return the opaque/hidden type pair from query, for caller of query to handle it.
Bubble,
/// Used to catch type mismatch errors when handling opaque types.
Error,
}

pub struct InferCtxt<'a, 'tcx> {
pub tcx: TyCtxt<'tcx>,

/// The `DefId` of the item in whose context we are performing inference or typeck.
/// It is used to check whether an opaque type use is a defining use.
///
/// If it is `None`, we can't resolve opaque types here and need to bubble up
/// If it is `DefiningAnchor::Bubble`, we can't resolve opaque types here and need to bubble up
/// the obligation. This frequently happens for
/// short lived InferCtxt within queries. The opaque type obligations are forwarded
/// to the outside until the end up in an `InferCtxt` for typeck or borrowck.
pub defining_use_anchor: Option<LocalDefId>,
///
/// It is default value is `DefiningAnchor::Error`, this way it is easier to catch errors that
/// might come up during inference or typeck.
pub defining_use_anchor: DefiningAnchor,

/// During type-checking/inference of a body, `in_progress_typeck_results`
/// contains a reference to the typeck results being built up, which are
Expand Down Expand Up @@ -526,7 +540,7 @@ impl<'tcx> fmt::Display for FixupError<'tcx> {
pub struct InferCtxtBuilder<'tcx> {
tcx: TyCtxt<'tcx>,
fresh_typeck_results: Option<RefCell<ty::TypeckResults<'tcx>>>,
defining_use_anchor: Option<LocalDefId>,
defining_use_anchor: DefiningAnchor,
}

pub trait TyCtxtInferExt<'tcx> {
Expand All @@ -535,7 +549,11 @@ pub trait TyCtxtInferExt<'tcx> {

impl<'tcx> TyCtxtInferExt<'tcx> for TyCtxt<'tcx> {
fn infer_ctxt(self) -> InferCtxtBuilder<'tcx> {
InferCtxtBuilder { tcx: self, defining_use_anchor: None, fresh_typeck_results: None }
InferCtxtBuilder {
tcx: self,
defining_use_anchor: DefiningAnchor::Error,
fresh_typeck_results: None,
}
}
}

Expand All @@ -545,7 +563,7 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
/// Will also change the scope for opaque type defining use checks to the given owner.
pub fn with_fresh_in_progress_typeck_results(mut self, table_owner: LocalDefId) -> Self {
self.fresh_typeck_results = Some(RefCell::new(ty::TypeckResults::new(table_owner)));
self.with_opaque_type_inference(table_owner)
self.with_opaque_type_inference(DefiningAnchor::Bind(table_owner))
}

/// Whenever the `InferCtxt` should be able to handle defining uses of opaque types,
Expand All @@ -554,8 +572,8 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
/// It is only meant to be called in two places, for typeck
/// (via `with_fresh_in_progress_typeck_results`) and for the inference context used
/// in mir borrowck.
pub fn with_opaque_type_inference(mut self, defining_use_anchor: LocalDefId) -> Self {
self.defining_use_anchor = Some(defining_use_anchor);
pub fn with_opaque_type_inference(mut self, defining_use_anchor: DefiningAnchor) -> Self {
self.defining_use_anchor = defining_use_anchor;
self
}

Expand Down
85 changes: 45 additions & 40 deletions compiler/rustc_infer/src/infer/opaque_types.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::infer::{InferCtxt, InferOk};
use crate::infer::{DefiningAnchor, InferCtxt, InferOk};
use crate::traits;
use hir::def_id::{DefId, LocalDefId};
use hir::{HirId, OpaqueTyOrigin};
Expand Down Expand Up @@ -101,44 +101,46 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
let process = |a: Ty<'tcx>, b: Ty<'tcx>| match *a.kind() {
ty::Opaque(def_id, substs) if def_id.is_local() => {
let def_id = def_id.expect_local();
let origin = if self.defining_use_anchor.is_some() {
// Check that this is `impl Trait` type is
// declared by `parent_def_id` -- i.e., one whose
// value we are inferring. At present, this is
// always true during the first phase of
// type-check, but not always true later on during
// NLL. Once we support named opaque types more fully,
// this same scenario will be able to arise during all phases.
//
// Here is an example using type alias `impl Trait`
// that indicates the distinction we are checking for:
//
// ```rust
// mod a {
// pub type Foo = impl Iterator;
// pub fn make_foo() -> Foo { .. }
// }
//
// mod b {
// fn foo() -> a::Foo { a::make_foo() }
// }
// ```
//
// Here, the return type of `foo` references an
// `Opaque` indeed, but not one whose value is
// presently being inferred. You can get into a
// similar situation with closure return types
// today:
//
// ```rust
// fn foo() -> impl Iterator { .. }
// fn bar() {
// let x = || foo(); // returns the Opaque assoc with `foo`
// }
// ```
self.opaque_type_origin(def_id, cause.span)?
} else {
self.opaque_ty_origin_unchecked(def_id, cause.span)
let origin = match self.defining_use_anchor {
DefiningAnchor::Bind(_) => {
// Check that this is `impl Trait` type is
// declared by `parent_def_id` -- i.e., one whose
// value we are inferring. At present, this is
// always true during the first phase of
// type-check, but not always true later on during
// NLL. Once we support named opaque types more fully,
// this same scenario will be able to arise during all phases.
//
// Here is an example using type alias `impl Trait`
// that indicates the distinction we are checking for:
//
// ```rust
// mod a {
// pub type Foo = impl Iterator;
// pub fn make_foo() -> Foo { .. }
// }
//
// mod b {
// fn foo() -> a::Foo { a::make_foo() }
// }
// ```
//
// Here, the return type of `foo` references an
// `Opaque` indeed, but not one whose value is
// presently being inferred. You can get into a
// similar situation with closure return types
// today:
//
// ```rust
// fn foo() -> impl Iterator { .. }
// fn bar() {
// let x = || foo(); // returns the Opaque assoc with `foo`
// }
// ```
self.opaque_type_origin(def_id, cause.span)?
}
DefiningAnchor::Bubble => self.opaque_ty_origin_unchecked(def_id, cause.span),
DefiningAnchor::Error => return None,
};
if let ty::Opaque(did2, _) = *b.kind() {
// We could accept this, but there are various ways to handle this situation, and we don't
Expand Down Expand Up @@ -407,7 +409,10 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
#[instrument(skip(self), level = "trace")]
pub fn opaque_type_origin(&self, def_id: LocalDefId, span: Span) -> Option<OpaqueTyOrigin> {
let opaque_hir_id = self.tcx.hir().local_def_id_to_hir_id(def_id);
let parent_def_id = self.defining_use_anchor?;
let parent_def_id = match self.defining_use_anchor {
DefiningAnchor::Bubble | DefiningAnchor::Error => return None,
DefiningAnchor::Bind(bind) => bind,
};
let item_kind = &self.tcx.hir().expect_item(def_id).kind;

let hir::ItemKind::OpaqueTy(hir::OpaqueTy { origin, .. }) = item_kind else {
Expand Down
9 changes: 6 additions & 3 deletions compiler/rustc_trait_selection/src/traits/codegen.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// seems likely that they should eventually be merged into more
// general routines.

use crate::infer::TyCtxtInferExt;
use crate::infer::{DefiningAnchor, TyCtxtInferExt};
use crate::traits::{
FulfillmentContext, ImplSource, Obligation, ObligationCause, SelectionContext, TraitEngine,
Unimplemented,
Expand All @@ -30,7 +30,9 @@ pub fn codegen_fulfill_obligation<'tcx>(

// Do the initial selection for the obligation. This yields the
// shallow result we are looking for -- that is, what specific impl.
tcx.infer_ctxt().enter(|infcx| {
tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bubble).enter(|infcx| {
//~^ HACK `Bubble` is required for
// this test to pass: type-alias-impl-trait/assoc-projection-ice.rs
let mut selcx = SelectionContext::new(&infcx);

let obligation_cause = ObligationCause::dummy();
Expand Down Expand Up @@ -69,7 +71,8 @@ pub fn codegen_fulfill_obligation<'tcx>(

// Opaque types may have gotten their hidden types constrained, but we can ignore them safely
// as they will get constrained elsewhere, too.
let _opaque_types = infcx.inner.borrow_mut().opaque_type_storage.take_opaque_types();
// (ouz-a) This is required for `type-alias-impl-trait/assoc-projection-ice.rs` to pass
let _ = infcx.inner.borrow_mut().opaque_type_storage.take_opaque_types();

debug!("Cache miss: {trait_ref:?} => {impl_source:?}");
Ok(&*tcx.arena.alloc(impl_source))
Expand Down
Loading

0 comments on commit eecfdfb

Please sign in to comment.