rustc_on_unimplemented cleanups

compiler/rustc_span/src/symbol.rs

@@ -398,7 +398,6 @@ symbols! {
         Wrapping,
         Yield,
         _DECLS,
-        _Self,
         __D,
         __H,
         __S,

compiler/rustc_trait_selection/messages.ftl

@@ -337,6 +337,8 @@ trait_selection_rustc_on_unimplemented_expected_one_predicate_in_not = expected
     .label = unexpected quantity of predicates here
 trait_selection_rustc_on_unimplemented_invalid_flag = invalid flag in `on`-clause
     .label = expected one of the `crate_local`, `direct` or `from_desugaring` flags, not `{$invalid_flag}`
+trait_selection_rustc_on_unimplemented_invalid_name = invalid name in `on`-clause
+    .label = expected one of `cause`, `from_desugaring`, `Self` or any generic parameter of the trait, not `{$invalid_name}`
 trait_selection_rustc_on_unimplemented_invalid_predicate = this predicate is invalid
     .label = expected one of `any`, `all` or `not` here, not `{$invalid_pred}`
 trait_selection_rustc_on_unimplemented_missing_value = this attribute must have a value

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

@@ -429,7 +429,19 @@ impl<'tcx> OnUnimplementedDirective {
                 .next()
                 .ok_or_else(|| tcx.dcx().emit_err(InvalidOnClause::Empty { span }))?;
 
-            match OnUnimplementedCondition::parse(cond) {
+            let generics: Vec<Symbol> = tcx
+                .generics_of(item_def_id)
+                .own_params
+                .iter()
+                .filter_map(|param| {
+                    if matches!(param.kind, GenericParamDefKind::Lifetime) {
+                        None
+                    } else {
+                        Some(param.name)
+                    }
+                })
+                .collect();
+            match OnUnimplementedCondition::parse(cond, &generics) {
                 Ok(condition) => Some(condition),
                 Err(e) => return Err(tcx.dcx().emit_err(e)),
             }

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

@@ -26,9 +26,12 @@ impl OnUnimplementedCondition {
         })
     }
 
-    pub(crate) fn parse(input: &MetaItemInner) -> Result<Self, InvalidOnClause> {
+    pub(crate) fn parse(
+        input: &MetaItemInner,
+        generics: &[Symbol],
+    ) -> Result<Self, InvalidOnClause> {
         let span = input.span();
-        let pred = Predicate::parse(input)?;
+        let pred = Predicate::parse(input, generics)?;
         Ok(OnUnimplementedCondition { span, pred })
     }
 }
@@ -52,7 +55,7 @@ enum Predicate {
 }
 
 impl Predicate {
-    fn parse(input: &MetaItemInner) -> Result<Self, InvalidOnClause> {
+    fn parse(input: &MetaItemInner, generics: &[Symbol]) -> Result<Self, InvalidOnClause> {
         let meta_item = match input {
             MetaItemInner::MetaItem(meta_item) => meta_item,
             MetaItemInner::Lit(lit) => {
@@ -69,10 +72,10 @@ impl Predicate {
 
         match meta_item.kind {
             MetaItemKind::List(ref mis) => match predicate.name {
-                sym::any => Ok(Predicate::Any(Predicate::parse_sequence(mis)?)),
-                sym::all => Ok(Predicate::All(Predicate::parse_sequence(mis)?)),
+                sym::any => Ok(Predicate::Any(Predicate::parse_sequence(mis, generics)?)),
+                sym::all => Ok(Predicate::All(Predicate::parse_sequence(mis, generics)?)),
                 sym::not => match &**mis {
-                    [one] => Ok(Predicate::Not(Box::new(Predicate::parse(one)?))),
+                    [one] => Ok(Predicate::Not(Box::new(Predicate::parse(one, generics)?))),
                     [first, .., last] => Err(InvalidOnClause::ExpectedOnePredInNot {
                         span: first.span().to(last.span()),
                     }),
@@ -83,7 +86,7 @@ impl Predicate {
                 }
             },
             MetaItemKind::NameValue(MetaItemLit { symbol, .. }) => {
-                let name = Name::parse(predicate);
+                let name = Name::parse(predicate, generics)?;
                 let value = FilterFormatString::parse(symbol);
                 let kv = NameValue { name, value };
                 Ok(Predicate::Match(kv))
@@ -95,8 +98,11 @@ impl Predicate {
         }
     }
 
-    fn parse_sequence(sequence: &[MetaItemInner]) -> Result<Vec<Self>, InvalidOnClause> {
-        sequence.iter().map(Predicate::parse).collect()
+    fn parse_sequence(
+        sequence: &[MetaItemInner],
+        generics: &[Symbol],
+    ) -> Result<Vec<Self>, InvalidOnClause> {
+        sequence.iter().map(|item| Predicate::parse(item, generics)).collect()
     }
 
     fn eval(&self, eval: &mut impl FnMut(FlagOrNv<'_>) -> bool) -> bool {
@@ -156,14 +162,13 @@ enum Name {
 }
 
 impl Name {
-    fn parse(Ident { name, .. }: Ident) -> Self {
+    fn parse(Ident { name, span }: Ident, generics: &[Symbol]) -> Result<Self, InvalidOnClause> {
         match name {
-            sym::_Self | kw::SelfUpper => Name::SelfUpper,
-            sym::from_desugaring => Name::FromDesugaring,
-            sym::cause => Name::Cause,
-            // FIXME(mejrs) Perhaps we should start checking that
-            // this actually is a valid generic parameter?
-            generic => Name::GenericArg(generic),
+            kw::SelfUpper => Ok(Name::SelfUpper),
+            sym::from_desugaring => Ok(Name::FromDesugaring),
+            sym::cause => Ok(Name::Cause),
+            generic if generics.contains(&generic) => Ok(Name::GenericArg(generic)),
+            invalid_name => Err(InvalidOnClause::InvalidName { invalid_name, span }),
         }
     }
 }

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

@@ -2,8 +2,8 @@ use std::fmt;
 use std::ops::Range;
 
 use errors::*;
-use rustc_middle::ty::TyCtxt;
 use rustc_middle::ty::print::TraitRefPrintSugared;
+use rustc_middle::ty::{GenericParamDefKind, TyCtxt};
 use rustc_parse_format::{
     Alignment, Argument, Count, FormatSpec, ParseError, ParseMode, Parser, Piece as RpfPiece,
     Position,
@@ -232,48 +232,16 @@ fn parse_arg<'tcx>(
 ) -> FormatArg {
     let (Ctx::RustcOnUnimplemented { tcx, trait_def_id }
     | Ctx::DiagnosticOnUnimplemented { tcx, trait_def_id }) = ctx;
-    let trait_name = tcx.item_ident(*trait_def_id);
-    let generics = tcx.generics_of(trait_def_id);
+
     let span = slice_span(input_span, arg.position_span.clone());
 
     match arg.position {
         // Something like "hello {name}"
         Position::ArgumentNamed(name) => match (ctx, Symbol::intern(name)) {
-            // accepted, but deprecated
-            (Ctx::RustcOnUnimplemented { .. }, sym::_Self) => {
-                warnings
-                    .push(FormatWarning::FutureIncompat { span, help: String::from("use {Self}") });
-                FormatArg::SelfUpper
-            }
-            (
-                Ctx::RustcOnUnimplemented { .. },
-                sym::from_desugaring
-                | sym::crate_local
-                | sym::direct
-                | sym::cause
-                | sym::float
-                | sym::integer_
-                | sym::integral,
-            ) => {
-                warnings.push(FormatWarning::FutureIncompat {
-                    span,
-                    help: String::from("don't use this in a format string"),
-                });
-                FormatArg::AsIs(String::new())
-            }
-
             // Only `#[rustc_on_unimplemented]` can use these
             (Ctx::RustcOnUnimplemented { .. }, sym::ItemContext) => FormatArg::ItemContext,
             (Ctx::RustcOnUnimplemented { .. }, sym::This) => FormatArg::This,
             (Ctx::RustcOnUnimplemented { .. }, sym::Trait) => FormatArg::Trait,
-            // `{ThisTraitsName}`. Some attrs in std use this, but I'd like to change it to the more general `{This}`
-            // because that'll be simpler to parse and extend in the future
-            (Ctx::RustcOnUnimplemented { .. }, name) if name == trait_name.name => {
-                warnings
-                    .push(FormatWarning::FutureIncompat { span, help: String::from("use {This}") });
-                FormatArg::This
-            }
-
             // Any attribute can use these
             (
                 Ctx::RustcOnUnimplemented { .. } | Ctx::DiagnosticOnUnimplemented { .. },
@@ -282,7 +250,10 @@ fn parse_arg<'tcx>(
             (
                 Ctx::RustcOnUnimplemented { .. } | Ctx::DiagnosticOnUnimplemented { .. },
                 generic_param,
-            ) if generics.own_params.iter().any(|param| param.name == generic_param) => {
+            ) if tcx.generics_of(trait_def_id).own_params.iter().any(|param| {
+                !matches!(param.kind, GenericParamDefKind::Lifetime) && param.name == generic_param
+            }) =>
+            {
                 FormatArg::GenericParam { generic_param }
             }
 
@@ -375,39 +346,4 @@ pub mod errors {
     #[diag(trait_selection_missing_options_for_on_unimplemented_attr)]
     #[help]
     pub struct MissingOptionsForOnUnimplementedAttr;
-
-    #[derive(LintDiagnostic)]
-    #[diag(trait_selection_ignored_diagnostic_option)]
-    pub struct IgnoredDiagnosticOption {
-        pub option_name: &'static str,
-        #[label]
-        pub span: Span,
-        #[label(trait_selection_other_label)]
-        pub prev_span: Span,
-    }
-
-    impl IgnoredDiagnosticOption {
-        pub fn maybe_emit_warning<'tcx>(
-            tcx: TyCtxt<'tcx>,
-            item_def_id: DefId,
-            new: Option<Span>,
-            old: Option<Span>,
-            option_name: &'static str,
-        ) {
-            if let (Some(new_item), Some(old_item)) = (new, old) {
-                if let Some(item_def_id) = item_def_id.as_local() {
-                    tcx.emit_node_span_lint(
-                        UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES,
-                        tcx.local_def_id_to_hir_id(item_def_id),
-                        new_item,
-                        IgnoredDiagnosticOption {
-                            span: new_item,
-                            prev_span: old_item,
-                            option_name,
-                        },
-                    );
-                }
-            }
-        }
-    }
 }

compiler/rustc_trait_selection/src/errors.rs

@@ -72,6 +72,13 @@ pub enum InvalidOnClause {
         span: Span,
         invalid_flag: Symbol,
     },
+    #[diag(trait_selection_rustc_on_unimplemented_invalid_name, code = E0232)]
+    InvalidName {
+        #[primary_span]
+        #[label]
+        span: Span,
+        invalid_name: Symbol,
+    },
 }
 
 #[derive(Diagnostic)]

library/core/src/convert/mod.rs

@@ -575,7 +575,7 @@ pub trait Into<T>: Sized {
 #[rustc_diagnostic_item = "From"]
 #[stable(feature = "rust1", since = "1.0.0")]
 #[rustc_on_unimplemented(on(
-    all(_Self = "&str", T = "alloc::string::String"),
+    all(Self = "&str", T = "alloc::string::String"),
     note = "to coerce a `{T}` into a `{Self}`, use `&*` as a prefix",
 ))]
 #[doc(search_unbox)]

library/core/src/fmt/mod.rs

@@ -856,10 +856,10 @@ impl Display for Arguments<'_> {
     on(
         crate_local,
         label = "`{Self}` cannot be formatted using `{{:?}}`",
-        note = "add `#[derive(Debug)]` to `{Self}` or manually `impl {Debug} for {Self}`"
+        note = "add `#[derive(Debug)]` to `{Self}` or manually `impl {This} for {Self}`"
     ),
-    message = "`{Self}` doesn't implement `{Debug}`",
-    label = "`{Self}` cannot be formatted using `{{:?}}` because it doesn't implement `{Debug}`"
+    message = "`{Self}` doesn't implement `{This}`",
+    label = "`{Self}` cannot be formatted using `{{:?}}` because it doesn't implement `{This}`"
 )]
 #[doc(alias = "{:?}")]
 #[rustc_diagnostic_item = "Debug"]
@@ -969,12 +969,12 @@ pub use macros::Debug;
 /// ```
 #[rustc_on_unimplemented(
     on(
-        any(_Self = "std::path::Path", _Self = "std::path::PathBuf"),
+        any(Self = "std::path::Path", Self = "std::path::PathBuf"),
         label = "`{Self}` cannot be formatted with the default formatter; call `.display()` on it",
         note = "call `.display()` or `.to_string_lossy()` to safely print paths, \
                 as they may contain non-Unicode data"
     ),
-    message = "`{Self}` doesn't implement `{Display}`",
+    message = "`{Self}` doesn't implement `{This}`",
     label = "`{Self}` cannot be formatted with the default formatter",
     note = "in format strings you may be able to use `{{:?}}` (or {{:#?}} for pretty-print) instead"
 )]

library/core/src/iter/traits/collect.rs

@@ -97,32 +97,32 @@ use super::TrustedLen;
 #[stable(feature = "rust1", since = "1.0.0")]
 #[rustc_on_unimplemented(
     on(
-        _Self = "&[{A}]",
+        Self = "&[{A}]",
         message = "a slice of type `{Self}` cannot be built since we need to store the elements somewhere",
         label = "try explicitly collecting into a `Vec<{A}>`",
     ),
     on(
-        all(A = "{integer}", any(_Self = "&[{integral}]",)),
+        all(A = "{integer}", any(Self = "&[{integral}]",)),
         message = "a slice of type `{Self}` cannot be built since we need to store the elements somewhere",
         label = "try explicitly collecting into a `Vec<{A}>`",
     ),
     on(
-        _Self = "[{A}]",
+        Self = "[{A}]",
         message = "a slice of type `{Self}` cannot be built since `{Self}` has no definite size",
         label = "try explicitly collecting into a `Vec<{A}>`",
     ),
     on(
-        all(A = "{integer}", any(_Self = "[{integral}]",)),
+        all(A = "{integer}", any(Self = "[{integral}]",)),
         message = "a slice of type `{Self}` cannot be built since `{Self}` has no definite size",
         label = "try explicitly collecting into a `Vec<{A}>`",
     ),
     on(
-        _Self = "[{A}; _]",
+        Self = "[{A}; _]",
         message = "an array of type `{Self}` cannot be built directly from an iterator",
         label = "try collecting into a `Vec<{A}>`, then using `.try_into()`",
     ),
     on(
-        all(A = "{integer}", any(_Self = "[{integral}; _]",)),
+        all(A = "{integer}", any(Self = "[{integral}; _]",)),
         message = "an array of type `{Self}` cannot be built directly from an iterator",
         label = "try collecting into a `Vec<{A}>`, then using `.try_into()`",
     ),
@@ -239,41 +239,38 @@ pub trait FromIterator<A>: Sized {
 #[rustc_diagnostic_item = "IntoIterator"]
 #[rustc_on_unimplemented(
     on(
-        _Self = "core::ops::range::RangeTo<Idx>",
+        Self = "core::ops::range::RangeTo<Idx>",
         label = "if you meant to iterate until a value, add a starting value",
         note = "`..end` is a `RangeTo`, which cannot be iterated on; you might have meant to have a \
               bounded `Range`: `0..end`"
     ),
     on(
-        _Self = "core::ops::range::RangeToInclusive<Idx>",
+        Self = "core::ops::range::RangeToInclusive<Idx>",
         label = "if you meant to iterate until a value (including it), add a starting value",
         note = "`..=end` is a `RangeToInclusive`, which cannot be iterated on; you might have meant \
               to have a bounded `RangeInclusive`: `0..=end`"
     ),
     on(
-        _Self = "[]",
+        Self = "[]",
         label = "`{Self}` is not an iterator; try calling `.into_iter()` or `.iter()`"
     ),
-    on(_Self = "&[]", label = "`{Self}` is not an iterator; try calling `.iter()`"),
+    on(Self = "&[]", label = "`{Self}` is not an iterator; try calling `.iter()`"),
     on(
-        _Self = "alloc::vec::Vec<T, A>",
+        Self = "alloc::vec::Vec<T, A>",
         label = "`{Self}` is not an iterator; try calling `.into_iter()` or `.iter()`"
     ),
+    on(Self = "&str", label = "`{Self}` is not an iterator; try calling `.chars()` or `.bytes()`"),
     on(
-        _Self = "&str",
+        Self = "alloc::string::String",
         label = "`{Self}` is not an iterator; try calling `.chars()` or `.bytes()`"
     ),
     on(
-        _Self = "alloc::string::String",
-        label = "`{Self}` is not an iterator; try calling `.chars()` or `.bytes()`"
-    ),
-    on(
-        _Self = "{integral}",
+        Self = "{integral}",
         note = "if you want to iterate between `start` until a value `end`, use the exclusive range \
               syntax `start..end` or the inclusive range syntax `start..=end`"
     ),
     on(
-        _Self = "{float}",
+        Self = "{float}",
         note = "if you want to iterate between `start` until a value `end`, use the exclusive range \
               syntax `start..end` or the inclusive range syntax `start..=end`"
     ),

library/core/src/iter/traits/iterator.rs

@@ -22,11 +22,11 @@ fn _assert_is_dyn_compatible(_: &dyn Iterator<Item = ()>) {}
 #[stable(feature = "rust1", since = "1.0.0")]
 #[rustc_on_unimplemented(
     on(
-        _Self = "core::ops::range::RangeTo<Idx>",
+        Self = "core::ops::range::RangeTo<Idx>",
         note = "you might have meant to use a bounded `Range`"
     ),
     on(
-        _Self = "core::ops::range::RangeToInclusive<Idx>",
+        Self = "core::ops::range::RangeToInclusive<Idx>",
         note = "you might have meant to use a bounded `RangeInclusive`"
     ),
     label = "`{Self}` is not an iterator",