Skip to content

Commit

Permalink
Expand "recursive opaque type" diagnostic
Browse files Browse the repository at this point in the history
Fix rust-lang#70968, partially address rust-lang#66523.
  • Loading branch information
estebank committed Jun 15, 2020
1 parent ff4a253 commit 96f5584
Show file tree
Hide file tree
Showing 13 changed files with 374 additions and 115 deletions.
12 changes: 12 additions & 0 deletions src/librustc_hir/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2726,6 +2726,18 @@ impl Node<'_> {
}
}

pub fn body_id(&self) -> Option<BodyId> {
match self {
Node::TraitItem(TraitItem {
kind: TraitItemKind::Fn(_, TraitFn::Provided(body_id)),
..
})
| Node::ImplItem(ImplItem { kind: ImplItemKind::Fn(_, body_id), .. })
| Node::Item(Item { kind: ItemKind::Fn(.., body_id), .. }) => Some(*body_id),
_ => None,
}
}

pub fn generics(&self) -> Option<&Generics<'_>> {
match self {
Node::TraitItem(TraitItem { generics, .. })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1992,8 +1992,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
/// Collect all the returned expressions within the input expression.
/// Used to point at the return spans when we want to suggest some change to them.
#[derive(Default)]
struct ReturnsVisitor<'v> {
returns: Vec<&'v hir::Expr<'v>>,
pub struct ReturnsVisitor<'v> {
pub returns: Vec<&'v hir::Expr<'v>>,
in_block_tail: bool,
}

Expand Down
195 changes: 181 additions & 14 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ use rustc_target::spec::abi::Abi;
use rustc_trait_selection::infer::InferCtxtExt as _;
use rustc_trait_selection::opaque_types::{InferCtxtExt as _, OpaqueTypeDecl};
use rustc_trait_selection::traits::error_reporting::recursive_type_with_infinite_size_error;
use rustc_trait_selection::traits::error_reporting::suggestions::ReturnsVisitor;
use rustc_trait_selection::traits::error_reporting::InferCtxtExt as _;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _;
use rustc_trait_selection::traits::{
Expand Down Expand Up @@ -1711,6 +1712,181 @@ fn check_opaque_for_inheriting_lifetimes(tcx: TyCtxt<'tcx>, def_id: LocalDefId,
}
}

/// Given a `DefId` for an opaque type in return position, find its parent item's return
/// expressions.
fn get_owner_return_paths(
tcx: TyCtxt<'tcx>,
def_id: LocalDefId,
) -> Option<(hir::HirId, ReturnsVisitor<'tcx>)> {
let hir_id = tcx.hir().as_local_hir_id(def_id);
let id = tcx.hir().get_parent_item(hir_id);
tcx.hir()
.find(id)
.map(|n| (id, n))
.and_then(|(hir_id, node)| node.body_id().map(|b| (hir_id, b)))
.map(|(hir_id, body_id)| {
let body = tcx.hir().body(body_id);
let mut visitor = ReturnsVisitor::default();
visitor.visit_body(body);
(hir_id, visitor)
})
}

/// Emit an error for recursive opaque types.
///
/// If this is a return `impl Trait`, find the item's return expressions and point at them. For
/// direct recursion this is enough, but for indirect recursion also point at the last intermediary
/// `impl Trait`.
///
/// If all the return expressions evaluate to `!`, then we explain that the error will go away
/// after changing it. This can happen when a user uses `panic!()` or similar as a placeholder.
fn opaque_type_cycle_error(tcx: TyCtxt<'tcx>, def_id: LocalDefId, span: Span) {
let mut err =
struct_span_err!(tcx.sess, span, E0720, "cannot resolve opaque type to a concrete type");

let mut label = false;
if let Some((hir_id, visitor)) = get_owner_return_paths(tcx, def_id) {
let tables = tcx.typeck_tables_of(tcx.hir().local_def_id(hir_id));
if visitor
.returns
.iter()
.filter_map(|expr| tables.node_type_opt(expr.hir_id))
.map(|ty| tcx.infer_ctxt().enter(|infcx| infcx.resolve_vars_if_possible(&ty)))
.all(|ty| matches!(ty.kind, ty::Never))
{
let spans = visitor
.returns
.iter()
.filter(|expr| tables.node_type_opt(expr.hir_id).is_some())
.map(|expr| expr.span)
.collect::<Vec<Span>>();
let span_len = spans.len();
if span_len == 1 {
err.span_label(spans[0], "this returned value is of `!` type");
} else {
let mut multispan: MultiSpan = spans.clone().into();
for span in spans {
multispan
.push_span_label(span, "this returned value is of `!` type".to_string());
}
err.span_note(multispan, "these returned values have a concrete \"never\" type");
}
err.help("this error will resolve once the item's body returns a concrete type");
} else {
let mut seen = FxHashSet::default();
seen.insert(span);
err.span_label(span, "recursive opaque type");
label = true;
for (sp, ty) in visitor
.returns
.iter()
.filter_map(|e| tables.node_type_opt(e.hir_id).map(|t| (e.span, t)))
.filter(|(_, ty)| !matches!(ty.kind, ty::Never))
.map(|(sp, ty)| {
(sp, tcx.infer_ctxt().enter(|infcx| infcx.resolve_vars_if_possible(&ty)))
})
{
struct VisitTypes(Vec<DefId>);
impl<'tcx> ty::fold::TypeVisitor<'tcx> for VisitTypes {
fn visit_ty(&mut self, t: Ty<'tcx>) -> bool {
match t.kind {
ty::Opaque(def, _) => {
self.0.push(def);
false
}
_ => t.super_visit_with(self),
}
}
}
let mut visitor = VisitTypes(vec![]);
ty.visit_with(&mut visitor);
for def_id in visitor.0 {
let ty_span = tcx.def_span(def_id);
if !seen.contains(&ty_span) {
err.span_label(ty_span, &format!("returning this opaque type `{}`", ty));
seen.insert(ty_span);
}
err.span_label(sp, &format!("returning here with type `{}`", ty));
}
}
}
}
if !label {
err.span_label(span, "cannot resolve to a concrete type");
}
err.emit();
}

/// Emit an error for recursive opaque types in a `let` binding.
fn binding_opaque_type_cycle_error(
tcx: TyCtxt<'tcx>,
def_id: LocalDefId,
span: Span,
partially_expanded_type: Ty<'tcx>,
) {
let mut err =
struct_span_err!(tcx.sess, span, E0720, "cannot resolve opaque type to a concrete type");
err.span_label(span, "cannot resolve to a concrete type");
let hir_id = tcx.hir().as_local_hir_id(def_id);
let mut prev_hir_id = hir_id;
let mut hir_id = tcx.hir().get_parent_node(hir_id);
while let Some(node) = tcx.hir().find(hir_id) {
match node {
hir::Node::Local(hir::Local {
pat,
init: None,
ty: Some(ty),
source: hir::LocalSource::Normal,
..
}) => {
err.span_label(pat.span, "this binding might not have a concrete type");
err.span_suggestion_verbose(
ty.span.shrink_to_hi(),
"set the binding to a value for a concrete type to be resolved",
" = /* value */".to_string(),
Applicability::HasPlaceholders,
);
}
hir::Node::Local(hir::Local {
init: Some(expr),
source: hir::LocalSource::Normal,
..
}) => {
let hir_id = tcx.hir().as_local_hir_id(def_id);
let tables =
tcx.typeck_tables_of(tcx.hir().local_def_id(tcx.hir().get_parent_item(hir_id)));
let ty = tables.node_type_opt(expr.hir_id);
if let Some(ty) =
tcx.infer_ctxt().enter(|infcx| infcx.resolve_vars_if_possible(&ty))
{
err.span_label(
expr.span,
&format!(
"this is of type `{}`, which doesn't constrain \
`{}` enough to arrive to a concrete type",
ty, partially_expanded_type
),
);
}
}
_ => {}
}
if prev_hir_id == hir_id {
break;
}
prev_hir_id = hir_id;
hir_id = tcx.hir().get_parent_node(hir_id);
}
err.emit();
}

fn async_opaque_type_cycle_error(tcx: TyCtxt<'tcx>, span: Span) {
struct_span_err!(tcx.sess, span, E0733, "recursion in an `async fn` requires boxing")
.span_label(span, "recursive `async fn`")
.note("a recursive `async fn` must be rewritten to return a boxed `dyn Future`")
.emit();
}

/// Checks that an opaque type does not contain cycles.
fn check_opaque_for_cycles<'tcx>(
tcx: TyCtxt<'tcx>,
Expand All @@ -1721,21 +1897,12 @@ fn check_opaque_for_cycles<'tcx>(
) {
if let Err(partially_expanded_type) = tcx.try_expand_impl_trait_type(def_id.to_def_id(), substs)
{
if let hir::OpaqueTyOrigin::AsyncFn = origin {
struct_span_err!(tcx.sess, span, E0733, "recursion in an `async fn` requires boxing",)
.span_label(span, "recursive `async fn`")
.note("a recursive `async fn` must be rewritten to return a boxed `dyn Future`")
.emit();
} else {
let mut err =
struct_span_err!(tcx.sess, span, E0720, "opaque type expands to a recursive type",);
err.span_label(span, "expands to a recursive type");
if let ty::Opaque(..) = partially_expanded_type.kind {
err.note("type resolves to itself");
} else {
err.note(&format!("expanded type is `{}`", partially_expanded_type));
match origin {
hir::OpaqueTyOrigin::AsyncFn => async_opaque_type_cycle_error(tcx, span),
hir::OpaqueTyOrigin::Binding => {
binding_opaque_type_cycle_error(tcx, def_id, span, partially_expanded_type)
}
err.emit();
_ => opaque_type_cycle_error(tcx, def_id, span),
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions src/test/ui/impl-trait/binding-without-value.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#![allow(incomplete_features)]
#![feature(impl_trait_in_bindings)]

fn foo() {
let _ : impl Copy;
//~^ ERROR cannot resolve opaque type
}

fn main() {}
16 changes: 16 additions & 0 deletions src/test/ui/impl-trait/binding-without-value.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error[E0720]: cannot resolve opaque type to a concrete type
--> $DIR/binding-without-value.rs:5:13
|
LL | let _ : impl Copy;
| - ^^^^^^^^^ cannot resolve to a concrete type
| |
| this binding might not have a concrete type
|
help: set the binding to a value for a concrete type to be resolved
|
LL | let _ : impl Copy = /* value */;
| ^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0720`.
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@

trait Quux {}

fn foo() -> impl Quux { //~ opaque type expands to a recursive type
fn foo() -> impl Quux { //~ ERROR cannot resolve opaque type
struct Foo<T>(T);
impl<T> Quux for Foo<T> {}
Foo(bar())
}

fn bar() -> impl Quux { //~ opaque type expands to a recursive type
fn bar() -> impl Quux { //~ ERROR cannot resolve opaque type
struct Bar<T>(T);
impl<T> Quux for Bar<T> {}
Bar(foo())
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,26 @@
error[E0720]: opaque type expands to a recursive type
error[E0720]: cannot resolve opaque type to a concrete type
--> $DIR/infinite-impl-trait-issue-38064.rs:8:13
|
LL | fn foo() -> impl Quux {
| ^^^^^^^^^ expands to a recursive type
|
= note: expanded type is `foo::Foo<bar::Bar<impl Quux>>`
| ^^^^^^^^^ recursive opaque type
...
LL | Foo(bar())
| ---------- returning here with type `foo::Foo<impl Quux>`
...
LL | fn bar() -> impl Quux {
| --------- returning this opaque type `foo::Foo<impl Quux>`

error[E0720]: opaque type expands to a recursive type
error[E0720]: cannot resolve opaque type to a concrete type
--> $DIR/infinite-impl-trait-issue-38064.rs:14:13
|
LL | fn foo() -> impl Quux {
| --------- returning this opaque type `bar::Bar<impl Quux>`
...
LL | fn bar() -> impl Quux {
| ^^^^^^^^^ expands to a recursive type
|
= note: expanded type is `bar::Bar<foo::Foo<impl Quux>>`
| ^^^^^^^^^ recursive opaque type
...
LL | Bar(foo())
| ---------- returning here with type `bar::Bar<impl Quux>`

error: aborting due to 2 previous errors

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
error[E0720]: opaque type expands to a recursive type
error[E0720]: cannot resolve opaque type to a concrete type
--> $DIR/recursive-impl-trait-type-direct.rs:5:14
|
LL | fn test() -> impl Sized {
| ^^^^^^^^^^ expands to a recursive type
|
= note: type resolves to itself
| ^^^^^^^^^^ recursive opaque type
LL |
LL | test()
| ------ returning here with type `impl Sized`

error: aborting due to previous error

Expand Down
Loading

0 comments on commit 96f5584

Please sign in to comment.