Skip to content

Commit

Permalink
Auto merge of #75020 - JohnTitor:fix-multispan, r=estebank,tmandry
Browse files Browse the repository at this point in the history 
Avoid complex diagnostics in snippets which contain newlines

Fixes #70935

r? `@estebank` `@tmandry`
  • Loading branch information
@bors
bors committed Nov 2, 2020
2 parents 3e93027 + 8d65101 commit 234099d
Show file tree
Hide file tree
Showing 4 changed files with 186 additions and 33 deletions.
152 changes: 123 additions & 29 deletions compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1570,36 +1570,130 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
format!("does not implement `{}`", trait_ref.print_only_trait_path())
};

let mut explain_yield = |interior_span: Span,
yield_span: Span,
scope_span: Option<Span>| {
let mut span = MultiSpan::from_span(yield_span);
if let Ok(snippet) = source_map.span_to_snippet(interior_span) {
span.push_span_label(
yield_span,
format!("{} occurs here, with `{}` maybe used later", await_or_yield, snippet),
);
// If available, use the scope span to annotate the drop location.
if let Some(scope_span) = scope_span {
span.push_span_label(
source_map.end_point(scope_span),
format!("`{}` is later dropped here", snippet),
);
let mut explain_yield =
|interior_span: Span, yield_span: Span, scope_span: Option<Span>| {
let mut span = MultiSpan::from_span(yield_span);
if let Ok(snippet) = source_map.span_to_snippet(interior_span) {
// #70935: If snippet contains newlines, display "the value" instead
// so that we do not emit complex diagnostics.
let snippet = &format!("`{}`", snippet);
let snippet = if snippet.contains('\n') { "the value" } else { snippet };
// The multispan can be complex here, like:
// note: future is not `Send` as this value is used across an await
// --> $DIR/issue-70935-complex-spans.rs:13:9
// |
// LL | baz(|| async{
// | __________^___-
// | | _________|
// | ||
// LL | || foo(tx.clone());
// LL | || }).await;
// | || - ^- value is later dropped here
// | ||_________|______|
// | |__________| await occurs here, with value maybe used later
// | has type `closure` which is not `Send`
//
// So, detect it and separate into some notes, like:
//
// note: future is not `Send` as this value is used across an await
// --> $DIR/issue-70935-complex-spans.rs:13:9
// |
// LL | / baz(|| async{
// LL | | foo(tx.clone());
// LL | | }).await;
// | |________________^ first, await occurs here, with the value maybe used later...
// note: the value is later dropped here
// --> $DIR/issue-70935-complex-spans.rs:15:17
// |
// LL | }).await;
// | ^
//
// If available, use the scope span to annotate the drop location.
if let Some(scope_span) = scope_span {
let scope_span = source_map.end_point(scope_span);
let is_overlapped =
yield_span.overlaps(scope_span) || yield_span.overlaps(interior_span);
if is_overlapped {
span.push_span_label(
yield_span,
format!(
"first, {} occurs here, with {} maybe used later...",
await_or_yield, snippet
),
);
err.span_note(
span,
&format!(
"{} {} as this value is used across {}",
future_or_generator, trait_explanation, an_await_or_yield
),
);
if source_map.is_multiline(interior_span) {
err.span_note(
scope_span,
&format!("{} is later dropped here", snippet),
);
err.span_note(
interior_span,
&format!(
"this has type `{}` which {}",
target_ty, trait_explanation
),
);
} else {
let mut span = MultiSpan::from_span(scope_span);
span.push_span_label(
interior_span,
format!("has type `{}` which {}", target_ty, trait_explanation),
);
err.span_note(span, &format!("{} is later dropped here", snippet));
}
} else {
span.push_span_label(
yield_span,
format!(
"{} occurs here, with {} maybe used later",
await_or_yield, snippet
),
);
span.push_span_label(
scope_span,
format!("{} is later dropped here", snippet),
);
span.push_span_label(
interior_span,
format!("has type `{}` which {}", target_ty, trait_explanation),
);
err.span_note(
span,
&format!(
"{} {} as this value is used across {}",
future_or_generator, trait_explanation, an_await_or_yield
),
);
}
} else {
span.push_span_label(
yield_span,
format!(
"{} occurs here, with {} maybe used later",
await_or_yield, snippet
),
);
span.push_span_label(
interior_span,
format!("has type `{}` which {}", target_ty, trait_explanation),
);
err.span_note(
span,
&format!(
"{} {} as this value is used across {}",
future_or_generator, trait_explanation, an_await_or_yield
),
);
}
}
}
span.push_span_label(
interior_span,
format!("has type `{}` which {}", target_ty, trait_explanation),
);

err.span_note(
span,
&format!(
"{} {} as this value is used across {}",
future_or_generator, trait_explanation, an_await_or_yield
),
);
};
};
match interior_or_upvar_span {
GeneratorInteriorOrUpvar::Interior(interior_span) => {
if let Some((scope_span, yield_span, expr, from_awaited_ty)) = interior_extra_info {
Expand Down
25 changes: 25 additions & 0 deletions src/test/ui/async-await/issue-70935-complex-spans.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// edition:2018
// #70935: Check if we do not emit snippet
// with newlines which lead complex diagnostics.

use std::future::Future;

async fn baz<T>(_c: impl FnMut() -> T) where T: Future<Output=()> {
}

fn foo(tx: std::sync::mpsc::Sender<i32>) -> impl Future + Send {
//~^ ERROR: future cannot be sent between threads safely
async move {
baz(|| async{
foo(tx.clone());
}).await;
}
}

fn bar(_s: impl Future + Send) {
}

fn main() {
let (tx, _rx) = std::sync::mpsc::channel();
bar(foo(tx));
}
30 changes: 30 additions & 0 deletions src/test/ui/async-await/issue-70935-complex-spans.stderr
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
error: future cannot be sent between threads safely
--> $DIR/issue-70935-complex-spans.rs:10:45
|
LL | fn foo(tx: std::sync::mpsc::Sender<i32>) -> impl Future + Send {
| ^^^^^^^^^^^^^^^^^^ future created by async block is not `Send`
|
= help: the trait `Sync` is not implemented for `Sender<i32>`
note: future is not `Send` as this value is used across an await
--> $DIR/issue-70935-complex-spans.rs:13:9
|
LL | / baz(|| async{
LL | | foo(tx.clone());
LL | | }).await;
| |________________^ first, await occurs here, with the value maybe used later...
note: the value is later dropped here
--> $DIR/issue-70935-complex-spans.rs:15:17
|
LL | }).await;
| ^
note: this has type `[closure@$DIR/issue-70935-complex-spans.rs:13:13: 15:10]` which is not `Send`
--> $DIR/issue-70935-complex-spans.rs:13:13
|
LL | baz(|| async{
| _____________^
LL | | foo(tx.clone());
LL | | }).await;
| |_________^

error: aborting due to previous error

12 changes: 8 additions & 4 deletions src/test/ui/async-await/issues/issue-65436-raw-ptr-not-send.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,14 @@ note: future is not `Send` as this value is used across an await
--> $DIR/issue-65436-raw-ptr-not-send.rs:14:9
|
LL | bar(Foo(std::ptr::null())).await;
| ^^^^^^^^----------------^^^^^^^^- `std::ptr::null()` is later dropped here
| | |
| | has type `*const u8` which is not `Send`
| await occurs here, with `std::ptr::null()` maybe used later
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ first, await occurs here, with `std::ptr::null()` maybe used later...
note: `std::ptr::null()` is later dropped here
--> $DIR/issue-65436-raw-ptr-not-send.rs:14:41
|
LL | bar(Foo(std::ptr::null())).await;
| ---------------- ^
| |
| has type `*const u8` which is not `Send`
help: consider moving this into a `let` binding to create a shorter lived borrow
--> $DIR/issue-65436-raw-ptr-not-send.rs:14:13
|
Expand Down

0 comments on commit 234099d

Please sign in to comment.