Skip to content

Commit 15a40c7

Browse files
estebankestebank
committed
Do not discard ?Sized type params and suggest their removal
1 parent 5fb3394 commit 15a40c7

32 files changed

+615
-19
lines changed

‎compiler/rustc_ast_lowering/src/item.rs‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1443,16 +1443,19 @@ impl<'hir> LoweringContext<'_, 'hir> {
14431443
ImplTraitContext::disallowed(),
14441444
),
14451445
bounded_ty: this.lower_ty(bounded_ty, ImplTraitContext::disallowed()),
1446-
bounds: this.arena.alloc_from_iter(bounds.iter().filter_map(|bound| {
1447-
match *bound {
1448-
// Ignore `?Trait` bounds.
1449-
// They were copied into type parameters already.
1450-
GenericBound::Trait(_, TraitBoundModifier::Maybe) => None,
1451-
_ => Some(
1452-
this.lower_param_bound(bound, ImplTraitContext::disallowed()),
1453-
),
1454-
}
1455-
})),
1446+
bounds: this.arena.alloc_from_iter(bounds.iter().map(
1447+
|bound| match bound {
1448+
// We used to ignore `?Trait` bounds, as they were copied into type
1449+
// parameters already, but we need to keep them around only for
1450+
// diagnostics when we suggest removal of `?Sized` bounds. See
1451+
// `suggest_constraining_type_param`. This will need to change if
1452+
// we ever allow something *other* than `?Sized`.
1453+
GenericBound::Trait(p, TraitBoundModifier::Maybe) => {
1454+
hir::GenericBound::Unsized(p.span)
1455+
}
1456+
_ => this.lower_param_bound(bound, ImplTraitContext::disallowed()),
1457+
},
1458+
)),
14561459
span,
14571460
})
14581461
})

‎compiler/rustc_ast_lowering/src/lib.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2160,12 +2160,12 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
21602160
tpb: &GenericBound,
21612161
itctx: ImplTraitContext<'_, 'hir>,
21622162
) -> hir::GenericBound<'hir> {
2163-
match *tpb {
2164-
GenericBound::Trait(ref ty, modifier) => hir::GenericBound::Trait(
2165-
self.lower_poly_trait_ref(ty, itctx),
2166-
self.lower_trait_bound_modifier(modifier),
2163+
match tpb {
2164+
GenericBound::Trait(p, modifier) => hir::GenericBound::Trait(
2165+
self.lower_poly_trait_ref(p, itctx),
2166+
self.lower_trait_bound_modifier(*modifier),
21672167
),
2168-
GenericBound::Outlives(ref lifetime) => {
2168+
GenericBound::Outlives(lifetime) => {
21692169
hir::GenericBound::Outlives(self.lower_lifetime(lifetime))
21702170
}
21712171
}

‎compiler/rustc_hir/src/hir.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,7 @@ pub enum GenericBound<'hir> {
442442
Trait(PolyTraitRef<'hir>, TraitBoundModifier),
443443
// FIXME(davidtwco): Introduce `PolyTraitRef::LangItem`
444444
LangItemTrait(LangItem, Span, HirId, &'hir GenericArgs<'hir>),
445+
Unsized(Span),
445446
Outlives(Lifetime),
446447
}
447448

@@ -458,6 +459,7 @@ impl GenericBound<'_> {
458459
GenericBound::Trait(t, ..) => t.span,
459460
GenericBound::LangItemTrait(_, span, ..) => *span,
460461
GenericBound::Outlives(l) => l.span,
462+
GenericBound::Unsized(span) => *span,
461463
}
462464
}
463465
}

‎compiler/rustc_hir/src/intravisit.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -889,6 +889,7 @@ pub fn walk_param_bound<'v, V: Visitor<'v>>(visitor: &mut V, bound: &'v GenericB
889889
visitor.visit_generic_args(span, args);
890890
}
891891
GenericBound::Outlives(ref lifetime) => visitor.visit_lifetime(lifetime),
892+
GenericBound::Unsized(_) => {}
892893
}
893894
}
894895

‎compiler/rustc_hir_pretty/src/lib.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2230,6 +2230,9 @@ impl<'a> State<'a> {
22302230
GenericBound::Outlives(lt) => {
22312231
self.print_lifetime(lt);
22322232
}
2233+
GenericBound::Unsized(_) => {
2234+
self.s.word("?Sized");
2235+
}
22332236
}
22342237
}
22352238
}

‎compiler/rustc_middle/src/ty/diagnostics.rs‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
33
use crate::ty::TyKind::*;
44
use crate::ty::{InferTy, TyCtxt, TyS};
5+
use rustc_data_structures::fx::FxHashSet;
56
use rustc_errors::{Applicability, DiagnosticBuilder};
67
use rustc_hir as hir;
78
use rustc_hir::def_id::DefId;
@@ -105,6 +106,116 @@ pub fn suggest_arbitrary_trait_bound(
105106
true
106107
}
107108

109+
fn suggest_removing_unsized_bound(
110+
generics: &hir::Generics<'_>,
111+
err: &mut DiagnosticBuilder<'_>,
112+
param_name: &str,
113+
param: &hir::GenericParam<'_>,
114+
def_id: Option<DefId>,
115+
) {
116+
// See if there's a `?Sized` bound that can be removed to suggest that.
117+
// First look at the `where` clause because we can have `where T: ?Sized`, but that
118+
// `?Sized` bound is *also* included in the `GenericParam` as a bound, which breaks
119+
// the spans. Hence the somewhat involved logic that follows.
120+
let mut where_unsized_bounds = FxHashSet::default();
121+
for (where_pos, predicate) in generics.where_clause.predicates.iter().enumerate() {
122+
match predicate {
123+
WherePredicate::BoundPredicate(WhereBoundPredicate {
124+
bounded_ty:
125+
hir::Ty {
126+
kind:
127+
hir::TyKind::Path(hir::QPath::Resolved(
128+
None,
129+
hir::Path {
130+
segments: [segment],
131+
res: hir::def::Res::Def(hir::def::DefKind::TyParam, _),
132+
..
133+
},
134+
)),
135+
..
136+
},
137+
bounds,
138+
span,
139+
..
140+
}) if segment.ident.as_str() == param_name => {
141+
for (pos, bound) in bounds.iter().enumerate() {
142+
match bound {
143+
hir::GenericBound::Unsized(_) => {}
144+
hir::GenericBound::Trait(poly, hir::TraitBoundModifier::Maybe)
145+
if poly.trait_ref.trait_def_id() == def_id => {}
146+
_ => continue,
147+
}
148+
let sp = match (
149+
bounds.len(),
150+
pos,
151+
generics.where_clause.predicates.len(),
152+
where_pos,
153+
) {
154+
// where T: ?Sized
155+
// ^^^^^^^^^^^^^^^
156+
(1, _, 1, _) => generics.where_clause.span,
157+
// where Foo: Bar, T: ?Sized,
158+
// ^^^^^^^^^^^
159+
(1, _, len, pos) if pos == len - 1 => generics.where_clause.predicates
160+
[pos - 1]
161+
.span()
162+
.shrink_to_hi()
163+
.to(*span),
164+
// where T: ?Sized, Foo: Bar,
165+
// ^^^^^^^^^^^
166+
(1, _, _, pos) => {
167+
span.until(generics.where_clause.predicates[pos + 1].span())
168+
}
169+
// where T: ?Sized + Bar, Foo: Bar,
170+
// ^^^^^^^^^
171+
(_, 0, _, _) => bound.span().to(bounds[1].span().shrink_to_lo()),
172+
// where T: Bar + ?Sized, Foo: Bar,
173+
// ^^^^^^^^^
174+
(_, pos, _, _) => bounds[pos - 1].span().shrink_to_hi().to(bound.span()),
175+
};
176+
where_unsized_bounds.insert(bound.span());
177+
err.span_suggestion_verbose(
178+
sp,
179+
"consider removing the `?Sized` bound to make the \
180+
type parameter `Sized`",
181+
String::new(),
182+
Applicability::MaybeIncorrect,
183+
);
184+
}
185+
}
186+
_ => {}
187+
}
188+
}
189+
for (pos, bound) in param.bounds.iter().enumerate() {
190+
match bound {
191+
hir::GenericBound::Trait(poly, hir::TraitBoundModifier::Maybe)
192+
if poly.trait_ref.trait_def_id() == def_id
193+
&& !where_unsized_bounds.contains(&bound.span()) =>
194+
{
195+
let sp = match (param.bounds.len(), pos) {
196+
// T: ?Sized,
197+
// ^^^^^^^^
198+
(1, _) => param.span.shrink_to_hi().to(bound.span()),
199+
// T: ?Sized + Bar,
200+
// ^^^^^^^^^
201+
(_, 0) => bound.span().to(param.bounds[1].span().shrink_to_lo()),
202+
// T: Bar + ?Sized,
203+
// ^^^^^^^^^
204+
(_, pos) => param.bounds[pos - 1].span().shrink_to_hi().to(bound.span()),
205+
};
206+
err.span_suggestion_verbose(
207+
sp,
208+
"consider removing the `?Sized` bound to make the type parameter \
209+
`Sized`",
210+
String::new(),
211+
Applicability::MaybeIncorrect,
212+
);
213+
}
214+
_ => {}
215+
}
216+
}
217+
}
218+
108219
/// Suggest restricting a type param with a new bound.
109220
pub fn suggest_constraining_type_param(
110221
tcx: TyCtxt<'_>,
@@ -130,6 +241,7 @@ pub fn suggest_constraining_type_param(
130241
if def_id == tcx.lang_items().sized_trait() {
131242
// Type parameters are already `Sized` by default.
132243
err.span_label(param.span, &format!("this type parameter needs to be `{}`", constraint));
244+
suggest_removing_unsized_bound(generics, err, param_name, param, def_id);
133245
return true;
134246
}
135247
let mut suggest_restrict = |span| {

‎compiler/rustc_save_analysis/src/dump_visitor.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -689,6 +689,7 @@ impl<'tcx> DumpVisitor<'tcx> {
689689
(Some(self.tcx.require_lang_item(lang_item, Some(span))), span)
690690
}
691691
hir::GenericBound::Outlives(..) => continue,
692+
hir::GenericBound::Unsized(_) => continue,
692693
};
693694

694695
if let Some(id) = def_id {

‎compiler/rustc_typeck/src/astconv/mod.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -943,7 +943,8 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
943943
false,
944944
);
945945
}
946-
hir::GenericBound::Trait(_, hir::TraitBoundModifier::Maybe) => {}
946+
hir::GenericBound::Trait(_, hir::TraitBoundModifier::Maybe)
947+
| hir::GenericBound::Unsized(_) => {}
947948
hir::GenericBound::LangItemTrait(lang_item, span, hir_id, args) => self
948949
.instantiate_lang_item_trait_ref(
949950
lang_item, span, hir_id, args, param_ty, bounds,

‎compiler/rustc_typeck/src/collect.rs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2230,7 +2230,9 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::GenericP
22302230
let constness = match modifier {
22312231
hir::TraitBoundModifier::MaybeConst => hir::Constness::NotConst,
22322232
hir::TraitBoundModifier::None => constness,
2233-
hir::TraitBoundModifier::Maybe => bug!("this wasn't handled"),
2233+
// We ignore `where T: ?Sized`, it is already part of
2234+
// type parameter `T`.
2235+
hir::TraitBoundModifier::Maybe => continue,
22342236
};
22352237

22362238
let mut bounds = Bounds::default();
@@ -2260,6 +2262,8 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::GenericP
22602262
predicates.extend(bounds.predicates(tcx, ty));
22612263
}
22622264

2265+
hir::GenericBound::Unsized(_) => {}
2266+
22632267
hir::GenericBound::Outlives(lifetime) => {
22642268
let region =
22652269
<dyn AstConv<'_>>::ast_region_to_region(&icx, lifetime, None);
@@ -2521,6 +2525,7 @@ fn predicates_from_bound<'tcx>(
25212525
);
25222526
bounds.predicates(astconv.tcx(), param_ty)
25232527
}
2528+
hir::GenericBound::Unsized(_) => vec![],
25242529
hir::GenericBound::Outlives(ref lifetime) => {
25252530
let region = astconv.ast_region_to_region(lifetime, None);
25262531
let pred = ty::PredicateKind::TypeOutlives(ty::OutlivesPredicate(param_ty, region))

‎src/librustdoc/clean/mod.rs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ impl Clean<GenericBound> for hir::GenericBound<'_> {
128128
fn clean(&self, cx: &mut DocContext<'_>) -> GenericBound {
129129
match *self {
130130
hir::GenericBound::Outlives(lt) => GenericBound::Outlives(lt.clean(cx)),
131+
hir::GenericBound::Unsized(_) => GenericBound::maybe_sized(cx),
131132
hir::GenericBound::LangItemTrait(lang_item, span, _, generic_args) => {
132133
let def_id = cx.tcx.require_lang_item(lang_item, Some(span));
133134

@@ -562,13 +563,19 @@ impl Clean<Generics> for hir::Generics<'_> {
562563
WherePredicate::BoundPredicate {
563564
ty: Generic(ref name), ref mut bounds, ..
564565
} => {
565-
if bounds.is_empty() {
566+
if let [] | [GenericBound::TraitBound(_, hir::TraitBoundModifier::Maybe)] =
567+
&bounds[..]
568+
{
566569
for param in &mut generics.params {
567570
match param.kind {
568571
GenericParamDefKind::Lifetime => {}
569572
GenericParamDefKind::Type { bounds: ref mut ty_bounds, .. } => {
570573
if &param.name == name {
571574
mem::swap(bounds, ty_bounds);
575+
// We now keep track of `?Sized` obligations in the HIR.
576+
// If we don't clear `ty_bounds` we end up with
577+
// `fn foo<X: ?Sized>(_: X) where X: ?Sized`.
578+
ty_bounds.clear();
572579
break;
573580
}
574581
}

0 commit comments

Comments
 (0)