Rollup merge of #119769 - fmease:rustdoc-off-by-one-dyn-trait-def-gen-args, r=GuillaumeGomez

rustdoc: offset generic args of cross-crate trait object types when cleaning

Fixes #119529.

This PR contains several refactorings apart from the bug fix.
Best reviewed commit by commit.
r? GuillaumeGomez

Author: matthiaskrgr

src/librustdoc/clean/mod.rs

@@ -207,8 +207,13 @@ pub(crate) fn clean_trait_ref_with_bindings<'tcx>(
         span_bug!(cx.tcx.def_span(trait_ref.def_id()), "`TraitRef` had unexpected kind {kind:?}");
     }
     inline::record_extern_fqn(cx, trait_ref.def_id(), kind);
-    let path =
-        external_path(cx, trait_ref.def_id(), true, bindings, trait_ref.map_bound(|tr| tr.args));
+    let path = clean_middle_path(
+        cx,
+        trait_ref.def_id(),
+        true,
+        bindings,
+        trait_ref.map_bound(|tr| tr.args),
+    );
 
     debug!(?trait_ref);
 
@@ -467,7 +472,7 @@ fn projection_to_path_segment<'tcx>(
     PathSegment {
         name: item.name,
         args: GenericArgs::AngleBracketed {
-            args: ty_args_to_args(
+            args: clean_middle_generic_args(
                 cx,
                 ty.map_bound(|ty| &ty.args[generics.parent_count..]),
                 false,
@@ -1903,7 +1908,7 @@ fn normalize<'tcx>(
 
 fn clean_trait_object_lifetime_bound<'tcx>(
     region: ty::Region<'tcx>,
-    container: Option<ContainerTy<'tcx>>,
+    container: Option<ContainerTy<'_, 'tcx>>,
     preds: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
     tcx: TyCtxt<'tcx>,
 ) -> Option<Lifetime> {
@@ -1932,7 +1937,7 @@ fn clean_trait_object_lifetime_bound<'tcx>(
 
 fn can_elide_trait_object_lifetime_bound<'tcx>(
     region: ty::Region<'tcx>,
-    container: Option<ContainerTy<'tcx>>,
+    container: Option<ContainerTy<'_, 'tcx>>,
     preds: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
     tcx: TyCtxt<'tcx>,
 ) -> bool {
@@ -1979,21 +1984,22 @@ fn can_elide_trait_object_lifetime_bound<'tcx>(
 }
 
 #[derive(Debug)]
-pub(crate) enum ContainerTy<'tcx> {
+pub(crate) enum ContainerTy<'a, 'tcx> {
     Ref(ty::Region<'tcx>),
     Regular {
         ty: DefId,
-        args: ty::Binder<'tcx, &'tcx [ty::GenericArg<'tcx>]>,
-        has_self: bool,
+        /// The arguments *have* to contain an arg for the self type if the corresponding generics
+        /// contain a self type.
+        args: ty::Binder<'tcx, &'a [ty::GenericArg<'tcx>]>,
         arg: usize,
     },
 }
 
-impl<'tcx> ContainerTy<'tcx> {
+impl<'tcx> ContainerTy<'_, 'tcx> {
     fn object_lifetime_default(self, tcx: TyCtxt<'tcx>) -> ObjectLifetimeDefault<'tcx> {
         match self {
             Self::Ref(region) => ObjectLifetimeDefault::Arg(region),
-            Self::Regular { ty: container, args, has_self, arg: index } => {
+            Self::Regular { ty: container, args, arg: index } => {
                 let (DefKind::Struct
                 | DefKind::Union
                 | DefKind::Enum
@@ -2006,14 +2012,7 @@ impl<'tcx> ContainerTy<'tcx> {
                 let generics = tcx.generics_of(container);
                 debug_assert_eq!(generics.parent_count, 0);
 
-                // If the container is a trait object type, the arguments won't contain the self type but the
-                // generics of the corresponding trait will. In such a case, offset the index by one.
-                // For comparison, if the container is a trait inside a bound, the arguments do contain the
-                // self type.
-                let offset =
-                    if !has_self && generics.parent.is_none() && generics.has_self { 1 } else { 0 };
-                let param = generics.params[index + offset].def_id;
-
+                let param = generics.params[index].def_id;
                 let default = tcx.object_lifetime_default(param);
                 match default {
                     rbv::ObjectLifetimeDefault::Param(lifetime) => {
@@ -2045,7 +2044,7 @@ pub(crate) fn clean_middle_ty<'tcx>(
     bound_ty: ty::Binder<'tcx, Ty<'tcx>>,
     cx: &mut DocContext<'tcx>,
     parent_def_id: Option<DefId>,
-    container: Option<ContainerTy<'tcx>>,
+    container: Option<ContainerTy<'_, 'tcx>>,
 ) -> Type {
     let bound_ty = normalize(cx, bound_ty).unwrap_or(bound_ty);
     match *bound_ty.skip_binder().kind() {
@@ -2096,12 +2095,12 @@ pub(crate) fn clean_middle_ty<'tcx>(
                 AdtKind::Enum => ItemType::Enum,
             };
             inline::record_extern_fqn(cx, did, kind);
-            let path = external_path(cx, did, false, ThinVec::new(), bound_ty.rebind(args));
+            let path = clean_middle_path(cx, did, false, ThinVec::new(), bound_ty.rebind(args));
             Type::Path { path }
         }
         ty::Foreign(did) => {
             inline::record_extern_fqn(cx, did, ItemType::ForeignType);
-            let path = external_path(
+            let path = clean_middle_path(
                 cx,
                 did,
                 false,
@@ -2132,7 +2131,7 @@ pub(crate) fn clean_middle_ty<'tcx>(
             let mut bounds = dids
                 .map(|did| {
                     let empty = ty::Binder::dummy(ty::GenericArgs::empty());
-                    let path = external_path(cx, did, false, ThinVec::new(), empty);
+                    let path = clean_middle_path(cx, did, false, ThinVec::new(), empty);
                     inline::record_extern_fqn(cx, did, ItemType::Trait);
                     PolyTrait { trait_: path, generic_params: Vec::new() }
                 })
@@ -2171,7 +2170,7 @@ pub(crate) fn clean_middle_ty<'tcx>(
                 .collect();
             let late_bound_regions = late_bound_regions.into_iter().collect();
 
-            let path = external_path(cx, did, false, bindings, args);
+            let path = clean_middle_path(cx, did, false, bindings, args);
             bounds.insert(0, PolyTrait { trait_: path, generic_params: late_bound_regions });
 
             DynTrait(bounds, lifetime)
@@ -2193,7 +2192,7 @@ pub(crate) fn clean_middle_ty<'tcx>(
                 assoc: PathSegment {
                     name: cx.tcx.associated_item(def_id).name,
                     args: GenericArgs::AngleBracketed {
-                        args: ty_args_to_args(
+                        args: clean_middle_generic_args(
                             cx,
                             alias_ty.map_bound(|ty| ty.args.as_slice()),
                             true,
@@ -2213,7 +2212,7 @@ pub(crate) fn clean_middle_ty<'tcx>(
             if cx.tcx.features().lazy_type_alias {
                 // Weak type alias `data` represents the `type X` in `type X = Y`. If we need `Y`,
                 // we need to use `type_of`.
-                let path = external_path(
+                let path = clean_middle_path(
                     cx,
                     data.def_id,
                     false,
@@ -2243,7 +2242,8 @@ pub(crate) fn clean_middle_ty<'tcx>(
         ty::Alias(ty::Opaque, ty::AliasTy { def_id, args, .. }) => {
             // If it's already in the same alias, don't get an infinite loop.
             if cx.current_type_aliases.contains_key(&def_id) {
-                let path = external_path(cx, def_id, false, ThinVec::new(), bound_ty.rebind(args));
+                let path =
+                    clean_middle_path(cx, def_id, false, ThinVec::new(), bound_ty.rebind(args));
                 Type::Path { path }
             } else {
                 *cx.current_type_aliases.entry(def_id).or_insert(0) += 1;

src/librustdoc/clean/types.rs

@@ -36,7 +36,7 @@ use rustc_target::abi::VariantIdx;
 use rustc_target::spec::abi::Abi;
 
 use crate::clean::cfg::Cfg;
-use crate::clean::external_path;
+use crate::clean::clean_middle_path;
 use crate::clean::inline::{self, print_inlined_const};
 use crate::clean::utils::{is_literal_expr, print_evaluated_const};
 use crate::core::DocContext;
@@ -1258,7 +1258,7 @@ impl GenericBound {
     fn sized_with(cx: &mut DocContext<'_>, modifier: hir::TraitBoundModifier) -> GenericBound {
         let did = cx.tcx.require_lang_item(LangItem::Sized, None);
         let empty = ty::Binder::dummy(ty::GenericArgs::empty());
-        let path = external_path(cx, did, false, ThinVec::new(), empty);
+        let path = clean_middle_path(cx, did, false, ThinVec::new(), empty);
         inline::record_extern_fqn(cx, did, ItemType::Trait);
         GenericBound::TraitBound(PolyTrait { trait_: path, generic_params: Vec::new() }, modifier)
     }

src/librustdoc/clean/utils.rs

@@ -16,9 +16,10 @@ use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE};
 use rustc_metadata::rendered_const;
 use rustc_middle::mir;
+use rustc_middle::ty::TypeVisitableExt;
 use rustc_middle::ty::{self, GenericArgKind, GenericArgsRef, TyCtxt};
-use rustc_middle::ty::{TypeVisitable, TypeVisitableExt};
 use rustc_span::symbol::{kw, sym, Symbol};
+use std::assert_matches::debug_assert_matches;
 use std::fmt::Write as _;
 use std::mem;
 use std::sync::LazyLock as Lazy;
@@ -75,94 +76,101 @@ pub(crate) fn krate(cx: &mut DocContext<'_>) -> Crate {
     Crate { module, external_traits: cx.external_traits.clone() }
 }
 
-pub(crate) fn ty_args_to_args<'tcx>(
+pub(crate) fn clean_middle_generic_args<'tcx>(
     cx: &mut DocContext<'tcx>,
-    ty_args: ty::Binder<'tcx, &'tcx [ty::GenericArg<'tcx>]>,
-    has_self: bool,
+    args: ty::Binder<'tcx, &'tcx [ty::GenericArg<'tcx>]>,
+    mut has_self: bool,
     owner: DefId,
 ) -> Vec<GenericArg> {
-    if ty_args.skip_binder().is_empty() {
+    let (args, bound_vars) = (args.skip_binder(), args.bound_vars());
+    if args.is_empty() {
         // Fast path which avoids executing the query `generics_of`.
         return Vec::new();
     }
 
-    let params = &cx.tcx.generics_of(owner).params;
-    let mut elision_has_failed_once_before = false;
+    // If the container is a trait object type, the arguments won't contain the self type but the
+    // generics of the corresponding trait will. In such a case, prepend a dummy self type in order
+    // to align the arguments and parameters for the iteration below and to enable us to correctly
+    // instantiate the generic parameter default later.
+    let generics = cx.tcx.generics_of(owner);
+    let args = if !has_self && generics.parent.is_none() && generics.has_self {
+        has_self = true;
+        [cx.tcx.types.trait_object_dummy_self.into()]
+            .into_iter()
+            .chain(args.iter().copied())
+            .collect::<Vec<_>>()
+            .into()
+    } else {
+        std::borrow::Cow::from(args)
+    };
 
-    let offset = if has_self { 1 } else { 0 };
-    let mut args = Vec::with_capacity(ty_args.skip_binder().len().saturating_sub(offset));
+    let mut elision_has_failed_once_before = false;
+    let clean_arg = |(index, &arg): (usize, &ty::GenericArg<'tcx>)| {
+        // Elide the self type.
+        if has_self && index == 0 {
+            return None;
+        }
 
-    let ty_arg_to_arg = |(index, arg): (usize, &ty::GenericArg<'tcx>)| match arg.unpack() {
-        GenericArgKind::Lifetime(lt) => {
-            Some(GenericArg::Lifetime(clean_middle_region(lt).unwrap_or(Lifetime::elided())))
+        // Elide internal host effect args.
+        let param = generics.param_at(index, cx.tcx);
+        if param.is_host_effect() {
+            return None;
         }
-        GenericArgKind::Type(_) if has_self && index == 0 => None,
-        GenericArgKind::Type(ty) => {
-            if !elision_has_failed_once_before
-                && let Some(default) = params[index].default_value(cx.tcx)
-            {
-                let default =
-                    ty_args.map_bound(|args| default.instantiate(cx.tcx, args).expect_ty());
-
-                if can_elide_generic_arg(ty_args.rebind(ty), default) {
-                    return None;
-                }
 
-                elision_has_failed_once_before = true;
+        let arg = ty::Binder::bind_with_vars(arg, bound_vars);
+
+        // Elide arguments that coincide with their default.
+        if !elision_has_failed_once_before && let Some(default) = param.default_value(cx.tcx) {
+            let default = default.instantiate(cx.tcx, args.as_ref());
+            if can_elide_generic_arg(arg, arg.rebind(default)) {
+                return None;
             }
+            elision_has_failed_once_before = true;
+        }
 
-            Some(GenericArg::Type(clean_middle_ty(
-                ty_args.rebind(ty),
+        match arg.skip_binder().unpack() {
+            GenericArgKind::Lifetime(lt) => {
+                Some(GenericArg::Lifetime(clean_middle_region(lt).unwrap_or(Lifetime::elided())))
+            }
+            GenericArgKind::Type(ty) => Some(GenericArg::Type(clean_middle_ty(
+                arg.rebind(ty),
                 cx,
                 None,
                 Some(crate::clean::ContainerTy::Regular {
                     ty: owner,
-                    args: ty_args,
-                    has_self,
+                    args: arg.rebind(args.as_ref()),
                     arg: index,
                 }),
-            )))
-        }
-        GenericArgKind::Const(ct) => {
-            if let ty::GenericParamDefKind::Const { is_host_effect: true, .. } = params[index].kind
-            {
-                return None;
-            }
-
-            if !elision_has_failed_once_before
-                && let Some(default) = params[index].default_value(cx.tcx)
-            {
-                let default =
-                    ty_args.map_bound(|args| default.instantiate(cx.tcx, args).expect_const());
-
-                if can_elide_generic_arg(ty_args.rebind(ct), default) {
-                    return None;
-                }
-
-                elision_has_failed_once_before = true;
+            ))),
+            GenericArgKind::Const(ct) => {
+                Some(GenericArg::Const(Box::new(clean_middle_const(arg.rebind(ct), cx))))
             }
-
-            Some(GenericArg::Const(Box::new(clean_middle_const(ty_args.rebind(ct), cx))))
         }
     };
 
-    args.extend(ty_args.skip_binder().iter().enumerate().rev().filter_map(ty_arg_to_arg));
-    args.reverse();
-    args
+    let offset = if has_self { 1 } else { 0 };
+    let mut clean_args = Vec::with_capacity(args.len().saturating_sub(offset));
+    clean_args.extend(args.iter().enumerate().rev().filter_map(clean_arg));
+    clean_args.reverse();
+    clean_args
 }
 
 /// Check if the generic argument `actual` coincides with the `default` and can therefore be elided.
 ///
 /// This uses a very conservative approach for performance and correctness reasons, meaning for
 /// several classes of terms it claims that they cannot be elided even if they theoretically could.
 /// This is absolutely fine since it mostly concerns edge cases.
-fn can_elide_generic_arg<'tcx, Term>(
-    actual: ty::Binder<'tcx, Term>,
-    default: ty::Binder<'tcx, Term>,
-) -> bool
-where
-    Term: Eq + TypeVisitable<TyCtxt<'tcx>>,
-{
+fn can_elide_generic_arg<'tcx>(
+    actual: ty::Binder<'tcx, ty::GenericArg<'tcx>>,
+    default: ty::Binder<'tcx, ty::GenericArg<'tcx>>,
+) -> bool {
+    debug_assert_matches!(
+        (actual.skip_binder().unpack(), default.skip_binder().unpack()),
+        (ty::GenericArgKind::Lifetime(_), ty::GenericArgKind::Lifetime(_))
+            | (ty::GenericArgKind::Type(_), ty::GenericArgKind::Type(_))
+            | (ty::GenericArgKind::Const(_), ty::GenericArgKind::Const(_))
+    );
+
     // In practice, we shouldn't have any inference variables at this point.
     // However to be safe, we bail out if we do happen to stumble upon them.
     if actual.has_infer() || default.has_infer() {
@@ -192,14 +200,14 @@ where
     actual.skip_binder() == default.skip_binder()
 }
 
-fn external_generic_args<'tcx>(
+fn clean_middle_generic_args_with_bindings<'tcx>(
     cx: &mut DocContext<'tcx>,
     did: DefId,
     has_self: bool,
     bindings: ThinVec<TypeBinding>,
     ty_args: ty::Binder<'tcx, GenericArgsRef<'tcx>>,
 ) -> GenericArgs {
-    let args = ty_args_to_args(cx, ty_args.map_bound(|args| &args[..]), has_self, did);
+    let args = clean_middle_generic_args(cx, ty_args.map_bound(|args| &args[..]), has_self, did);
 
     if cx.tcx.fn_trait_kind_from_def_id(did).is_some() {
         let ty = ty_args
@@ -225,7 +233,7 @@ fn external_generic_args<'tcx>(
     }
 }
 
-pub(super) fn external_path<'tcx>(
+pub(super) fn clean_middle_path<'tcx>(
     cx: &mut DocContext<'tcx>,
     did: DefId,
     has_self: bool,
@@ -238,7 +246,7 @@ pub(super) fn external_path<'tcx>(
         res: Res::Def(def_kind, did),
         segments: thin_vec![PathSegment {
             name,
-            args: external_generic_args(cx, did, has_self, bindings, args),
+            args: clean_middle_generic_args_with_bindings(cx, did, has_self, bindings, args),
         }],
     }
 }

tests/rustdoc/inline_cross/auxiliary/default-generic-args.rs

@@ -40,6 +40,11 @@ pub struct Multi<A = u64, B = u64>(A, B);
 
 pub type M0 = Multi<u64, ()>;
 
-pub trait Trait<'a, T = &'a ()> {}
-
-pub type F = dyn for<'a> Trait<'a>;
+pub trait Trait0<'a, T = &'a ()> {}
+pub type D0 = dyn for<'a> Trait0<'a>;
+
+// Regression test for issue #119529.
+pub trait Trait1<T = (), const K: u32 = 0> {}
+pub type D1<T> = dyn Trait1<T>;
+pub type D2<const K: u32> = dyn Trait1<(), K>;
+pub type D3 = dyn Trait1;

tests/rustdoc/inline_cross/auxiliary/u_default_generic_args.rs

@@ -0,0 +1 @@
+pub use default_generic_args::*;