Skip to content

Commit bd5fa75

Browse files
nikomatsakisnikomatsakis
committed
cleanup error reporting and add ui tests
1 parent e77cc9c commit bd5fa75

15 files changed

+229
-56
lines changed

‎src/librustc/infer/error_reporting.rs

Lines changed: 66 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
245245
debug!("report_region_errors: {} errors after preprocessing", errors.len());
246246

247247
for error in errors {
248+
debug!("report_region_errors: error = {:?}", error);
248249
match error.clone() {
249250
ConcreteFailure(origin, sub, sup) => {
250251
self.report_concrete_failure(origin, sub, sup).emit();
@@ -299,44 +300,58 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
299300
let mut bound_failures = Vec::new();
300301

301302
for error in errors {
303+
// Check whether we can process this error into some other
304+
// form; if not, fall through.
302305
match *error {
303306
ConcreteFailure(ref origin, sub, sup) => {
304307
debug!("processing ConcreteFailure");
305-
match free_regions_from_same_fn(self.tcx, sub, sup) {
306-
Some(ref same_frs) => {
307-
origins.push(
308-
ProcessedErrorOrigin::ConcreteFailure(
309-
origin.clone(),
310-
sub,
311-
sup));
312-
append_to_same_regions(&mut same_regions, same_frs);
313-
}
314-
_ => {
315-
other_errors.push(error.clone());
316-
}
308+
if let SubregionOrigin::CompareImplMethodObligation { .. } = *origin {
309+
// When comparing an impl method against a
310+
// trait method, it is not helpful to suggest
311+
// changes to the impl method. This is
312+
// because the impl method signature is being
313+
// checked using the trait's environment, so
314+
// usually the changes we suggest would
315+
// actually have to be applied to the *trait*
316+
// method (and it's not clear that the trait
317+
// method is even under the user's control).
318+
} else if let Some(same_frs) = free_regions_from_same_fn(self.tcx, sub, sup) {
319+
origins.push(
320+
ProcessedErrorOrigin::ConcreteFailure(
321+
origin.clone(),
322+
sub,
323+
sup));
324+
append_to_same_regions(&mut same_regions, &same_frs);
325+
continue;
317326
}
318327
}
319-
SubSupConflict(ref var_origin, _, sub_r, _, sup_r) => {
320-
debug!("processing SubSupConflict sub: {:?} sup: {:?}", sub_r, sup_r);
321-
match free_regions_from_same_fn(self.tcx, sub_r, sup_r) {
322-
Some(ref same_frs) => {
323-
origins.push(
324-
ProcessedErrorOrigin::VariableFailure(
325-
var_origin.clone()));
326-
append_to_same_regions(&mut same_regions, same_frs);
327-
}
328-
None => {
329-
other_errors.push(error.clone());
330-
}
328+
SubSupConflict(ref var_origin, ref sub_origin, sub, ref sup_origin, sup) => {
329+
debug!("processing SubSupConflict sub: {:?} sup: {:?}", sub, sup);
330+
if let SubregionOrigin::CompareImplMethodObligation { .. } = *sub_origin {
331+
// As above, when comparing an impl method
332+
// against a trait method, it is not helpful
333+
// to suggest changes to the impl method.
334+
} else if let SubregionOrigin::CompareImplMethodObligation { .. } = *sup_origin {
335+
// See above.
336+
} else if let Some(same_frs) = free_regions_from_same_fn(self.tcx, sub, sup) {
337+
origins.push(
338+
ProcessedErrorOrigin::VariableFailure(
339+
var_origin.clone()));
340+
append_to_same_regions(&mut same_regions, &same_frs);
341+
continue;
331342
}
332343
}
333344
GenericBoundFailure(ref origin, ref kind, region) => {
334345
bound_failures.push((origin.clone(), kind.clone(), region));
346+
continue;
335347
}
336348
ProcessedErrors(..) => {
337349
bug!("should not encounter a `ProcessedErrors` yet: {:?}", error)
338350
}
339351
}
352+
353+
// No changes to this error.
354+
other_errors.push(error.clone());
340355
}
341356

342357
// ok, let's pull together the errors, sorted in an order that
@@ -630,6 +645,18 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
630645
format!("the associated type `{}`", p),
631646
};
632647

648+
if let SubregionOrigin::CompareImplMethodObligation {
649+
span, item_name, impl_item_def_id, trait_item_def_id
650+
} = origin {
651+
self.report_extra_impl_obligation(span,
652+
item_name,
653+
impl_item_def_id,
654+
trait_item_def_id,
655+
&format!("`{}: {}`", bound_kind, sub))
656+
.emit();
657+
return;
658+
}
659+
633660
let mut err = match *sub {
634661
ty::ReFree(ty::FreeRegion {bound_region: ty::BrNamed(..), ..}) => {
635662
// Does the required lifetime have a nice name we can print?
@@ -947,6 +974,16 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
947974
"");
948975
err
949976
}
977+
infer::CompareImplMethodObligation { span,
978+
item_name,
979+
impl_item_def_id,
980+
trait_item_def_id } => {
981+
self.report_extra_impl_obligation(span,
982+
item_name,
983+
impl_item_def_id,
984+
trait_item_def_id,
985+
&format!("`{}: {}`", sup, sub))
986+
}
950987
}
951988
}
952989

@@ -1792,6 +1829,11 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
17921829
"...so that references are valid when the destructor \
17931830
runs");
17941831
}
1832+
infer::CompareImplMethodObligation { span, .. } => {
1833+
err.span_note(
1834+
span,
1835+
"...so that the definition in impl matches the definition from the trait");
1836+
}
17951837
}
17961838
}
17971839
}

‎src/librustc/infer/mod.rs

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,15 @@ pub enum SubregionOrigin<'tcx> {
360360

361361
// Region constraint arriving from destructor safety
362362
SafeDestructor(Span),
363+
364+
// Comparing the signature and requirements of an impl method against
365+
// the containing trait.
366+
CompareImplMethodObligation {
367+
span: Span,
368+
item_name: ast::Name,
369+
impl_item_def_id: DefId,
370+
trait_item_def_id: DefId,
371+
},
363372
}
364373

365374
/// Places that type/region parameters can appear.
@@ -1152,16 +1161,16 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
11521161
}
11531162

11541163
pub fn region_outlives_predicate(&self,
1155-
span: Span,
1164+
cause: &traits::ObligationCause<'tcx>,
11561165
predicate: &ty::PolyRegionOutlivesPredicate<'tcx>)
11571166
-> UnitResult<'tcx>
11581167
{
11591168
self.commit_if_ok(|snapshot| {
11601169
let (ty::OutlivesPredicate(r_a, r_b), skol_map) =
11611170
self.skolemize_late_bound_regions(predicate, snapshot);
1162-
let origin = RelateRegionParamBound(span);
1171+
let origin = SubregionOrigin::from_cause(cause, || RelateRegionParamBound(cause.span));
11631172
self.sub_regions(origin, r_b, r_a); // `b : a` ==> `a <= b`
1164-
self.leak_check(false, span, &skol_map, snapshot)?;
1173+
self.leak_check(false, cause.span, &skol_map, snapshot)?;
11651174
Ok(self.pop_skolemized(skol_map, snapshot))
11661175
})
11671176
}
@@ -1792,6 +1801,30 @@ impl<'tcx> SubregionOrigin<'tcx> {
17921801
AddrOf(a) => a,
17931802
AutoBorrow(a) => a,
17941803
SafeDestructor(a) => a,
1804+
CompareImplMethodObligation { span, .. } => span,
1805+
}
1806+
}
1807+
1808+
pub fn from_cause<F>(cause: &traits::ObligationCause<'tcx>,
1809+
default: F)
1810+
-> Self
1811+
where F: FnOnce() -> Self
1812+
{
1813+
match cause.code {
1814+
traits::ObligationCauseCode::ReferenceOutlivesReferent(ref_type) =>
1815+
SubregionOrigin::ReferenceOutlivesReferent(ref_type, cause.span),
1816+
1817+
traits::ObligationCauseCode::CompareImplMethodObligation { item_name,
1818+
impl_item_def_id,
1819+
trait_item_def_id } =>
1820+
SubregionOrigin::CompareImplMethodObligation {
1821+
span: cause.span,
1822+
item_name: item_name,
1823+
impl_item_def_id: impl_item_def_id,
1824+
trait_item_def_id: trait_item_def_id,
1825+
},
1826+
1827+
_ => default(),
17951828
}
17961829
}
17971830
}

‎src/librustc/traits/error_reporting.rs

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ use util::nodemap::{FnvHashMap, FnvHashSet};
3636

3737
use std::cmp;
3838
use std::fmt;
39+
use syntax::ast;
3940
use syntax_pos::Span;
4041
use errors::DiagnosticBuilder;
4142

@@ -417,19 +418,49 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
417418
self.report_overflow_error(&cycle[0], false);
418419
}
419420

421+
pub fn report_extra_impl_obligation(&self,
422+
error_span: Span,
423+
item_name: ast::Name,
424+
_impl_item_def_id: DefId,
425+
trait_item_def_id: DefId,
426+
requirement: &fmt::Display)
427+
-> DiagnosticBuilder<'tcx>
428+
{
429+
let mut err =
430+
struct_span_err!(self.tcx.sess,
431+
error_span,
432+
E0276,
433+
"impl has stricter requirements than trait");
434+
435+
if let Some(trait_item_span) = self.tcx.map.span_if_local(trait_item_def_id) {
436+
err.span_label(trait_item_span,
437+
&format!("definition of `{}` from trait", item_name));
438+
}
439+
440+
err.span_label(
441+
error_span,
442+
&format!("impl has extra requirement {}", requirement));
443+
444+
err
445+
}
446+
420447
pub fn report_selection_error(&self,
421448
obligation: &PredicateObligation<'tcx>,
422449
error: &SelectionError<'tcx>)
423450
{
424451
let span = obligation.cause.span;
425452
let mut err = match *error {
426453
SelectionError::Unimplemented => {
427-
if let ObligationCauseCode::CompareImplMethodObligation = obligation.cause.code {
428-
span_err!(
429-
self.tcx.sess, span, E0276,
430-
"the requirement `{}` appears on the impl \
431-
method but not on the corresponding trait method",
432-
obligation.predicate);
454+
if let ObligationCauseCode::CompareImplMethodObligation {
455+
item_name, impl_item_def_id, trait_item_def_id
456+
} = obligation.cause.code {
457+
self.report_extra_impl_obligation(
458+
span,
459+
item_name,
460+
impl_item_def_id,
461+
trait_item_def_id,
462+
&format!("`{}`", obligation.predicate))
463+
.emit();
433464
return;
434465
} else {
435466
match obligation.predicate {
@@ -492,7 +523,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
492523

493524
ty::Predicate::RegionOutlives(ref predicate) => {
494525
let predicate = self.resolve_type_vars_if_possible(predicate);
495-
let err = self.region_outlives_predicate(span,
526+
let err = self.region_outlives_predicate(&obligation.cause,
496527
&predicate).err().unwrap();
497528
struct_span_err!(self.tcx.sess, span, E0279,
498529
"the requirement `{}` is not satisfied (`{}`)",
@@ -886,7 +917,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
886917
&parent_predicate,
887918
&data.parent_code);
888919
}
889-
ObligationCauseCode::CompareImplMethodObligation => {
920+
ObligationCauseCode::CompareImplMethodObligation { .. } => {
890921
err.note(
891922
&format!("the requirement `{}` appears on the impl method \
892923
but not on the corresponding trait method",

‎src/librustc/traits/fulfill.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,7 @@ fn process_predicate<'a, 'gcx, 'tcx>(
526526
}
527527

528528
ty::Predicate::RegionOutlives(ref binder) => {
529-
match selcx.infcx().region_outlives_predicate(obligation.cause.span, binder) {
529+
match selcx.infcx().region_outlives_predicate(&obligation.cause, binder) {
530530
Ok(()) => Ok(Some(Vec::new())),
531531
Err(_) => Err(CodeSelectionError(Unimplemented)),
532532
}

‎src/librustc/traits/mod.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,11 @@ pub enum ObligationCauseCode<'tcx> {
138138

139139
ImplDerivedObligation(DerivedObligationCause<'tcx>),
140140

141-
CompareImplMethodObligation,
141+
CompareImplMethodObligation {
142+
item_name: ast::Name,
143+
impl_item_def_id: DefId,
144+
trait_item_def_id: DefId
145+
},
142146
}
143147

144148
#[derive(Clone, Debug, PartialEq, Eq)]

‎src/librustc/traits/structural_impls.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,14 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
195195
super::ImplDerivedObligation(ref cause) => {
196196
tcx.lift(cause).map(super::ImplDerivedObligation)
197197
}
198-
super::CompareImplMethodObligation => {
199-
Some(super::CompareImplMethodObligation)
198+
super::CompareImplMethodObligation { item_name,
199+
impl_item_def_id,
200+
trait_item_def_id } => {
201+
Some(super::CompareImplMethodObligation {
202+
item_name: item_name,
203+
impl_item_def_id: impl_item_def_id,
204+
trait_item_def_id: trait_item_def_id,
205+
})
200206
}
201207
}
202208
}
@@ -459,7 +465,7 @@ impl<'tcx> TypeFoldable<'tcx> for traits::ObligationCauseCode<'tcx> {
459465
super::FieldSized |
460466
super::ConstSized |
461467
super::SharedStatic |
462-
super::CompareImplMethodObligation => self.clone(),
468+
super::CompareImplMethodObligation { .. } => self.clone(),
463469

464470
super::ProjectionWf(proj) => super::ProjectionWf(proj.fold_with(folder)),
465471
super::ReferenceOutlivesReferent(ty) => {
@@ -492,7 +498,7 @@ impl<'tcx> TypeFoldable<'tcx> for traits::ObligationCauseCode<'tcx> {
492498
super::FieldSized |
493499
super::ConstSized |
494500
super::SharedStatic |
495-
super::CompareImplMethodObligation => false,
501+
super::CompareImplMethodObligation { .. } => false,
496502

497503
super::ProjectionWf(proj) => proj.visit_with(visitor),
498504
super::ReferenceOutlivesReferent(ty) => ty.visit_with(visitor),

‎src/librustc_typeck/check/compare_method.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,11 @@ pub fn compare_impl_method<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
363363
let cause = traits::ObligationCause {
364364
span: impl_m_span,
365365
body_id: impl_m_body_id,
366-
code: traits::ObligationCauseCode::CompareImplMethodObligation,
366+
code: traits::ObligationCauseCode::CompareImplMethodObligation {
367+
item_name: impl_m.name,
368+
impl_item_def_id: impl_m.def_id,
369+
trait_item_def_id: trait_m.def_id,
370+
},
367371
};
368372

369373
fulfillment_cx.borrow_mut().register_predicate_obligation(

‎src/librustc_typeck/check/regionck.rs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> {
356356
debug!("visit_region_obligations: r_o={:?} cause={:?}",
357357
r_o, r_o.cause);
358358
let sup_type = self.resolve_type(r_o.sup_type);
359-
let origin = self.code_to_origin(r_o.cause.span, sup_type, &r_o.cause.code);
359+
let origin = self.code_to_origin(&r_o.cause, sup_type);
360360
self.type_must_outlive(origin, sup_type, r_o.sub_region);
361361
}
362362

@@ -366,16 +366,10 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> {
366366
}
367367

368368
fn code_to_origin(&self,
369-
span: Span,
370-
sup_type: Ty<'tcx>,
371-
code: &traits::ObligationCauseCode<'tcx>)
369+
cause: &traits::ObligationCause<'tcx>,
370+
sup_type: Ty<'tcx>)
372371
-> SubregionOrigin<'tcx> {
373-
match *code {
374-
traits::ObligationCauseCode::ReferenceOutlivesReferent(ref_type) =>
375-
infer::ReferenceOutlivesReferent(ref_type, span),
376-
_ =>
377-
infer::RelateParamBound(span, sup_type),
378-
}
372+
SubregionOrigin::from_cause(cause, || infer::RelateParamBound(cause.span, sup_type))
379373
}
380374

381375
/// This method populates the region map's `free_region_map`. It walks over the transformed

‎src/test/compile-fail/traits-elaborate-type-region-proj.rs renamed to ‎src/test/ui/compare-method/proj-outlives-region.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ trait Master<'a, T: ?Sized, U> {
1818

1919
// `U::Item: 'a` does not imply that `U: 'a`
2020
impl<'a, U: Iterator> Master<'a, U::Item, U> for () {
21-
fn foo() where U: 'a { }
22-
//~^ ERROR parameter type `V` may not live long enough
21+
fn foo() where U: 'a { } //~ ERROR E0276
2322
}
2423

2524
fn main() {

0 commit comments

Comments
 (0)