Skip to content

Commit f338867

Browse files
estebankestebank
committed
Suggest removal of ?Sized bound when appropriate
1 parent 6fdf73f commit f338867

File tree

56 files changed

+715
-175
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

56 files changed

+715
-175
lines changed

‎compiler/rustc_ast_lowering/src/item.rs‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1477,15 +1477,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
14771477
ImplTraitContext::disallowed(),
14781478
),
14791479
bounded_ty: this.lower_ty(bounded_ty, ImplTraitContext::disallowed()),
1480-
bounds: this.arena.alloc_from_iter(bounds.iter().filter_map(|bound| {
1481-
match *bound {
1482-
// Ignore `?Trait` bounds.
1483-
// They were copied into type parameters already.
1484-
GenericBound::Trait(_, TraitBoundModifier::Maybe) => None,
1485-
_ => Some(
1486-
this.lower_param_bound(bound, ImplTraitContext::disallowed()),
1487-
),
1488-
}
1480+
bounds: this.arena.alloc_from_iter(bounds.iter().map(|bound| {
1481+
// We used to ignore `?Trait` bounds, as they were copied into type
1482+
// parameters already, but we need to keep them around only for
1483+
// diagnostics when we suggest removal of `?Sized` bounds. See
1484+
// `suggest_constraining_type_param`.
1485+
this.lower_param_bound(bound, ImplTraitContext::disallowed())
14891486
})),
14901487
span,
14911488
})

‎compiler/rustc_infer/src/traits/engine.rs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,10 @@ pub trait TraitEngine<'tcx>: 'tcx {
2727
cause: ObligationCause<'tcx>,
2828
) {
2929
let trait_ref = ty::TraitRef { def_id, substs: infcx.tcx.mk_substs_trait(ty, &[]) };
30+
let predicate = trait_ref.without_const().to_predicate(infcx.tcx);
3031
self.register_predicate_obligation(
3132
infcx,
32-
Obligation {
33-
cause,
34-
recursion_depth: 0,
35-
param_env,
36-
predicate: trait_ref.without_const().to_predicate(infcx.tcx),
37-
},
33+
Obligation { cause, recursion_depth: 0, param_env, predicate },
3834
);
3935
}
4036

‎compiler/rustc_infer/src/traits/util.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ impl Elaborator<'tcx> {
124124

125125
let bound_predicate = obligation.predicate.kind();
126126
match bound_predicate.skip_binder() {
127-
ty::PredicateKind::Trait(data, ..) => {
127+
ty::PredicateKind::Trait(data, _, _) => {
128128
// Get predicates declared on the trait.
129129
let predicates = tcx.super_predicates_of(data.def_id());
130130

‎compiler/rustc_middle/src/traits/mod.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,9 @@ pub enum ObligationCauseCode<'tcx> {
195195
/// Like `ItemObligation`, but with extra detail on the source of the obligation.
196196
BindingObligation(DefId, Span),
197197

198+
/// Like `ItemObligation`, but with extra detail on the source of the obligation.
199+
ImplicitSizedObligation(DefId, Span),
200+
198201
/// A type like `&'a T` is WF only if `T: 'a`.
199202
ReferenceOutlivesReferent(Ty<'tcx>),
200203

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

Lines changed: 116 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;
@@ -130,6 +131,121 @@ pub fn suggest_constraining_type_param(
130131
if def_id == tcx.lang_items().sized_trait() {
131132
// Type parameters are already `Sized` by default.
132133
err.span_label(param.span, &format!("this type parameter needs to be `{}`", constraint));
134+
// See if there's a `?Sized` bound that can be removed to suggest that.
135+
match param.bounds {
136+
[] => {}
137+
bounds => {
138+
// First look at the `where` clause because we can have `where T: ?Sized`, but that
139+
// `?Sized` bound is *also* included in the `GenericParam` as a bound, which breaks
140+
// the spans. Hence the somewhat involved logic that follows.
141+
let mut where_unsized_bounds = FxHashSet::default();
142+
for (where_pos, predicate) in generics.where_clause.predicates.iter().enumerate() {
143+
match predicate {
144+
WherePredicate::BoundPredicate(WhereBoundPredicate {
145+
bounded_ty:
146+
hir::Ty {
147+
kind:
148+
hir::TyKind::Path(hir::QPath::Resolved(
149+
None,
150+
hir::Path {
151+
segments: [segment],
152+
res:
153+
hir::def::Res::Def(hir::def::DefKind::TyParam, _),
154+
..
155+
},
156+
)),
157+
..
158+
},
159+
bounds,
160+
span,
161+
..
162+
}) if segment.ident.as_str() == param_name => {
163+
for (pos, bound) in bounds.iter().enumerate() {
164+
match bound {
165+
hir::GenericBound::Trait(
166+
poly,
167+
hir::TraitBoundModifier::Maybe,
168+
) if poly.trait_ref.trait_def_id() == def_id => {
169+
let sp = match (
170+
bounds.len(),
171+
pos,
172+
generics.where_clause.predicates.len(),
173+
where_pos,
174+
) {
175+
// where T: ?Sized
176+
// ^^^^^^^^^^^^^^^
177+
(1, _, 1, _) => generics.where_clause.span,
178+
// where Foo: Bar, T: ?Sized,
179+
// ^^^^^^^^^^^
180+
(1, _, len, pos) if pos == len - 1 => {
181+
generics.where_clause.predicates[pos - 1]
182+
.span()
183+
.shrink_to_hi()
184+
.to(*span)
185+
}
186+
// where T: ?Sized, Foo: Bar,
187+
// ^^^^^^^^^^^
188+
(1, _, _, pos) => span.until(
189+
generics.where_clause.predicates[pos + 1].span(),
190+
),
191+
// where T: ?Sized + Bar, Foo: Bar,
192+
// ^^^^^^^^^
193+
(_, 0, _, _) => {
194+
bound.span().to(bounds[1].span().shrink_to_lo())
195+
}
196+
// where T: Bar + ?Sized, Foo: Bar,
197+
// ^^^^^^^^^
198+
(_, pos, _, _) => bounds[pos - 1]
199+
.span()
200+
.shrink_to_hi()
201+
.to(bound.span()),
202+
};
203+
where_unsized_bounds.insert(bound.span());
204+
err.span_suggestion_verbose(
205+
sp,
206+
"consider removing the `?Sized` bound to make the \
207+
type parameter `Sized`",
208+
String::new(),
209+
Applicability::MaybeIncorrect,
210+
);
211+
}
212+
_ => {}
213+
}
214+
}
215+
}
216+
_ => {}
217+
}
218+
}
219+
for (pos, bound) in bounds.iter().enumerate() {
220+
match bound {
221+
hir::GenericBound::Trait(poly, hir::TraitBoundModifier::Maybe)
222+
if poly.trait_ref.trait_def_id() == def_id
223+
&& !where_unsized_bounds.contains(&bound.span()) =>
224+
{
225+
let sp = match (bounds.len(), pos) {
226+
// T: ?Sized,
227+
// ^^^^^^^^
228+
(1, _) => param.span.shrink_to_hi().to(bound.span()),
229+
// T: ?Sized + Bar,
230+
// ^^^^^^^^^
231+
(_, 0) => bound.span().to(bounds[1].span().shrink_to_lo()),
232+
// T: Bar + ?Sized,
233+
// ^^^^^^^^^
234+
(_, pos) => bounds[pos - 1].span().shrink_to_hi().to(bound.span()),
235+
};
236+
err.span_suggestion_verbose(
237+
sp,
238+
"consider removing the `?Sized` bound to make the type parameter \
239+
`Sized`",
240+
String::new(),
241+
Applicability::MaybeIncorrect,
242+
);
243+
}
244+
_ => {}
245+
}
246+
}
247+
}
248+
}
133249
return true;
134250
}
135251
let mut suggest_restrict = |span| {

‎compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs‎

Lines changed: 10 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
202202
&obligation.cause.code,
203203
&mut vec![],
204204
&mut Default::default(),
205-
None,
206205
);
207206

208207
err.emit();
@@ -233,7 +232,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
233232
) {
234233
let tcx = self.tcx;
235234
let span = obligation.cause.span;
236-
let mut obligation_note = None;
237235

238236
let mut err = match *error {
239237
SelectionError::Unimplemented => {
@@ -261,7 +259,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
261259

262260
let bound_predicate = obligation.predicate.kind();
263261
match bound_predicate.skip_binder() {
264-
ty::PredicateKind::Trait(trait_predicate, _, implicit) => {
262+
ty::PredicateKind::Trait(trait_predicate, _, _) => {
265263
let trait_predicate = bound_predicate.rebind(trait_predicate);
266264
let trait_predicate = self.resolve_vars_if_possible(trait_predicate);
267265

@@ -313,61 +311,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
313311
post_message,
314312
))
315313
);
316-
if let ty::ImplicitTraitPredicate::Yes = implicit {
317-
if let ObligationCauseCode::BindingObligation(item_def_id, _) =
318-
obligation.cause.code.peel_derives()
319-
{
320-
match self.tcx.hir().get_if_local(*item_def_id) {
321-
Some(hir::Node::TraitItem(hir::TraitItem {
322-
kind: hir::TraitItemKind::Type(bounds, ty),
323-
ident,
324-
..
325-
})) => {
326-
obligation_note = Some(
327-
"associated types introduce an implicit `Sized` \
328-
obligation",
329-
);
330-
match (bounds, ty) {
331-
([], None) => {
332-
err.span_suggestion_verbose(
333-
ident.span.shrink_to_hi(),
334-
"consider relaxing the `Sized` obligation",
335-
": ?Sized".to_string(),
336-
Applicability::MaybeIncorrect,
337-
);
338-
}
339-
([.., bound], None) => {
340-
err.span_suggestion_verbose(
341-
bound.span().shrink_to_hi(),
342-
"consider relaxing the `Sized` obligation",
343-
" + ?Sized".to_string(),
344-
Applicability::MaybeIncorrect,
345-
);
346-
}
347-
_ => {}
348-
}
349-
}
350-
Some(hir::Node::ImplItem(hir::ImplItem {
351-
kind: hir::ImplItemKind::TyAlias(_),
352-
..
353-
})) => {
354-
obligation_note = Some(
355-
"associated types on `impl` blocks for types, have an \
356-
implicit mandatory `Sized` obligation; associated \
357-
types from `trait`s can be relaxed to `?Sized`",
358-
);
359-
}
360-
_ => {
361-
// This is (likely?) a type parameter. The suggestion is handled
362-
// in `rustc_middle/src/ty/diagnostics.rs`.
363-
obligation_note = Some(
364-
"type parameters introduce an implicit `Sized` \
365-
obligation",
366-
);
367-
}
368-
}
369-
}
370-
}
371314

372315
if is_try_conversion {
373316
let none_error = self
@@ -425,7 +368,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
425368
points_at_arg,
426369
have_alt_message,
427370
) {
428-
self.note_obligation_cause(&mut err, obligation, obligation_note);
371+
self.note_obligation_cause(&mut err, obligation);
429372
err.emit();
430373
return;
431374
}
@@ -854,7 +797,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
854797
}
855798
};
856799

857-
self.note_obligation_cause(&mut err, obligation, obligation_note);
800+
self.note_obligation_cause(&mut err, obligation);
858801
self.point_at_returns_when_relevant(&mut err, &obligation);
859802

860803
err.emit();
@@ -1137,7 +1080,6 @@ trait InferCtxtPrivExt<'tcx> {
11371080
&self,
11381081
err: &mut DiagnosticBuilder<'tcx>,
11391082
obligation: &PredicateObligation<'tcx>,
1140-
note: Option<&str>,
11411083
);
11421084

11431085
fn suggest_unsized_bound_if_applicable(
@@ -1287,6 +1229,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
12871229
obligation.cause.code.peel_derives(),
12881230
ObligationCauseCode::ItemObligation(_)
12891231
| ObligationCauseCode::BindingObligation(_, _)
1232+
| ObligationCauseCode::ImplicitSizedObligation(_, _)
12901233
| ObligationCauseCode::ObjectCastObligation(_)
12911234
| ObligationCauseCode::OpaqueType
12921235
);
@@ -1358,7 +1301,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
13581301
_ => None,
13591302
};
13601303
self.note_type_err(&mut diag, &obligation.cause, secondary_span, values, err, true);
1361-
self.note_obligation_cause(&mut diag, obligation, None);
1304+
self.note_obligation_cause(&mut diag, obligation);
13621305
diag.emit();
13631306
}
13641307
});
@@ -1651,7 +1594,8 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
16511594
self.suggest_fully_qualified_path(&mut err, def_id, span, trait_ref.def_id());
16521595
} else if let (
16531596
Ok(ref snippet),
1654-
ObligationCauseCode::BindingObligation(ref def_id, _),
1597+
ObligationCauseCode::BindingObligation(ref def_id, _)
1598+
| ObligationCauseCode::ImplicitSizedObligation(ref def_id, _),
16551599
) =
16561600
(self.tcx.sess.source_map().span_to_snippet(span), &obligation.cause.code)
16571601
{
@@ -1767,7 +1711,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
17671711
err
17681712
}
17691713
};
1770-
self.note_obligation_cause(&mut err, obligation, None);
1714+
self.note_obligation_cause(&mut err, obligation);
17711715
err.emit();
17721716
}
17731717

@@ -1831,7 +1775,6 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
18311775
&self,
18321776
err: &mut DiagnosticBuilder<'tcx>,
18331777
obligation: &PredicateObligation<'tcx>,
1834-
note: Option<&str>,
18351778
) {
18361779
// First, attempt to add note to this error with an async-await-specific
18371780
// message, and fall back to regular note otherwise.
@@ -1842,7 +1785,6 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
18421785
&obligation.cause.code,
18431786
&mut vec![],
18441787
&mut Default::default(),
1845-
note,
18461788
);
18471789
self.suggest_unsized_bound_if_applicable(err, obligation);
18481790
}
@@ -1858,7 +1800,8 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
18581800
{
18591801
(
18601802
ty::PredicateKind::Trait(pred, _, _),
1861-
&ObligationCauseCode::BindingObligation(item_def_id, span),
1803+
&ObligationCauseCode::BindingObligation(item_def_id, span)
1804+
| &ObligationCauseCode::ImplicitSizedObligation(item_def_id, span),
18621805
) => (pred, item_def_id, span),
18631806
_ => return,
18641807
};

‎compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
142142
}
143143

144144
if let ObligationCauseCode::ItemObligation(item)
145-
| ObligationCauseCode::BindingObligation(item, _) = obligation.cause.code
145+
| ObligationCauseCode::BindingObligation(item, _)
146+
| ObligationCauseCode::ImplicitSizedObligation(item, _) = obligation.cause.code
146147
{
147148
// FIXME: maybe also have some way of handling methods
148149
// from other traits? That would require name resolution,

0 commit comments

Comments
 (0)