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

Pass list of defineable opaque types into canonical queries #122077

Merged
merged 13 commits into from
Apr 9, 2024
Merged
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
3 changes: 1 addition & 2 deletions compiler/rustc_borrowck/src/consumers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use rustc_hir::def_id::LocalDefId;
use rustc_index::{IndexSlice, IndexVec};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::mir::{Body, Promoted};
use rustc_middle::traits::DefiningAnchor;
use rustc_middle::ty::TyCtxt;
use std::rc::Rc;

Expand Down Expand Up @@ -106,7 +105,7 @@ pub fn get_body_with_borrowck_facts(
options: ConsumerOptions,
) -> BodyWithBorrowckFacts<'_> {
let (input_body, promoted) = tcx.mir_promoted(def);
let infcx = tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::bind(tcx, def)).build();
let infcx = tcx.infer_ctxt().with_opaque_type_inference(def).build();
let input_body: &Body<'_> = &input_body.borrow();
let promoted: &IndexSlice<_, _> = &promoted.borrow();
*super::do_mir_borrowck(&infcx, input_body, promoted, Some(options)).1.unwrap()
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ use rustc_infer::infer::{
use rustc_middle::mir::tcx::PlaceTy;
use rustc_middle::mir::*;
use rustc_middle::query::Providers;
use rustc_middle::traits::DefiningAnchor;
use rustc_middle::ty::{self, ParamEnv, RegionVid, TyCtxt};
use rustc_session::lint::builtin::UNUSED_MUT;
use rustc_span::{Span, Symbol};
Expand Down Expand Up @@ -126,7 +125,7 @@ fn mir_borrowck(tcx: TyCtxt<'_>, def: LocalDefId) -> &BorrowCheckResult<'_> {
return tcx.arena.alloc(result);
}

let infcx = tcx.infer_ctxt().with_opaque_type_inference(DefiningAnchor::bind(tcx, def)).build();
let infcx = tcx.infer_ctxt().with_opaque_type_inference(def).build();
let promoted: &IndexSlice<_, _> = &promoted.borrow();
let opt_closure_req = do_mir_borrowck(&infcx, input_body, promoted, None).0;
debug!("mir_borrowck done");
Expand Down
19 changes: 15 additions & 4 deletions compiler/rustc_borrowck/src/region_infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use rustc_infer::infer::TyCtxtInferExt as _;
use rustc_infer::infer::{InferCtxt, NllRegionVariableOrigin};
use rustc_infer::traits::{Obligation, ObligationCause};
use rustc_macros::extension;
use rustc_middle::traits::DefiningAnchor;
use rustc_middle::ty::visit::TypeVisitableExt;
use rustc_middle::ty::{self, OpaqueHiddenType, OpaqueTypeKey, Ty, TyCtxt, TypeFoldable};
use rustc_middle::ty::{GenericArgKind, GenericArgs};
Expand Down Expand Up @@ -133,6 +132,18 @@ impl<'tcx> RegionInferenceContext<'tcx> {

let ty =
infcx.infer_opaque_definition_from_instantiation(opaque_type_key, concrete_type);

// Sometimes, when the hidden type is an inference variable, it can happen that
// the hidden type becomes the opaque type itself. In this case, this was an opaque
// usage of the opaque type and we can ignore it. This check is mirrored in typeck's
// writeback.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// writeback.
// FIXME(-Znext-solver): This should be unnecessary with the new solver.

Also, can this not result in an overflow? If you have Opaque = OPaque in the storage and then equate it with u32.

It tries to assign u32 to Opaque, which fetches the hidden type of Opaque which is OPaque again, causing the equate of the previous hidden type with u32 to use handle_opaque_type again?

Copy link
Member

@aliemjay aliemjay Mar 22, 2024

Choose a reason for hiding this comment

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

By taking a glance, I guess it fixes the first example of #115017. Please add it as a test.
It is interesting to see how it interacts with this issue as a whole.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The first example already works on master

// FIXME(-Znext-solver): This should be unnecessary with the new solver.
if let ty::Alias(ty::Opaque, alias_ty) = ty.kind()
&& alias_ty.def_id == opaque_type_key.def_id.to_def_id()
&& alias_ty.args == opaque_type_key.args
{
continue;
}
// 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
Expand Down Expand Up @@ -321,13 +332,13 @@ fn check_opaque_type_well_formed<'tcx>(
parent_def_id = tcx.local_parent(parent_def_id);
}

// FIXME(-Znext-solver): We probably should use `DefiningAnchor::Bind(&[])`
// FIXME(-Znext-solver): We probably should use `&[]` instead of
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
// and prepopulate this `InferCtxt` with known opaque values, rather than
// using the `Bind` anchor here. For now it's fine.
// allowing opaque types to be defined and checking them after the fact.
let infcx = tcx
.infer_ctxt()
.with_next_trait_solver(next_trait_solver)
.with_opaque_type_inference(DefiningAnchor::bind(tcx, parent_def_id))
.with_opaque_type_inference(parent_def_id)
.build();
let ocx = ObligationCtxt::new(&infcx);
let identity_args = GenericArgs::identity_for_item(tcx, def_id);
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_borrowck/src/type_check/canonical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {

let TypeOpOutput { output, constraints, error_info } =
op.fully_perform(self.infcx, locations.span(self.body))?;
if cfg!(debug_assertions) {
let data = self.infcx.take_and_reset_region_constraints();
if !data.is_empty() {
panic!("leftover region constraints: {data:#?}");
}
}

debug!(?output, ?constraints);

Expand Down
9 changes: 3 additions & 6 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_infer::infer::{RegionVariableOrigin, TyCtxtInferExt};
use rustc_infer::traits::{Obligation, TraitEngineExt as _};
use rustc_lint_defs::builtin::REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS;
use rustc_middle::middle::stability::EvalResult;
use rustc_middle::traits::{DefiningAnchor, ObligationCauseCode};
use rustc_middle::traits::ObligationCauseCode;
use rustc_middle::ty::fold::BottomUpFolder;
use rustc_middle::ty::layout::{LayoutError, MAX_SIMD_LANES};
use rustc_middle::ty::util::{Discr, InspectCoroutineFields, IntTypeExt};
Expand Down Expand Up @@ -345,10 +345,7 @@ fn check_opaque_meets_bounds<'tcx>(
};
let param_env = tcx.param_env(defining_use_anchor);

let infcx = tcx
.infer_ctxt()
.with_opaque_type_inference(DefiningAnchor::bind(tcx, defining_use_anchor))
.build();
let infcx = tcx.infer_ctxt().with_opaque_type_inference(defining_use_anchor).build();
let ocx = ObligationCtxt::new(&infcx);

let args = match *origin {
Expand Down Expand Up @@ -1567,7 +1564,7 @@ pub(super) fn check_coroutine_obligations(
.ignoring_regions()
// Bind opaque types to type checking root, as they should have been checked by borrowck,
// but may show up in some cases, like when (root) obligations are stalled in the new solver.
.with_opaque_type_inference(DefiningAnchor::bind(tcx, typeck.hir_owner.def_id))
.with_opaque_type_inference(typeck.hir_owner.def_id)
.build();

let mut fulfillment_cx = <dyn TraitEngine<'_>>::new(&infcx);
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -730,8 +730,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
for ty in ret_ty.walk() {
if let ty::GenericArgKind::Type(ty) = ty.unpack()
&& let ty::Alias(ty::Opaque, ty::AliasTy { def_id, .. }) = *ty.kind()
&& let Some(def_id) = def_id.as_local()
&& self.opaque_type_origin(def_id).is_some()
&& self.can_define_opaque_ty(def_id)
{
return None;
}
Expand Down
7 changes: 1 addition & 6 deletions compiler/rustc_hir_typeck/src/typeck_root_ctxt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use rustc_hir as hir;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::HirIdMap;
use rustc_infer::infer::{InferCtxt, InferOk, TyCtxtInferExt};
use rustc_middle::traits::DefiningAnchor;
use rustc_middle::ty::visit::TypeVisitableExt;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_span::def_id::LocalDefIdMap;
Expand Down Expand Up @@ -78,11 +77,7 @@ impl<'tcx> TypeckRootCtxt<'tcx> {
pub fn new(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> Self {
let hir_owner = tcx.local_def_id_to_hir_id(def_id).owner;

let infcx = tcx
.infer_ctxt()
.ignoring_regions()
.with_opaque_type_inference(DefiningAnchor::bind(tcx, def_id))
.build();
let infcx = tcx.infer_ctxt().ignoring_regions().with_opaque_type_inference(def_id).build();
let typeck_results = RefCell::new(ty::TypeckResults::new(hir_owner));

TypeckRootCtxt {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/at.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl<'tcx> InferCtxt<'tcx> {
pub fn fork_with_intercrate(&self, intercrate: bool) -> Self {
Self {
tcx: self.tcx,
defining_use_anchor: self.defining_use_anchor,
defining_opaque_types: self.defining_opaque_types,
considering_regions: self.considering_regions,
skip_leak_check: self.skip_leak_check,
inner: self.inner.clone(),
Expand Down
15 changes: 13 additions & 2 deletions compiler/rustc_infer/src/infer/canonical/canonicalizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl<'tcx> InferCtxt<'tcx> {
V: TypeFoldable<TyCtxt<'tcx>>,
{
let (param_env, value) = value.into_parts();
let param_env = self.tcx.canonical_param_env_cache.get_or_insert(
let mut param_env = self.tcx.canonical_param_env_cache.get_or_insert(
self.tcx,
param_env,
query_state,
Expand All @@ -59,6 +59,8 @@ impl<'tcx> InferCtxt<'tcx> {
},
);

param_env.defining_opaque_types = self.defining_opaque_types;

Canonicalizer::canonicalize_with_base(
param_env,
value,
Expand Down Expand Up @@ -540,6 +542,7 @@ impl<'cx, 'tcx> Canonicalizer<'cx, 'tcx> {
max_universe: ty::UniverseIndex::ROOT,
variables: List::empty(),
value: (),
defining_opaque_types: infcx.map(|i| i.defining_opaque_types).unwrap_or_default(),
};
Canonicalizer::canonicalize_with_base(
base,
Expand Down Expand Up @@ -609,7 +612,15 @@ impl<'cx, 'tcx> Canonicalizer<'cx, 'tcx> {
.max()
.unwrap_or(ty::UniverseIndex::ROOT);

Canonical { max_universe, variables: canonical_variables, value: (base.value, out_value) }
assert!(
!infcx.is_some_and(|infcx| infcx.defining_opaque_types != base.defining_opaque_types)
);
Canonical {
aliemjay marked this conversation as resolved.
Show resolved Hide resolved
max_universe,
variables: canonical_variables,
value: (base.value, out_value),
defining_opaque_types: base.defining_opaque_types,
}
}

/// Creates a canonical variable replacing `kind` from the input,
Expand Down
9 changes: 3 additions & 6 deletions compiler/rustc_infer/src/infer/canonical/query_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,12 +505,9 @@ impl<'tcx> InferCtxt<'tcx> {
let b = instantiate_value(self.tcx, &result_args, b);
debug!(?a, ?b, "constrain opaque type");
// We use equate here instead of, for example, just registering the
// opaque type's hidden value directly, because we may be instantiating
// a query response that was canonicalized in an InferCtxt that had
// a different defining anchor. In that case, we may have inferred
// `NonLocalOpaque := LocalOpaque` but can only instantiate it in
// the other direction as `LocalOpaque := NonLocalOpaque`. Using eq
// here allows us to try both directions (in `InferCtxt::handle_opaque_type`).
// opaque type's hidden value directly, because the hidden type may have been an inference
// variable that got constrained to the opaque type itself. In that case we want to equate
// the generic args of the opaque with the generic params of its hidden type version.
obligations.extend(
self.at(cause, param_env)
.eq(
Expand Down
50 changes: 29 additions & 21 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use rustc_middle::infer::unify_key::{ConstVariableOrigin, ConstVariableOriginKin
use rustc_middle::infer::unify_key::{ConstVidKey, EffectVidKey};
use rustc_middle::mir::interpret::{ErrorHandled, EvalToValTreeResult};
use rustc_middle::mir::ConstraintCategory;
use rustc_middle::traits::{select, DefiningAnchor};
use rustc_middle::traits::select;
use rustc_middle::ty::error::{ExpectedFound, TypeError};
use rustc_middle::ty::fold::BoundVarReplacerDelegate;
use rustc_middle::ty::fold::{TypeFoldable, TypeFolder, TypeSuperFoldable};
Expand Down Expand Up @@ -243,18 +243,8 @@ impl<'tcx> InferCtxtInner<'tcx> {
pub struct InferCtxt<'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 `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.
///
/// Its default value is `DefiningAnchor::Bind(&[])`, which means no opaque types may be defined.
/// This way it is easier to catch errors that
/// might come up during inference or typeck.
pub defining_use_anchor: DefiningAnchor<'tcx>,
/// The `DefIds` of the opaque types that may have their hidden types constrained.
defining_opaque_types: &'tcx ty::List<LocalDefId>,

/// Whether this inference context should care about region obligations in
/// the root universe. Most notably, this is used during hir typeck as region
Expand Down Expand Up @@ -401,6 +391,10 @@ impl<'tcx> ty::InferCtxtLike for InferCtxt<'tcx> {
fn probe_ct_var(&self, vid: ConstVid) -> Option<ty::Const<'tcx>> {
self.probe_const_var(vid).ok()
}

fn defining_opaque_types(&self) -> &'tcx ty::List<LocalDefId> {
self.defining_opaque_types
}
}

/// See the `error_reporting` module for more details.
Expand Down Expand Up @@ -615,7 +609,7 @@ impl fmt::Display for FixupError {
/// Used to configure inference contexts before their creation.
pub struct InferCtxtBuilder<'tcx> {
tcx: TyCtxt<'tcx>,
defining_use_anchor: DefiningAnchor<'tcx>,
defining_opaque_types: &'tcx ty::List<LocalDefId>,
considering_regions: bool,
skip_leak_check: bool,
/// Whether we are in coherence mode.
Expand All @@ -630,7 +624,7 @@ impl<'tcx> TyCtxt<'tcx> {
fn infer_ctxt(self) -> InferCtxtBuilder<'tcx> {
InferCtxtBuilder {
tcx: self,
defining_use_anchor: DefiningAnchor::Bind(ty::List::empty()),
defining_opaque_types: ty::List::empty(),
considering_regions: true,
skip_leak_check: false,
intercrate: false,
Expand All @@ -646,8 +640,16 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
/// It is only meant to be called in two places, for typeck
/// (via `Inherited::build`) and for the inference context used
/// in mir borrowck.
pub fn with_opaque_type_inference(mut self, defining_use_anchor: DefiningAnchor<'tcx>) -> Self {
self.defining_use_anchor = defining_use_anchor;
pub fn with_opaque_type_inference(mut self, defining_anchor: LocalDefId) -> Self {
self.defining_opaque_types = self.tcx.opaque_types_defined_by(defining_anchor);
self
}

pub fn with_defining_opaque_types(
mut self,
defining_opaque_types: &'tcx ty::List<LocalDefId>,
) -> Self {
self.defining_opaque_types = defining_opaque_types;
self
}

Expand Down Expand Up @@ -679,30 +681,30 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
/// the bound values in `C` to their instantiated values in `V`
/// (in other words, `S(C) = V`).
pub fn build_with_canonical<T>(
&mut self,
self,
span: Span,
canonical: &Canonical<'tcx, T>,
) -> (InferCtxt<'tcx>, T, CanonicalVarValues<'tcx>)
where
T: TypeFoldable<TyCtxt<'tcx>>,
{
let infcx = self.build();
let infcx = self.with_defining_opaque_types(canonical.defining_opaque_types).build();
let (value, args) = infcx.instantiate_canonical(span, canonical);
(infcx, value, args)
}

pub fn build(&mut self) -> InferCtxt<'tcx> {
let InferCtxtBuilder {
tcx,
defining_use_anchor,
defining_opaque_types,
considering_regions,
skip_leak_check,
intercrate,
next_trait_solver,
} = *self;
InferCtxt {
tcx,
defining_use_anchor,
defining_opaque_types,
considering_regions,
skip_leak_check,
inner: RefCell::new(InferCtxtInner::new()),
Expand Down Expand Up @@ -1230,6 +1232,12 @@ impl<'tcx> InferCtxt<'tcx> {
self.inner.borrow().opaque_type_storage.opaque_types.clone()
}

#[inline(always)]
pub fn can_define_opaque_ty(&self, id: impl Into<DefId>) -> bool {
let Some(id) = id.into().as_local() else { return false };
self.defining_opaque_types.contains(&id)
}

pub fn ty_to_string(&self, t: Ty<'tcx>) -> String {
self.resolve_vars_if_possible(t).to_string()
}
Expand Down
Loading
Loading