Skip to content

Commit 8c84920

Browse files
authored
Unrolled build for rust-lang#119769
Rollup merge of rust-lang#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 rust-lang#119529. This PR contains several refactorings apart from the bug fix. Best reviewed commit by commit. r? GuillaumeGomez
2 parents 9273d63 + 2d010bc commit 8c84920

File tree

6 files changed

+129
-105
lines changed

6 files changed

+129
-105
lines changed

src/librustdoc/clean/mod.rs

+26-26
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,13 @@ pub(crate) fn clean_trait_ref_with_bindings<'tcx>(
207207
span_bug!(cx.tcx.def_span(trait_ref.def_id()), "`TraitRef` had unexpected kind {kind:?}");
208208
}
209209
inline::record_extern_fqn(cx, trait_ref.def_id(), kind);
210-
let path =
211-
external_path(cx, trait_ref.def_id(), true, bindings, trait_ref.map_bound(|tr| tr.args));
210+
let path = clean_middle_path(
211+
cx,
212+
trait_ref.def_id(),
213+
true,
214+
bindings,
215+
trait_ref.map_bound(|tr| tr.args),
216+
);
212217

213218
debug!(?trait_ref);
214219

@@ -467,7 +472,7 @@ fn projection_to_path_segment<'tcx>(
467472
PathSegment {
468473
name: item.name,
469474
args: GenericArgs::AngleBracketed {
470-
args: ty_args_to_args(
475+
args: clean_middle_generic_args(
471476
cx,
472477
ty.map_bound(|ty| &ty.args[generics.parent_count..]),
473478
false,
@@ -1903,7 +1908,7 @@ fn normalize<'tcx>(
19031908

19041909
fn clean_trait_object_lifetime_bound<'tcx>(
19051910
region: ty::Region<'tcx>,
1906-
container: Option<ContainerTy<'tcx>>,
1911+
container: Option<ContainerTy<'_, 'tcx>>,
19071912
preds: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
19081913
tcx: TyCtxt<'tcx>,
19091914
) -> Option<Lifetime> {
@@ -1932,7 +1937,7 @@ fn clean_trait_object_lifetime_bound<'tcx>(
19321937

19331938
fn can_elide_trait_object_lifetime_bound<'tcx>(
19341939
region: ty::Region<'tcx>,
1935-
container: Option<ContainerTy<'tcx>>,
1940+
container: Option<ContainerTy<'_, 'tcx>>,
19361941
preds: &'tcx ty::List<ty::PolyExistentialPredicate<'tcx>>,
19371942
tcx: TyCtxt<'tcx>,
19381943
) -> bool {
@@ -1979,21 +1984,22 @@ fn can_elide_trait_object_lifetime_bound<'tcx>(
19791984
}
19801985

19811986
#[derive(Debug)]
1982-
pub(crate) enum ContainerTy<'tcx> {
1987+
pub(crate) enum ContainerTy<'a, 'tcx> {
19831988
Ref(ty::Region<'tcx>),
19841989
Regular {
19851990
ty: DefId,
1986-
args: ty::Binder<'tcx, &'tcx [ty::GenericArg<'tcx>]>,
1987-
has_self: bool,
1991+
/// The arguments *have* to contain an arg for the self type if the corresponding generics
1992+
/// contain a self type.
1993+
args: ty::Binder<'tcx, &'a [ty::GenericArg<'tcx>]>,
19881994
arg: usize,
19891995
},
19901996
}
19911997

1992-
impl<'tcx> ContainerTy<'tcx> {
1998+
impl<'tcx> ContainerTy<'_, 'tcx> {
19931999
fn object_lifetime_default(self, tcx: TyCtxt<'tcx>) -> ObjectLifetimeDefault<'tcx> {
19942000
match self {
19952001
Self::Ref(region) => ObjectLifetimeDefault::Arg(region),
1996-
Self::Regular { ty: container, args, has_self, arg: index } => {
2002+
Self::Regular { ty: container, args, arg: index } => {
19972003
let (DefKind::Struct
19982004
| DefKind::Union
19992005
| DefKind::Enum
@@ -2006,14 +2012,7 @@ impl<'tcx> ContainerTy<'tcx> {
20062012
let generics = tcx.generics_of(container);
20072013
debug_assert_eq!(generics.parent_count, 0);
20082014

2009-
// If the container is a trait object type, the arguments won't contain the self type but the
2010-
// generics of the corresponding trait will. In such a case, offset the index by one.
2011-
// For comparison, if the container is a trait inside a bound, the arguments do contain the
2012-
// self type.
2013-
let offset =
2014-
if !has_self && generics.parent.is_none() && generics.has_self { 1 } else { 0 };
2015-
let param = generics.params[index + offset].def_id;
2016-
2015+
let param = generics.params[index].def_id;
20172016
let default = tcx.object_lifetime_default(param);
20182017
match default {
20192018
rbv::ObjectLifetimeDefault::Param(lifetime) => {
@@ -2045,7 +2044,7 @@ pub(crate) fn clean_middle_ty<'tcx>(
20452044
bound_ty: ty::Binder<'tcx, Ty<'tcx>>,
20462045
cx: &mut DocContext<'tcx>,
20472046
parent_def_id: Option<DefId>,
2048-
container: Option<ContainerTy<'tcx>>,
2047+
container: Option<ContainerTy<'_, 'tcx>>,
20492048
) -> Type {
20502049
let bound_ty = normalize(cx, bound_ty).unwrap_or(bound_ty);
20512050
match *bound_ty.skip_binder().kind() {
@@ -2096,12 +2095,12 @@ pub(crate) fn clean_middle_ty<'tcx>(
20962095
AdtKind::Enum => ItemType::Enum,
20972096
};
20982097
inline::record_extern_fqn(cx, did, kind);
2099-
let path = external_path(cx, did, false, ThinVec::new(), bound_ty.rebind(args));
2098+
let path = clean_middle_path(cx, did, false, ThinVec::new(), bound_ty.rebind(args));
21002099
Type::Path { path }
21012100
}
21022101
ty::Foreign(did) => {
21032102
inline::record_extern_fqn(cx, did, ItemType::ForeignType);
2104-
let path = external_path(
2103+
let path = clean_middle_path(
21052104
cx,
21062105
did,
21072106
false,
@@ -2132,7 +2131,7 @@ pub(crate) fn clean_middle_ty<'tcx>(
21322131
let mut bounds = dids
21332132
.map(|did| {
21342133
let empty = ty::Binder::dummy(ty::GenericArgs::empty());
2135-
let path = external_path(cx, did, false, ThinVec::new(), empty);
2134+
let path = clean_middle_path(cx, did, false, ThinVec::new(), empty);
21362135
inline::record_extern_fqn(cx, did, ItemType::Trait);
21372136
PolyTrait { trait_: path, generic_params: Vec::new() }
21382137
})
@@ -2171,7 +2170,7 @@ pub(crate) fn clean_middle_ty<'tcx>(
21712170
.collect();
21722171
let late_bound_regions = late_bound_regions.into_iter().collect();
21732172

2174-
let path = external_path(cx, did, false, bindings, args);
2173+
let path = clean_middle_path(cx, did, false, bindings, args);
21752174
bounds.insert(0, PolyTrait { trait_: path, generic_params: late_bound_regions });
21762175

21772176
DynTrait(bounds, lifetime)
@@ -2193,7 +2192,7 @@ pub(crate) fn clean_middle_ty<'tcx>(
21932192
assoc: PathSegment {
21942193
name: cx.tcx.associated_item(def_id).name,
21952194
args: GenericArgs::AngleBracketed {
2196-
args: ty_args_to_args(
2195+
args: clean_middle_generic_args(
21972196
cx,
21982197
alias_ty.map_bound(|ty| ty.args.as_slice()),
21992198
true,
@@ -2213,7 +2212,7 @@ pub(crate) fn clean_middle_ty<'tcx>(
22132212
if cx.tcx.features().lazy_type_alias {
22142213
// Weak type alias `data` represents the `type X` in `type X = Y`. If we need `Y`,
22152214
// we need to use `type_of`.
2216-
let path = external_path(
2215+
let path = clean_middle_path(
22172216
cx,
22182217
data.def_id,
22192218
false,
@@ -2243,7 +2242,8 @@ pub(crate) fn clean_middle_ty<'tcx>(
22432242
ty::Alias(ty::Opaque, ty::AliasTy { def_id, args, .. }) => {
22442243
// If it's already in the same alias, don't get an infinite loop.
22452244
if cx.current_type_aliases.contains_key(&def_id) {
2246-
let path = external_path(cx, def_id, false, ThinVec::new(), bound_ty.rebind(args));
2245+
let path =
2246+
clean_middle_path(cx, def_id, false, ThinVec::new(), bound_ty.rebind(args));
22472247
Type::Path { path }
22482248
} else {
22492249
*cx.current_type_aliases.entry(def_id).or_insert(0) += 1;

src/librustdoc/clean/types.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ use rustc_target::abi::VariantIdx;
3636
use rustc_target::spec::abi::Abi;
3737

3838
use crate::clean::cfg::Cfg;
39-
use crate::clean::external_path;
39+
use crate::clean::clean_middle_path;
4040
use crate::clean::inline::{self, print_inlined_const};
4141
use crate::clean::utils::{is_literal_expr, print_evaluated_const};
4242
use crate::core::DocContext;
@@ -1258,7 +1258,7 @@ impl GenericBound {
12581258
fn sized_with(cx: &mut DocContext<'_>, modifier: hir::TraitBoundModifier) -> GenericBound {
12591259
let did = cx.tcx.require_lang_item(LangItem::Sized, None);
12601260
let empty = ty::Binder::dummy(ty::GenericArgs::empty());
1261-
let path = external_path(cx, did, false, ThinVec::new(), empty);
1261+
let path = clean_middle_path(cx, did, false, ThinVec::new(), empty);
12621262
inline::record_extern_fqn(cx, did, ItemType::Trait);
12631263
GenericBound::TraitBound(PolyTrait { trait_: path, generic_params: Vec::new() }, modifier)
12641264
}

src/librustdoc/clean/utils.rs

+71-63
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,10 @@ use rustc_hir::def::{DefKind, Res};
1616
use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE};
1717
use rustc_metadata::rendered_const;
1818
use rustc_middle::mir;
19+
use rustc_middle::ty::TypeVisitableExt;
1920
use rustc_middle::ty::{self, GenericArgKind, GenericArgsRef, TyCtxt};
20-
use rustc_middle::ty::{TypeVisitable, TypeVisitableExt};
2121
use rustc_span::symbol::{kw, sym, Symbol};
22+
use std::assert_matches::debug_assert_matches;
2223
use std::fmt::Write as _;
2324
use std::mem;
2425
use std::sync::LazyLock as Lazy;
@@ -75,94 +76,101 @@ pub(crate) fn krate(cx: &mut DocContext<'_>) -> Crate {
7576
Crate { module, external_traits: cx.external_traits.clone() }
7677
}
7778

78-
pub(crate) fn ty_args_to_args<'tcx>(
79+
pub(crate) fn clean_middle_generic_args<'tcx>(
7980
cx: &mut DocContext<'tcx>,
80-
ty_args: ty::Binder<'tcx, &'tcx [ty::GenericArg<'tcx>]>,
81-
has_self: bool,
81+
args: ty::Binder<'tcx, &'tcx [ty::GenericArg<'tcx>]>,
82+
mut has_self: bool,
8283
owner: DefId,
8384
) -> Vec<GenericArg> {
84-
if ty_args.skip_binder().is_empty() {
85+
let (args, bound_vars) = (args.skip_binder(), args.bound_vars());
86+
if args.is_empty() {
8587
// Fast path which avoids executing the query `generics_of`.
8688
return Vec::new();
8789
}
8890

89-
let params = &cx.tcx.generics_of(owner).params;
90-
let mut elision_has_failed_once_before = false;
91+
// If the container is a trait object type, the arguments won't contain the self type but the
92+
// generics of the corresponding trait will. In such a case, prepend a dummy self type in order
93+
// to align the arguments and parameters for the iteration below and to enable us to correctly
94+
// instantiate the generic parameter default later.
95+
let generics = cx.tcx.generics_of(owner);
96+
let args = if !has_self && generics.parent.is_none() && generics.has_self {
97+
has_self = true;
98+
[cx.tcx.types.trait_object_dummy_self.into()]
99+
.into_iter()
100+
.chain(args.iter().copied())
101+
.collect::<Vec<_>>()
102+
.into()
103+
} else {
104+
std::borrow::Cow::from(args)
105+
};
91106

92-
let offset = if has_self { 1 } else { 0 };
93-
let mut args = Vec::with_capacity(ty_args.skip_binder().len().saturating_sub(offset));
107+
let mut elision_has_failed_once_before = false;
108+
let clean_arg = |(index, &arg): (usize, &ty::GenericArg<'tcx>)| {
109+
// Elide the self type.
110+
if has_self && index == 0 {
111+
return None;
112+
}
94113

95-
let ty_arg_to_arg = |(index, arg): (usize, &ty::GenericArg<'tcx>)| match arg.unpack() {
96-
GenericArgKind::Lifetime(lt) => {
97-
Some(GenericArg::Lifetime(clean_middle_region(lt).unwrap_or(Lifetime::elided())))
114+
// Elide internal host effect args.
115+
let param = generics.param_at(index, cx.tcx);
116+
if param.is_host_effect() {
117+
return None;
98118
}
99-
GenericArgKind::Type(_) if has_self && index == 0 => None,
100-
GenericArgKind::Type(ty) => {
101-
if !elision_has_failed_once_before
102-
&& let Some(default) = params[index].default_value(cx.tcx)
103-
{
104-
let default =
105-
ty_args.map_bound(|args| default.instantiate(cx.tcx, args).expect_ty());
106-
107-
if can_elide_generic_arg(ty_args.rebind(ty), default) {
108-
return None;
109-
}
110119

111-
elision_has_failed_once_before = true;
120+
let arg = ty::Binder::bind_with_vars(arg, bound_vars);
121+
122+
// Elide arguments that coincide with their default.
123+
if !elision_has_failed_once_before && let Some(default) = param.default_value(cx.tcx) {
124+
let default = default.instantiate(cx.tcx, args.as_ref());
125+
if can_elide_generic_arg(arg, arg.rebind(default)) {
126+
return None;
112127
}
128+
elision_has_failed_once_before = true;
129+
}
113130

114-
Some(GenericArg::Type(clean_middle_ty(
115-
ty_args.rebind(ty),
131+
match arg.skip_binder().unpack() {
132+
GenericArgKind::Lifetime(lt) => {
133+
Some(GenericArg::Lifetime(clean_middle_region(lt).unwrap_or(Lifetime::elided())))
134+
}
135+
GenericArgKind::Type(ty) => Some(GenericArg::Type(clean_middle_ty(
136+
arg.rebind(ty),
116137
cx,
117138
None,
118139
Some(crate::clean::ContainerTy::Regular {
119140
ty: owner,
120-
args: ty_args,
121-
has_self,
141+
args: arg.rebind(args.as_ref()),
122142
arg: index,
123143
}),
124-
)))
125-
}
126-
GenericArgKind::Const(ct) => {
127-
if let ty::GenericParamDefKind::Const { is_host_effect: true, .. } = params[index].kind
128-
{
129-
return None;
130-
}
131-
132-
if !elision_has_failed_once_before
133-
&& let Some(default) = params[index].default_value(cx.tcx)
134-
{
135-
let default =
136-
ty_args.map_bound(|args| default.instantiate(cx.tcx, args).expect_const());
137-
138-
if can_elide_generic_arg(ty_args.rebind(ct), default) {
139-
return None;
140-
}
141-
142-
elision_has_failed_once_before = true;
144+
))),
145+
GenericArgKind::Const(ct) => {
146+
Some(GenericArg::Const(Box::new(clean_middle_const(arg.rebind(ct), cx))))
143147
}
144-
145-
Some(GenericArg::Const(Box::new(clean_middle_const(ty_args.rebind(ct), cx))))
146148
}
147149
};
148150

149-
args.extend(ty_args.skip_binder().iter().enumerate().rev().filter_map(ty_arg_to_arg));
150-
args.reverse();
151-
args
151+
let offset = if has_self { 1 } else { 0 };
152+
let mut clean_args = Vec::with_capacity(args.len().saturating_sub(offset));
153+
clean_args.extend(args.iter().enumerate().rev().filter_map(clean_arg));
154+
clean_args.reverse();
155+
clean_args
152156
}
153157

154158
/// Check if the generic argument `actual` coincides with the `default` and can therefore be elided.
155159
///
156160
/// This uses a very conservative approach for performance and correctness reasons, meaning for
157161
/// several classes of terms it claims that they cannot be elided even if they theoretically could.
158162
/// This is absolutely fine since it mostly concerns edge cases.
159-
fn can_elide_generic_arg<'tcx, Term>(
160-
actual: ty::Binder<'tcx, Term>,
161-
default: ty::Binder<'tcx, Term>,
162-
) -> bool
163-
where
164-
Term: Eq + TypeVisitable<TyCtxt<'tcx>>,
165-
{
163+
fn can_elide_generic_arg<'tcx>(
164+
actual: ty::Binder<'tcx, ty::GenericArg<'tcx>>,
165+
default: ty::Binder<'tcx, ty::GenericArg<'tcx>>,
166+
) -> bool {
167+
debug_assert_matches!(
168+
(actual.skip_binder().unpack(), default.skip_binder().unpack()),
169+
(ty::GenericArgKind::Lifetime(_), ty::GenericArgKind::Lifetime(_))
170+
| (ty::GenericArgKind::Type(_), ty::GenericArgKind::Type(_))
171+
| (ty::GenericArgKind::Const(_), ty::GenericArgKind::Const(_))
172+
);
173+
166174
// In practice, we shouldn't have any inference variables at this point.
167175
// However to be safe, we bail out if we do happen to stumble upon them.
168176
if actual.has_infer() || default.has_infer() {
@@ -192,14 +200,14 @@ where
192200
actual.skip_binder() == default.skip_binder()
193201
}
194202

195-
fn external_generic_args<'tcx>(
203+
fn clean_middle_generic_args_with_bindings<'tcx>(
196204
cx: &mut DocContext<'tcx>,
197205
did: DefId,
198206
has_self: bool,
199207
bindings: ThinVec<TypeBinding>,
200208
ty_args: ty::Binder<'tcx, GenericArgsRef<'tcx>>,
201209
) -> GenericArgs {
202-
let args = ty_args_to_args(cx, ty_args.map_bound(|args| &args[..]), has_self, did);
210+
let args = clean_middle_generic_args(cx, ty_args.map_bound(|args| &args[..]), has_self, did);
203211

204212
if cx.tcx.fn_trait_kind_from_def_id(did).is_some() {
205213
let ty = ty_args
@@ -225,7 +233,7 @@ fn external_generic_args<'tcx>(
225233
}
226234
}
227235

228-
pub(super) fn external_path<'tcx>(
236+
pub(super) fn clean_middle_path<'tcx>(
229237
cx: &mut DocContext<'tcx>,
230238
did: DefId,
231239
has_self: bool,
@@ -238,7 +246,7 @@ pub(super) fn external_path<'tcx>(
238246
res: Res::Def(def_kind, did),
239247
segments: thin_vec![PathSegment {
240248
name,
241-
args: external_generic_args(cx, did, has_self, bindings, args),
249+
args: clean_middle_generic_args_with_bindings(cx, did, has_self, bindings, args),
242250
}],
243251
}
244252
}

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

+8-3
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@ pub struct Multi<A = u64, B = u64>(A, B);
4040

4141
pub type M0 = Multi<u64, ()>;
4242

43-
pub trait Trait<'a, T = &'a ()> {}
44-
45-
pub type F = dyn for<'a> Trait<'a>;
43+
pub trait Trait0<'a, T = &'a ()> {}
44+
pub type D0 = dyn for<'a> Trait0<'a>;
45+
46+
// Regression test for issue #119529.
47+
pub trait Trait1<T = (), const K: u32 = 0> {}
48+
pub type D1<T> = dyn Trait1<T>;
49+
pub type D2<const K: u32> = dyn Trait1<(), K>;
50+
pub type D3 = dyn Trait1;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
pub use default_generic_args::*;

0 commit comments

Comments
 (0)