Skip to content

small opaque types refactor #111949

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

Closed
wants to merge 2 commits into from
Closed
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
6 changes: 3 additions & 3 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ fn do_mir_borrowck<'tcx>(
// Compute non-lexical lifetimes.
let nll::NllOutput {
regioncx,
opaque_type_values,
concrete_opaque_types,
polonius_input,
polonius_output,
opt_closure_req,
Expand Down Expand Up @@ -274,7 +274,7 @@ fn do_mir_borrowck<'tcx>(
&body,
&regioncx,
&opt_closure_req,
&opaque_type_values,
&concrete_opaque_types,
&mut errors,
);

Expand Down Expand Up @@ -434,7 +434,7 @@ fn do_mir_borrowck<'tcx>(
let tainted_by_errors = mbcx.emit_errors();

let result = BorrowCheckResult {
concrete_opaque_types: opaque_type_values,
concrete_opaque_types,
closure_requirements: opt_closure_req,
used_mut_upvars: mbcx.used_mut_upvars,
tainted_by_errors,
Expand Down
16 changes: 8 additions & 8 deletions compiler/rustc_borrowck/src/nll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub type PoloniusOutput = Output<RustcFacts>;
/// closure requirements to propagate, and any generated errors.
pub(crate) struct NllOutput<'tcx> {
pub regioncx: RegionInferenceContext<'tcx>,
pub opaque_type_values: FxIndexMap<LocalDefId, OpaqueHiddenType<'tcx>>,
pub concrete_opaque_types: FxIndexMap<LocalDefId, ty::EarlyBinder<OpaqueHiddenType<'tcx>>>,
pub polonius_input: Option<Box<AllFacts>>,
pub polonius_output: Option<Rc<PoloniusOutput>>,
pub opt_closure_req: Option<ClosureRegionRequirements<'tcx>>,
Expand Down Expand Up @@ -302,7 +302,7 @@ pub(crate) fn compute_regions<'cx, 'tcx>(
});

// Solve the region constraints.
let (closure_region_requirements, nll_errors) =
let (opt_closure_req, nll_errors) =
regioncx.solve(infcx, param_env, &body, polonius_output.clone());

if !nll_errors.is_empty() {
Expand All @@ -313,14 +313,14 @@ pub(crate) fn compute_regions<'cx, 'tcx>(
));
}

let remapped_opaque_tys = regioncx.infer_opaque_types(&infcx, opaque_type_values);
let concrete_opaque_types = regioncx.infer_opaque_types(&infcx, opaque_type_values);

NllOutput {
regioncx,
opaque_type_values: remapped_opaque_tys,
concrete_opaque_types,
polonius_input: all_facts.map(Box::new),
polonius_output,
opt_closure_req: closure_region_requirements,
opt_closure_req,
nll_errors,
}
}
Expand Down Expand Up @@ -382,7 +382,7 @@ pub(super) fn dump_annotation<'tcx>(
body: &Body<'tcx>,
regioncx: &RegionInferenceContext<'tcx>,
closure_region_requirements: &Option<ClosureRegionRequirements<'tcx>>,
opaque_type_values: &FxIndexMap<LocalDefId, OpaqueHiddenType<'tcx>>,
concrete_opaque_types: &FxIndexMap<LocalDefId, ty::EarlyBinder<OpaqueHiddenType<'tcx>>>,
errors: &mut crate::error::BorrowckErrors<'tcx>,
) {
let tcx = infcx.tcx;
Expand Down Expand Up @@ -425,8 +425,8 @@ pub(super) fn dump_annotation<'tcx>(
err
};

if !opaque_type_values.is_empty() {
err.note(format!("Inferred opaque type values:\n{:#?}", opaque_type_values));
if !concrete_opaque_types.is_empty() {
err.note(format!("Inferred opaque type values:\n{:#?}", concrete_opaque_types));
}

errors.buffer_non_error_diag(err);
Expand Down
36 changes: 23 additions & 13 deletions compiler/rustc_borrowck/src/region_infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use rustc_infer::traits::{Obligation, ObligationCause};
use rustc_middle::traits::DefiningAnchor;
use rustc_middle::ty::subst::{GenericArgKind, InternalSubsts};
use rustc_middle::ty::visit::TypeVisitableExt;
use rustc_middle::ty::EarlyBinder;
use rustc_middle::ty::{self, OpaqueHiddenType, OpaqueTypeKey, Ty, TyCtxt, TypeFoldable};
use rustc_span::Span;
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _;
Expand Down Expand Up @@ -62,8 +63,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {
&self,
infcx: &InferCtxt<'tcx>,
opaque_ty_decls: FxIndexMap<OpaqueTypeKey<'tcx>, (OpaqueHiddenType<'tcx>, OpaqueTyOrigin)>,
) -> FxIndexMap<LocalDefId, OpaqueHiddenType<'tcx>> {
let mut result: FxIndexMap<LocalDefId, OpaqueHiddenType<'tcx>> = FxIndexMap::default();
) -> FxIndexMap<LocalDefId, EarlyBinder<OpaqueHiddenType<'tcx>>> {
let mut result: FxIndexMap<LocalDefId, EarlyBinder<OpaqueHiddenType<'tcx>>> =
FxIndexMap::default();

let member_constraints: FxIndexMap<_, _> = self
.member_constraints
Expand Down Expand Up @@ -144,11 +146,14 @@ impl<'tcx> RegionInferenceContext<'tcx> {
universal_concrete_type,
origin,
);

// Sometimes two opaque types are the same only after we remap the generic parameters
// back to the opaque type definition. E.g. we may have `OpaqueType<X, Y>` mapped to `(X, Y)`
// and `OpaqueType<Y, X>` mapped to `(Y, X)`, and those are the same, but we only know that
// once we convert the generic parameters to those of the opaque type.
if let Some(prev) = result.get_mut(&opaque_type_key.def_id) {
if let Some(stored_hidden_ty) = result.get_mut(&opaque_type_key.def_id) {
let mut prev = stored_hidden_ty.subst_identity();
let ty = ty.subst_identity();
if prev.ty != ty {
let guar = ty.error_reported().err().unwrap_or_else(|| {
prev.report_mismatch(
Expand All @@ -163,10 +168,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// Pick a better span if there is one.
// FIXME(oli-obk): collect multiple spans for better diagnostics down the road.
prev.span = prev.span.substitute_dummy(concrete_type.span);
*stored_hidden_ty = EarlyBinder(prev);
} else {
result.insert(
opaque_type_key.def_id,
OpaqueHiddenType { ty, span: concrete_type.span },
ty.map_bound(|ty| OpaqueHiddenType { ty, span: concrete_type.span }),
);
}
}
Expand Down Expand Up @@ -215,7 +221,7 @@ pub trait InferCtxtExt<'tcx> {
opaque_type_key: OpaqueTypeKey<'tcx>,
instantiated_ty: OpaqueHiddenType<'tcx>,
origin: OpaqueTyOrigin,
) -> Ty<'tcx>;
) -> EarlyBinder<Ty<'tcx>>;
}

impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> {
Expand Down Expand Up @@ -248,22 +254,22 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> {
opaque_type_key: OpaqueTypeKey<'tcx>,
instantiated_ty: OpaqueHiddenType<'tcx>,
origin: OpaqueTyOrigin,
) -> Ty<'tcx> {
) -> EarlyBinder<Ty<'tcx>> {
if let Some(e) = self.tainted_by_errors() {
return self.tcx.ty_error(e);
return EarlyBinder::dummy(self.tcx.ty_error(e));
}

let definition_ty = instantiated_ty
.remap_generic_params_to_declaration_params(opaque_type_key, self.tcx, false)
.ty;
.map_bound(|ty| ty.ty);

if let Err(guar) = check_opaque_type_parameter_valid(
self.tcx,
opaque_type_key,
origin,
instantiated_ty.span,
) {
return self.tcx.ty_error(guar);
return EarlyBinder::dummy(self.tcx.ty_error(guar));
}

// Only check this for TAIT. RPIT already supports `tests/ui/impl-trait/nested-return-type2.rs`
Expand All @@ -274,17 +280,21 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> {
let def_id = opaque_type_key.def_id;
// This logic duplicates most of `check_opaque_meets_bounds`.
// FIXME(oli-obk): Also do region checks here and then consider removing `check_opaque_meets_bounds` entirely.
//
// FIXME(lcnr): Move this check into a separate function which cannot access `self` here.
// This check runs in the context of the opaque type, not the defining scope.
let param_env = self.tcx.param_env(def_id);
// HACK This bubble is required for this tests to pass:
// nested-return-type2-tait2.rs
// nested-return-type2-tait3.rs
let infcx =
self.tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bubble).build();
let ocx = ObligationCtxt::new(&infcx);
let hidden_type = definition_ty.subst_identity();
// 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()));
let predicate = ty::PredicateKind::WellFormed(hidden_type.into());

let id_substs = InternalSubsts::identity_for_item(self.tcx, def_id);

Expand All @@ -295,14 +305,14 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> {
&ObligationCause::misc(instantiated_ty.span, def_id),
param_env,
opaque_ty,
definition_ty,
hidden_type,
) {
infcx
.err_ctxt()
.report_mismatched_types(
&ObligationCause::misc(instantiated_ty.span, def_id),
opaque_ty,
definition_ty,
hidden_type,
err,
)
.emit();
Expand All @@ -328,7 +338,7 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> {
definition_ty
} else {
let reported = infcx.err_ctxt().report_fulfillment_errors(&errors);
self.tcx.ty_error(reported)
EarlyBinder::dummy(self.tcx.ty_error(reported))
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1043,8 +1043,10 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
.concrete_opaque_types
.iter()
.map(|(&def_id, &hidden_ty)| {
// FIXME(-Ztrait-solver=next): Using the identity substs for the opaques
// is incorrect and should be fixed.
let substs = ty::InternalSubsts::identity_for_item(self.infcx.tcx, def_id);
(ty::OpaqueTypeKey { def_id, substs }, hidden_ty)
(ty::OpaqueTypeKey { def_id, substs }, hidden_ty.subst_identity())
})
.collect();

Expand Down
72 changes: 55 additions & 17 deletions compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{self as hir, Expr, ImplItem, Item, Node, TraitItem};
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt};
use rustc_span::DUMMY_SP;
use rustc_span::{ErrorGuaranteed, DUMMY_SP};

use crate::errors::UnconstrainedOpaqueType;

Expand Down Expand Up @@ -134,6 +134,8 @@ impl TaitConstraintLocator<'_> {
debug!("no constraints in typeck results");
return;
};

let typeck_hidden_ty = typeck_hidden_ty.subst_identity();
if self.typeck_types.iter().all(|prev| prev.ty != typeck_hidden_ty.ty) {
self.typeck_types.push(typeck_hidden_ty);
}
Expand All @@ -142,6 +144,7 @@ impl TaitConstraintLocator<'_> {
let concrete_opaque_types = &self.tcx.mir_borrowck(item_def_id).concrete_opaque_types;
debug!(?concrete_opaque_types);
if let Some(&concrete_type) = concrete_opaque_types.get(&self.def_id) {
let concrete_type = concrete_type.subst_identity();
debug!(?concrete_type, "found constraint");
if let Some(prev) = &mut self.found {
if concrete_type.ty != prev.ty && !(concrete_type, prev.ty).references_error() {
Expand Down Expand Up @@ -197,38 +200,70 @@ pub(super) fn find_opaque_ty_constraints_for_rpit(
) -> Ty<'_> {
let concrete = tcx.mir_borrowck(owner_def_id).concrete_opaque_types.get(&def_id).copied();

// In most cases the opaque type is defined by MIR typeck,
// in this case we check whether all nested bodies, e.g. closures,
// also have the same hidden type.
//
// If this is not the case we instead lookup the opaque type in the
// hir typeck results.
if let Some(concrete) = concrete {
let concrete = concrete.subst_identity();
let scope = tcx.hir().local_def_id_to_hir_id(owner_def_id);
debug!(?scope);
let mut locator = RpitConstraintChecker { def_id, tcx, found: concrete };
let mut locator =
RpitConstraintChecker { def_id, tcx, found: concrete, tainted_by_errors: None };

match tcx.hir().get(scope) {
Node::Item(it) => intravisit::walk_item(&mut locator, it),
Node::ImplItem(it) => intravisit::walk_impl_item(&mut locator, it),
Node::TraitItem(it) => intravisit::walk_trait_item(&mut locator, it),
other => bug!("{:?} is not a valid scope for an opaque type item", other),
}
}

concrete.map(|concrete| concrete.ty).unwrap_or_else(|| {
if let Some(guar) = locator.tainted_by_errors {
tcx.ty_error(guar)
} else {
// As a sanity check we also compare the type defined by mir typeck to
// the one defined by hir typeck. This should never fail unless there
// is some different error somewhere.
//
// We still return the type inferred by mir borrowck as we otherwise get
// ICE in tests with recursive opaque types. For those we only error
// when checking the opaque returned by `type_of`.
if let Some(typeck_ty) = tcx.typeck(owner_def_id).concrete_opaque_types.get(&def_id) {
let typeck_ty = typeck_ty.subst_identity();
if tcx.erase_regions(concrete.ty) != typeck_ty.ty {
concrete.report_mismatch(&typeck_ty, def_id, tcx).delay_as_bug();
}
} else {
tcx.sess.delay_span_bug(
concrete.span,
format!("opaque type defined in MIR not in HIR: {concrete:?}"),
);
}

concrete.ty
}
} else {
// If that isn't the case we use the type from HIR typeck.
let table = tcx.typeck(owner_def_id);
if let Some(guar) = table.tainted_by_errors {
// Some error in the
// owner fn prevented us from populating
// the `concrete_opaque_types` table.
tcx.ty_error(guar)
} else if let Some(ty) = table.concrete_opaque_types.get(&def_id) {
ty.subst_identity().ty
} else {
table.concrete_opaque_types.get(&def_id).map(|ty| ty.ty).unwrap_or_else(|| {
// We failed to resolve the opaque type or it
// resolves to itself. We interpret this as the
// no values of the hidden type ever being constructed,
// so we can just make the hidden type be `!`.
// For backwards compatibility reasons, we fall back to
// `()` until we the diverging default is changed.
tcx.mk_diverging_default()
})
// We failed to resolve the opaque type or it
// resolves to itself. We interpret this as the
// no values of the hidden type ever being constructed,
// so we can just make the hidden type be `!`.
// For backwards compatibility reasons, we fall back to
// `()` until we the diverging default is changed.
tcx.mk_diverging_default()
}
})
}
}

struct RpitConstraintChecker<'tcx> {
Expand All @@ -238,11 +273,13 @@ struct RpitConstraintChecker<'tcx> {
def_id: LocalDefId,

found: ty::OpaqueHiddenType<'tcx>,

tainted_by_errors: Option<ErrorGuaranteed>,
}

impl RpitConstraintChecker<'_> {
#[instrument(skip(self), level = "debug")]
fn check(&self, def_id: LocalDefId) {
fn check(&mut self, def_id: LocalDefId) {
// Use borrowck to get the type with unerased regions.
let concrete_opaque_types = &self.tcx.mir_borrowck(def_id).concrete_opaque_types;
debug!(?concrete_opaque_types);
Expand All @@ -253,10 +290,11 @@ impl RpitConstraintChecker<'_> {
}

debug!(?concrete_type, "found constraint");

let concrete_type = concrete_type.subst_identity();
if concrete_type.ty != self.found.ty && !(concrete_type, self.found).references_error()
{
self.found.report_mismatch(&concrete_type, self.def_id, self.tcx).emit();
let guar = self.found.report_mismatch(&concrete_type, self.def_id, self.tcx).emit();
self.tainted_by_errors = Some(guar);
}
}
}
Expand Down
Loading