Rollup merge of #75711 - CohenArthur:split-up-astconv, r=oli-obk

Split `astconv.rs` into its own submodule

Fixes #67418

This changed induced a few changes across the Type checker, but only there. Mostly, it was just renaming `Self::` into something else to call specific methods from a subtrait instead of having a 2500+ lines one.

I split up the `astconv.rs` file into its own module. This way, directives such as
```rust
use crate::astconv::AstConv;
```
are still valid, and doing
```rust
use crate::astconv::{AstConv, AstConvGeneric};
```
is possible

(instead of having two modules, one named `astconv_generic.rs` for example and `astconv.rs`)

I'm not entirely sure that the name `AstConvGeneric` is a good one. However, only methods related to lifetimes or generics have been moved over to this module. Sorry about the large diff.

I'd be very happy to make any correction you deem necessary.

r? @oli-obk

src/librustc_typeck/astconv/errors.rs

+use crate::astconv::AstConv;
+use rustc_ast::util::lev_distance::find_best_match_for_name;
+use rustc_data_structures::fx::FxHashMap;
+use rustc_errors::{pluralize, struct_span_err, Applicability};
+use rustc_hir as hir;
+use rustc_hir::def_id::DefId;
+use rustc_middle::ty;
+use rustc_session::parse::feature_err;
+use rustc_span::symbol::{sym, Ident};
+use rustc_span::{Span, DUMMY_SP};
+
+use std::collections::BTreeSet;
+
+impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
+    /// On missing type parameters, emit an E0393 error and provide a structured suggestion using
+    /// the type parameter's name as a placeholder.
+    pub(crate) fn complain_about_missing_type_params(
+        &self,
+        missing_type_params: Vec<String>,
+        def_id: DefId,
+        span: Span,
+        empty_generic_args: bool,
+    ) {
+        if missing_type_params.is_empty() {
+            return;
+        }
+        let display =
+            missing_type_params.iter().map(|n| format!("`{}`", n)).collect::<Vec<_>>().join(", ");
+        let mut err = struct_span_err!(
+            self.tcx().sess,
+            span,
+            E0393,
+            "the type parameter{} {} must be explicitly specified",
+            pluralize!(missing_type_params.len()),
+            display,
+        );
+        err.span_label(
+            self.tcx().def_span(def_id),
+            &format!(
+                "type parameter{} {} must be specified for this",
+                pluralize!(missing_type_params.len()),
+                display,
+            ),
+        );
+        let mut suggested = false;
+        if let (Ok(snippet), true) = (
+            self.tcx().sess.source_map().span_to_snippet(span),
+            // Don't suggest setting the type params if there are some already: the order is
+            // tricky to get right and the user will already know what the syntax is.
+            empty_generic_args,
+        ) {
+            if snippet.ends_with('>') {
+                // The user wrote `Trait<'a, T>` or similar. To provide an accurate suggestion
+                // we would have to preserve the right order. For now, as clearly the user is
+                // aware of the syntax, we do nothing.
+            } else {
+                // The user wrote `Iterator`, so we don't have a type we can suggest, but at
+                // least we can clue them to the correct syntax `Iterator<Type>`.
+                err.span_suggestion(
+                    span,
+                    &format!(
+                        "set the type parameter{plural} to the desired type{plural}",
+                        plural = pluralize!(missing_type_params.len()),
+                    ),
+                    format!("{}<{}>", snippet, missing_type_params.join(", ")),
+                    Applicability::HasPlaceholders,
+                );
+                suggested = true;
+            }
+        }
+        if !suggested {
+            err.span_label(
+                span,
+                format!(
+                    "missing reference{} to {}",
+                    pluralize!(missing_type_params.len()),
+                    display,
+                ),
+            );
+        }
+        err.note(
+            "because of the default `Self` reference, type parameters must be \
+                  specified on object types",
+        );
+        err.emit();
+    }
+
+    /// When the code is using the `Fn` traits directly, instead of the `Fn(A) -> B` syntax, emit
+    /// an error and attempt to build a reasonable structured suggestion.
+    pub(crate) fn complain_about_internal_fn_trait(
+        &self,
+        span: Span,
+        trait_def_id: DefId,
+        trait_segment: &'a hir::PathSegment<'a>,
+    ) {
+        let trait_def = self.tcx().trait_def(trait_def_id);
+
+        if !self.tcx().features().unboxed_closures
+            && trait_segment.generic_args().parenthesized != trait_def.paren_sugar
+        {
+            let sess = &self.tcx().sess.parse_sess;
+            // For now, require that parenthetical notation be used only with `Fn()` etc.
+            let (msg, sugg) = if trait_def.paren_sugar {
+                (
+                    "the precise format of `Fn`-family traits' type parameters is subject to \
+                     change",
+                    Some(format!(
+                        "{}{} -> {}",
+                        trait_segment.ident,
+                        trait_segment
+                            .args
+                            .as_ref()
+                            .and_then(|args| args.args.get(0))
+                            .and_then(|arg| match arg {
+                                hir::GenericArg::Type(ty) => match ty.kind {
+                                    hir::TyKind::Tup(t) => t
+                                        .iter()
+                                        .map(|e| sess.source_map().span_to_snippet(e.span))
+                                        .collect::<Result<Vec<_>, _>>()
+                                        .map(|a| a.join(", ")),
+                                    _ => sess.source_map().span_to_snippet(ty.span),
+                                }
+                                .map(|s| format!("({})", s))
+                                .ok(),
+                                _ => None,
+                            })
+                            .unwrap_or_else(|| "()".to_string()),
+                        trait_segment
+                            .generic_args()
+                            .bindings
+                            .iter()
+                            .find_map(|b| match (b.ident.name == sym::Output, &b.kind) {
+                                (true, hir::TypeBindingKind::Equality { ty }) => {
+                                    sess.source_map().span_to_snippet(ty.span).ok()
+                                }
+                                _ => None,
+                            })
+                            .unwrap_or_else(|| "()".to_string()),
+                    )),
+                )
+            } else {
+                ("parenthetical notation is only stable when used with `Fn`-family traits", None)
+            };
+            let mut err = feature_err(sess, sym::unboxed_closures, span, msg);
+            if let Some(sugg) = sugg {
+                let msg = "use parenthetical notation instead";
+                err.span_suggestion(span, msg, sugg, Applicability::MaybeIncorrect);
+            }
+            err.emit();
+        }
+    }
+
+    pub(crate) fn complain_about_assoc_type_not_found<I>(
+        &self,
+        all_candidates: impl Fn() -> I,
+        ty_param_name: &str,
+        assoc_name: Ident,
+        span: Span,
+    ) where
+        I: Iterator<Item = ty::PolyTraitRef<'tcx>>,
+    {
+        // The fallback span is needed because `assoc_name` might be an `Fn()`'s `Output` without a
+        // valid span, so we point at the whole path segment instead.
+        let span = if assoc_name.span != DUMMY_SP { assoc_name.span } else { span };
+        let mut err = struct_span_err!(
+            self.tcx().sess,
+            span,
+            E0220,
+            "associated type `{}` not found for `{}`",
+            assoc_name,
+            ty_param_name
+        );
+
+        let all_candidate_names: Vec<_> = all_candidates()
+            .map(|r| self.tcx().associated_items(r.def_id()).in_definition_order())
+            .flatten()
+            .filter_map(
+                |item| if item.kind == ty::AssocKind::Type { Some(item.ident.name) } else { None },
+            )
+            .collect();
+
+        if let (Some(suggested_name), true) = (
+            find_best_match_for_name(all_candidate_names.iter(), assoc_name.name, None),
+            assoc_name.span != DUMMY_SP,
+        ) {
+            err.span_suggestion(
+                assoc_name.span,
+                "there is an associated type with a similar name",
+                suggested_name.to_string(),
+                Applicability::MaybeIncorrect,
+            );
+        } else {
+            err.span_label(span, format!("associated type `{}` not found", assoc_name));
+        }
+
+        err.emit();
+    }
+
+    /// When there are any missing associated types, emit an E0191 error and attempt to supply a
+    /// reasonable suggestion on how to write it. For the case of multiple associated types in the
+    /// same trait bound have the same name (as they come from different super-traits), we instead
+    /// emit a generic note suggesting using a `where` clause to constraint instead.
+    pub(crate) fn complain_about_missing_associated_types(
+        &self,
+        associated_types: FxHashMap<Span, BTreeSet<DefId>>,
+        potential_assoc_types: Vec<Span>,
+        trait_bounds: &[hir::PolyTraitRef<'_>],
+    ) {
+        if associated_types.values().all(|v| v.is_empty()) {
+            return;
+        }
+        let tcx = self.tcx();
+        // FIXME: Marked `mut` so that we can replace the spans further below with a more
+        // appropriate one, but this should be handled earlier in the span assignment.
+        let mut associated_types: FxHashMap<Span, Vec<_>> = associated_types
+            .into_iter()
+            .map(|(span, def_ids)| {
+                (span, def_ids.into_iter().map(|did| tcx.associated_item(did)).collect())
+            })
+            .collect();
+        let mut names = vec![];
+
+        // Account for things like `dyn Foo + 'a`, like in tests `issue-22434.rs` and
+        // `issue-22560.rs`.
+        let mut trait_bound_spans: Vec<Span> = vec![];
+        for (span, items) in &associated_types {
+            if !items.is_empty() {
+                trait_bound_spans.push(*span);
+            }
+            for assoc_item in items {
+                let trait_def_id = assoc_item.container.id();
+                names.push(format!(
+                    "`{}` (from trait `{}`)",
+                    assoc_item.ident,
+                    tcx.def_path_str(trait_def_id),
+                ));
+            }
+        }
+        if let ([], [bound]) = (&potential_assoc_types[..], &trait_bounds) {
+            match &bound.trait_ref.path.segments[..] {
+                // FIXME: `trait_ref.path.span` can point to a full path with multiple
+                // segments, even though `trait_ref.path.segments` is of length `1`. Work
+                // around that bug here, even though it should be fixed elsewhere.
+                // This would otherwise cause an invalid suggestion. For an example, look at
+                // `src/test/ui/issues/issue-28344.rs` where instead of the following:
+                //
+                //   error[E0191]: the value of the associated type `Output`
+                //                 (from trait `std::ops::BitXor`) must be specified
+                //   --> $DIR/issue-28344.rs:4:17
+                //    |
+                // LL |     let x: u8 = BitXor::bitor(0 as u8, 0 as u8);
+                //    |                 ^^^^^^ help: specify the associated type:
+                //    |                              `BitXor<Output = Type>`
+                //
+                // we would output:
+                //
+                //   error[E0191]: the value of the associated type `Output`
+                //                 (from trait `std::ops::BitXor`) must be specified
+                //   --> $DIR/issue-28344.rs:4:17
+                //    |
+                // LL |     let x: u8 = BitXor::bitor(0 as u8, 0 as u8);
+                //    |                 ^^^^^^^^^^^^^ help: specify the associated type:
+                //    |                                     `BitXor::bitor<Output = Type>`
+                [segment] if segment.args.is_none() => {
+                    trait_bound_spans = vec![segment.ident.span];
+                    associated_types = associated_types
+                        .into_iter()
+                        .map(|(_, items)| (segment.ident.span, items))
+                        .collect();
+                }
+                _ => {}
+            }
+        }
+        names.sort();
+        trait_bound_spans.sort();
+        let mut err = struct_span_err!(
+            tcx.sess,
+            trait_bound_spans,
+            E0191,
+            "the value of the associated type{} {} must be specified",
+            pluralize!(names.len()),
+            names.join(", "),
+        );
+        let mut suggestions = vec![];
+        let mut types_count = 0;
+        let mut where_constraints = vec![];
+        for (span, assoc_items) in &associated_types {
+            let mut names: FxHashMap<_, usize> = FxHashMap::default();
+            for item in assoc_items {
+                types_count += 1;
+                *names.entry(item.ident.name).or_insert(0) += 1;
+            }
+            let mut dupes = false;
+            for item in assoc_items {
+                let prefix = if names[&item.ident.name] > 1 {
+                    let trait_def_id = item.container.id();
+                    dupes = true;
+                    format!("{}::", tcx.def_path_str(trait_def_id))
+                } else {
+                    String::new()
+                };
+                if let Some(sp) = tcx.hir().span_if_local(item.def_id) {
+                    err.span_label(sp, format!("`{}{}` defined here", prefix, item.ident));
+                }
+            }
+            if potential_assoc_types.len() == assoc_items.len() {
+                // Only suggest when the amount of missing associated types equals the number of
+                // extra type arguments present, as that gives us a relatively high confidence
+                // that the user forgot to give the associtated type's name. The canonical
+                // example would be trying to use `Iterator<isize>` instead of
+                // `Iterator<Item = isize>`.
+                for (potential, item) in potential_assoc_types.iter().zip(assoc_items.iter()) {
+                    if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(*potential) {
+                        suggestions.push((*potential, format!("{} = {}", item.ident, snippet)));
+                    }
+                }
+            } else if let (Ok(snippet), false) =
+                (tcx.sess.source_map().span_to_snippet(*span), dupes)
+            {
+                let types: Vec<_> =
+                    assoc_items.iter().map(|item| format!("{} = Type", item.ident)).collect();
+                let code = if snippet.ends_with('>') {
+                    // The user wrote `Trait<'a>` or similar and we don't have a type we can
+                    // suggest, but at least we can clue them to the correct syntax
+                    // `Trait<'a, Item = Type>` while accounting for the `<'a>` in the
+                    // suggestion.
+                    format!("{}, {}>", &snippet[..snippet.len() - 1], types.join(", "))
+                } else {
+                    // The user wrote `Iterator`, so we don't have a type we can suggest, but at
+                    // least we can clue them to the correct syntax `Iterator<Item = Type>`.
+                    format!("{}<{}>", snippet, types.join(", "))
+                };
+                suggestions.push((*span, code));
+            } else if dupes {
+                where_constraints.push(*span);
+            }
+        }
+        let where_msg = "consider introducing a new type parameter, adding `where` constraints \
+                         using the fully-qualified path to the associated types";
+        if !where_constraints.is_empty() && suggestions.is_empty() {
+            // If there are duplicates associated type names and a single trait bound do not
+            // use structured suggestion, it means that there are multiple super-traits with
+            // the same associated type name.
+            err.help(where_msg);
+        }
+        if suggestions.len() != 1 {
+            // We don't need this label if there's an inline suggestion, show otherwise.
+            for (span, assoc_items) in &associated_types {
+                let mut names: FxHashMap<_, usize> = FxHashMap::default();
+                for item in assoc_items {
+                    types_count += 1;
+                    *names.entry(item.ident.name).or_insert(0) += 1;
+                }
+                let mut label = vec![];
+                for item in assoc_items {
+                    let postfix = if names[&item.ident.name] > 1 {
+                        let trait_def_id = item.container.id();
+                        format!(" (from trait `{}`)", tcx.def_path_str(trait_def_id))
+                    } else {
+                        String::new()
+                    };
+                    label.push(format!("`{}`{}", item.ident, postfix));
+                }
+                if !label.is_empty() {
+                    err.span_label(
+                        *span,
+                        format!(
+                            "associated type{} {} must be specified",
+                            pluralize!(label.len()),
+                            label.join(", "),
+                        ),
+                    );
+                }
+            }
+        }
+        if !suggestions.is_empty() {
+            err.multipart_suggestion(
+                &format!("specify the associated type{}", pluralize!(types_count)),
+                suggestions,
+                Applicability::HasPlaceholders,
+            );
+            if !where_constraints.is_empty() {
+                err.span_help(where_constraints, where_msg);
+            }
+        }
+        err.emit();
+    }
+}

src/librustc_typeck/bounds.rs

+//! Bounds are restrictions applied to some types after they've been converted into the
+//! `ty` form from the HIR.
+
+use rustc_hir::Constness;
+use rustc_middle::ty::{self, ToPredicate, Ty, TyCtxt, WithConstness};
+use rustc_span::Span;
+
+/// Collects together a list of type bounds. These lists of bounds occur in many places
+/// in Rust's syntax:
+///
+/// ```text
+/// trait Foo: Bar + Baz { }
+///            ^^^^^^^^^ supertrait list bounding the `Self` type parameter
+///
+/// fn foo<T: Bar + Baz>() { }
+///           ^^^^^^^^^ bounding the type parameter `T`
+///
+/// impl dyn Bar + Baz
+///          ^^^^^^^^^ bounding the forgotten dynamic type
+/// ```
+///
+/// Our representation is a bit mixed here -- in some cases, we
+/// include the self type (e.g., `trait_bounds`) but in others we do not
+#[derive(Default, PartialEq, Eq, Clone, Debug)]
+pub struct Bounds<'tcx> {
+    /// A list of region bounds on the (implicit) self type. So if you
+    /// had `T: 'a + 'b` this might would be a list `['a, 'b]` (but
+    /// the `T` is not explicitly included).
+    pub region_bounds: Vec<(ty::Region<'tcx>, Span)>,
+
+    /// A list of trait bounds. So if you had `T: Debug` this would be
+    /// `T: Debug`. Note that the self-type is explicit here.
+    pub trait_bounds: Vec<(ty::PolyTraitRef<'tcx>, Span, Constness)>,
+
+    /// A list of projection equality bounds. So if you had `T:
+    /// Iterator<Item = u32>` this would include `<T as
+    /// Iterator>::Item => u32`. Note that the self-type is explicit
+    /// here.
+    pub projection_bounds: Vec<(ty::PolyProjectionPredicate<'tcx>, Span)>,
+
+    /// `Some` if there is *no* `?Sized` predicate. The `span`
+    /// is the location in the source of the `T` declaration which can
+    /// be cited as the source of the `T: Sized` requirement.
+    pub implicitly_sized: Option<Span>,
+}
+
+impl<'tcx> Bounds<'tcx> {
+    /// Converts a bounds list into a flat set of predicates (like
+    /// where-clauses). Because some of our bounds listings (e.g.,
+    /// regions) don't include the self-type, you must supply the
+    /// self-type here (the `param_ty` parameter).
+    pub fn predicates(
+        &self,
+        tcx: TyCtxt<'tcx>,
+        param_ty: Ty<'tcx>,
+    ) -> Vec<(ty::Predicate<'tcx>, Span)> {
+        // If it could be sized, and is, add the `Sized` predicate.
+        let sized_predicate = self.implicitly_sized.and_then(|span| {
+            tcx.lang_items().sized_trait().map(|sized| {
+                let trait_ref = ty::Binder::bind(ty::TraitRef {
+                    def_id: sized,
+                    substs: tcx.mk_substs_trait(param_ty, &[]),
+                });
+                (trait_ref.without_const().to_predicate(tcx), span)
+            })
+        });
+
+        sized_predicate
+            .into_iter()
+            .chain(
+                self.region_bounds
+                    .iter()
+                    .map(|&(region_bound, span)| {
+                        // Account for the binder being introduced below; no need to shift `param_ty`
+                        // because, at present at least, it either only refers to early-bound regions,
+                        // or it's a generic associated type that deliberately has escaping bound vars.
+                        let region_bound = ty::fold::shift_region(tcx, region_bound, 1);
+                        let outlives = ty::OutlivesPredicate(param_ty, region_bound);
+                        (ty::Binder::bind(outlives).to_predicate(tcx), span)
+                    })
+                    .chain(self.trait_bounds.iter().map(|&(bound_trait_ref, span, constness)| {
+                        let predicate = bound_trait_ref.with_constness(constness).to_predicate(tcx);
+                        (predicate, span)
+                    }))
+                    .chain(
+                        self.projection_bounds
+                            .iter()
+                            .map(|&(projection, span)| (projection.to_predicate(tcx), span)),
+                    ),
+            )
+            .collect()
+    }
+}

src/librustc_typeck/collect.rs

@@ -14,7 +14,8 @@
 //! At present, however, we do run collection across all items in the
 //! crate as a kind of pass. This should eventually be factored away.
 
-use crate::astconv::{AstConv, Bounds, SizedByDefault};
+use crate::astconv::{AstConv, SizedByDefault};
+use crate::bounds::Bounds;
 use crate::check::intrinsic::intrinsic_operation_unsafety;
 use crate::constrained_generic_params as cgp;
 use crate::middle::resolve_lifetime as rl;

src/librustc_typeck/lib.rs

@@ -79,6 +79,7 @@
 pub mod expr_use_visitor;
 
 mod astconv;
+mod bounds;
 mod check_unused;
 mod coherence;
 mod collect;
@@ -109,7 +110,8 @@
 
 use std::iter;
 
-use astconv::{AstConv, Bounds};
+use astconv::AstConv;
+use bounds::Bounds;
 
 fn require_c_abi_if_c_variadic(tcx: TyCtxt<'_>, decl: &hir::FnDecl<'_>, abi: Abi, span: Span) {
     if decl.c_variadic && !(abi == Abi::C || abi == Abi::Cdecl) {