Auto merge of #151816 - Zalathar:rollup-z5YytdB, r=Zalathar

Rollup of 5 pull requests

Successful merges:

 - #151775 (Portable SIMD subtree update)
 - #151488 (Tweak E0599 to consolidate unsatisfied trait bound messages)
 - #149823 (fix(parser): Disallow CR in frontmatter )
 - #151475 (add foregin type tests for issue 64458)
 - #151657 (Cleanup of `#[derive(Diagnostic)]` attribute parsers)

Author: bors

compiler/rustc_hir_typeck/src/method/suggest.rs

@@ -13,7 +13,9 @@ use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
 use rustc_data_structures::sorted_map::SortedMap;
 use rustc_data_structures::unord::UnordSet;
 use rustc_errors::codes::*;
-use rustc_errors::{Applicability, Diag, MultiSpan, StashKey, pluralize, struct_span_code_err};
+use rustc_errors::{
+    Applicability, Diag, MultiSpan, StashKey, listify, pluralize, struct_span_code_err,
+};
 use rustc_hir::attrs::AttributeKind;
 use rustc_hir::def::{CtorKind, DefKind, Res};
 use rustc_hir::def_id::DefId;
@@ -50,6 +52,51 @@ use crate::errors::{self, CandidateTraitNote, NoAssociatedItem};
 use crate::method::probe::UnsatisfiedPredicates;
 use crate::{Expectation, FnCtxt};
 
+/// Tracks trait bounds and detects duplicates between ref and non-ref versions of self types.
+/// This is used to condense error messages when the same trait bound appears for both
+/// `T` and `&T` (or `&mut T`).
+struct TraitBoundDuplicateTracker {
+    trait_def_ids: FxIndexSet<DefId>,
+    seen_ref: FxIndexSet<DefId>,
+    seen_non_ref: FxIndexSet<DefId>,
+    has_ref_dupes: bool,
+}
+
+impl TraitBoundDuplicateTracker {
+    fn new() -> Self {
+        Self {
+            trait_def_ids: FxIndexSet::default(),
+            seen_ref: FxIndexSet::default(),
+            seen_non_ref: FxIndexSet::default(),
+            has_ref_dupes: false,
+        }
+    }
+
+    /// Track a trait bound. `is_ref` indicates whether the self type is a reference.
+    fn track(&mut self, def_id: DefId, is_ref: bool) {
+        self.trait_def_ids.insert(def_id);
+        if is_ref {
+            if self.seen_non_ref.contains(&def_id) {
+                self.has_ref_dupes = true;
+            }
+            self.seen_ref.insert(def_id);
+        } else {
+            if self.seen_ref.contains(&def_id) {
+                self.has_ref_dupes = true;
+            }
+            self.seen_non_ref.insert(def_id);
+        }
+    }
+
+    fn has_ref_dupes(&self) -> bool {
+        self.has_ref_dupes
+    }
+
+    fn into_trait_def_ids(self) -> FxIndexSet<DefId> {
+        self.trait_def_ids
+    }
+}
+
 impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     fn is_slice_ty(&self, ty: Ty<'tcx>, span: Span) -> bool {
         self.autoderef(span, ty)
@@ -1004,6 +1051,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         item_ident: Ident,
         item_kind: &str,
         bound_spans: SortedMap<Span, Vec<String>>,
+        unsatisfied_predicates: &UnsatisfiedPredicates<'tcx>,
     ) {
         let mut ty_span = match rcvr_ty.kind() {
             ty::Param(param_type) => {
@@ -1012,13 +1060,61 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             ty::Adt(def, _) if def.did().is_local() => Some(self.tcx.def_span(def.did())),
             _ => None,
         };
+        let rcvr_ty_str = self.tcx.short_string(rcvr_ty, err.long_ty_path());
+        let mut tracker = TraitBoundDuplicateTracker::new();
+        for (predicate, _parent_pred, _cause) in unsatisfied_predicates {
+            if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) =
+                predicate.kind().skip_binder()
+                && let self_ty = pred.trait_ref.self_ty()
+                && self_ty.peel_refs() == rcvr_ty
+            {
+                let is_ref = matches!(self_ty.kind(), ty::Ref(..));
+                tracker.track(pred.trait_ref.def_id, is_ref);
+            }
+        }
+        let has_ref_dupes = tracker.has_ref_dupes();
+        let mut missing_trait_names = tracker
+            .into_trait_def_ids()
+            .into_iter()
+            .map(|def_id| format!("`{}`", self.tcx.def_path_str(def_id)))
+            .collect::<Vec<_>>();
+        missing_trait_names.sort();
+        let should_condense =
+            has_ref_dupes && missing_trait_names.len() > 1 && matches!(rcvr_ty.kind(), ty::Adt(..));
+        let missing_trait_list = if should_condense {
+            Some(match missing_trait_names.as_slice() {
+                [only] => only.clone(),
+                [first, second] => format!("{first} or {second}"),
+                [rest @ .., last] => format!("{} or {last}", rest.join(", ")),
+                [] => String::new(),
+            })
+        } else {
+            None
+        };
         for (span, mut bounds) in bound_spans {
             if !self.tcx.sess.source_map().is_span_accessible(span) {
                 continue;
             }
             bounds.sort();
             bounds.dedup();
-            let pre = if Some(span) == ty_span {
+            let is_ty_span = Some(span) == ty_span;
+            if is_ty_span && should_condense {
+                ty_span.take();
+                let label = if let Some(missing_trait_list) = &missing_trait_list {
+                    format!(
+                        "{item_kind} `{item_ident}` not found for this {} because `{rcvr_ty_str}` doesn't implement {missing_trait_list}",
+                        rcvr_ty.prefix_string(self.tcx)
+                    )
+                } else {
+                    format!(
+                        "{item_kind} `{item_ident}` not found for this {}",
+                        rcvr_ty.prefix_string(self.tcx)
+                    )
+                };
+                err.span_label(span, label);
+                continue;
+            }
+            let pre = if is_ty_span {
                 ty_span.take();
                 format!(
                     "{item_kind} `{item_ident}` not found for this {} because it ",
@@ -1248,6 +1344,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             item_ident,
             item_kind,
             bound_spans,
+            unsatisfied_predicates,
         );
 
         self.note_derefed_ty_has_method(&mut err, source, rcvr_ty, item_ident, expected);
@@ -1507,6 +1604,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         bound_spans: &mut SortedMap<Span, Vec<String>>,
     ) {
         let tcx = self.tcx;
+        let rcvr_ty_str = self.tcx.short_string(rcvr_ty, err.long_ty_path());
         let mut type_params = FxIndexMap::default();
 
         // Pick out the list of unimplemented traits on the receiver.
@@ -1798,14 +1896,55 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let mut spanned_predicates: Vec<_> = spanned_predicates.into_iter().collect();
         spanned_predicates.sort_by_key(|(span, _)| *span);
         for (_, (primary_spans, span_labels, predicates)) in spanned_predicates {
+            let mut tracker = TraitBoundDuplicateTracker::new();
+            let mut all_trait_bounds_for_rcvr = true;
+            for pred in &predicates {
+                match pred.kind().skip_binder() {
+                    ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) => {
+                        let self_ty = pred.trait_ref.self_ty();
+                        if self_ty.peel_refs() != rcvr_ty {
+                            all_trait_bounds_for_rcvr = false;
+                            break;
+                        }
+                        let is_ref = matches!(self_ty.kind(), ty::Ref(..));
+                        tracker.track(pred.trait_ref.def_id, is_ref);
+                    }
+                    _ => {
+                        all_trait_bounds_for_rcvr = false;
+                        break;
+                    }
+                }
+            }
+            let has_ref_dupes = tracker.has_ref_dupes();
+            let trait_def_ids = tracker.into_trait_def_ids();
             let mut preds: Vec<_> = predicates
                 .iter()
                 .filter_map(|pred| format_pred(**pred))
                 .map(|(p, _)| format!("`{p}`"))
                 .collect();
             preds.sort();
             preds.dedup();
-            let msg = if let [pred] = &preds[..] {
+            let availability_note = if all_trait_bounds_for_rcvr
+                && has_ref_dupes
+                && trait_def_ids.len() > 1
+                && matches!(rcvr_ty.kind(), ty::Adt(..))
+            {
+                let mut trait_names = trait_def_ids
+                    .into_iter()
+                    .map(|def_id| format!("`{}`", tcx.def_path_str(def_id)))
+                    .collect::<Vec<_>>();
+                trait_names.sort();
+                listify(&trait_names, |name| name.to_string()).map(|traits| {
+                        format!(
+                            "for `{item_ident}` to be available, `{rcvr_ty_str}` must implement {traits}"
+                        )
+                    })
+            } else {
+                None
+            };
+            let msg = if let Some(availability_note) = availability_note {
+                availability_note
+            } else if let [pred] = &preds[..] {
                 format!("trait bound {pred} was not satisfied")
             } else {
                 format!("the following trait bounds were not satisfied:\n{}", preds.join("\n"),)

compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs

@@ -2,6 +2,7 @@
 
 use proc_macro2::{Ident, Span, TokenStream};
 use quote::{format_ident, quote, quote_spanned};
+use syn::parse::ParseStream;
 use syn::spanned::Spanned;
 use syn::{Attribute, Meta, Path, Token, Type, parse_quote};
 use synstructure::{BindingInfo, Structure, VariantInfo};
@@ -11,7 +12,7 @@ use crate::diagnostics::error::{
     DiagnosticDeriveError, span_err, throw_invalid_attr, throw_span_err,
 };
 use crate::diagnostics::utils::{
-    FieldInfo, FieldInnerTy, FieldMap, HasFieldMap, SetOnce, SpannedOption, SubdiagnosticKind,
+    FieldInfo, FieldInnerTy, FieldMap, SetOnce, SpannedOption, SubdiagnosticKind,
     build_field_mapping, is_doc_comment, report_error_if_not_applied_to_span, report_type_error,
     should_generate_arg, type_is_bool, type_is_unit, type_matches_path,
 };
@@ -42,19 +43,13 @@ pub(crate) struct DiagnosticDeriveVariantBuilder {
 
     /// Slug is a mandatory part of the struct attribute as corresponds to the Fluent message that
     /// has the actual diagnostic message.
-    pub slug: SpannedOption<Path>,
+    pub slug: Option<Path>,
 
     /// Error codes are a optional part of the struct attribute - this is only set to detect
     /// multiple specifications.
     pub code: SpannedOption<()>,
 }
 
-impl HasFieldMap for DiagnosticDeriveVariantBuilder {
-    fn get_field_binding(&self, field: &String) -> Option<&TokenStream> {
-        self.field_map.get(field)
-    }
-}
-
 impl DiagnosticDeriveKind {
     /// Call `f` for the struct or for each variant of the enum, returning a `TokenStream` with the
     /// tokens from `f` wrapped in an `match` expression. Emits errors for use of derive on unions
@@ -111,7 +106,7 @@ impl DiagnosticDeriveKind {
 
 impl DiagnosticDeriveVariantBuilder {
     pub(crate) fn primary_message(&self) -> Option<&Path> {
-        match self.slug.value_ref() {
+        match self.slug.as_ref() {
             None => {
                 span_err(self.span, "diagnostic slug not specified")
                     .help(
@@ -169,7 +164,7 @@ impl DiagnosticDeriveVariantBuilder {
         &self,
         attr: &Attribute,
     ) -> Result<Option<(SubdiagnosticKind, Path, bool)>, DiagnosticDeriveError> {
-        let Some(subdiag) = SubdiagnosticVariant::from_attr(attr, self)? else {
+        let Some(subdiag) = SubdiagnosticVariant::from_attr(attr, &self.field_map)? else {
             // Some attributes aren't errors - like documentation comments - but also aren't
             // subdiagnostics.
             return Ok(None);
@@ -191,7 +186,7 @@ impl DiagnosticDeriveVariantBuilder {
             SubdiagnosticKind::MultipartSuggestion { .. } => unreachable!(),
         });
 
-        Ok(Some((subdiag.kind, slug, subdiag.no_span)))
+        Ok(Some((subdiag.kind, slug, false)))
     }
 
     /// Establishes state in the `DiagnosticDeriveBuilder` resulting from the struct
@@ -209,47 +204,54 @@ impl DiagnosticDeriveVariantBuilder {
         let name = attr.path().segments.last().unwrap().ident.to_string();
         let name = name.as_str();
 
-        let mut first = true;
-
         if name == "diag" {
             let mut tokens = TokenStream::new();
-            attr.parse_nested_meta(|nested| {
-                let path = &nested.path;
+            attr.parse_args_with(|input: ParseStream<'_>| {
+                let mut input = &*input;
+                let slug_recovery_point = input.fork();
 
-                if first && (nested.input.is_empty() || nested.input.peek(Token![,])) {
-                    self.slug.set_once(path.clone(), path.span().unwrap());
-                    first = false;
-                    return Ok(());
+                let slug = input.parse::<Path>()?;
+                if input.is_empty() || input.peek(Token![,]) {
+                    self.slug = Some(slug);
+                } else {
+                    input = &slug_recovery_point;
                 }
 
-                first = false;
-
-                let Ok(nested) = nested.value() else {
-                    span_err(
-                        nested.input.span().unwrap(),
-                        "diagnostic slug must be the first argument",
-                    )
-                    .emit();
-                    return Ok(());
-                };
-
-                if path.is_ident("code") {
-                    self.code.set_once((), path.span().unwrap());
-
-                    let code = nested.parse::<syn::Expr>()?;
-                    tokens.extend(quote! {
-                        diag.code(#code);
-                    });
-                } else {
-                    span_err(path.span().unwrap(), "unknown argument")
-                        .note("only the `code` parameter is valid after the slug")
+                while !input.is_empty() {
+                    input.parse::<Token![,]>()?;
+                    // Allow trailing comma
+                    if input.is_empty() {
+                        break;
+                    }
+                    let arg_name: Path = input.parse::<Path>()?;
+                    if input.peek(Token![,]) {
+                        span_err(
+                            arg_name.span().unwrap(),
+                            "diagnostic slug must be the first argument",
+                        )
                         .emit();
-
-                    // consume the buffer so we don't have syntax errors from syn
-                    let _ = nested.parse::<TokenStream>();
+                        continue;
+                    }
+                    let arg_name = arg_name.require_ident()?;
+                    input.parse::<Token![=]>()?;
+                    let arg_value = input.parse::<syn::Expr>()?;
+                    match arg_name.to_string().as_str() {
+                        "code" => {
+                            self.code.set_once((), arg_name.span().unwrap());
+                            tokens.extend(quote! {
+                                diag.code(#arg_value);
+                            });
+                        }
+                        _ => {
+                            span_err(arg_name.span().unwrap(), "unknown argument")
+                                .note("only the `code` parameter is valid after the slug")
+                                .emit();
+                        }
+                    }
                 }
                 Ok(())
             })?;
+
             return Ok(tokens);
         }