Skip to content

Commit

Permalink
Stop masking overflow and propagate it out more aggressively; also im…
Browse files Browse the repository at this point in the history
…prove error reporting to suggest to user how to fix.
  • Loading branch information
nikomatsakis committed Dec 8, 2014
1 parent 3ee85d8 commit 34812b8
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 17 deletions.
33 changes: 20 additions & 13 deletions src/librustc/middle/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,10 @@ enum BuiltinBoundConditions<'tcx> {
}

#[deriving(Show)]
enum EvaluationResult {
enum EvaluationResult<'tcx> {
EvaluatedToOk,
EvaluatedToErr,
EvaluatedToAmbig,
EvaluatedToErr(SelectionError<'tcx>),
}

impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
Expand Down Expand Up @@ -272,7 +272,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
bound: ty::BuiltinBound,
previous_stack: &ObligationStack<'o, 'tcx>,
ty: Ty<'tcx>)
-> EvaluationResult
-> EvaluationResult<'tcx>
{
let obligation =
util::obligation_for_builtin_bound(
Expand All @@ -295,7 +295,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
fn evaluate_obligation_recursively<'o>(&mut self,
previous_stack: Option<&ObligationStack<'o, 'tcx>>,
obligation: &Obligation<'tcx>)
-> EvaluationResult
-> EvaluationResult<'tcx>
{
debug!("evaluate_obligation_recursively({})",
obligation.repr(self.tcx()));
Expand All @@ -310,7 +310,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {

fn evaluate_stack<'o>(&mut self,
stack: &ObligationStack<'o, 'tcx>)
-> EvaluationResult
-> EvaluationResult<'tcx>
{
// In intercrate mode, whenever any of the types are unbound,
// there can always be an impl. Even if there are no impls in
Expand Down Expand Up @@ -381,7 +381,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
match self.candidate_from_obligation(stack) {
Ok(Some(c)) => self.winnow_candidate(stack, &c),
Ok(None) => EvaluatedToAmbig,
Err(_) => EvaluatedToErr,
Err(e) => EvaluatedToErr(e),
}
}

Expand Down Expand Up @@ -812,27 +812,27 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
fn winnow_candidate<'o>(&mut self,
stack: &ObligationStack<'o, 'tcx>,
candidate: &Candidate<'tcx>)
-> EvaluationResult
-> EvaluationResult<'tcx>
{
debug!("winnow_candidate: candidate={}", candidate.repr(self.tcx()));
self.infcx.probe(|| {
let candidate = (*candidate).clone();
match self.confirm_candidate(stack.obligation, candidate) {
Ok(selection) => self.winnow_selection(Some(stack), selection),
Err(_) => EvaluatedToErr,
Err(error) => EvaluatedToErr(error),
}
})
}

fn winnow_selection<'o>(&mut self,
stack: Option<&ObligationStack<'o, 'tcx>>,
selection: Selection<'tcx>)
-> EvaluationResult
-> EvaluationResult<'tcx>
{
let mut result = EvaluatedToOk;
for obligation in selection.iter_nested() {
match self.evaluate_obligation_recursively(stack, obligation) {
EvaluatedToErr => { return EvaluatedToErr; }
EvaluatedToErr(e) => { return EvaluatedToErr(e); }
EvaluatedToAmbig => { result = EvaluatedToAmbig; }
EvaluatedToOk => { }
}
Expand Down Expand Up @@ -1847,11 +1847,18 @@ impl<'o, 'tcx> Repr<'tcx> for ObligationStack<'o, 'tcx> {
}
}

impl EvaluationResult {
impl<'tcx> EvaluationResult<'tcx> {
fn may_apply(&self) -> bool {
match *self {
EvaluatedToOk | EvaluatedToAmbig => true,
EvaluatedToErr => false,
EvaluatedToOk |
EvaluatedToAmbig |
EvaluatedToErr(Overflow) |
EvaluatedToErr(OutputTypeParameterMismatch(..)) => {
true
}
EvaluatedToErr(Unimplemented) => {
false
}
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions src/librustc_typeck/check/vtable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,15 @@ pub fn report_selection_error<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
"overflow evaluating the trait `{}` for the type `{}`",
trait_ref.user_string(fcx.tcx()),
self_ty.user_string(fcx.tcx())).as_slice());

let current_limit = fcx.tcx().sess.recursion_limit.get();
let suggested_limit = current_limit * 2;
fcx.tcx().sess.span_note(
obligation.cause.span,
format!(
"consider adding a `#![recursion_limit=\"{}\"]` attribute to your crate",
suggested_limit)[]);

note_obligation_cause(fcx, obligation);
}
Unimplemented => {
Expand Down
8 changes: 6 additions & 2 deletions src/test/compile-fail/recursion_limit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ fn is_send<T:Send>() { }

fn main() {
is_send::<A>();
//~^ ERROR not implemented
//~^^ ERROR not implemented
//~^ ERROR overflow evaluating
//~^^ NOTE consider adding a `#![recursion_limit="20"]` attribute to your crate
//~^^^ NOTE must be implemented
//~^^^^ ERROR overflow evaluating
//~^^^^^ NOTE consider adding a `#![recursion_limit="20"]` attribute to your crate
//~^^^^^^ NOTE must be implemented
}
2 changes: 1 addition & 1 deletion src/test/compile-fail/unboxed-closures-type-mismatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ use std::ops::FnMut;

pub fn main() {
let mut f = |&mut: x: int, y: int| -> int { x + y };
let z = f.call_mut((1u, 2)); //~ ERROR not implemented
let z = f.call_mut((1u, 2)); //~ ERROR type mismatch
println!("{}", z);
}
2 changes: 1 addition & 1 deletion src/test/compile-fail/unboxed-closures-vtable-mismatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn call_it<F:FnMut<(int,int),int>>(y: int, mut f: F) -> int {

pub fn main() {
let f = |&mut: x: uint, y: int| -> int { (x as int) + y };
let z = call_it(3, f); //~ ERROR not implemented
let z = call_it(3, f); //~ ERROR type mismatch
println!("{}", z);
}

2 comments on commit 34812b8

@nikomatsakis
Copy link
Owner Author

Choose a reason for hiding this comment

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

r=eddyb

@alexcrichton
Copy link

Choose a reason for hiding this comment

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

@bors: retry

Please sign in to comment.