From 66226ca1574fb936743a32f8b584fcfd1712a385 Mon Sep 17 00:00:00 2001
From: Yuki Okushi <huyuumi.dev@gmail.com>
Date: Sun, 11 Oct 2020 04:48:17 +0900
Subject: [PATCH] Address some code reviews

---
 .../src/traits/error_reporting/suggestions.rs | 179 +++++++++---------
 .../issue-70935-complex-spans.stderr          |  11 +-
 .../issue-65436-raw-ptr-not-send.stderr       |   9 +-
 3 files changed, 89 insertions(+), 110 deletions(-)

diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
index 56eb5e3686a..e0c46a65b56 100644
--- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
+++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
@@ -1570,99 +1570,66 @@ 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) {
-                    // #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`
-                    //    = note: the return type of a function must have a statically known size
-                    // 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;
-                    //    | |________________^
-                    // note: first, await occurs here, with the value maybe used later
-                    //   --> $DIR/issue-70935-complex-spans.rs:13:9
-                    //    |
-                    // LL | /         baz(|| async{
-                    // LL | |             foo(tx.clone());
-                    // LL | |         }).await;
-                    //    | |________________^
-                    // note: ...but, the value is later dropped here
-                    //   --> $DIR/issue-70935-complex-spans.rs:15:17
-                    //    |
-                    // LL |         }).await;
-                    //    |                 ^
-                    //    = note: the return type of a function must have a statically known size
-
-                    // 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 {
-                            err.span_note(
-                                span,
-                                &format!(
-                                    "{} {} as this value is used across {}",
-                                    future_or_generator, trait_explanation, an_await_or_yield
-                                ),
-                            );
-                            err.span_note(
-                                yield_span,
-                                &format!(
-                                    "first, {} occurs here, with {} maybe used later",
-                                    await_or_yield, snippet
-                                ),
-                            );
-                            err.span_note(
-                                scope_span,
-                                &format!("...but, {} 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
-                                ),
-                            );
-                        }
+        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
+                            ),
+                        );
+                        err.span_note(scope_span, &format!("{} is later dropped here", snippet));
                     } else {
                         span.push_span_label(
                             yield_span,
@@ -1671,6 +1638,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                                 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),
@@ -1683,8 +1654,28 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                             ),
                         );
                     }
+                } 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
+                        ),
+                    );
                 }
-            };
+            }
+        };
         match interior_or_upvar_span {
             GeneratorInteriorOrUpvar::Interior(interior_span) => {
                 if let Some((scope_span, yield_span, expr, from_awaited_ty)) = interior_extra_info {
diff --git a/src/test/ui/async-await/issue-70935-complex-spans.stderr b/src/test/ui/async-await/issue-70935-complex-spans.stderr
index c55f8abc42d..6b599b5d603 100644
--- a/src/test/ui/async-await/issue-70935-complex-spans.stderr
+++ b/src/test/ui/async-await/issue-70935-complex-spans.stderr
@@ -11,15 +11,8 @@ note: future is not `Send` as this value is used across an await
 LL | /         baz(|| async{
 LL | |             foo(tx.clone());
 LL | |         }).await;
-   | |________________^
-note: first, await occurs here, with the value maybe used later
-  --> $DIR/issue-70935-complex-spans.rs:13:9
-   |
-LL | /         baz(|| async{
-LL | |             foo(tx.clone());
-LL | |         }).await;
-   | |________________^
-note: ...but, the value is later dropped here
+   | |________________^ 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;
diff --git a/src/test/ui/async-await/issues/issue-65436-raw-ptr-not-send.stderr b/src/test/ui/async-await/issues/issue-65436-raw-ptr-not-send.stderr
index 6ffbcd7ea65..9d3b10c8663 100644
--- a/src/test/ui/async-await/issues/issue-65436-raw-ptr-not-send.stderr
+++ b/src/test/ui/async-await/issues/issue-65436-raw-ptr-not-send.stderr
@@ -12,13 +12,8 @@ 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;
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-note: first, await occurs here, with `std::ptr::null()` maybe used later
-  --> $DIR/issue-65436-raw-ptr-not-send.rs:14:9
-   |
-LL |         bar(Foo(std::ptr::null())).await;
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-note: ...but, `std::ptr::null()` is later dropped here
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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;