Auto merge of rust-lang#77524 - Patryk27:fixes/66228, r=estebank

Rework diagnostics for wrong number of generic args (fixes rust-lang#66228 and rust-lang#71924)

This PR reworks the `wrong number of {} arguments` message, so that it provides more details and contextual hints.

Author: bors

compiler/rustc_ast/src/ast.rs

@@ -242,12 +242,21 @@ impl Into<Option<P<GenericArgs>>> for ParenthesizedArgs {
 /// A path like `Foo(A, B) -> C`.
 #[derive(Clone, Encodable, Decodable, Debug)]
 pub struct ParenthesizedArgs {
-    /// Overall span
+    /// ```text
+    /// Foo(A, B) -> C
+    /// ^^^^^^^^^^^^^^
+    /// ```
     pub span: Span,
 
     /// `(A, B)`
     pub inputs: Vec<P<Ty>>,
 
+    /// ```text
+    /// Foo(A, B) -> C
+    ///    ^^^^^^
+    /// ```
+    pub inputs_span: Span,
+
     /// `C`
     pub output: FnRetTy,
 }

compiler/rustc_ast/src/lib.rs

@@ -13,6 +13,7 @@
 #![feature(const_fn_transmute)]
 #![feature(const_panic)]
 #![feature(crate_visibility_modifier)]
+#![feature(iterator_fold_self)]
 #![feature(label_break_value)]
 #![feature(nll)]
 #![feature(or_patterns)]

compiler/rustc_ast/src/mut_visit.rs

@@ -565,7 +565,7 @@ pub fn noop_visit_parenthesized_parameter_data<T: MutVisitor>(
     args: &mut ParenthesizedArgs,
     vis: &mut T,
 ) {
-    let ParenthesizedArgs { inputs, output, span } = args;
+    let ParenthesizedArgs { inputs, output, span, .. } = args;
     visit_vec(inputs, |input| vis.visit_ty(input));
     noop_visit_fn_ret_ty(output, vis);
     vis.visit_span(span);

compiler/rustc_ast_lowering/src/path.rs

@@ -401,15 +401,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         // compatibility, even in contexts like an impl header where
         // we generally don't permit such things (see #51008).
         self.with_anonymous_lifetime_mode(AnonymousLifetimeMode::PassThrough, |this| {
-            let &ParenthesizedArgs { ref inputs, ref output, span } = data;
+            let ParenthesizedArgs { span, inputs, inputs_span, output } = data;
             let inputs = this.arena.alloc_from_iter(
                 inputs.iter().map(|ty| this.lower_ty_direct(ty, ImplTraitContext::disallowed())),
             );
             let output_ty = match output {
                 FnRetTy::Ty(ty) => this.lower_ty(&ty, ImplTraitContext::disallowed()),
-                FnRetTy::Default(_) => this.arena.alloc(this.ty_tup(span, &[])),
+                FnRetTy::Default(_) => this.arena.alloc(this.ty_tup(*span, &[])),
             };
-            let args = smallvec![GenericArg::Type(this.ty_tup(span, inputs))];
+            let args = smallvec![GenericArg::Type(this.ty_tup(*inputs_span, inputs))];
             let binding = this.output_ty_binding(output_ty.span, output_ty);
             (
                 GenericArgsCtor { args, bindings: arena_vec![this; binding], parenthesized: true },

compiler/rustc_hir/src/hir.rs

@@ -11,9 +11,9 @@ pub use rustc_ast::{CaptureBy, Movability, Mutability};
 use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
 use rustc_data_structures::sync::{par_for_each_in, Send, Sync};
 use rustc_macros::HashStable_Generic;
-use rustc_span::def_id::LocalDefId;
-use rustc_span::source_map::Spanned;
+use rustc_span::source_map::{SourceMap, Spanned};
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
+use rustc_span::{def_id::LocalDefId, BytePos};
 use rustc_span::{MultiSpan, Span, DUMMY_SP};
 use rustc_target::asm::InlineAsmRegOrRegClass;
 use rustc_target::spec::abi::Abi;
@@ -231,7 +231,11 @@ impl<'hir> PathSegment<'hir> {
         PathSegment { ident, hir_id: None, res: None, infer_args: true, args: None }
     }
 
-    pub fn generic_args(&self) -> &GenericArgs<'hir> {
+    pub fn invalid() -> Self {
+        Self::from_ident(Ident::invalid())
+    }
+
+    pub fn args(&self) -> &GenericArgs<'hir> {
         if let Some(ref args) = self.args {
             args
         } else {
@@ -275,19 +279,15 @@ impl GenericArg<'_> {
         matches!(self, GenericArg::Const(_))
     }
 
-    pub fn descr(&self) -> &'static str {
-        match self {
-            GenericArg::Lifetime(_) => "lifetime",
-            GenericArg::Type(_) => "type",
-            GenericArg::Const(_) => "constant",
-        }
+    pub fn is_synthetic(&self) -> bool {
+        matches!(self, GenericArg::Lifetime(lifetime) if lifetime.name.ident() == Ident::invalid())
     }
 
-    pub fn short_descr(&self) -> &'static str {
+    pub fn descr(&self) -> &'static str {
         match self {
             GenericArg::Lifetime(_) => "lifetime",
             GenericArg::Type(_) => "type",
-            GenericArg::Const(_) => "const",
+            GenericArg::Const(_) => "constant",
         }
     }
 
@@ -352,6 +352,39 @@ impl GenericArgs<'_> {
 
         own_counts
     }
+
+    pub fn span(&self) -> Option<Span> {
+        self.args
+            .iter()
+            .filter(|arg| !arg.is_synthetic())
+            .map(|arg| arg.span())
+            .fold_first(|span1, span2| span1.to(span2))
+    }
+
+    /// Returns span encompassing arguments and their surrounding `<>` or `()`
+    pub fn span_ext(&self, sm: &SourceMap) -> Option<Span> {
+        let mut span = self.span()?;
+
+        let (o, c) = if self.parenthesized { ('(', ')') } else { ('<', '>') };
+
+        if let Ok(snippet) = sm.span_to_snippet(span) {
+            let snippet = snippet.as_bytes();
+
+            if snippet[0] != (o as u8) || snippet[snippet.len() - 1] != (c as u8) {
+                span = sm.span_extend_to_prev_char(span, o, true);
+                span = span.with_lo(span.lo() - BytePos(1));
+
+                span = sm.span_extend_to_next_char(span, c, true);
+                span = span.with_hi(span.hi() + BytePos(1));
+            }
+        }
+
+        Some(span)
+    }
+
+    pub fn is_empty(&self) -> bool {
+        self.args.is_empty()
+    }
 }
 
 /// A modifier on a bound, currently this is only used for `?Sized`, where the

compiler/rustc_hir/src/lang_items.rs

@@ -157,7 +157,7 @@ where
 }
 
 language_item_table! {
-//  Variant name,            Name,                    Method name,             Target;
+//  Variant name,            Name,                     Method name,                Target;
     Bool,                    sym::bool,                bool_impl,                  Target::Impl;
     Char,                    sym::char,                char_impl,                  Target::Impl;
     Str,                     sym::str,                 str_impl,                   Target::Impl;

compiler/rustc_hir/src/lib.rs

@@ -7,6 +7,7 @@
 #![feature(const_fn)] // For the unsizing cast on `&[]`
 #![feature(const_panic)]
 #![feature(in_band_lifetimes)]
+#![feature(iterator_fold_self)]
 #![feature(once_cell)]
 #![feature(or_patterns)]
 #![recursion_limit = "256"]

compiler/rustc_hir_pretty/src/lib.rs

@@ -1213,7 +1213,7 @@ impl<'a> State<'a> {
         self.s.word(".");
         self.print_ident(segment.ident);
 
-        let generic_args = segment.generic_args();
+        let generic_args = segment.args();
         if !generic_args.args.is_empty() || !generic_args.bindings.is_empty() {
             self.print_generic_args(generic_args, segment.infer_args, true);
         }
@@ -1661,19 +1661,15 @@ impl<'a> State<'a> {
             }
             if segment.ident.name != kw::PathRoot {
                 self.print_ident(segment.ident);
-                self.print_generic_args(
-                    segment.generic_args(),
-                    segment.infer_args,
-                    colons_before_params,
-                );
+                self.print_generic_args(segment.args(), segment.infer_args, colons_before_params);
             }
         }
     }
 
     pub fn print_path_segment(&mut self, segment: &hir::PathSegment<'_>) {
         if segment.ident.name != kw::PathRoot {
             self.print_ident(segment.ident);
-            self.print_generic_args(segment.generic_args(), segment.infer_args, false);
+            self.print_generic_args(segment.args(), segment.infer_args, false);
         }
     }
 
@@ -1693,7 +1689,7 @@ impl<'a> State<'a> {
                     if segment.ident.name != kw::PathRoot {
                         self.print_ident(segment.ident);
                         self.print_generic_args(
-                            segment.generic_args(),
+                            segment.args(),
                             segment.infer_args,
                             colons_before_params,
                         );
@@ -1705,7 +1701,7 @@ impl<'a> State<'a> {
                 let item_segment = path.segments.last().unwrap();
                 self.print_ident(item_segment.ident);
                 self.print_generic_args(
-                    item_segment.generic_args(),
+                    item_segment.args(),
                     item_segment.infer_args,
                     colons_before_params,
                 )
@@ -1725,7 +1721,7 @@ impl<'a> State<'a> {
                 self.s.word("::");
                 self.print_ident(item_segment.ident);
                 self.print_generic_args(
-                    item_segment.generic_args(),
+                    item_segment.args(),
                     item_segment.infer_args,
                     colons_before_params,
                 )

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -721,21 +721,25 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
         &self.raw_proc_macros.unwrap()[pos]
     }
 
-    fn item_ident(&self, item_index: DefIndex, sess: &Session) -> Ident {
+    fn try_item_ident(&self, item_index: DefIndex, sess: &Session) -> Result<Ident, String> {
         let name = self
             .def_key(item_index)
             .disambiguated_data
             .data
             .get_opt_name()
-            .expect("no name in item_ident");
+            .ok_or_else(|| format!("Missing opt name for {:?}", item_index))?;
         let span = self
             .root
             .tables
             .ident_span
             .get(self, item_index)
-            .map(|data| data.decode((self, sess)))
-            .unwrap_or_else(|| panic!("Missing ident span for {:?} ({:?})", name, item_index));
-        Ident::new(name, span)
+            .ok_or_else(|| format!("Missing ident span for {:?} ({:?})", name, item_index))?
+            .decode((self, sess));
+        Ok(Ident::new(name, span))
+    }
+
+    fn item_ident(&self, item_index: DefIndex, sess: &Session) -> Ident {
+        self.try_item_ident(item_index, sess).unwrap()
     }
 
     fn def_kind(&self, index: DefIndex) -> DefKind {

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

@@ -129,6 +129,9 @@ provide! { <'tcx> tcx, def_id, other, cdata,
     generator_kind => { cdata.generator_kind(def_id.index) }
     def_kind => { cdata.def_kind(def_id.index) }
     def_span => { cdata.get_span(def_id.index, &tcx.sess) }
+    def_ident_span => {
+        cdata.try_item_ident(def_id.index, &tcx.sess).ok().map(|ident| ident.span)
+    }
     lookup_stability => {
         cdata.get_stability(def_id.index).map(|s| tcx.intern_stability(s))
     }

compiler/rustc_middle/src/query/mod.rs

@@ -881,6 +881,7 @@ rustc_queries! {
         query def_kind(def_id: DefId) -> DefKind {
             desc { |tcx| "looking up definition kind of `{}`", tcx.def_path_str(def_id) }
         }
+
         query def_span(def_id: DefId) -> Span {
             desc { |tcx| "looking up span for `{}`", tcx.def_path_str(def_id) }
             // FIXME(mw): DefSpans are not really inputs since they are derived from
@@ -890,15 +891,23 @@ rustc_queries! {
             // regardless of HIR hashing.
             eval_always
         }
+
+        query def_ident_span(def_id: DefId) -> Option<Span> {
+            desc { |tcx| "looking up span for `{}`'s identifier", tcx.def_path_str(def_id) }
+        }
+
         query lookup_stability(def_id: DefId) -> Option<&'tcx attr::Stability> {
             desc { |tcx| "looking up stability of `{}`", tcx.def_path_str(def_id) }
         }
+
         query lookup_const_stability(def_id: DefId) -> Option<&'tcx attr::ConstStability> {
             desc { |tcx| "looking up const stability of `{}`", tcx.def_path_str(def_id) }
         }
+
         query lookup_deprecation_entry(def_id: DefId) -> Option<DeprecationEntry> {
             desc { |tcx| "checking whether `{}` is deprecated", tcx.def_path_str(def_id) }
         }
+
         query item_attrs(def_id: DefId) -> &'tcx [ast::Attribute] {
             desc { |tcx| "collecting attributes of `{}`", tcx.def_path_str(def_id) }
         }

compiler/rustc_middle/src/ty/error.rs

@@ -847,7 +847,7 @@ fn foo(&self) -> Self::T { String::new() }
     }
 
     /// Given a slice of `hir::GenericBound`s, if any of them corresponds to the `trait_ref`
-    /// requirement, provide a strucuted suggestion to constrain it to a given type `ty`.
+    /// requirement, provide a structured suggestion to constrain it to a given type `ty`.
     fn constrain_generic_bound_associated_type_structured_suggestion(
         self,
         db: &mut DiagnosticBuilder<'_>,

compiler/rustc_middle/src/ty/mod.rs

@@ -871,19 +871,37 @@ impl<'tcx> Generics {
         // We could cache this as a property of `GenericParamCount`, but
         // the aim is to refactor this away entirely eventually and the
         // presence of this method will be a constant reminder.
-        let mut own_counts: GenericParamCount = Default::default();
+        let mut own_counts = GenericParamCount::default();
 
         for param in &self.params {
             match param.kind {
                 GenericParamDefKind::Lifetime => own_counts.lifetimes += 1,
                 GenericParamDefKind::Type { .. } => own_counts.types += 1,
                 GenericParamDefKind::Const => own_counts.consts += 1,
-            };
+            }
         }
 
         own_counts
     }
 
+    pub fn own_defaults(&self) -> GenericParamCount {
+        let mut own_defaults = GenericParamCount::default();
+
+        for param in &self.params {
+            match param.kind {
+                GenericParamDefKind::Lifetime => (),
+                GenericParamDefKind::Type { has_default, .. } => {
+                    own_defaults.types += has_default as usize;
+                }
+                GenericParamDefKind::Const => {
+                    // FIXME(const_generics:defaults)
+                }
+            }
+        }
+
+        own_defaults
+    }
+
     pub fn requires_monomorphization(&self, tcx: TyCtxt<'tcx>) -> bool {
         if self.own_requires_monomorphization() {
             return true;

compiler/rustc_parse/src/parser/path.rs

@@ -230,10 +230,11 @@ impl<'a> Parser<'a> {
                 } else {
                     // `(T, U) -> R`
                     let (inputs, _) = self.parse_paren_comma_seq(|p| p.parse_ty())?;
+                    let inputs_span = lo.to(self.prev_token.span);
                     let span = ident.span.to(self.prev_token.span);
                     let output =
                         self.parse_ret_ty(AllowPlus::No, RecoverQPath::No, RecoverReturnSign::No)?;
-                    ParenthesizedArgs { inputs, output, span }.into()
+                    ParenthesizedArgs { span, inputs, inputs_span, output }.into()
                 };
 
                 PathSegment { ident, args, id: ast::DUMMY_NODE_ID }

compiler/rustc_resolve/src/late/lifetimes.rs

@@ -1433,7 +1433,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
                     hir::TyKind::Path(ref qpath) => {
                         if let QPath::Resolved(_, path) = qpath {
                             let last_segment = &path.segments[path.segments.len() - 1];
-                            let generics = last_segment.generic_args();
+                            let generics = last_segment.args();
                             for arg in generics.args.iter() {
                                 if let GenericArg::Lifetime(lt) = arg {
                                     if lt.name.ident() == name {