Skip to content

Commit

Permalink
Rollup merge of rust-lang#66463 - estebank:point-at-closure-and-opaqu…
Browse files Browse the repository at this point in the history
…e-types, r=Centril

Point at opaque and closure type definitions in type errors

Fixes rust-lang#57266, fixes rust-lang#67117.
  • Loading branch information
Centril authored Jan 10, 2020
2 parents 72b2bd5 + 33ae322 commit aabb037
Show file tree
Hide file tree
Showing 22 changed files with 255 additions and 21 deletions.
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);
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 @@ -4746,14 +4746,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 @@ -112,6 +112,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

0 comments on commit aabb037

Please sign in to comment.