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

Stop handling opaque types in queries and leave it to typeck #107891

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 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
15 changes: 7 additions & 8 deletions compiler/rustc_borrowck/src/region_infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,13 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> {
// 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.
let param_env = self.tcx.param_env(def_id);
// HACK This bubble is required for this tests to pass:
// type-alias-impl-trait/issue-67844-nested-opaque.rs
let infcx =
self.tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bubble).build();
let infcx = self
.tcx
.infer_ctxt()
// HACK This bubble is required for this tests to pass:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// HACK This bubble is required for this tests to pass:
// HACK This ignore is required for this tests to pass:

// type-alias-impl-trait/issue-67844-nested-opaque.rs
.with_opaque_type_inference(DefiningAnchor::Ignore)
.build();
let ocx = ObligationCtxt::new(&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
Expand Down Expand Up @@ -316,10 +319,6 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> {
// version.
let errors = ocx.select_all_or_error();

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

if errors.is_empty() {
definition_ty
} else {
Expand Down
17 changes: 9 additions & 8 deletions compiler/rustc_const_eval/src/util/compare_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,15 @@ pub fn is_subtype<'tcx>(
return true;
}

let mut builder =
tcx.infer_ctxt().ignoring_regions().with_opaque_type_inference(DefiningAnchor::Bubble);
let mut builder = tcx
.infer_ctxt()
.ignoring_regions()
// With `Reveal::All`, opaque types get normalized away, with `Reveal::UserFacing`
// we would get unification errors because we're unable to look into opaque types,
// even if they're constrained in our current function.
//
// It seems very unlikely that this hides any bugs.
.with_opaque_type_inference(DefiningAnchor::Ignore);
let infcx = builder.build();
let ocx = ObligationCtxt::new(&infcx);
let cause = ObligationCause::dummy();
Expand All @@ -53,11 +60,5 @@ pub fn is_subtype<'tcx>(
Err(_) => return false,
};
let errors = ocx.select_all_or_error();
// With `Reveal::All`, opaque types get normalized away, with `Reveal::UserFacing`
// we would get unification errors because we're unable to look into opaque types,
// even if they're constrained in our current function.
//
// It seems very unlikely that this hides any bugs.
let _ = infcx.take_opaque_types();
errors.is_empty()
}
39 changes: 5 additions & 34 deletions compiler/rustc_infer/src/infer/canonical/query_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ impl<'tcx> InferCtxt<'tcx> {
var_values: inference_vars,
region_constraints: QueryRegionConstraints::default(),
certainty: Certainty::Proven, // Ambiguities are OK!
opaque_types: vec![],
value: answer,
})
}
Expand Down Expand Up @@ -140,28 +139,14 @@ impl<'tcx> InferCtxt<'tcx> {
let certainty =
if ambig_errors.is_empty() { Certainty::Proven } else { Certainty::Ambiguous };

let opaque_types = self.take_opaque_types_for_query_response();

Ok(QueryResponse {
var_values: inference_vars,
region_constraints,
certainty,
value: answer,
opaque_types,
})
}

/// FIXME: This method should only be used for canonical queries and therefore be private.
///
/// As the new solver does canonicalization slightly differently, this is also used there
/// for now. This should hopefully change fairly soon.
pub fn take_opaque_types_for_query_response(&self) -> Vec<(Ty<'tcx>, Ty<'tcx>)> {
std::mem::take(&mut self.inner.borrow_mut().opaque_type_storage.opaque_types)
.into_iter()
.map(|(k, v)| (self.tcx.mk_opaque(k.def_id.to_def_id(), k.substs), v.hidden_type.ty))
.collect()
}

/// Given the (canonicalized) result to a canonical query,
/// instantiates the result so it can be used, plugging in the
/// values from the canonical query. (Note that the result may
Expand Down Expand Up @@ -244,8 +229,8 @@ impl<'tcx> InferCtxt<'tcx> {
where
R: Debug + TypeFoldable<'tcx>,
{
let InferOk { value: result_subst, mut obligations } = self
.query_response_substitution_guess(cause, param_env, original_values, query_response)?;
let InferOk { value: result_subst, mut obligations } =
self.query_response_substitution_guess(cause, original_values, query_response)?;

// Compute `QueryOutlivesConstraint` values that unify each of
// the original values `v_o` that was canonicalized into a
Expand Down Expand Up @@ -363,12 +348,8 @@ impl<'tcx> InferCtxt<'tcx> {
original_values, query_response,
);

let mut value = self.query_response_substitution_guess(
cause,
param_env,
original_values,
query_response,
)?;
let mut value =
self.query_response_substitution_guess(cause, original_values, query_response)?;

value.obligations.extend(
self.unify_query_response_substitution_guess(
Expand Down Expand Up @@ -396,7 +377,6 @@ impl<'tcx> InferCtxt<'tcx> {
fn query_response_substitution_guess<R>(
&self,
cause: &ObligationCause<'tcx>,
param_env: ty::ParamEnv<'tcx>,
original_values: &OriginalQueryValues<'tcx>,
query_response: &Canonical<'tcx, QueryResponse<'tcx, R>>,
) -> InferResult<'tcx, CanonicalVarValues<'tcx>>
Expand Down Expand Up @@ -496,16 +476,7 @@ impl<'tcx> InferCtxt<'tcx> {
)),
};

let mut obligations = vec![];

// Carry all newly resolved opaque types to the caller's scope
for &(a, b) in &query_response.value.opaque_types {
let a = substitute_value(self.tcx, &result_subst, a);
let b = substitute_value(self.tcx, &result_subst, b);
obligations.extend(self.at(cause, param_env).eq(a, b)?.obligations);
}

Ok(InferOk { value: result_subst, obligations })
Ok(InferOk { value: result_subst, obligations: vec![] })
}

/// Given a "guess" at the values for the canonical variables in
Expand Down
17 changes: 11 additions & 6 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,12 @@ impl<'tcx> InferCtxtInner<'tcx> {

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum DefiningAnchor {
/// `DefId` of the item.
/// `DefId` of the item for which we check whether any opaque types may
/// have their hidden types registered.
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 treat all opaque types as in their defining scope.
/// Used for coherence, codegen and sanity checks.
Ignore,
/// Used to catch type mismatch errors when handling opaque types.
Error,
}
Expand All @@ -259,7 +260,7 @@ pub struct InferCtxt<'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 `DefiningAnchor::Bubble`, we can't resolve opaque types here and need to bubble up
/// If it is `DefiningAnchor::Ignore`, 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.
Expand Down Expand Up @@ -1340,7 +1341,11 @@ impl<'tcx> InferCtxt<'tcx> {

#[instrument(level = "debug", skip(self), ret)]
pub fn take_opaque_types(&self) -> opaque_types::OpaqueTypeMap<'tcx> {
debug_assert_ne!(self.defining_use_anchor, DefiningAnchor::Error);
debug_assert!(
matches!(self.defining_use_anchor, DefiningAnchor::Bind(_)),
"{:?}",
self.defining_use_anchor
);
std::mem::take(&mut self.inner.borrow_mut().opaque_type_storage.opaque_types)
}

Expand Down
22 changes: 12 additions & 10 deletions compiler/rustc_infer/src/infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl<'tcx> InferCtxt<'tcx> {
// ```
self.opaque_type_origin(def_id)?
}
DefiningAnchor::Bubble => self.opaque_type_origin_unchecked(def_id),
DefiningAnchor::Ignore => self.opaque_type_origin_unchecked(def_id),
DefiningAnchor::Error => return None,
};
if let ty::Alias(ty::Opaque, ty::AliasTy { def_id: b_def_id, .. }) = *b.kind() {
Expand Down Expand Up @@ -374,7 +374,7 @@ impl<'tcx> InferCtxt<'tcx> {
pub fn opaque_type_origin(&self, def_id: LocalDefId) -> Option<OpaqueTyOrigin> {
let opaque_hir_id = self.tcx.hir().local_def_id_to_hir_id(def_id);
let parent_def_id = match self.defining_use_anchor {
DefiningAnchor::Bubble | DefiningAnchor::Error => return None,
DefiningAnchor::Ignore | DefiningAnchor::Error => return None,
DefiningAnchor::Bind(bind) => bind,
};

Expand Down Expand Up @@ -539,14 +539,16 @@ impl<'tcx> InferCtxt<'tcx> {
let span = cause.span;

let mut obligations = vec![];
let prev = self.inner.borrow_mut().opaque_types().register(
OpaqueTypeKey { def_id, substs },
OpaqueHiddenType { ty: hidden_ty, span },
origin,
);
if let Some(prev) = prev {
obligations =
self.at(&cause, param_env).eq_exp(a_is_expected, prev, hidden_ty)?.obligations;
if !matches!(self.defining_use_anchor, DefiningAnchor::Ignore) {
let prev = self.inner.borrow_mut().opaque_types().register(
OpaqueTypeKey { def_id, substs },
OpaqueHiddenType { ty: hidden_ty, span },
origin,
);
if let Some(prev) = prev {
obligations =
self.at(&cause, param_env).eq_exp(a_is_expected, prev, hidden_ty)?.obligations;
}
}

let item_bounds = tcx.bound_explicit_item_bounds(def_id.to_def_id());
Expand Down
6 changes: 0 additions & 6 deletions compiler/rustc_middle/src/infer/canonical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,6 @@ pub struct QueryResponse<'tcx, R> {
pub var_values: CanonicalVarValues<'tcx>,
pub region_constraints: QueryRegionConstraints<'tcx>,
pub certainty: Certainty,
/// List of opaque types which we tried to compare to another type.
/// Inside the query we don't know yet whether the opaque type actually
/// should get its hidden type inferred. So we bubble the opaque type
/// and the type it was compared against upwards and let the query caller
/// handle it.
pub opaque_types: Vec<(Ty<'tcx>, Ty<'tcx>)>,
pub value: R,
}

Expand Down
32 changes: 13 additions & 19 deletions compiler/rustc_middle/src/traits/solve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustc_data_structures::intern::Interned;

use crate::ty::{
ir::{self, TypeFoldable, TypeVisitable},
FallibleTypeFolder, Ty, TyCtxt, TypeFolder, TypeVisitor,
FallibleTypeFolder, TyCtxt, TypeFolder, TypeVisitor,
};

#[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)]
Expand All @@ -19,32 +19,27 @@ impl<'tcx> std::ops::Deref for ExternalConstraints<'tcx> {
}

/// Additional constraints returned on success.
#[derive(Debug, PartialEq, Eq, Clone, Hash, Default)]
#[derive(Debug, PartialEq, Eq, Clone, Hash)]
pub struct ExternalConstraintsData<'tcx> {
// FIXME: implement this.
pub regions: (),
pub opaque_types: Vec<(Ty<'tcx>, Ty<'tcx>)>,
pub regions: &'tcx (),
}

impl<'tcx> Default for ExternalConstraintsData<'tcx> {
fn default() -> Self {
Self { regions: &() }
}
}

impl<'tcx> TypeFoldable<TyCtxt<'tcx>> for ExternalConstraints<'tcx> {
fn try_fold_with<F: FallibleTypeFolder<'tcx>>(self, folder: &mut F) -> Result<Self, F::Error> {
Ok(ir::FallibleTypeFolder::interner(folder).intern_external_constraints(
ExternalConstraintsData {
regions: (),
opaque_types: self
.opaque_types
.iter()
.map(|opaque| opaque.try_fold_with(folder))
.collect::<Result<_, F::Error>>()?,
},
))
Ok(ir::FallibleTypeFolder::interner(folder)
.intern_external_constraints(ExternalConstraintsData { regions: &() }))
}

fn fold_with<F: TypeFolder<'tcx>>(self, folder: &mut F) -> Self {
ir::TypeFolder::interner(folder).intern_external_constraints(ExternalConstraintsData {
regions: (),
opaque_types: self.opaque_types.iter().map(|opaque| opaque.fold_with(folder)).collect(),
})
ir::TypeFolder::interner(folder)
.intern_external_constraints(ExternalConstraintsData { regions: &() })
}
}

Expand All @@ -54,7 +49,6 @@ impl<'tcx> TypeVisitable<TyCtxt<'tcx>> for ExternalConstraints<'tcx> {
visitor: &mut V,
) -> std::ops::ControlFlow<V::BreakTy> {
self.regions.visit_with(visitor)?;
self.opaque_types.visit_with(visitor)?;
ControlFlow::Continue(())
}
}
1 change: 1 addition & 0 deletions compiler/rustc_trait_selection/src/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ pub trait InferCtxtExt<'tcx> {
param_env: ty::ParamEnv<'tcx>,
) -> traits::EvaluationResult;
}

impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> {
fn type_is_copy_modulo_regions(
&self,
Expand Down
20 changes: 6 additions & 14 deletions compiler/rustc_trait_selection/src/solve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,9 @@ impl<'tcx> CanonicalResponseExt for Canonical<'tcx, Response<'tcx>> {
fn has_no_inference_or_external_constraints(&self) -> bool {
// so that we get a compile error when regions are supported
// so this code can be checked for being correct
let _: () = self.value.external_constraints.regions;
let _: &() = self.value.external_constraints.regions;

self.value.var_values.is_identity()
&& self.value.external_constraints.opaque_types.is_empty()
}
}

Expand Down Expand Up @@ -582,14 +581,11 @@ fn compute_external_query_constraints<'tcx>(
infcx: &InferCtxt<'tcx>,
) -> Result<ExternalConstraints<'tcx>, NoSolution> {
let region_obligations = infcx.take_registered_region_obligations();
let opaque_types = infcx.take_opaque_types_for_query_response();
Ok(infcx.tcx.intern_external_constraints(ExternalConstraintsData {
// FIXME: Now that's definitely wrong :)
//
// Should also do the leak check here I think
regions: drop(region_obligations),
opaque_types,
}))
// FIXME: Now that's definitely wrong :)
//
// Should also do the leak check here I think
drop(region_obligations);
Ok(infcx.tcx.intern_external_constraints(ExternalConstraintsData { regions: &() }))
}

fn instantiate_canonical_query_response<'tcx>(
Expand All @@ -609,10 +605,6 @@ fn instantiate_canonical_query_response<'tcx>(
Certainty::Yes => OldCertainty::Proven,
Certainty::Maybe(_) => OldCertainty::Ambiguous,
},
// FIXME: This to_owned makes me sad, but we should eventually impl
// `instantiate_query_response_and_region_obligations` separately
// instead of piggybacking off of the old implementation.
opaque_types: resp.external_constraints.opaque_types.to_owned(),
value: resp.certainty,
}),
) else { bug!(); };
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ pub fn overlapping_impls(
}

let infcx =
tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bubble).intercrate().build();
tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Ignore).intercrate().build();
let selcx = &mut SelectionContext::new(&infcx);
let overlaps =
overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id, overlap_mode).is_some();
Expand All @@ -108,7 +108,7 @@ pub fn overlapping_impls(
// this time tracking intercrate ambiguity causes for better
// diagnostics. (These take time and can lead to false errors.)
let infcx =
tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Bubble).intercrate().build();
tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::Ignore).intercrate().build();
let selcx = &mut SelectionContext::new(&infcx);
selcx.enable_tracking_intercrate_ambiguity_causes();
Some(overlap(selcx, skip_leak_check, impl1_def_id, impl2_def_id, overlap_mode).unwrap())
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_traits/src/chalk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ pub(crate) fn evaluate_goal<'tcx>(
var_values: CanonicalVarValues { var_values },
region_constraints: QueryRegionConstraints::default(),
certainty: Certainty::Proven,
opaque_types: vec![],
value: (),
},
};
Expand Down Expand Up @@ -159,7 +158,6 @@ pub(crate) fn evaluate_goal<'tcx>(
var_values: CanonicalVarValues::dummy(),
region_constraints: QueryRegionConstraints::default(),
certainty: Certainty::Ambiguous,
opaque_types: vec![],
value: (),
},
};
Expand Down
Loading