Skip to content

Commit 2c71894

Browse files
estebankestebank
committed
Tweak suggest_constraining_type_param
Some of the bound restriction structured suggestions were incorrect while others had subpar output.
1 parent 285519d commit 2c71894

File tree

70 files changed

+439
-640
lines changed

Some content is hidden

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

70 files changed

+439
-640
lines changed

‎src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
222222
&mut err,
223223
&param.name.as_str(),
224224
"Copy",
225-
tcx.sess.source_map(),
226-
span,
227225
None,
228226
);
229227
}

‎src/librustc_trait_selection/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#![feature(drain_filter)]
1616
#![feature(in_band_lifetimes)]
1717
#![feature(crate_visibility_modifier)]
18+
#![feature(or_patterns)]
1819
#![recursion_limit = "512"] // For rustdoc
1920

2021
#[macro_use]

‎src/librustc_trait_selection/traits/error_reporting/mod.rs

Lines changed: 40 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ use rustc_hir as hir;
2525
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
2626
use rustc_hir::{Node, QPath, TyKind, WhereBoundPredicate, WherePredicate};
2727
use rustc_session::DiagnosticMessageId;
28-
use rustc_span::source_map::SourceMap;
29-
use rustc_span::{ExpnKind, Span, DUMMY_SP};
28+
use rustc_span::{BytePos, ExpnKind, Span, DUMMY_SP};
3029
use std::fmt;
3130

3231
use crate::traits::query::evaluate_obligation::InferCtxtExt as _;
@@ -1679,14 +1678,8 @@ pub fn suggest_constraining_type_param(
16791678
err: &mut DiagnosticBuilder<'_>,
16801679
param_name: &str,
16811680
constraint: &str,
1682-
source_map: &SourceMap,
1683-
span: Span,
16841681
def_id: Option<DefId>,
16851682
) -> bool {
1686-
const MSG_RESTRICT_BOUND_FURTHER: &str = "consider further restricting this bound with";
1687-
const MSG_RESTRICT_TYPE: &str = "consider restricting this type parameter with";
1688-
const MSG_RESTRICT_TYPE_FURTHER: &str = "consider further restricting this type parameter with";
1689-
16901683
let param = generics.params.iter().find(|p| p.name.ident().as_str() == param_name);
16911684

16921685
let param = if let Some(param) = param {
@@ -1695,11 +1688,24 @@ pub fn suggest_constraining_type_param(
16951688
return false;
16961689
};
16971690

1691+
const MSG_RESTRICT_BOUND_FURTHER: &str = "consider further restricting this bound";
1692+
let msg_restrict_type = format!("consider restricting type parameter `{}`", param_name);
1693+
let msg_restrict_type_further =
1694+
format!("consider further restricting type parameter `{}`", param_name);
1695+
16981696
if def_id == tcx.lang_items().sized_trait() {
16991697
// Type parameters are already `Sized` by default.
17001698
err.span_label(param.span, &format!("this type parameter needs to be `{}`", constraint));
17011699
return true;
17021700
}
1701+
let mut suggest_restrict = |span| {
1702+
err.span_suggestion_verbose(
1703+
span,
1704+
MSG_RESTRICT_BOUND_FURTHER,
1705+
format!(" + {}", constraint),
1706+
Applicability::MachineApplicable,
1707+
);
1708+
};
17031709

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

1720-
err.span_help(param.span, &format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint));
1721-
1722-
err.tool_only_span_suggestion(
1723-
param.span,
1724-
MSG_RESTRICT_BOUND_FURTHER,
1725-
format!("{} + {}", param_name, constraint),
1726-
Applicability::MachineApplicable,
1727-
);
1728-
1726+
suggest_restrict(param.span.shrink_to_hi());
17291727
return true;
17301728
}
17311729

1732-
if generics.where_clause.predicates.is_empty() {
1730+
if generics.where_clause.predicates.is_empty()
1731+
// Given `trait Base<T = String>: Super<T>` where `T: Copy`, suggest restricting in the
1732+
// `where` clause instead of `trait Base<T: Copy = String>: Super<T>`.
1733+
&& !matches!(param.kind, hir::GenericParamKind::Type { default: Some(_), .. })
1734+
{
17331735
if let Some(bounds_span) = param.bounds_span() {
17341736
// If user has provided some bounds, suggest restricting them:
17351737
//
@@ -1744,38 +1746,16 @@ pub fn suggest_constraining_type_param(
17441746
// --
17451747
// |
17461748
// replace with: `T: Bar +`
1747-
1748-
err.span_help(
1749-
bounds_span,
1750-
&format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint),
1751-
);
1752-
1753-
let span_hi = param.span.with_hi(span.hi());
1754-
let span_with_colon = source_map.span_through_char(span_hi, ':');
1755-
1756-
if span_hi != param.span && span_with_colon != span_hi {
1757-
err.tool_only_span_suggestion(
1758-
span_with_colon,
1759-
MSG_RESTRICT_BOUND_FURTHER,
1760-
format!("{}: {} + ", param_name, constraint),
1761-
Applicability::MachineApplicable,
1762-
);
1763-
}
1749+
suggest_restrict(bounds_span.shrink_to_hi());
17641750
} else {
17651751
// If user hasn't provided any bounds, suggest adding a new one:
17661752
//
17671753
// fn foo<T>(t: T) { ... }
17681754
// - help: consider restricting this type parameter with `T: Foo`
1769-
1770-
err.span_help(
1771-
param.span,
1772-
&format!("{} `{}: {}`", MSG_RESTRICT_TYPE, param_name, constraint),
1773-
);
1774-
1775-
err.tool_only_span_suggestion(
1776-
param.span,
1777-
MSG_RESTRICT_TYPE,
1778-
format!("{}: {}", param_name, constraint),
1755+
err.span_suggestion_verbose(
1756+
param.span.shrink_to_hi(),
1757+
&msg_restrict_type,
1758+
format!(": {}", constraint),
17791759
Applicability::MachineApplicable,
17801760
);
17811761
}
@@ -1839,55 +1819,25 @@ pub fn suggest_constraining_type_param(
18391819
}
18401820
}
18411821

1842-
let where_clause_span =
1843-
generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi();
1822+
let where_clause_span = generics.where_clause.span_for_predicates_or_empty_place();
1823+
// Account for `fn foo<T>(t: T) where T: Foo,` so we don't suggest two trailing commas.
1824+
let mut trailing_comma = false;
1825+
if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(where_clause_span) {
1826+
trailing_comma = snippet.ends_with(",");
1827+
}
1828+
let where_clause_span = if trailing_comma {
1829+
let hi = where_clause_span.hi();
1830+
Span::new(hi - BytePos(1), hi, where_clause_span.ctxt())
1831+
} else {
1832+
where_clause_span.shrink_to_hi()
1833+
};
18441834

18451835
match &param_spans[..] {
1846-
&[] => {
1847-
err.span_help(
1848-
param.span,
1849-
&format!("{} `where {}: {}`", MSG_RESTRICT_TYPE, param_name, constraint),
1850-
);
1851-
1852-
err.tool_only_span_suggestion(
1853-
where_clause_span,
1854-
MSG_RESTRICT_TYPE,
1855-
format!(", {}: {}", param_name, constraint),
1856-
Applicability::MachineApplicable,
1857-
);
1858-
}
1859-
1860-
&[&param_span] => {
1861-
err.span_help(
1862-
param_span,
1863-
&format!("{} `+ {}`", MSG_RESTRICT_BOUND_FURTHER, constraint),
1864-
);
1865-
1866-
let span_hi = param_span.with_hi(span.hi());
1867-
let span_with_colon = source_map.span_through_char(span_hi, ':');
1868-
1869-
if span_hi != param_span && span_with_colon != span_hi {
1870-
err.tool_only_span_suggestion(
1871-
span_with_colon,
1872-
MSG_RESTRICT_BOUND_FURTHER,
1873-
format!("{}: {} +", param_name, constraint),
1874-
Applicability::MachineApplicable,
1875-
);
1876-
}
1877-
}
1878-
1836+
&[&param_span] => suggest_restrict(param_span.shrink_to_hi()),
18791837
_ => {
1880-
err.span_help(
1881-
param.span,
1882-
&format!(
1883-
"{} `where {}: {}`",
1884-
MSG_RESTRICT_TYPE_FURTHER, param_name, constraint,
1885-
),
1886-
);
1887-
1888-
err.tool_only_span_suggestion(
1838+
err.span_suggestion_verbose(
18891839
where_clause_span,
1890-
MSG_RESTRICT_BOUND_FURTHER,
1840+
&msg_restrict_type_further,
18911841
format!(", {}: {}", param_name, constraint),
18921842
Applicability::MachineApplicable,
18931843
);

‎src/librustc_trait_selection/traits/error_reporting/suggestions.rs

Lines changed: 21 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
195195
return;
196196
}
197197

198-
hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(_, generics, _), .. })
199-
| hir::Node::TraitItem(hir::TraitItem {
198+
hir::Node::TraitItem(hir::TraitItem {
200199
generics,
201200
kind: hir::TraitItemKind::Fn(..),
202201
..
@@ -206,63 +205,31 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
206205
kind: hir::ImplItemKind::Fn(..),
207206
..
208207
})
209-
| hir::Node::Item(hir::Item {
210-
kind: hir::ItemKind::Trait(_, _, generics, _, _),
211-
..
212-
})
213-
| hir::Node::Item(hir::Item {
214-
kind: hir::ItemKind::Impl { generics, .. }, ..
215-
}) if projection.is_some() => {
208+
| hir::Node::Item(
209+
hir::Item { kind: hir::ItemKind::Fn(_, generics, _), .. }
210+
| hir::Item { kind: hir::ItemKind::Trait(_, _, generics, _, _), .. }
211+
| hir::Item { kind: hir::ItemKind::Impl { generics, .. }, .. },
212+
) if projection.is_some() => {
216213
// Missing associated type bound.
217214
suggest_restriction(&generics, "the associated type", err);
218215
return;
219216
}
220217

221-
hir::Node::Item(hir::Item {
222-
kind: hir::ItemKind::Struct(_, generics),
223-
span,
224-
..
225-
})
226-
| hir::Node::Item(hir::Item {
227-
kind: hir::ItemKind::Enum(_, generics), span, ..
228-
})
229-
| hir::Node::Item(hir::Item {
230-
kind: hir::ItemKind::Union(_, generics),
231-
span,
232-
..
233-
})
234-
| hir::Node::Item(hir::Item {
235-
kind: hir::ItemKind::Trait(_, _, generics, ..),
236-
span,
237-
..
238-
})
239-
| hir::Node::Item(hir::Item {
240-
kind: hir::ItemKind::Impl { generics, .. },
241-
span,
242-
..
243-
})
244-
| hir::Node::Item(hir::Item {
245-
kind: hir::ItemKind::Fn(_, generics, _),
246-
span,
247-
..
248-
})
249-
| hir::Node::Item(hir::Item {
250-
kind: hir::ItemKind::TyAlias(_, generics),
251-
span,
252-
..
253-
})
254-
| hir::Node::Item(hir::Item {
255-
kind: hir::ItemKind::TraitAlias(generics, _),
256-
span,
257-
..
258-
})
259-
| hir::Node::Item(hir::Item {
260-
kind: hir::ItemKind::OpaqueTy(hir::OpaqueTy { generics, .. }),
261-
span,
262-
..
263-
})
264-
| hir::Node::TraitItem(hir::TraitItem { generics, span, .. })
265-
| hir::Node::ImplItem(hir::ImplItem { generics, span, .. })
218+
hir::Node::Item(
219+
hir::Item { kind: hir::ItemKind::Struct(_, generics), .. }
220+
| hir::Item { kind: hir::ItemKind::Enum(_, generics), .. }
221+
| hir::Item { kind: hir::ItemKind::Union(_, generics), .. }
222+
| hir::Item { kind: hir::ItemKind::Trait(_, _, generics, ..), .. }
223+
| hir::Item { kind: hir::ItemKind::Impl { generics, .. }, .. }
224+
| hir::Item { kind: hir::ItemKind::Fn(_, generics, _), .. }
225+
| hir::Item { kind: hir::ItemKind::TyAlias(_, generics), .. }
226+
| hir::Item { kind: hir::ItemKind::TraitAlias(generics, _), .. }
227+
| hir::Item {
228+
kind: hir::ItemKind::OpaqueTy(hir::OpaqueTy { generics, .. }), ..
229+
},
230+
)
231+
| hir::Node::TraitItem(hir::TraitItem { generics, .. })
232+
| hir::Node::ImplItem(hir::ImplItem { generics, .. })
266233
if param_ty =>
267234
{
268235
// Missing generic type parameter bound.
@@ -274,8 +241,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
274241
&mut err,
275242
&param_name,
276243
&constraint,
277-
self.tcx.sess.source_map(),
278-
*span,
279244
Some(trait_ref.def_id()),
280245
) {
281246
return;

‎src/test/ui/associated-const/associated-const-type-parameter-arrays-2.stderr

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,10 @@ LL | const Y: usize;
77
LL | let _array = [4; <A as Foo>::Y];
88
| ^^^^^^^^^^^^^ the trait `Foo` is not implemented for `A`
99
|
10-
help: consider further restricting this bound with `+ Foo`
11-
--> $DIR/associated-const-type-parameter-arrays-2.rs:15:16
10+
help: consider further restricting this bound
1211
|
13-
LL | pub fn test<A: Foo, B: Foo>() {
14-
| ^^^
12+
LL | pub fn test<A: Foo + Foo, B: Foo>() {
13+
| ^^^^^
1514

1615
error: aborting due to previous error
1716

‎src/test/ui/associated-const/associated-const-type-parameter-arrays.stderr

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,10 @@ LL | const Y: usize;
77
LL | let _array: [u32; <A as Foo>::Y];
88
| ^^^^^^^^^^^^^ the trait `Foo` is not implemented for `A`
99
|
10-
help: consider further restricting this bound with `+ Foo`
11-
--> $DIR/associated-const-type-parameter-arrays.rs:15:16
10+
help: consider further restricting this bound
1211
|
13-
LL | pub fn test<A: Foo, B: Foo>() {
14-
| ^^^
12+
LL | pub fn test<A: Foo + Foo, B: Foo>() {
13+
| ^^^^^
1514

1615
error: aborting due to previous error
1716

‎src/test/ui/associated-types/associated-types-invalid-trait-ref-issue-18865.stderr

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@ error[E0277]: the trait bound `T: Foo<usize>` is not satisfied
44
LL | let u: <T as Foo<usize>>::Bar = t.get_bar();
55
| ^^^^^^^^^^^^^^^^^^^^^^ the trait `Foo<usize>` is not implemented for `T`
66
|
7-
help: consider further restricting this bound with `+ Foo<usize>`
8-
--> $DIR/associated-types-invalid-trait-ref-issue-18865.rs:9:8
7+
help: consider further restricting this bound
98
|
10-
LL | fn f<T:Foo<isize>>(t: &T) {
11-
| ^^^^^^^^^^
9+
LL | fn f<T:Foo<isize> + Foo<usize>>(t: &T) {
10+
| ^^^^^^^^^^^^
1211

1312
error: aborting due to previous error
1413

‎src/test/ui/associated-types/associated-types-no-suitable-bound.stderr

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@ error[E0277]: the trait bound `T: Get` is not satisfied
44
LL | fn uhoh<T>(foo: <T as Get>::Value) {}
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Get` is not implemented for `T`
66
|
7-
help: consider restricting this type parameter with `T: Get`
8-
--> $DIR/associated-types-no-suitable-bound.rs:11:13
7+
help: consider restricting type parameter `T`
98
|
10-
LL | fn uhoh<T>(foo: <T as Get>::Value) {}
11-
| ^
9+
LL | fn uhoh<T: Get>(foo: <T as Get>::Value) {}
10+
| ^^^^^
1211

1312
error: aborting due to previous error
1413

0 commit comments

Comments
 (0)