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.

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

Tweak obligation error output #68377

Merged
merged 23 commits into from
Feb 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
fca5c64
Point at arguments or output when fn obligations come from them, or i…
estebank Jan 18, 2020
1c9242f
Point at `Sized` bound
estebank Jan 19, 2020
d72bcdb
When object unsafe trait uses itself in associated item suggest using…
estebank Jan 20, 2020
0eb29d1
fix test
estebank Jan 20, 2020
d137b7a
review comments
estebank Jan 25, 2020
4b2f1db
Tweak `Self: Sized` restriction diagnostic output
estebank Jan 29, 2020
972ae5a
Point at the `Sized` obligation in `where` clauses
estebank Jan 29, 2020
8d48597
Point at return type obligations instead of at `fn` ident
estebank Jan 29, 2020
6870f79
Use more accurate failed predicate spans
estebank Jan 30, 2020
144e259
Slight rewording of diagnostic message
estebank Jan 30, 2020
132921b
Remove duplicated code
estebank Jan 30, 2020
06fea92
review comments
estebank Jan 31, 2020
3ca1c5d
Point at `Sized` requirements
estebank Jan 31, 2020
413bfa4
Wording changes to object unsafe trait errors
estebank Feb 1, 2020
a52ec87
Use more appropriate spans on object unsafe traits and provide struct…
estebank Feb 1, 2020
542130b
add tests for structured suggestion
estebank Feb 1, 2020
cb6dfea
Suggest `?Sized` on type parameters
estebank Feb 2, 2020
d216b73
Remove duplicated code
estebank Feb 2, 2020
342db71
Account for `?Sized` type parameter bounds
estebank Feb 2, 2020
16d935e
fix rebase
estebank Feb 2, 2020
865216b
Point at reason in object unsafe trait with `Self` in supertraits or …
estebank Feb 2, 2020
b9c125a
Deal with spans showing `std` lib
estebank Feb 2, 2020
0e58411
Change wording for object unsafe because of assoc const
estebank Feb 3, 2020
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
151 changes: 76 additions & 75 deletions src/librustc/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_span::source_map::SourceMap;
use rustc_span::{ExpnKind, Span, DUMMY_SP};
use std::fmt;
use syntax::ast;
Expand Down Expand Up @@ -1034,6 +1033,10 @@ pub fn report_object_safety_error(
violations: Vec<ObjectSafetyViolation>,
) -> DiagnosticBuilder<'tcx> {
let trait_str = tcx.def_path_str(trait_def_id);
let trait_span = tcx.hir().get_if_local(trait_def_id).and_then(|node| match node {
hir::Node::Item(item) => Some(item.ident.span),
_ => None,
});
let span = tcx.sess.source_map().def_span(span);
let mut err = struct_span_err!(
tcx.sess,
Expand All @@ -1045,14 +1048,45 @@ pub fn report_object_safety_error(
err.span_label(span, format!("the trait `{}` cannot be made into an object", trait_str));

let mut reported_violations = FxHashSet::default();
let mut had_span_label = false;
for violation in violations {
if let ObjectSafetyViolation::SizedSelf(sp) = &violation {
if !sp.is_empty() {
// Do not report `SizedSelf` without spans pointing at `SizedSelf` obligations
// with a `Span`.
reported_violations.insert(ObjectSafetyViolation::SizedSelf(vec![].into()));
}
}
if reported_violations.insert(violation.clone()) {
match violation.span() {
Some(span) => err.span_label(span, violation.error_msg()),
None => err.note(&violation.error_msg()),
let spans = violation.spans();
let msg = if trait_span.is_none() || spans.is_empty() {
format!("the trait cannot be made into an object because {}", violation.error_msg())
} else {
had_span_label = true;
format!("...because {}", violation.error_msg())
};
if spans.is_empty() {
err.note(&msg);
} else {
for span in spans {
err.span_label(span, &msg);
}
}
match (trait_span, violation.solution()) {
(Some(_), Some((note, None))) => {
err.help(&note);
}
(Some(_), Some((note, Some((sugg, span))))) => {
err.span_suggestion(span, &note, sugg, Applicability::MachineApplicable);
}
// Only provide the help if its a local trait, otherwise it's not actionable.
_ => {}
}
}
}
if let (Some(trait_span), true) = (trait_span, had_span_label) {
err.span_label(trait_span, "this trait cannot be made into an object...");
}

if tcx.sess.trait_methods_not_found.borrow().contains(&span) {
// Avoid emitting error caused by non-existing method (#58734)
Expand Down Expand Up @@ -1305,6 +1339,44 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
&obligation.cause.code,
&mut vec![],
);
self.suggest_unsized_bound_if_applicable(err, obligation);
}
}

fn suggest_unsized_bound_if_applicable(
&self,
err: &mut DiagnosticBuilder<'_>,
obligation: &PredicateObligation<'tcx>,
) {
if let (
ty::Predicate::Trait(pred, _),
ObligationCauseCode::BindingObligation(item_def_id, span),
) = (&obligation.predicate, &obligation.cause.code)
{
if let (Some(generics), true) = (
self.tcx.hir().get_if_local(*item_def_id).as_ref().and_then(|n| n.generics()),
Some(pred.def_id()) == self.tcx.lang_items().sized_trait(),
) {
for param in generics.params {
if param.span == *span
&& !param.bounds.iter().any(|bound| {
bound.trait_def_id() == self.tcx.lang_items().sized_trait()
})
{
let (span, separator) = match param.bounds {
[] => (span.shrink_to_hi(), ":"),
[.., bound] => (bound.span().shrink_to_hi(), " + "),
};
err.span_suggestion(
span,
"consider relaxing the implicit `Sized` restriction",
format!("{} ?Sized", separator),
Applicability::MachineApplicable,
);
return;
}
}
}
}
}

Expand Down Expand Up @@ -1354,74 +1426,3 @@ impl ArgKind {
}
}
}

/// Suggest restricting a type param with a new bound.
pub fn suggest_constraining_type_param(
generics: &hir::Generics<'_>,
err: &mut DiagnosticBuilder<'_>,
param_name: &str,
constraint: &str,
source_map: &SourceMap,
span: Span,
) -> bool {
let restrict_msg = "consider further restricting this bound";
if let Some(param) =
generics.params.iter().filter(|p| p.name.ident().as_str() == param_name).next()
{
if param_name.starts_with("impl ") {
// `impl Trait` in argument:
// `fn foo(x: impl Trait) {}` → `fn foo(t: impl Trait + Trait2) {}`
err.span_suggestion(
param.span,
restrict_msg,
// `impl CurrentTrait + MissingTrait`
format!("{} + {}", param_name, constraint),
Applicability::MachineApplicable,
);
} else if generics.where_clause.predicates.is_empty() && param.bounds.is_empty() {
// If there are no bounds whatsoever, suggest adding a constraint
// to the type parameter:
// `fn foo<T>(t: T) {}` → `fn foo<T: Trait>(t: T) {}`
err.span_suggestion(
param.span,
"consider restricting this bound",
format!("{}: {}", param_name, constraint),
Applicability::MachineApplicable,
);
} else if !generics.where_clause.predicates.is_empty() {
// There is a `where` clause, so suggest expanding it:
// `fn foo<T>(t: T) where T: Debug {}` →
// `fn foo<T>(t: T) where T: Debug, T: Trait {}`
err.span_suggestion(
generics.where_clause.span().unwrap().shrink_to_hi(),
&format!("consider further restricting type parameter `{}`", param_name),
format!(", {}: {}", param_name, constraint),
Applicability::MachineApplicable,
);
} else {
// If there is no `where` clause lean towards constraining to the
// type parameter:
// `fn foo<X: Bar, T>(t: T, x: X) {}` → `fn foo<T: Trait>(t: T) {}`
// `fn foo<T: Bar>(t: T) {}` → `fn foo<T: Bar + Trait>(t: T) {}`
let sp = param.span.with_hi(span.hi());
let span = source_map.span_through_char(sp, ':');
if sp != param.span && sp != span {
// Only suggest if we have high certainty that the span
// covers the colon in `foo<T: Trait>`.
err.span_suggestion(
span,
restrict_msg,
format!("{}: {} + ", param_name, constraint),
Applicability::MachineApplicable,
);
} else {
err.span_label(
param.span,
&format!("consider adding a `where {}: {}` bound", param_name, constraint),
);
}
}
return true;
}
false
}
12 changes: 11 additions & 1 deletion src/librustc/traits/error_reporting/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,14 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
let param_name = self_ty.to_string();
let constraint = trait_ref.print_only_trait_path().to_string();
if suggest_constraining_type_param(
self.tcx,
generics,
&mut err,
&param_name,
&constraint,
self.tcx.sess.source_map(),
*span,
Some(trait_ref.def_id()),
) {
return;
}
Expand Down Expand Up @@ -1652,18 +1654,26 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {

/// Suggest restricting a type param with a new bound.
pub fn suggest_constraining_type_param(
tcx: TyCtxt<'_>,
generics: &hir::Generics<'_>,
err: &mut DiagnosticBuilder<'_>,
param_name: &str,
constraint: &str,
source_map: &SourceMap,
span: Span,
def_id: Option<DefId>,
) -> bool {
let restrict_msg = "consider further restricting this bound";
if let Some(param) =
generics.params.iter().filter(|p| p.name.ident().as_str() == param_name).next()
{
if param_name.starts_with("impl ") {
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),
);
} else if param_name.starts_with("impl ") {
// `impl Trait` in argument:
// `fn foo(x: impl Trait) {}` → `fn foo(t: impl Trait + Trait2) {}`
err.span_suggestion(
Expand Down
Loading