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

Point at opaque and closure type definitions in type errors #66463

Merged
merged 8 commits into from
Jan 10, 2020
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
144 changes: 139 additions & 5 deletions src/librustc/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,12 @@ use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_hir::Node;

use errors::{struct_span_err, Applicability, DiagnosticBuilder, DiagnosticStyledString};
use errors::{
pluralize, struct_span_err, Applicability, DiagnosticBuilder, DiagnosticStyledString,
};
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_error_codes::*;
use rustc_span::{Pos, Span};
use rustc_span::{DesugaringKind, Pos, Span};
use rustc_target::spec::abi;
use std::{cmp, fmt};

Expand Down Expand Up @@ -1289,6 +1292,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
mut values: Option<ValuePairs<'tcx>>,
terr: &TypeError<'tcx>,
) {
let span = cause.span(self.tcx);

// For some types of errors, expected-found does not make
// sense, so just ignore the values we were given.
match terr {
Expand All @@ -1298,6 +1303,100 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
_ => {}
}

struct OpaqueTypesVisitor<'tcx> {
types: FxHashMap<TyCategory, FxHashSet<Span>>,
expected: FxHashMap<TyCategory, FxHashSet<Span>>,
found: FxHashMap<TyCategory, FxHashSet<Span>>,
ignore_span: Span,
tcx: TyCtxt<'tcx>,
}

impl<'tcx> OpaqueTypesVisitor<'tcx> {
fn visit_expected_found(
tcx: TyCtxt<'tcx>,
expected: Ty<'tcx>,
found: Ty<'tcx>,
ignore_span: Span,
) -> Self {
let mut types_visitor = OpaqueTypesVisitor {
types: Default::default(),
expected: Default::default(),
found: Default::default(),
ignore_span,
tcx,
};
// The visitor puts all the relevant encountered types in `self.types`, but in
// here we want to visit two separate types with no relation to each other, so we
// move the results from `types` to `expected` or `found` as appropriate.
expected.visit_with(&mut types_visitor);
std::mem::swap(&mut types_visitor.expected, &mut types_visitor.types);
found.visit_with(&mut types_visitor);
std::mem::swap(&mut types_visitor.found, &mut types_visitor.types);
estebank marked this conversation as resolved.
Show resolved Hide resolved
types_visitor
}

fn report(&self, err: &mut DiagnosticBuilder<'_>) {
self.add_labels_for_types(err, "expected", &self.expected);
self.add_labels_for_types(err, "found", &self.found);
}

fn add_labels_for_types(
&self,
err: &mut DiagnosticBuilder<'_>,
target: &str,
types: &FxHashMap<TyCategory, FxHashSet<Span>>,
) {
for (key, values) in types.iter() {
let count = values.len();
let kind = key.descr();
for sp in values {
err.span_label(
*sp,
format!(
"{}{}{} {}{}",
if sp.is_desugaring(DesugaringKind::Async) {
"the `Output` of this `async fn`'s "
} else if count == 1 {
"the "
} else {
""
},
if count > 1 { "one of the " } else { "" },
target,
kind,
pluralize!(count),
),
);
}
}
}
}

impl<'tcx> ty::fold::TypeVisitor<'tcx> for OpaqueTypesVisitor<'tcx> {
fn visit_ty(&mut self, t: Ty<'tcx>) -> bool {
if let Some((kind, def_id)) = TyCategory::from_ty(t) {
let span = self.tcx.def_span(def_id);
// Avoid cluttering the output when the "found" and error span overlap:
//
// error[E0308]: mismatched types
// --> $DIR/issue-20862.rs:2:5
// |
// LL | |y| x + y
// | ^^^^^^^^^
// | |
// | the found closure
// | expected `()`, found closure
// |
// = note: expected unit type `()`
// found closure `[closure@$DIR/issue-20862.rs:2:5: 2:14 x:_]`
if !self.ignore_span.overlaps(span) {
self.types.entry(kind).or_default().insert(span);
}
}
t.super_visit_with(self)
}
}

debug!("note_type_err(diag={:?})", diag);
let (expected_found, exp_found, is_simple_error) = match values {
None => (None, None, false),
Expand All @@ -1306,6 +1405,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
ValuePairs::Types(exp_found) => {
let is_simple_err =
exp_found.expected.is_simple_text() && exp_found.found.is_simple_text();
OpaqueTypesVisitor::visit_expected_found(
self.tcx,
exp_found.expected,
exp_found.found,
span,
)
.report(diag);

(is_simple_err, Some(exp_found))
}
Expand All @@ -1323,8 +1429,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
};

let span = cause.span(self.tcx);

// Ignore msg for object safe coercion
// since E0038 message will be printed
match terr {
Expand All @@ -1336,7 +1440,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
}
};

if let Some((expected, found)) = expected_found {
let expected_label = exp_found.map_or("type".into(), |ef| ef.expected.prefix_string());
let found_label = exp_found.map_or("type".into(), |ef| ef.found.prefix_string());
Expand Down Expand Up @@ -1933,3 +2036,34 @@ impl<'tcx> ObligationCause<'tcx> {
}
}
}

/// This is a bare signal of what kind of type we're dealing with. `ty::TyKind` tracks
/// extra information about each type, but we only care about the category.
#[derive(Clone, Copy, PartialEq, Eq, Hash)]
crate enum TyCategory {
Closure,
Opaque,
Generator,
Foreign,
}

impl TyCategory {
fn descr(&self) -> &'static str {
match self {
Self::Closure => "closure",
Self::Opaque => "opaque type",
Self::Generator => "generator",
Self::Foreign => "foreign type",
}
}

pub fn from_ty(ty: Ty<'_>) -> Option<(Self, DefId)> {
match ty.kind {
ty::Closure(def_id, _) => Some((Self::Closure, def_id)),
ty::Opaque(def_id, _) => Some((Self::Opaque, def_id)),
ty::Generator(def_id, ..) => Some((Self::Generator, def_id)),
ty::Foreign(def_id) => Some((Self::Foreign, def_id)),
_ => None,
}
}
}
29 changes: 23 additions & 6 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use super::{
TraitNotObjectSafe,
};

use crate::infer::error_reporting::TypeAnnotationNeeded as ErrorCode;
use crate::infer::error_reporting::{TyCategory, TypeAnnotationNeeded as ErrorCode};
use crate::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use crate::infer::{self, InferCtxt};
use crate::mir::interpret::ErrorHandled;
Expand Down Expand Up @@ -446,7 +446,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
flags.push((sym::from_method, Some(method.to_string())));
}
}
if let Some(t) = self.get_parent_trait_ref(&obligation.cause.code) {
if let Some((t, _)) = self.get_parent_trait_ref(&obligation.cause.code) {
flags.push((sym::parent_trait, Some(t)));
}

Expand Down Expand Up @@ -665,13 +665,21 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}

/// Gets the parent trait chain start
fn get_parent_trait_ref(&self, code: &ObligationCauseCode<'tcx>) -> Option<String> {
fn get_parent_trait_ref(
&self,
code: &ObligationCauseCode<'tcx>,
) -> Option<(String, Option<Span>)> {
match code {
&ObligationCauseCode::BuiltinDerivedObligation(ref data) => {
let parent_trait_ref = self.resolve_vars_if_possible(&data.parent_trait_ref);
match self.get_parent_trait_ref(&data.parent_code) {
Some(t) => Some(t),
None => Some(parent_trait_ref.skip_binder().self_ty().to_string()),
None => {
let ty = parent_trait_ref.skip_binder().self_ty();
let span =
TyCategory::from_ty(ty).map(|(_, def_id)| self.tcx.def_span(def_id));
Some((ty.to_string(), span))
}
}
}
_ => None,
Expand Down Expand Up @@ -719,9 +727,15 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
return;
}
let trait_ref = trait_predicate.to_poly_trait_ref();
let (post_message, pre_message) = self
let (post_message, pre_message, type_def) = self
.get_parent_trait_ref(&obligation.cause.code)
.map(|t| (format!(" in `{}`", t), format!("within `{}`, ", t)))
.map(|(t, s)| {
(
format!(" in `{}`", t),
format!("within `{}`, ", t),
s.map(|s| (format!("within this `{}`", t), s)),
)
})
.unwrap_or_default();

let OnUnimplementedNote { message, label, note, enclosing_scope } =
Expand Down Expand Up @@ -795,6 +809,9 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
} else {
err.span_label(span, explanation);
}
if let Some((msg, span)) = type_def {
err.span_label(span, &msg);
}
if let Some(ref s) = note {
// If it has a custom `#[rustc_on_unimplemented]` note, let's display it
err.note(s.as_str());
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4743,14 +4743,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.join(", ");
}
Some(Node::Expr(hir::Expr {
kind: ExprKind::Closure(_, _, body_id, closure_span, _),
kind: ExprKind::Closure(_, _, body_id, _, _),
span: full_closure_span,
..
})) => {
if *full_closure_span == expr.span {
return false;
}
err.span_label(*closure_span, "closure defined here");
msg = "call this closure";
let body = hir.body(*body_id);
sugg_call = body
Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/async-await/dont-suggest-missing-await.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
error[E0308]: mismatched types
--> $DIR/dont-suggest-missing-await.rs:14:18
|
LL | async fn make_u32() -> u32 {
| --- the `Output` of this `async fn`'s found opaque type
...
LL | take_u32(x)
| ^ expected `u32`, found opaque type
|
Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/async-await/issue-64130-3-other.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ error[E0277]: the trait bound `Foo: Qux` is not satisfied in `impl std::future::
|
LL | fn is_qux<T: Qux>(t: T) { }
| ------ --- required by this bound in `is_qux`
LL |
LL | async fn bar() {
| - within this `impl std::future::Future`
...
LL | is_qux(bar());
| ^^^^^^ within `impl std::future::Future`, the trait `Qux` is not implemented for `Foo`
Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/async-await/suggest-missing-await-closure.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
error[E0308]: mismatched types
--> $DIR/suggest-missing-await-closure.rs:16:18
|
LL | async fn make_u32() -> u32 {
| --- the `Output` of this `async fn`'s found opaque type
...
LL | take_u32(x)
| ^
| |
Expand Down
6 changes: 6 additions & 0 deletions src/test/ui/async-await/suggest-missing-await.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
error[E0308]: mismatched types
--> $DIR/suggest-missing-await.rs:13:14
|
LL | async fn make_u32() -> u32 {
| --- the `Output` of this `async fn`'s found opaque type
...
LL | take_u32(x)
| ^
| |
Expand All @@ -13,6 +16,9 @@ LL | take_u32(x)
error[E0308]: mismatched types
--> $DIR/suggest-missing-await.rs:23:5
|
LL | async fn dummy() {}
| - the `Output` of this `async fn`'s found opaque type
...
LL | dummy()
| ^^^^^^^ expected `()`, found opaque type
|
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/closures/closure-reform-bad.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
error[E0308]: mismatched types
--> $DIR/closure-reform-bad.rs:11:15
|
LL | let f = |s: &str| println!("{}{}", s, string);
| ------------------------------------- the found closure
LL | call_bare(f)
| ^ expected fn pointer, found closure
|
Expand Down
5 changes: 5 additions & 0 deletions src/test/ui/extern/extern-types-distinct-types.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
error[E0308]: mismatched types
--> $DIR/extern-types-distinct-types.rs:9:5
|
LL | type A;
| ------- the found foreign type
LL | type B;
| ------- the expected foreign type
...
LL | r
| ^ expected extern type `B`, found extern type `A`
|
Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/impl-trait/auto-trait-leak.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ LL | fn send<T: Send>(_: T) {}
...
LL | send(cycle2().clone());
| ^^^^ `std::rc::Rc<std::string::String>` cannot be sent between threads safely
...
LL | fn cycle2() -> impl Clone {
| ---------- within this `impl std::clone::Clone`
|
= help: within `impl std::clone::Clone`, the trait `std::marker::Send` is not implemented for `std::rc::Rc<std::string::String>`
= note: required because it appears within the type `impl std::clone::Clone`
Expand Down
6 changes: 6 additions & 0 deletions src/test/ui/impl-trait/auto-trait-leak2.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
error[E0277]: `std::rc::Rc<std::cell::Cell<i32>>` cannot be sent between threads safely
--> $DIR/auto-trait-leak2.rs:13:5
|
LL | fn before() -> impl Fn(i32) {
| ------------ within this `impl std::ops::Fn<(i32,)>`
...
LL | fn send<T: Send>(_: T) {}
| ---- ---- required by this bound in `send`
...
Expand All @@ -19,6 +22,9 @@ LL | fn send<T: Send>(_: T) {}
...
LL | send(after());
| ^^^^ `std::rc::Rc<std::cell::Cell<i32>>` cannot be sent between threads safely
...
LL | fn after() -> impl Fn(i32) {
| ------------ within this `impl std::ops::Fn<(i32,)>`
|
= help: within `impl std::ops::Fn<(i32,)>`, the trait `std::marker::Send` is not implemented for `std::rc::Rc<std::cell::Cell<i32>>`
= note: required because it appears within the type `[closure@$DIR/auto-trait-leak2.rs:24:5: 24:22 p:std::rc::Rc<std::cell::Cell<i32>>]`
Expand Down
Loading