Skip to content

Commit

Permalink
Suggest removal of ?Sized bound when appropriate
Browse files Browse the repository at this point in the history
  • Loading branch information
@estebank
estebank committed Jun 11, 2021
1 parent 6fdf73f commit f338867
Show file tree
Hide file tree
Showing 56 changed files with 715 additions and 175 deletions.
15 changes: 6 additions & 9 deletions compiler/rustc_ast_lowering/src/item.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1477,15 +1477,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
ImplTraitContext::disallowed(),
),
bounded_ty: this.lower_ty(bounded_ty, ImplTraitContext::disallowed()),
bounds: this.arena.alloc_from_iter(bounds.iter().filter_map(|bound| {
match *bound {
// Ignore `?Trait` bounds.
// They were copied into type parameters already.
GenericBound::Trait(_, TraitBoundModifier::Maybe) => None,
_ => Some(
this.lower_param_bound(bound, ImplTraitContext::disallowed()),
),
}
bounds: this.arena.alloc_from_iter(bounds.iter().map(|bound| {
// We used to ignore `?Trait` bounds, as they were copied into type
// parameters already, but we need to keep them around only for
// diagnostics when we suggest removal of `?Sized` bounds. See
// `suggest_constraining_type_param`.
this.lower_param_bound(bound, ImplTraitContext::disallowed())
})),
span,
})
Expand Down
8 changes: 2 additions & 6 deletions compiler/rustc_infer/src/traits/engine.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,10 @@ pub trait TraitEngine<'tcx>: 'tcx {
cause: ObligationCause<'tcx>,
) {
let trait_ref = ty::TraitRef { def_id, substs: infcx.tcx.mk_substs_trait(ty, &[]) };
let predicate = trait_ref.without_const().to_predicate(infcx.tcx);
self.register_predicate_obligation(
infcx,
Obligation {
cause,
recursion_depth: 0,
param_env,
predicate: trait_ref.without_const().to_predicate(infcx.tcx),
},
Obligation { cause, recursion_depth: 0, param_env, predicate },
);
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/traits/util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ impl Elaborator<'tcx> {

let bound_predicate = obligation.predicate.kind();
match bound_predicate.skip_binder() {
ty::PredicateKind::Trait(data, ..) => {
ty::PredicateKind::Trait(data, _, _) => {
// Get predicates declared on the trait.
let predicates = tcx.super_predicates_of(data.def_id());

Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_middle/src/traits/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,9 @@ pub enum ObligationCauseCode<'tcx> {
/// Like `ItemObligation`, but with extra detail on the source of the obligation.
BindingObligation(DefId, Span),

/// Like `ItemObligation`, but with extra detail on the source of the obligation.
ImplicitSizedObligation(DefId, Span),

/// A type like `&'a T` is WF only if `T: 'a`.
ReferenceOutlivesReferent(Ty<'tcx>),

Expand Down
116 changes: 116 additions & 0 deletions compiler/rustc_middle/src/ty/diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use crate::ty::TyKind::*;
use crate::ty::{InferTy, TyCtxt, TyS};
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{Applicability, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
Expand Down Expand Up @@ -130,6 +131,121 @@ pub fn suggest_constraining_type_param(
if def_id == tcx.lang_items().sized_trait() {
// Type parameters are already `Sized` by default.
err.span_label(param.span, &format!("this type parameter needs to be `{}`", constraint));
// See if there's a `?Sized` bound that can be removed to suggest that.
match param.bounds {
[] => {}
bounds => {
// First look at the `where` clause because we can have `where T: ?Sized`, but that
// `?Sized` bound is *also* included in the `GenericParam` as a bound, which breaks
// the spans. Hence the somewhat involved logic that follows.
let mut where_unsized_bounds = FxHashSet::default();
for (where_pos, predicate) in generics.where_clause.predicates.iter().enumerate() {
match predicate {
WherePredicate::BoundPredicate(WhereBoundPredicate {
bounded_ty:
hir::Ty {
kind:
hir::TyKind::Path(hir::QPath::Resolved(
None,
hir::Path {
segments: [segment],
res:
hir::def::Res::Def(hir::def::DefKind::TyParam, _),
..
},
)),
..
},
bounds,
span,
..
}) if segment.ident.as_str() == param_name => {
for (pos, bound) in bounds.iter().enumerate() {
match bound {
hir::GenericBound::Trait(
poly,
hir::TraitBoundModifier::Maybe,
) if poly.trait_ref.trait_def_id() == def_id => {
let sp = match (
bounds.len(),
pos,
generics.where_clause.predicates.len(),
where_pos,
) {
// where T: ?Sized
// ^^^^^^^^^^^^^^^
(1, _, 1, _) => generics.where_clause.span,
// where Foo: Bar, T: ?Sized,
// ^^^^^^^^^^^
(1, _, len, pos) if pos == len - 1 => {
generics.where_clause.predicates[pos - 1]
.span()
.shrink_to_hi()
.to(*span)
}
// where T: ?Sized, Foo: Bar,
// ^^^^^^^^^^^
(1, _, _, pos) => span.until(
generics.where_clause.predicates[pos + 1].span(),
),
// where T: ?Sized + Bar, Foo: Bar,
// ^^^^^^^^^
(_, 0, _, _) => {
bound.span().to(bounds[1].span().shrink_to_lo())
}
// where T: Bar + ?Sized, Foo: Bar,
// ^^^^^^^^^
(_, pos, _, _) => bounds[pos - 1]
.span()
.shrink_to_hi()
.to(bound.span()),
};
where_unsized_bounds.insert(bound.span());
err.span_suggestion_verbose(
sp,
"consider removing the `?Sized` bound to make the \
type parameter `Sized`",
String::new(),
Applicability::MaybeIncorrect,
);
}
_ => {}
}
}
}
_ => {}
}
}
for (pos, bound) in bounds.iter().enumerate() {
match bound {
hir::GenericBound::Trait(poly, hir::TraitBoundModifier::Maybe)
if poly.trait_ref.trait_def_id() == def_id
&& !where_unsized_bounds.contains(&bound.span()) =>
{
let sp = match (bounds.len(), pos) {
// T: ?Sized,
// ^^^^^^^^
(1, _) => param.span.shrink_to_hi().to(bound.span()),
// T: ?Sized + Bar,
// ^^^^^^^^^
(_, 0) => bound.span().to(bounds[1].span().shrink_to_lo()),
// T: Bar + ?Sized,
// ^^^^^^^^^
(_, pos) => bounds[pos - 1].span().shrink_to_hi().to(bound.span()),
};
err.span_suggestion_verbose(
sp,
"consider removing the `?Sized` bound to make the type parameter \
`Sized`",
String::new(),
Applicability::MaybeIncorrect,
);
}
_ => {}
}
}
}
}
return true;
}
let mut suggest_restrict = |span| {
Expand Down
77 changes: 10 additions & 67 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
&obligation.cause.code,
&mut vec![],
&mut Default::default(),
None,
);

err.emit();
Expand Down Expand Up @@ -233,7 +232,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
) {
let tcx = self.tcx;
let span = obligation.cause.span;
let mut obligation_note = None;

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

let bound_predicate = obligation.predicate.kind();
match bound_predicate.skip_binder() {
ty::PredicateKind::Trait(trait_predicate, _, implicit) => {
ty::PredicateKind::Trait(trait_predicate, _, _) => {
let trait_predicate = bound_predicate.rebind(trait_predicate);
let trait_predicate = self.resolve_vars_if_possible(trait_predicate);

Expand Down Expand Up @@ -313,61 +311,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
post_message,
))
);
if let ty::ImplicitTraitPredicate::Yes = implicit {
if let ObligationCauseCode::BindingObligation(item_def_id, _) =
obligation.cause.code.peel_derives()
{
match self.tcx.hir().get_if_local(*item_def_id) {
Some(hir::Node::TraitItem(hir::TraitItem {
kind: hir::TraitItemKind::Type(bounds, ty),
ident,
..
})) => {
obligation_note = Some(
"associated types introduce an implicit `Sized` \
obligation",
);
match (bounds, ty) {
([], None) => {
err.span_suggestion_verbose(
ident.span.shrink_to_hi(),
"consider relaxing the `Sized` obligation",
": ?Sized".to_string(),
Applicability::MaybeIncorrect,
);
}
([.., bound], None) => {
err.span_suggestion_verbose(
bound.span().shrink_to_hi(),
"consider relaxing the `Sized` obligation",
" + ?Sized".to_string(),
Applicability::MaybeIncorrect,
);
}
_ => {}
}
}
Some(hir::Node::ImplItem(hir::ImplItem {
kind: hir::ImplItemKind::TyAlias(_),
..
})) => {
obligation_note = Some(
"associated types on `impl` blocks for types, have an \
implicit mandatory `Sized` obligation; associated \
types from `trait`s can be relaxed to `?Sized`",
);
}
_ => {
// This is (likely?) a type parameter. The suggestion is handled
// in `rustc_middle/src/ty/diagnostics.rs`.
obligation_note = Some(
"type parameters introduce an implicit `Sized` \
obligation",
);
}
}
}
}

if is_try_conversion {
let none_error = self
Expand Down Expand Up @@ -425,7 +368,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
points_at_arg,
have_alt_message,
) {
self.note_obligation_cause(&mut err, obligation, obligation_note);
self.note_obligation_cause(&mut err, obligation);
err.emit();
return;
}
Expand Down Expand Up @@ -854,7 +797,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}
};

self.note_obligation_cause(&mut err, obligation, obligation_note);
self.note_obligation_cause(&mut err, obligation);
self.point_at_returns_when_relevant(&mut err, &obligation);

err.emit();
Expand Down Expand Up @@ -1137,7 +1080,6 @@ trait InferCtxtPrivExt<'tcx> {
&self,
err: &mut DiagnosticBuilder<'tcx>,
obligation: &PredicateObligation<'tcx>,
note: Option<&str>,
);

fn suggest_unsized_bound_if_applicable(
Expand Down Expand Up @@ -1287,6 +1229,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
obligation.cause.code.peel_derives(),
ObligationCauseCode::ItemObligation(_)
| ObligationCauseCode::BindingObligation(_, _)
| ObligationCauseCode::ImplicitSizedObligation(_, _)
| ObligationCauseCode::ObjectCastObligation(_)
| ObligationCauseCode::OpaqueType
);
Expand Down Expand Up @@ -1358,7 +1301,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
_ => None,
};
self.note_type_err(&mut diag, &obligation.cause, secondary_span, values, err, true);
self.note_obligation_cause(&mut diag, obligation, None);
self.note_obligation_cause(&mut diag, obligation);
diag.emit();
}
});
Expand Down Expand Up @@ -1651,7 +1594,8 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
self.suggest_fully_qualified_path(&mut err, def_id, span, trait_ref.def_id());
} else if let (
Ok(ref snippet),
ObligationCauseCode::BindingObligation(ref def_id, _),
ObligationCauseCode::BindingObligation(ref def_id, _)
| ObligationCauseCode::ImplicitSizedObligation(ref def_id, _),
) =
(self.tcx.sess.source_map().span_to_snippet(span), &obligation.cause.code)
{
Expand Down Expand Up @@ -1767,7 +1711,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
err
}
};
self.note_obligation_cause(&mut err, obligation, None);
self.note_obligation_cause(&mut err, obligation);
err.emit();
}

Expand Down Expand Up @@ -1831,7 +1775,6 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
&self,
err: &mut DiagnosticBuilder<'tcx>,
obligation: &PredicateObligation<'tcx>,
note: Option<&str>,
) {
// First, attempt to add note to this error with an async-await-specific
// message, and fall back to regular note otherwise.
Expand All @@ -1842,7 +1785,6 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
&obligation.cause.code,
&mut vec![],
&mut Default::default(),
note,
);
self.suggest_unsized_bound_if_applicable(err, obligation);
}
Expand All @@ -1858,7 +1800,8 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
{
(
ty::PredicateKind::Trait(pred, _, _),
&ObligationCauseCode::BindingObligation(item_def_id, span),
&ObligationCauseCode::BindingObligation(item_def_id, span)
| &ObligationCauseCode::ImplicitSizedObligation(item_def_id, span),
) => (pred, item_def_id, span),
_ => return,
};
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}

if let ObligationCauseCode::ItemObligation(item)
| ObligationCauseCode::BindingObligation(item, _) = obligation.cause.code
| ObligationCauseCode::BindingObligation(item, _)
| ObligationCauseCode::ImplicitSizedObligation(item, _) = obligation.cause.code
{
// FIXME: maybe also have some way of handling methods
// from other traits? That would require name resolution,
Expand Down
Loading

0 comments on commit f338867

Please sign in to comment.