Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tweak suggest_constraining_type_param #70009

Merged
merged 1 commit into from
Mar 30, 2020
Merged

Tweak suggest_constraining_type_param #70009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
&mut err,
&param.name.as_str(),
"Copy",
tcx.sess.source_map(),
span,
None,
);
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_trait_selection/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#![feature(drain_filter)]
#![feature(in_band_lifetimes)]
#![feature(crate_visibility_modifier)]
#![feature(or_patterns)]
#![recursion_limit = "512"] // For rustdoc

#[macro_use]
Expand Down
130 changes: 40 additions & 90 deletions src/librustc_trait_selection/traits/error_reporting/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_hir::{Node, QPath, TyKind, WhereBoundPredicate, WherePredicate};
use rustc_session::DiagnosticMessageId;
use rustc_span::source_map::SourceMap;
use rustc_span::{ExpnKind, Span, DUMMY_SP};
use rustc_span::{BytePos, ExpnKind, Span, DUMMY_SP};
use std::fmt;

use crate::traits::query::evaluate_obligation::InferCtxtExt as _;
Expand Down Expand Up @@ -1679,14 +1678,8 @@ pub fn suggest_constraining_type_param(
err: &mut DiagnosticBuilder<'_>,
param_name: &str,
constraint: &str,
source_map: &SourceMap,
span: Span,
def_id: Option<DefId>,
) -> bool {
const MSG_RESTRICT_BOUND_FURTHER: &str = "consider further restricting this bound with";
const MSG_RESTRICT_TYPE: &str = "consider restricting this type parameter with";
const MSG_RESTRICT_TYPE_FURTHER: &str = "consider further restricting this type parameter with";

let param = generics.params.iter().find(|p| p.name.ident().as_str() == param_name);

let param = if let Some(param) = param {
Expand All @@ -1695,11 +1688,24 @@ pub fn suggest_constraining_type_param(
return false;
};

const MSG_RESTRICT_BOUND_FURTHER: &str = "consider further restricting this bound";
let msg_restrict_type = format!("consider restricting type parameter `{}`", param_name);
let msg_restrict_type_further =
format!("consider further restricting type parameter `{}`", param_name);

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));
return true;
}
let mut suggest_restrict = |span| {
err.span_suggestion_verbose(
span,
MSG_RESTRICT_BOUND_FURTHER,
format!(" + {}", constraint),
Applicability::MachineApplicable,
);
};

if param_name.starts_with("impl ") {
// If there's an `impl Trait` used in argument position, suggest
Expand All @@ -1717,19 +1723,15 @@ pub fn suggest_constraining_type_param(
// |
// replace with: `impl Foo + Bar`

err.span_help(param.span, &format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint));

err.tool_only_span_suggestion(
param.span,
MSG_RESTRICT_BOUND_FURTHER,
format!("{} + {}", param_name, constraint),
Applicability::MachineApplicable,
);

suggest_restrict(param.span.shrink_to_hi());
return true;
}

if generics.where_clause.predicates.is_empty() {
if generics.where_clause.predicates.is_empty()
// Given `trait Base<T = String>: Super<T>` where `T: Copy`, suggest restricting in the
// `where` clause instead of `trait Base<T: Copy = String>: Super<T>`.
&& !matches!(param.kind, hir::GenericParamKind::Type { default: Some(_), .. })
{
if let Some(bounds_span) = param.bounds_span() {
// If user has provided some bounds, suggest restricting them:
//
Expand All @@ -1744,38 +1746,16 @@ pub fn suggest_constraining_type_param(
// --
// |
// replace with: `T: Bar +`

err.span_help(
bounds_span,
&format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint),
);

let span_hi = param.span.with_hi(span.hi());
let span_with_colon = source_map.span_through_char(span_hi, ':');

if span_hi != param.span && span_with_colon != span_hi {
err.tool_only_span_suggestion(
span_with_colon,
MSG_RESTRICT_BOUND_FURTHER,
format!("{}: {} + ", param_name, constraint),
Applicability::MachineApplicable,
);
}
suggest_restrict(bounds_span.shrink_to_hi());
} else {
// If user hasn't provided any bounds, suggest adding a new one:
//
// fn foo<T>(t: T) { ... }
// - help: consider restricting this type parameter with `T: Foo`

err.span_help(
param.span,
&format!("{} `{}: {}`", MSG_RESTRICT_TYPE, param_name, constraint),
);

err.tool_only_span_suggestion(
param.span,
MSG_RESTRICT_TYPE,
format!("{}: {}", param_name, constraint),
err.span_suggestion_verbose(
param.span.shrink_to_hi(),
&msg_restrict_type,
format!(": {}", constraint),
Applicability::MachineApplicable,
);
}
Expand Down Expand Up @@ -1839,55 +1819,25 @@ pub fn suggest_constraining_type_param(
}
}

let where_clause_span =
generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi();
let where_clause_span = generics.where_clause.span_for_predicates_or_empty_place();
// Account for `fn foo<T>(t: T) where T: Foo,` so we don't suggest two trailing commas.
let mut trailing_comma = false;
if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(where_clause_span) {
trailing_comma = snippet.ends_with(",");
}
let where_clause_span = if trailing_comma {
let hi = where_clause_span.hi();
Span::new(hi - BytePos(1), hi, where_clause_span.ctxt())
estebank marked this conversation as resolved.
Show resolved Hide resolved
} else {
where_clause_span.shrink_to_hi()
};

match &param_spans[..] {
&[] => {
err.span_help(
param.span,
&format!("{} `where {}: {}`", MSG_RESTRICT_TYPE, param_name, constraint),
);

err.tool_only_span_suggestion(
where_clause_span,
MSG_RESTRICT_TYPE,
format!(", {}: {}", param_name, constraint),
Applicability::MachineApplicable,
);
}

&[&param_span] => {
err.span_help(
param_span,
&format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint),
);

let span_hi = param_span.with_hi(span.hi());
let span_with_colon = source_map.span_through_char(span_hi, ':');

if span_hi != param_span && span_with_colon != span_hi {
err.tool_only_span_suggestion(
span_with_colon,
MSG_RESTRICT_BOUND_FURTHER,
format!("{}: {} +", param_name, constraint),
Applicability::MachineApplicable,
);
}
}

&[&param_span] => suggest_restrict(param_span.shrink_to_hi()),
_ => {
err.span_help(
param.span,
&format!(
"{} `where {}: {}`",
MSG_RESTRICT_TYPE_FURTHER, param_name, constraint,
),
);

err.tool_only_span_suggestion(
err.span_suggestion_verbose(
where_clause_span,
MSG_RESTRICT_BOUND_FURTHER,
&msg_restrict_type_further,
format!(", {}: {}", param_name, constraint),
Applicability::MachineApplicable,
);
Expand Down
77 changes: 21 additions & 56 deletions src/librustc_trait_selection/traits/error_reporting/suggestions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
return;
}

hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, generics, _), .. })
| hir::Node::TraitItem(hir::TraitItem {
hir::Node::TraitItem(hir::TraitItem {
generics,
kind: hir::TraitItemKind::Fn(..),
..
Expand All @@ -206,63 +205,31 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
kind: hir::ImplItemKind::Fn(..),
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::Trait(_, _, generics, _, _),
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::Impl { generics, .. }, ..
}) if projection.is_some() => {
| hir::Node::Item(
hir::Item { kind: hir::ItemKind::Fn(_, generics, _), .. }
| hir::Item { kind: hir::ItemKind::Trait(_, _, generics, _, _), .. }
| hir::Item { kind: hir::ItemKind::Impl { generics, .. }, .. },
) if projection.is_some() => {
// Missing associated type bound.
suggest_restriction(&generics, "the associated type", err);
return;
}

hir::Node::Item(hir::Item {
kind: hir::ItemKind::Struct(_, generics),
span,
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::Enum(_, generics), span, ..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::Union(_, generics),
span,
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::Trait(_, _, generics, ..),
span,
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::Impl { generics, .. },
span,
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::Fn(_, generics, _),
span,
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::TyAlias(_, generics),
span,
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::TraitAlias(generics, _),
span,
..
})
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::OpaqueTy(hir::OpaqueTy { generics, .. }),
span,
..
})
| hir::Node::TraitItem(hir::TraitItem { generics, span, .. })
| hir::Node::ImplItem(hir::ImplItem { generics, span, .. })
hir::Node::Item(
hir::Item { kind: hir::ItemKind::Struct(_, generics), .. }
| hir::Item { kind: hir::ItemKind::Enum(_, generics), .. }
| hir::Item { kind: hir::ItemKind::Union(_, generics), .. }
| hir::Item { kind: hir::ItemKind::Trait(_, _, generics, ..), .. }
| hir::Item { kind: hir::ItemKind::Impl { generics, .. }, .. }
| hir::Item { kind: hir::ItemKind::Fn(_, generics, _), .. }
| hir::Item { kind: hir::ItemKind::TyAlias(_, generics), .. }
| hir::Item { kind: hir::ItemKind::TraitAlias(generics, _), .. }
| hir::Item {
kind: hir::ItemKind::OpaqueTy(hir::OpaqueTy { generics, .. }), ..
},
Comment on lines +219 to +229
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you can further simplify this by moving the or-pattern all the way into the kind: field-pattern (same above).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that first but I got a syntax error so I assumed it wasn't possible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, that shouldn't happen... Let's merge this without and I'll see what's up with this myself.

)
| hir::Node::TraitItem(hir::TraitItem { generics, .. })
| hir::Node::ImplItem(hir::ImplItem { generics, .. })
if param_ty =>
{
// Missing generic type parameter bound.
Expand All @@ -274,8 +241,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
&mut err,
&param_name,
&constraint,
self.tcx.sess.source_map(),
*span,
Some(trait_ref.def_id()),
) {
return;
Expand Down
7 changes: 3 additions & 4 deletions src/test/ui/associated-const/associated-const-type-parameter-arrays-2.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@ LL | const Y: usize;
LL | let _array = [4; <A as Foo>::Y];
| ^^^^^^^^^^^^^ the trait `Foo` is not implemented for `A`
|
help: consider further restricting this bound with `+ Foo`
--> $DIR/associated-const-type-parameter-arrays-2.rs:15:16
help: consider further restricting this bound
|
LL | pub fn test<A: Foo, B: Foo>() {
| ^^^
LL | pub fn test<A: Foo + Foo, B: Foo>() {
Centril marked this conversation as resolved.
Show resolved Hide resolved
| ^^^^^

error: aborting due to previous error

Expand Down
7 changes: 3 additions & 4 deletions src/test/ui/associated-const/associated-const-type-parameter-arrays.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@ LL | const Y: usize;
LL | let _array: [u32; <A as Foo>::Y];
| ^^^^^^^^^^^^^ the trait `Foo` is not implemented for `A`
|
help: consider further restricting this bound with `+ Foo`
--> $DIR/associated-const-type-parameter-arrays.rs:15:16
help: consider further restricting this bound
|
LL | pub fn test<A: Foo, B: Foo>() {
| ^^^
LL | pub fn test<A: Foo + Foo, B: Foo>() {
| ^^^^^

error: aborting due to previous error

Expand Down
7 changes: 3 additions & 4 deletions src/test/ui/associated-types/associated-types-invalid-trait-ref-issue-18865.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ error[E0277]: the trait bound `T: Foo<usize>` is not satisfied
LL | let u: <T as Foo<usize>>::Bar = t.get_bar();
| ^^^^^^^^^^^^^^^^^^^^^^ the trait `Foo<usize>` is not implemented for `T`
|
help: consider further restricting this bound with `+ Foo<usize>`
--> $DIR/associated-types-invalid-trait-ref-issue-18865.rs:9:8
help: consider further restricting this bound
|
LL | fn f<T:Foo<isize>>(t: &T) {
| ^^^^^^^^^^
LL | fn f<T:Foo<isize> + Foo<usize>>(t: &T) {
| ^^^^^^^^^^^^

error: aborting due to previous error

Expand Down
7 changes: 3 additions & 4 deletions src/test/ui/associated-types/associated-types-no-suitable-bound.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ error[E0277]: the trait bound `T: Get` is not satisfied
LL | fn uhoh<T>(foo: <T as Get>::Value) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `T`
|
help: consider restricting this type parameter with `T: Get`
--> $DIR/associated-types-no-suitable-bound.rs:11:13
help: consider restricting type parameter `T`
|
LL | fn uhoh<T>(foo: <T as Get>::Value) {}
| ^
LL | fn uhoh<T: Get>(foo: <T as Get>::Value) {}
| ^^^^^

error: aborting due to previous error

Expand Down
Loading