Auto merge of #101296 - compiler-errors:head-span-for-enclosing-scope, r=oli-obk

Use head span for `rustc_on_unimplemented`'s `enclosing_scope` attr

This may make #101281 slightly easier to understand

Author: bors

compiler/rustc_span/src/symbol.rs

@@ -662,7 +662,6 @@ symbols! {
         emit_struct,
         emit_struct_field,
         enable,
-        enclosing_scope,
         encode,
         end,
         env,
@@ -1065,6 +1064,7 @@ symbols! {
         panic_unwind,
         panicking,
         param_attrs,
+        parent_label,
         partial_cmp,
         partial_ord,
         passes,

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

@@ -349,7 +349,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                             message,
                             label,
                             note,
-                            enclosing_scope,
+                            parent_label,
                             append_const_msg,
                         } = self.on_unimplemented_note(trait_ref, &obligation);
                         let have_alt_message = message.is_some() || label.is_some();
@@ -530,7 +530,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for 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 {
+                        if let Some(ref s) = parent_label {
                             let body = tcx
                                 .hir()
                                 .opt_local_def_id(obligation.cause.body_id)
@@ -539,11 +539,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                                         hir_id: obligation.cause.body_id,
                                     })
                                 });
-
-                            let enclosing_scope_span =
-                                tcx.hir().span_with_body(tcx.hir().local_def_id_to_hir_id(body));
-
-                            err.span_label(enclosing_scope_span, s);
+                            err.span_label(tcx.def_span(body), s);
                         }
 
                         self.suggest_floating_point_literal(&obligation, &mut err, &trait_ref);

compiler/rustc_trait_selection/src/traits/on_unimplemented.rs

@@ -22,7 +22,7 @@ pub struct OnUnimplementedDirective {
     pub message: Option<OnUnimplementedFormatString>,
     pub label: Option<OnUnimplementedFormatString>,
     pub note: Option<OnUnimplementedFormatString>,
-    pub enclosing_scope: Option<OnUnimplementedFormatString>,
+    pub parent_label: Option<OnUnimplementedFormatString>,
     pub append_const_msg: Option<Option<Symbol>>,
 }
 
@@ -31,7 +31,7 @@ pub struct OnUnimplementedNote {
     pub message: Option<String>,
     pub label: Option<String>,
     pub note: Option<String>,
-    pub enclosing_scope: Option<String>,
+    pub parent_label: Option<String>,
     /// Append a message for `~const Trait` errors. `None` means not requested and
     /// should fallback to a generic message, `Some(None)` suggests using the default
     /// appended message, `Some(Some(s))` suggests use the `s` message instead of the
@@ -74,7 +74,7 @@ impl<'tcx> OnUnimplementedDirective {
         let mut message = None;
         let mut label = None;
         let mut note = None;
-        let mut enclosing_scope = None;
+        let mut parent_label = None;
         let mut subcommands = vec![];
         let mut append_const_msg = None;
 
@@ -94,9 +94,9 @@ impl<'tcx> OnUnimplementedDirective {
                     note = parse_value(note_)?;
                     continue;
                 }
-            } else if item.has_name(sym::enclosing_scope) && enclosing_scope.is_none() {
-                if let Some(enclosing_scope_) = item.value_str() {
-                    enclosing_scope = parse_value(enclosing_scope_)?;
+            } else if item.has_name(sym::parent_label) && parent_label.is_none() {
+                if let Some(parent_label_) = item.value_str() {
+                    parent_label = parse_value(parent_label_)?;
                     continue;
                 }
             } else if item.has_name(sym::on)
@@ -135,7 +135,7 @@ impl<'tcx> OnUnimplementedDirective {
                 message,
                 label,
                 note,
-                enclosing_scope,
+                parent_label,
                 append_const_msg,
             })
         }
@@ -160,7 +160,7 @@ impl<'tcx> OnUnimplementedDirective {
                     attr.span,
                 )?),
                 note: None,
-                enclosing_scope: None,
+                parent_label: None,
                 append_const_msg: None,
             }))
         } else {
@@ -181,7 +181,7 @@ impl<'tcx> OnUnimplementedDirective {
         let mut message = None;
         let mut label = None;
         let mut note = None;
-        let mut enclosing_scope = None;
+        let mut parent_label = None;
         let mut append_const_msg = None;
         info!("evaluate({:?}, trait_ref={:?}, options={:?})", self, trait_ref, options);
 
@@ -217,8 +217,8 @@ impl<'tcx> OnUnimplementedDirective {
                 note = Some(note_.clone());
             }
 
-            if let Some(ref enclosing_scope_) = command.enclosing_scope {
-                enclosing_scope = Some(enclosing_scope_.clone());
+            if let Some(ref parent_label_) = command.parent_label {
+                parent_label = Some(parent_label_.clone());
             }
 
             append_const_msg = command.append_const_msg;
@@ -228,7 +228,7 @@ impl<'tcx> OnUnimplementedDirective {
             label: label.map(|l| l.format(tcx, trait_ref, &options_map)),
             message: message.map(|m| m.format(tcx, trait_ref, &options_map)),
             note: note.map(|n| n.format(tcx, trait_ref, &options_map)),
-            enclosing_scope: enclosing_scope.map(|e_s| e_s.format(tcx, trait_ref, &options_map)),
+            parent_label: parent_label.map(|e_s| e_s.format(tcx, trait_ref, &options_map)),
             append_const_msg,
         }
     }

library/core/src/ops/try_trait.rs

@@ -222,7 +222,87 @@ pub trait Try: FromResidual {
 /// Every `Try` type needs to be recreatable from its own associated
 /// `Residual` type, but can also have additional `FromResidual` implementations
 /// to support interconversion with other `Try` types.
-#[rustc_on_unimplemented(
+#[cfg_attr(not(bootstrap), rustc_on_unimplemented(
+    on(
+        all(
+            from_desugaring = "QuestionMark",
+            _Self = "std::result::Result<T, E>",
+            R = "std::option::Option<std::convert::Infallible>"
+        ),
+        message = "the `?` operator can only be used on `Result`s, not `Option`s, \
+            in {ItemContext} that returns `Result`",
+        label = "use `.ok_or(...)?` to provide an error compatible with `{Self}`",
+        parent_label = "this function returns a `Result`"
+    ),
+    on(
+        all(
+            from_desugaring = "QuestionMark",
+            _Self = "std::result::Result<T, E>",
+        ),
+        // There's a special error message in the trait selection code for
+        // `From` in `?`, so this is not shown for result-in-result errors,
+        // and thus it can be phrased more strongly than `ControlFlow`'s.
+        message = "the `?` operator can only be used on `Result`s \
+            in {ItemContext} that returns `Result`",
+        label = "this `?` produces `{R}`, which is incompatible with `{Self}`",
+        parent_label = "this function returns a `Result`"
+    ),
+    on(
+        all(
+            from_desugaring = "QuestionMark",
+            _Self = "std::option::Option<T>",
+            R = "std::result::Result<T, E>",
+        ),
+        message = "the `?` operator can only be used on `Option`s, not `Result`s, \
+            in {ItemContext} that returns `Option`",
+        label = "use `.ok()?` if you want to discard the `{R}` error information",
+        parent_label = "this function returns an `Option`"
+    ),
+    on(
+        all(
+            from_desugaring = "QuestionMark",
+            _Self = "std::option::Option<T>",
+        ),
+        // `Option`-in-`Option` always works, as there's only one possible
+        // residual, so this can also be phrased strongly.
+        message = "the `?` operator can only be used on `Option`s \
+            in {ItemContext} that returns `Option`",
+        label = "this `?` produces `{R}`, which is incompatible with `{Self}`",
+        parent_label = "this function returns an `Option`"
+    ),
+    on(
+        all(
+            from_desugaring = "QuestionMark",
+            _Self = "std::ops::ControlFlow<B, C>",
+            R = "std::ops::ControlFlow<B, C>",
+        ),
+        message = "the `?` operator in {ItemContext} that returns `ControlFlow<B, _>` \
+            can only be used on other `ControlFlow<B, _>`s (with the same Break type)",
+        label = "this `?` produces `{R}`, which is incompatible with `{Self}`",
+        parent_label = "this function returns a `ControlFlow`",
+        note = "unlike `Result`, there's no `From`-conversion performed for `ControlFlow`"
+    ),
+    on(
+        all(
+            from_desugaring = "QuestionMark",
+            _Self = "std::ops::ControlFlow<B, C>",
+            // `R` is not a `ControlFlow`, as that case was matched previously
+        ),
+        message = "the `?` operator can only be used on `ControlFlow`s \
+            in {ItemContext} that returns `ControlFlow`",
+        label = "this `?` produces `{R}`, which is incompatible with `{Self}`",
+        parent_label = "this function returns a `ControlFlow`",
+    ),
+    on(
+        all(from_desugaring = "QuestionMark"),
+        message = "the `?` operator can only be used in {ItemContext} \
+                    that returns `Result` or `Option` \
+                    (or another type that implements `{FromResidual}`)",
+        label = "cannot use the `?` operator in {ItemContext} that returns `{Self}`",
+        parent_label = "this function should return `Result` or `Option` to accept `?`"
+    ),
+))]
+#[cfg_attr(bootstrap, rustc_on_unimplemented(
     on(
         all(
             from_desugaring = "QuestionMark",
@@ -301,7 +381,7 @@ pub trait Try: FromResidual {
         label = "cannot use the `?` operator in {ItemContext} that returns `{Self}`",
         enclosing_scope = "this function should return `Result` or `Option` to accept `?`"
     ),
-)]
+))]
 #[rustc_diagnostic_item = "FromResidual"]
 #[unstable(feature = "try_trait_v2", issue = "84277")]
 pub trait FromResidual<R = <Self as Try>::Residual> {

src/test/ui/on-unimplemented/enclosing-scope.rs

@@ -0,0 +1,27 @@
+// Test scope annotations from `parent_label` parameter
+
+#![feature(rustc_attrs)]
+
+#[rustc_on_unimplemented(parent_label = "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
+}