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

Preserve substs in opaques recorded in typeck results #111980

Merged
merged 3 commits into from
Jun 1, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
14 changes: 12 additions & 2 deletions compiler/rustc_borrowck/src/region_infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,18 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> {
// 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();
// FIXME(-Ztrait-solver=next): We probably should use `DefiningAnchor::Error`
// and prepopulate this `InferCtxt` with known opaque values, rather than
// using the `Bind` anchor here. For now it's fine.
let infcx = self
.tcx
.infer_ctxt()
.with_opaque_type_inference(if self.tcx.trait_solver_next() {
DefiningAnchor::Bind(def_id)
} else {
DefiningAnchor::Bubble
})
compiler-errors marked this conversation as resolved.
Show resolved Hide resolved
.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
8 changes: 1 addition & 7 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,6 @@ pub(crate) fn type_check<'mir, 'tcx>(

// FIXME(-Ztrait-solver=next): A bit dubious that we're only registering
// predefined opaques in the typeck root.
// FIXME(-Ztrait-solver=next): This is also totally wrong for TAITs, since
// the HIR typeck map defining usages back to their definition params,
// they won't actually match up with the usages in this body...
if infcx.tcx.trait_solver_next() && !infcx.tcx.is_typeck_child(body.source.def_id()) {
checker.register_predefined_opaques_in_new_solver();
}
Expand Down Expand Up @@ -1042,10 +1039,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
.typeck(self.body.source.def_id().expect_local())
.concrete_opaque_types
.iter()
.map(|(&def_id, &hidden_ty)| {
let substs = ty::InternalSubsts::identity_for_item(self.infcx.tcx, def_id);
(ty::OpaqueTypeKey { def_id, substs }, hidden_ty)
})
.map(|(k, v)| (*k, *v))
.collect();

let renumbered_opaques = self.infcx.tcx.fold_regions(opaques, |_, _| {
Expand Down
94 changes: 66 additions & 28 deletions compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,20 @@ pub(super) fn find_opaque_ty_constraints_for_tait(tcx: TyCtxt<'_>, def_id: Local
}
}

let Some(hidden) = locator.found else {
if let Some(hidden) = locator.found {
// Only check against typeck if we didn't already error
if !hidden.ty.references_error() {
for concrete_type in locator.typeck_types {
if concrete_type.ty != tcx.erase_regions(hidden.ty)
&& !(concrete_type, hidden).references_error()
{
hidden.report_mismatch(&concrete_type, def_id, tcx).emit();
}
}
}

hidden.ty
} else {
let reported = tcx.sess.emit_err(UnconstrainedOpaqueType {
span: tcx.def_span(def_id),
name: tcx.item_name(tcx.local_parent(def_id).to_def_id()),
Expand All @@ -70,21 +83,8 @@ pub(super) fn find_opaque_ty_constraints_for_tait(tcx: TyCtxt<'_>, def_id: Local
_ => "item",
},
});
return tcx.ty_error(reported);
};

// Only check against typeck if we didn't already error
if !hidden.ty.references_error() {
for concrete_type in locator.typeck_types {
if concrete_type.ty != tcx.erase_regions(hidden.ty)
&& !(concrete_type, hidden).references_error()
{
hidden.report_mismatch(&concrete_type, def_id, tcx).emit();
}
}
tcx.ty_error(reported)
}

hidden.ty
}

struct TaitConstraintLocator<'tcx> {
Expand Down Expand Up @@ -130,13 +130,28 @@ impl TaitConstraintLocator<'_> {
self.found = Some(ty::OpaqueHiddenType { span: DUMMY_SP, ty: self.tcx.ty_error(guar) });
return;
}
let Some(&typeck_hidden_ty) = tables.concrete_opaque_types.get(&self.def_id) else {

let mut constrained = false;
for (&opaque_type_key, &hidden_type) in &tables.concrete_opaque_types {
if opaque_type_key.def_id != self.def_id {
continue;
}
constrained = true;
let concrete_type =
self.tcx.erase_regions(hidden_type.remap_generic_params_to_declaration_params(
opaque_type_key,
self.tcx,
true,
));
if self.typeck_types.iter().all(|prev| prev.ty != concrete_type.ty) {
self.typeck_types.push(concrete_type);
}
}

if !constrained {
debug!("no constraints in typeck results");
return;
};
if self.typeck_types.iter().all(|prev| prev.ty != typeck_hidden_ty.ty) {
self.typeck_types.push(typeck_hidden_ty);
}

// Use borrowck to get the type with unerased regions.
let concrete_opaque_types = &self.tcx.mir_borrowck(item_def_id).concrete_opaque_types;
Expand Down Expand Up @@ -190,8 +205,8 @@ impl<'tcx> intravisit::Visitor<'tcx> for TaitConstraintLocator<'tcx> {
}
}

pub(super) fn find_opaque_ty_constraints_for_rpit(
tcx: TyCtxt<'_>,
pub(super) fn find_opaque_ty_constraints_for_rpit<'tcx>(
tcx: TyCtxt<'tcx>,
def_id: LocalDefId,
owner_def_id: LocalDefId,
) -> Ty<'_> {
Expand All @@ -208,27 +223,50 @@ pub(super) fn find_opaque_ty_constraints_for_rpit(
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(|| {
let table = tcx.typeck(owner_def_id);
if let Some(guar) = table.tainted_by_errors {
concrete.ty
} else {
let tables = tcx.typeck(owner_def_id);
if let Some(guar) = tables.tainted_by_errors {
// Some error in the
// owner fn prevented us from populating
// the `concrete_opaque_types` table.
tcx.ty_error(guar)
} else {
table.concrete_opaque_types.get(&def_id).map(|ty| ty.ty).unwrap_or_else(|| {
// Fall back to the RPIT we inferred during HIR typeck
let mut opaque_ty: Option<ty::OpaqueHiddenType<'tcx>> = None;
for (&opaque_type_key, &hidden_type) in &tables.concrete_opaque_types {
if opaque_type_key.def_id != def_id {
continue;
}
let concrete_type =
tcx.erase_regions(hidden_type.remap_generic_params_to_declaration_params(
opaque_type_key,
tcx,
true,
));
if let Some(prev) = &mut opaque_ty {
if concrete_type.ty != prev.ty && !(concrete_type, prev.ty).references_error() {
prev.report_mismatch(&concrete_type, def_id, tcx).emit();
}
} else {
opaque_ty = Some(concrete_type);
}
}

if let Some(opaque_ty) = opaque_ty {
opaque_ty.ty
} 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()
})
}
}
})
}
}

struct RpitConstraintChecker<'tcx> {
Expand Down
12 changes: 4 additions & 8 deletions compiler/rustc_hir_typeck/src/writeback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -583,19 +583,15 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
continue;
}

let hidden_type =
self.tcx().erase_regions(hidden_type.remap_generic_params_to_declaration_params(
opaque_type_key,
self.tcx(),
true,
));

// Here we only detect impl trait definition conflicts when they
// are equal modulo regions.
if let Some(last_opaque_ty) = self
.typeck_results
.concrete_opaque_types
.insert(opaque_type_key.def_id, hidden_type)
.insert(opaque_type_key, hidden_type)
&& last_opaque_ty.ty != hidden_type.ty
{
assert!(!self.tcx().trait_solver_next());
hidden_type
.report_mismatch(&last_opaque_ty, opaque_type_key.def_id, self.tcx())
.stash(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/typeck_results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ pub struct TypeckResults<'tcx> {
/// These types are mapped back to the opaque's identity substitutions
/// (with erased regions), which is why we don't associated substs with any
/// of these usages.
compiler-errors marked this conversation as resolved.
Show resolved Hide resolved
pub concrete_opaque_types: FxIndexMap<LocalDefId, ty::OpaqueHiddenType<'tcx>>,
pub concrete_opaque_types: FxIndexMap<ty::OpaqueTypeKey<'tcx>, ty::OpaqueHiddenType<'tcx>>,

/// Tracks the minimum captures required for a closure;
/// see `MinCaptureInformationMap` for more details.
Expand Down
19 changes: 19 additions & 0 deletions tests/ui/traits/new-solver/dont-remap-tait-substs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// compile-flags: -Ztrait-solver=next
// check-pass

// Makes sure we don't prepopulate the MIR typeck of `define`
// with `Foo<T, U> = T`, but instead, `Foo<B, A> = B`, so that
// the param-env predicates actually apply.

#![feature(type_alias_impl_trait)]

type Foo<T: Send, U> = impl NeedsSend<T>;

trait NeedsSend<T> {}
impl<T: Send> NeedsSend<T> for T {}

fn define<A, B: Send>(a: A, b: B) {
let y: Option<Foo<B, A>> = Some(b);
}

fn main() {}
2 changes: 2 additions & 0 deletions tests/ui/type-alias-impl-trait/cross_inference.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// revisions: current next
//[next] compile-flags: -Ztrait-solver=next
// check-pass

#![feature(type_alias_impl_trait)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ type X<A, B> = impl Into<&'static A>;

fn f<A, B: 'static>(a: &'static A, b: B) -> (X<A, B>, X<B, A>) {
//~^ ERROR the trait bound `&'static B: From<&A>` is not satisfied
//~| ERROR concrete type differs from previous defining opaque type use
(a, a)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,6 @@ help: consider introducing a `where` clause, but there might be an alternative b
LL | fn f<A, B: 'static>(a: &'static A, b: B) -> (X<A, B>, X<B, A>) where &'static B: From<&A> {
| ++++++++++++++++++++++++++

error: concrete type differs from previous defining opaque type use
--> $DIR/multiple-def-uses-in-one-fn.rs:9:45
|
LL | fn f<A, B: 'static>(a: &'static A, b: B) -> (X<A, B>, X<B, A>) {
| ^^^^^^^^^^^^^^^^^^
| |
| expected `&B`, got `&A`
| this expression supplies two conflicting concrete types for the same opaque type

error: aborting due to 2 previous errors
error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.