Skip to content

Commit

Permalink
Rollup merge of #99079 - compiler-errors:issue-99073, r=oli-obk
Browse files Browse the repository at this point in the history
Check that RPITs constrained by a recursive call in a closure are compatible

Fixes #99073

Adapts a similar visitor pattern to `find_opaque_ty_constraints` (that we use to check TAITs), but with some changes:
0. Only walk the "OnlyBody" children, instead of all items in the RPIT's defining scope
1. Only walk through the body's children if we found a constraining usage
2. Don't actually do any inference, just do a comparison and error if they're mismatched

----

r? `@oli-obk` -- you know all this impl-trait stuff best... is this the right approach? I can explain the underlying issue better if you'd like, in case that might reveal a better solution. Not sure if it's possible to gather up the closure's defining usages of the RPIT while borrowck'ing the outer function, that might be a better place to put this check...
  • Loading branch information
JohnTitor authored Jul 27, 2022
2 parents ff693dc + 5e8f1e8 commit 3b780fc
Show file tree
Hide file tree
Showing 7 changed files with 140 additions and 72 deletions.
21 changes: 0 additions & 21 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKi
use rustc_infer::infer::{
InferCtxt, InferOk, LateBoundRegion, LateBoundRegionConversionTime, NllRegionVariableOrigin,
};
use rustc_infer::traits::ObligationCause;
use rustc_middle::mir::tcx::PlaceTy;
use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor};
use rustc_middle::mir::AssertKind;
Expand Down Expand Up @@ -225,26 +224,6 @@ pub(crate) fn type_check<'mir, 'tcx>(
)
.unwrap();
let mut hidden_type = infcx.resolve_vars_if_possible(decl.hidden_type);
// Check that RPITs are only constrained in their outermost
// function, otherwise report a mismatched types error.
if let OpaqueTyOrigin::FnReturn(parent) | OpaqueTyOrigin::AsyncFn(parent)
= infcx.opaque_ty_origin_unchecked(opaque_type_key.def_id, hidden_type.span)
&& parent.to_def_id() != body.source.def_id()
{
infcx
.report_mismatched_types(
&ObligationCause::misc(
hidden_type.span,
infcx.tcx.hir().local_def_id_to_hir_id(
body.source.def_id().expect_local(),
),
),
infcx.tcx.mk_opaque(opaque_type_key.def_id.to_def_id(), opaque_type_key.substs),
hidden_type.ty,
ty::error::TypeError::Mismatch,
)
.emit();
}
trace!(
"finalized opaque type {:?} to {:#?}",
opaque_type_key,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}

#[instrument(skip(self), level = "trace")]
pub fn opaque_ty_origin_unchecked(&self, def_id: LocalDefId, span: Span) -> OpaqueTyOrigin {
fn opaque_ty_origin_unchecked(&self, def_id: LocalDefId, span: Span) -> OpaqueTyOrigin {
let origin = match self.tcx.hir().expect_item(def_id).kind {
hir::ItemKind::OpaqueTy(hir::OpaqueTy { origin, .. }) => origin,
ref itemkind => {
Expand Down
148 changes: 119 additions & 29 deletions compiler/rustc_typeck/src/collect/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,37 +335,11 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
tcx.mk_adt(def, substs)
}
ItemKind::OpaqueTy(OpaqueTy { origin: hir::OpaqueTyOrigin::TyAlias, .. }) => {
find_opaque_ty_constraints(tcx, def_id)
find_opaque_ty_constraints_for_tait(tcx, def_id)
}
// Opaque types desugared from `impl Trait`.
ItemKind::OpaqueTy(OpaqueTy { origin: hir::OpaqueTyOrigin::FnReturn(owner) | hir::OpaqueTyOrigin::AsyncFn(owner), .. }) => {
let concrete_ty = tcx
.mir_borrowck(owner)
.concrete_opaque_types
.get(&def_id)
.copied()
.map(|concrete| concrete.ty)
.unwrap_or_else(|| {
let table = tcx.typeck(owner);
if let Some(_) = table.tainted_by_errors {
// Some error in the
// owner fn prevented us from populating
// the `concrete_opaque_types` table.
tcx.ty_error()
} else {
table.concrete_opaque_types.get(&def_id).copied().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.
Some(tcx.mk_diverging_default())
}).expect("RPIT always have a hidden type from typeck")
}
});
debug!("concrete_ty = {:?}", concrete_ty);
concrete_ty
find_opaque_ty_constraints_for_rpit(tcx, def_id, owner)
}
ItemKind::Trait(..)
| ItemKind::TraitAlias(..)
Expand Down Expand Up @@ -519,7 +493,7 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
/// fn b<T>() -> Foo<T, u32> { .. }
/// ```
///
fn find_opaque_ty_constraints(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Ty<'_> {
fn find_opaque_ty_constraints_for_tait(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Ty<'_> {
use rustc_hir::{Expr, ImplItem, Item, TraitItem};

struct ConstraintLocator<'tcx> {
Expand Down Expand Up @@ -660,6 +634,122 @@ fn find_opaque_ty_constraints(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Ty<'_> {
}
}

fn find_opaque_ty_constraints_for_rpit(
tcx: TyCtxt<'_>,
def_id: LocalDefId,
owner_def_id: LocalDefId,
) -> Ty<'_> {
use rustc_hir::{Expr, ImplItem, Item, TraitItem};

struct ConstraintChecker<'tcx> {
tcx: TyCtxt<'tcx>,

/// def_id of the opaque type whose defining uses are being checked
def_id: LocalDefId,

found: ty::OpaqueHiddenType<'tcx>,
}

impl ConstraintChecker<'_> {
#[instrument(skip(self), level = "debug")]
fn check(&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);
for &(def_id, concrete_type) in concrete_opaque_types {
if def_id != self.def_id {
// Ignore constraints for other opaque types.
continue;
}

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

if concrete_type.ty != self.found.ty
&& !(concrete_type, self.found).references_error()
{
self.found.report_mismatch(&concrete_type, self.tcx);
}
}
}
}

impl<'tcx> intravisit::Visitor<'tcx> for ConstraintChecker<'tcx> {
type NestedFilter = nested_filter::OnlyBodies;

fn nested_visit_map(&mut self) -> Self::Map {
self.tcx.hir()
}
fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
if let hir::ExprKind::Closure { .. } = ex.kind {
let def_id = self.tcx.hir().local_def_id(ex.hir_id);
self.check(def_id);
}
intravisit::walk_expr(self, ex);
}
fn visit_item(&mut self, it: &'tcx Item<'tcx>) {
trace!(?it.def_id);
// The opaque type itself or its children are not within its reveal scope.
if it.def_id != self.def_id {
self.check(it.def_id);
intravisit::walk_item(self, it);
}
}
fn visit_impl_item(&mut self, it: &'tcx ImplItem<'tcx>) {
trace!(?it.def_id);
// The opaque type itself or its children are not within its reveal scope.
if it.def_id != self.def_id {
self.check(it.def_id);
intravisit::walk_impl_item(self, it);
}
}
fn visit_trait_item(&mut self, it: &'tcx TraitItem<'tcx>) {
trace!(?it.def_id);
self.check(it.def_id);
intravisit::walk_trait_item(self, it);
}
}

let concrete = tcx.mir_borrowck(owner_def_id).concrete_opaque_types.get(&def_id).copied();

if let Some(concrete) = concrete {
let scope = tcx.hir().local_def_id_to_hir_id(owner_def_id);
debug!(?scope);
let mut locator = ConstraintChecker { def_id: def_id, tcx, found: concrete };

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(|| {
let table = tcx.typeck(owner_def_id);
if let Some(_) = table.tainted_by_errors {
// Some error in the
// owner fn prevented us from populating
// the `concrete_opaque_types` table.
tcx.ty_error()
} else {
table
.concrete_opaque_types
.get(&def_id)
.copied()
.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.
Some(tcx.mk_diverging_default())
})
.expect("RPIT always have a hidden type from typeck")
}
})
}

fn infer_placeholder_type<'a>(
tcx: TyCtxt<'a>,
def_id: LocalDefId,
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/impl-trait/issue-99073-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ fn main() {
fn test<T: Display>(t: T, recurse: bool) -> impl Display {
let f = || {
let i: u32 = test::<i32>(-1, false);
//~^ ERROR mismatched types
//~^ ERROR concrete type differs from previous defining opaque type use
println!("{i}");
};
if recurse {
Expand Down
15 changes: 7 additions & 8 deletions src/test/ui/impl-trait/issue-99073-2.stderr
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
error[E0308]: mismatched types
error: concrete type differs from previous defining opaque type use
--> $DIR/issue-99073-2.rs:9:22
|
LL | fn test<T: Display>(t: T, recurse: bool) -> impl Display {
| ------------ the expected opaque type
LL | let f = || {
LL | let i: u32 = test::<i32>(-1, false);
| ^^^^^^^^^^^^^^^^^^^^^^ types differ
| ^^^^^^^^^^^^^^^^^^^^^^ expected `T`, got `u32`
|
= note: expected opaque type `impl std::fmt::Display`
found type `u32`
note: previous use here
--> $DIR/issue-99073-2.rs:16:5
|
LL | t
| ^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
6 changes: 3 additions & 3 deletions src/test/ui/impl-trait/issue-99073.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
fn main() {
let _ = fix(|_: &dyn Fn()| {});
let _ = fix(|_: &dyn Fn()| {});
}

fn fix<F: Fn(G), G: Fn()>(f: F) -> impl Fn() {
move || f(fix(&f))
//~^ ERROR mismatched types
move || f(fix(&f))
//~^ ERROR concrete type differs from previous defining opaque type use
}
18 changes: 9 additions & 9 deletions src/test/ui/impl-trait/issue-99073.stderr
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
error[E0308]: mismatched types
--> $DIR/issue-99073.rs:6:13
error: concrete type differs from previous defining opaque type use
--> $DIR/issue-99073.rs:6:11
|
LL | fn fix<F: Fn(G), G: Fn()>(f: F) -> impl Fn() {
| --------- the expected opaque type
LL | move || f(fix(&f))
| ^^^^^^^^^^ types differ
LL | move || f(fix(&f))
| ^^^^^^^^^^ expected `[closure@$DIR/issue-99073.rs:6:3: 6:10]`, got `G`
|
= note: expected opaque type `impl Fn()`
found type parameter `G`
note: previous use here
--> $DIR/issue-99073.rs:6:3
|
LL | move || f(fix(&f))
| ^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

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

0 comments on commit 3b780fc

Please sign in to comment.