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.

Sign up for GitHub

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

Formalize defining_use_anchor #99383

Merged
merged 2 commits into from
Jul 20, 2022
Merged

Formalize defining_use_anchor #99383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c0fb38
take opaq types
ouz-a Jul 2, 2022
64dc377
use def_id
ouz-a Jul 20, 2022
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
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/consumers.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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(
ouz-a marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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,
Copy link
Member

@bjorn3 bjorn3 Jul 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This used to be the equivalent to DefiningAnchor::Bubble. I don't see any other behavior changes.

Copy link
Member

Choose a reason for hiding this comment

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

@bjorn3 so could that be the cause of the ICE?

@ouz-a any chance you could look into why Miri started ICEing after this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah sorry, I totally forgot about this I will look into this asap.

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
Open in desktop
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
Open in desktop
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| {
ouz-a marked this conversation as resolved.
Show resolved Hide resolved
//~^ 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