Rollup merge of #66651 - Areredify:on-unimplemented-scope, r=davidtwco

Add `enclosing scope` parameter to `rustc_on_unimplemented` Adds a new parameter to `#[rustc_on_unimplemented]`, `enclosing scope`, which highlights the function or closure scope with a message. The wip part refers to adding this annotation to `Try` trait to improve ergonomics (which I don't know how to do since I change both std and librustc) Closes #61709.
rust-lang · Dec 3, 2019 · 8dacfc2 · 8dacfc2
2 parents 3045d22 + 1d0c015
commit 8dacfc2
Show file tree

Hide file tree

Showing 11 changed files with 235 additions and 37 deletions.
diff --git a/src/libcore/ops/try.rs b/src/libcore/ops/try.rs
@@ -5,19 +5,20 @@
 /// extracting those success or failure values from an existing instance and
 /// creating a new instance from a success or failure value.
 #[unstable(feature = "try_trait", issue = "42327")]
-#[rustc_on_unimplemented(
+#[cfg_attr(not(bootstrap), rustc_on_unimplemented(
 on(all(
 any(from_method="from_error", from_method="from_ok"),
 from_desugaring="QuestionMark"),
 message="the `?` operator can only be used in {ItemContext} \
                that returns `Result` or `Option` \
                (or another type that implements `{Try}`)",
-label="cannot use the `?` operator in {ItemContext} that returns `{Self}`"),
+label="cannot use the `?` operator in {ItemContext} that returns `{Self}`",
+enclosing_scope="this function should return `Result` or `Option` to accept `?`"),
 on(all(from_method="into_result", from_desugaring="QuestionMark"),
 message="the `?` operator can only be applied to values \
                that implement `{Try}`",
 label="the `?` operator cannot be applied to type `{Self}`")
-)]
+))]
 #[doc(alias = "?")]
 pub trait Try {
     /// The type of this value when viewed as successful.

diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs
@@ -521,7 +521,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         ) {
             command.evaluate(self.tcx, trait_ref, &flags[..])
         } else {
-            OnUnimplementedNote::empty()
+            OnUnimplementedNote::default()
         }
     }
 
@@ -697,6 +697,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         fallback_has_occurred: bool,
         points_at_arg: bool,
     ) {
+        let tcx = self.tcx;
         let span = obligation.cause.span;
 
         let mut err = match *error {
@@ -732,6 +733,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                             message,
                             label,
                             note,
+                            enclosing_scope,
                         } = self.on_unimplemented_note(trait_ref, obligation);
                         let have_alt_message = message.is_some() || label.is_some();
                         let is_try = self.tcx.sess.source_map().span_to_snippet(span)
@@ -798,6 +800,19 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                             // If it has a custom `#[rustc_on_unimplemented]` note, let's display it
                             err.note(s.as_str());
                         }
+                        if let Some(ref s) = enclosing_scope {
+                            let enclosing_scope_span = tcx.def_span(
+                                tcx.hir()
+                                    .opt_local_def_id(obligation.cause.body_id)
+                                    .unwrap_or_else(|| {
+                                        tcx.hir().body_owner_def_id(hir::BodyId {
+                                            hir_id: obligation.cause.body_id,
+                                        })
+                                    }),
+                            );
+
+                            err.span_label(enclosing_scope_span, s.as_str());
+                        }
 
                         self.suggest_borrow_on_unsized_slice(&obligation.cause.code, &mut err);
                         self.suggest_fn_call(&obligation, &mut err, &trait_ref, points_at_arg);

diff --git a/src/librustc/traits/on_unimplemented.rs b/src/librustc/traits/on_unimplemented.rs
@@ -22,18 +22,15 @@ pub struct OnUnimplementedDirective {
     pub message: Option<OnUnimplementedFormatString>,
     pub label: Option<OnUnimplementedFormatString>,
     pub note: Option<OnUnimplementedFormatString>,
+    pub enclosing_scope: Option<OnUnimplementedFormatString>,
 }
 
+#[derive(Default)]
 pub struct OnUnimplementedNote {
     pub message: Option<String>,
     pub label: Option<String>,
     pub note: Option<String>,
-}
-
-impl OnUnimplementedNote {
-    pub fn empty() -> Self {
-        OnUnimplementedNote { message: None, label: None, note: None }
-    }
+    pub enclosing_scope: Option<String>,
 }
 
 fn parse_error(
@@ -85,24 +82,33 @@ impl<'tcx> OnUnimplementedDirective {
         let mut message = None;
         let mut label = None;
         let mut note = None;
+        let mut enclosing_scope = None;
         let mut subcommands = vec![];
+
+        let parse_value = |value_str| {
+                OnUnimplementedFormatString::try_parse(tcx, trait_def_id, value_str, span)
+                    .map(Some)
+            };
+
         for item in item_iter {
             if item.check_name(sym::message) && message.is_none() {
                 if let Some(message_) = item.value_str() {
-                    message = Some(OnUnimplementedFormatString::try_parse(
-                        tcx, trait_def_id, message_, span)?);
+                    message = parse_value(message_)?;
                     continue;
                 }
             } else if item.check_name(sym::label) && label.is_none() {
                 if let Some(label_) = item.value_str() {
-                    label = Some(OnUnimplementedFormatString::try_parse(
-                        tcx, trait_def_id, label_, span)?);
+                    label = parse_value(label_)?;
                     continue;
                 }
             } else if item.check_name(sym::note) && note.is_none() {
                 if let Some(note_) = item.value_str() {
-                    note = Some(OnUnimplementedFormatString::try_parse(
-                        tcx, trait_def_id, note_, span)?);
+                    note = parse_value(note_)?;
+                    continue;
+                }
+            } else if item.check_name(sym::enclosing_scope) && enclosing_scope.is_none() {
+                if let Some(enclosing_scope_) = item.value_str() {
+                    enclosing_scope = parse_value(enclosing_scope_)?;
                     continue;
                 }
             } else if item.check_name(sym::on) && is_root &&
@@ -130,7 +136,14 @@ impl<'tcx> OnUnimplementedDirective {
         if errored {
             Err(ErrorReported)
         } else {
-            Ok(OnUnimplementedDirective { condition, message, label, subcommands, note })
+            Ok(OnUnimplementedDirective {
+                condition,
+                subcommands,
+                message,
+                label,
+                note,
+                enclosing_scope
+            })
         }
     }
 
@@ -157,6 +170,7 @@ impl<'tcx> OnUnimplementedDirective {
                 label: Some(OnUnimplementedFormatString::try_parse(
                     tcx, trait_def_id, value, attr.span)?),
                 note: None,
+                enclosing_scope: None,
             }))
         } else {
             return Err(ErrorReported);
@@ -174,6 +188,7 @@ impl<'tcx> OnUnimplementedDirective {
         let mut message = None;
         let mut label = None;
         let mut note = None;
+        let mut enclosing_scope = None;
         info!("evaluate({:?}, trait_ref={:?}, options={:?})", self, trait_ref, options);
 
         for command in self.subcommands.iter().chain(Some(self)).rev() {
@@ -202,6 +217,10 @@ impl<'tcx> OnUnimplementedDirective {
             if let Some(ref note_) = command.note {
                 note = Some(note_.clone());
             }
+
+            if let Some(ref enclosing_scope_) = command.enclosing_scope {
+                enclosing_scope = Some(enclosing_scope_.clone());
+            }
         }
 
         let options: FxHashMap<Symbol, String> = options.into_iter()
@@ -211,6 +230,7 @@ impl<'tcx> OnUnimplementedDirective {
             label: label.map(|l| l.format(tcx, trait_ref, &options)),
             message: message.map(|m| m.format(tcx, trait_ref, &options)),
             note: note.map(|n| n.format(tcx, trait_ref, &options)),
+            enclosing_scope: enclosing_scope.map(|e_s| e_s.format(tcx, trait_ref, &options)),
         }
     }
 }

diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs
@@ -280,6 +280,7 @@ symbols! {
         Err,
         Eq,
         Equal,
+        enclosing_scope,
         except,
         exclusive_range_pattern,
         exhaustive_integer_patterns,

diff --git a/src/test/ui/async-await/try-on-option-in-async.stderr b/src/test/ui/async-await/try-on-option-in-async.stderr
@@ -1,26 +1,44 @@
 error[E0277]: the `?` operator can only be used in an async block that returns `Result` or `Option` (or another type that implements `std::ops::Try`)
   --> $DIR/try-on-option-in-async.rs:8:9
    |
-LL |         x?;
-   |         ^^ cannot use the `?` operator in an async block that returns `{integer}`
+LL |       async {
+   |  ___________-
+LL | |         let x: Option<u32> = None;
+LL | |         x?;
+   | |         ^^ cannot use the `?` operator in an async block that returns `{integer}`
+LL | |         22
+LL | |     }.await
+   | |_____- this function should return `Result` or `Option` to accept `?`
    |
    = help: the trait `std::ops::Try` is not implemented for `{integer}`
    = note: required by `std::ops::Try::from_error`
 
 error[E0277]: the `?` operator can only be used in an async closure that returns `Result` or `Option` (or another type that implements `std::ops::Try`)
   --> $DIR/try-on-option-in-async.rs:16:9
    |
-LL |         x?;
-   |         ^^ cannot use the `?` operator in an async closure that returns `u32`
+LL |       let async_closure = async || {
+   |  __________________________________-
+LL | |         let x: Option<u32> = None;
+LL | |         x?;
+   | |         ^^ cannot use the `?` operator in an async closure that returns `u32`
+LL | |         22_u32
+LL | |     };
+   | |_____- this function should return `Result` or `Option` to accept `?`
    |
    = help: the trait `std::ops::Try` is not implemented for `u32`
    = note: required by `std::ops::Try::from_error`
 
 error[E0277]: the `?` operator can only be used in an async function that returns `Result` or `Option` (or another type that implements `std::ops::Try`)
   --> $DIR/try-on-option-in-async.rs:25:5
    |
-LL |     x?;
-   |     ^^ cannot use the `?` operator in an async function that returns `u32`
+LL |   async fn an_async_function() -> u32 {
+   |  _____________________________________-
+LL | |     let x: Option<u32> = None;
+LL | |     x?;
+   | |     ^^ cannot use the `?` operator in an async function that returns `u32`
+LL | |     22
+LL | | }
+   | |_- this function should return `Result` or `Option` to accept `?`
    |
    = help: the trait `std::ops::Try` is not implemented for `u32`
    = note: required by `std::ops::Try::from_error`

diff --git a/src/test/ui/on-unimplemented/enclosing-scope.rs b/src/test/ui/on-unimplemented/enclosing-scope.rs
@@ -0,0 +1,27 @@
+// Test scope annotations from `enclosing_scope` parameter
+
+#![feature(rustc_attrs)]
+
+#[rustc_on_unimplemented(enclosing_scope="in this scope")]
+trait Trait{}
+
+struct Foo;
+
+fn f<T: Trait>(x: T) {}
+
+fn main() {
+    let x = || {
+        f(Foo{}); //~ ERROR the trait bound `Foo: Trait` is not satisfied
+        let y = || {
+            f(Foo{}); //~ ERROR the trait bound `Foo: Trait` is not satisfied
+        };
+    };
+
+    {
+        {
+            f(Foo{}); //~ ERROR the trait bound `Foo: Trait` is not satisfied
+        }
+    }
+
+    f(Foo{}); //~ ERROR the trait bound `Foo: Trait` is not satisfied
+}
diff --git a/src/test/ui/on-unimplemented/enclosing-scope.stderr b/src/test/ui/on-unimplemented/enclosing-scope.stderr
@@ -0,0 +1,66 @@
+error[E0277]: the trait bound `Foo: Trait` is not satisfied
+  --> $DIR/enclosing-scope.rs:14:11
+   |
+LL |   fn f<T: Trait>(x: T) {}
+   |      -    ----- required by this bound in `f`
+...
+LL |       let x = || {
+   |  _____________-
+LL | |         f(Foo{});
+   | |           ^^^^^ the trait `Trait` is not implemented for `Foo`
+LL | |         let y = || {
+LL | |             f(Foo{});
+LL | |         };
+LL | |     };
+   | |_____- in this scope
+
+error[E0277]: the trait bound `Foo: Trait` is not satisfied
+  --> $DIR/enclosing-scope.rs:16:15
+   |
+LL |   fn f<T: Trait>(x: T) {}
+   |      -    ----- required by this bound in `f`
+...
+LL |           let y = || {
+   |  _________________-
+LL | |             f(Foo{});
+   | |               ^^^^^ the trait `Trait` is not implemented for `Foo`
+LL | |         };
+   | |_________- in this scope
+
+error[E0277]: the trait bound `Foo: Trait` is not satisfied
+  --> $DIR/enclosing-scope.rs:22:15
+   |
+LL |   fn f<T: Trait>(x: T) {}
+   |      -    ----- required by this bound in `f`
+LL | 
+LL | / fn main() {
+LL | |     let x = || {
+LL | |         f(Foo{});
+LL | |         let y = || {
+...  |
+LL | |             f(Foo{});
+   | |               ^^^^^ the trait `Trait` is not implemented for `Foo`
+...  |
+LL | |     f(Foo{});
+LL | | }
+   | |_- in this scope
+
+error[E0277]: the trait bound `Foo: Trait` is not satisfied
+  --> $DIR/enclosing-scope.rs:26:7
+   |
+LL |   fn f<T: Trait>(x: T) {}
+   |      -    ----- required by this bound in `f`
+LL | 
+LL | / fn main() {
+LL | |     let x = || {
+LL | |         f(Foo{});
+LL | |         let y = || {
+...  |
+LL | |     f(Foo{});
+   | |       ^^^^^ the trait `Trait` is not implemented for `Foo`
+LL | | }
+   | |_- in this scope
+
+error: aborting due to 4 previous errors
+
+For more information about this error, try `rustc --explain E0277`.
diff --git a/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr b/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr
@@ -575,8 +575,17 @@ LL |     if (let 0 = 0)? {}
 error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `std::ops::Try`)
   --> $DIR/disallowed-positions.rs:46:8
    |
-LL |     if (let 0 = 0)? {}
-   |        ^^^^^^^^^^^^ cannot use the `?` operator in a function that returns `()`
+LL | / fn nested_within_if_expr() {
+LL | |     if &let 0 = 0 {}
+LL | |
+LL | |
+...  |
+LL | |     if (let 0 = 0)? {}
+   | |        ^^^^^^^^^^^^ cannot use the `?` operator in a function that returns `()`
+...  |
+LL | |     if let true = let true = true {}
+LL | | }
+   | |_- this function should return `Result` or `Option` to accept `?`
    |
    = help: the trait `std::ops::Try` is not implemented for `()`
    = note: required by `std::ops::Try::from_error`
@@ -754,8 +763,17 @@ LL |     while (let 0 = 0)? {}
 error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `std::ops::Try`)
   --> $DIR/disallowed-positions.rs:110:11
    |
-LL |     while (let 0 = 0)? {}
-   |           ^^^^^^^^^^^^ cannot use the `?` operator in a function that returns `()`
+LL | / fn nested_within_while_expr() {
+LL | |     while &let 0 = 0 {}
+LL | |
+LL | |
+...  |
+LL | |     while (let 0 = 0)? {}
+   | |           ^^^^^^^^^^^^ cannot use the `?` operator in a function that returns `()`
+...  |
+LL | |     while let true = let true = true {}
+LL | | }
+   | |_- this function should return `Result` or `Option` to accept `?`
    |
    = help: the trait `std::ops::Try` is not implemented for `()`
    = note: required by `std::ops::Try::from_error`
@@ -924,8 +942,17 @@ LL |     (let 0 = 0)?;
 error[E0277]: the `?` operator can only be used in a function that returns `Result` or `Option` (or another type that implements `std::ops::Try`)
   --> $DIR/disallowed-positions.rs:183:5
    |
-LL |     (let 0 = 0)?;
-   |     ^^^^^^^^^^^^ cannot use the `?` operator in a function that returns `()`
+LL | / fn outside_if_and_while_expr() {
+LL | |     &let 0 = 0;
+LL | |
+LL | |     !let 0 = 0;
+...  |
+LL | |     (let 0 = 0)?;
+   | |     ^^^^^^^^^^^^ cannot use the `?` operator in a function that returns `()`
+...  |
+LL | |
+LL | | }
+   | |_- this function should return `Result` or `Option` to accept `?`
    |
    = help: the trait `std::ops::Try` is not implemented for `()`
    = note: required by `std::ops::Try::from_error`
-Original file line number
+Diff line change
@@ Expand Up / @@ -280,6 +280,7 @@ symbols! { @@
             Err,
             Eq,
             Equal,
+            enclosing_scope,
             except,
             exclusive_range_pattern,
             exhaustive_integer_patterns,
@@ Expand Down @@