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

Remove duplicated errors for closure type mismatch #41760

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,11 @@ pub struct InferCtxt<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
/// Caches the results of trait evaluation.
pub evaluation_cache: traits::EvaluationCache<'tcx>,

// the set of predicates on which errors have been reported, to
// avoid reporting the same error twice.
/// Set of predicates on which errors have been reported, to
/// avoid reporting the same error twice.
pub reported_trait_errors: RefCell<FxHashSet<traits::TraitErrorKey<'tcx>>>,
/// Set of selection errors have been reported, to avoid reporting them twice.
pub reported_selection_errors: RefCell<FxHashSet<(Span, traits::SelectionError<'tcx>)>>,

// Sadly, the behavior of projection varies a bit depending on the
// stage of compilation. The specifics are given in the
Expand Down Expand Up @@ -502,6 +504,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'gcx> {
evaluation_cache: traits::EvaluationCache::new(),
projection_cache: RefCell::new(traits::ProjectionCache::new()),
reported_trait_errors: RefCell::new(FxHashSet()),
reported_selection_errors: RefCell::new(FxHashSet()),
projection_mode: Reveal::UserFacing,
tainted_by_errors_flag: Cell::new(false),
err_count_on_creation: self.sess.err_count(),
Expand Down Expand Up @@ -540,6 +543,7 @@ impl<'a, 'gcx, 'tcx> InferCtxtBuilder<'a, 'gcx, 'tcx> {
selection_cache: traits::SelectionCache::new(),
evaluation_cache: traits::EvaluationCache::new(),
reported_trait_errors: RefCell::new(FxHashSet()),
reported_selection_errors: RefCell::new(FxHashSet()),
projection_mode: projection_mode,
tainted_by_errors_flag: Cell::new(false),
err_count_on_creation: tcx.sess.err_count(),
Expand Down
69 changes: 23 additions & 46 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use super::{
Obligation,
ObligationCause,
ObligationCauseCode,
ParameterCountMismatch,
OutputTypeParameterMismatch,
TraitNotObjectSafe,
PredicateObligation,
Expand Down Expand Up @@ -54,11 +55,11 @@ pub struct TraitErrorKey<'tcx> {
impl<'a, 'gcx, 'tcx> TraitErrorKey<'tcx> {
fn from_error(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
e: &FulfillmentError<'tcx>) -> Self {
let predicate =
infcx.resolve_type_vars_if_possible(&e.obligation.predicate);
let predicate = infcx.resolve_type_vars_if_possible(&e.obligation.predicate);
let predicate = infcx.tcx.erase_regions(&predicate);
TraitErrorKey {
span: e.obligation.cause.span,
predicate: infcx.tcx.erase_regions(&predicate)
predicate: predicate,
}
}
}
Expand Down Expand Up @@ -523,6 +524,11 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
{
let span = obligation.cause.span;

if !self.reported_selection_errors.borrow_mut().insert((span, error.clone())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This sounds overly excessive. Outside of closures all trait errors are Unimplemented, so you are only showing 1 error per span.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it was surprising to me how many tests were affected by this. I'll think of a different way to deduplicate these errors...

Originally I was trying to get to the point where only one of FnMut and FnOnce were required, which feels like the correct way to accomplish this, but could not find where to tackle that (even pouring through the code and with full debugging output on :-/ ).

debug!("report_selection_error: skipping duplicate {:?}", error);
return;
}

let mut err = match *error {
SelectionError::Unimplemented => {
if let ObligationCauseCode::CompareImplMethodObligation {
Expand Down Expand Up @@ -657,6 +663,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}
}

ParameterCountMismatch(expected_found, ty, def_id) => {
let found_span = self.tcx.hir.span_if_local(def_id);
self.report_arg_count_mismatch(span,
found_span,
expected_found.expected,
expected_found.found,
ty.is_closure())
}
OutputTypeParameterMismatch(ref expected_trait_ref, ref actual_trait_ref, ref e) => {
let expected_trait_ref = self.resolve_type_vars_if_possible(&*expected_trait_ref);
let actual_trait_ref = self.resolve_type_vars_if_possible(&*actual_trait_ref);
Expand All @@ -668,49 +682,12 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
self.tcx.hir.span_if_local(did)
});

if let &TypeError::TupleSize(ref expected_found) = e {
// Expected `|x| { }`, found `|x, y| { }`
self.report_arg_count_mismatch(span,
found_span,
expected_found.expected,
expected_found.found,
expected_trait_ty.is_closure())
} else if let &TypeError::Sorts(ref expected_found) = e {
let expected = if let ty::TyTuple(tys, _) = expected_found.expected.sty {
tys.len()
} else {
1
};
let found = if let ty::TyTuple(tys, _) = expected_found.found.sty {
tys.len()
} else {
1
};

if expected != found {
// Expected `|| { }`, found `|x, y| { }`
// Expected `fn(x) -> ()`, found `|| { }`
self.report_arg_count_mismatch(span,
found_span,
expected,
found,
expected_trait_ty.is_closure())
} else {
self.report_type_argument_mismatch(span,
found_span,
expected_trait_ty,
expected_trait_ref,
actual_trait_ref,
e)
}
} else {
self.report_type_argument_mismatch(span,
found_span,
expected_trait_ty,
expected_trait_ref,
actual_trait_ref,
e)
}
self.report_type_argument_mismatch(span,
found_span,
expected_trait_ty,
expected_trait_ref,
actual_trait_ref,
e)
}

TraitNotObjectSafe(did) => {
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,13 @@ pub type TraitObligations<'tcx> = Vec<TraitObligation<'tcx>>;

pub type Selection<'tcx> = Vtable<'tcx, PredicateObligation<'tcx>>;

#[derive(Clone,Debug)]
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub enum SelectionError<'tcx> {
Unimplemented,
OutputTypeParameterMismatch(ty::PolyTraitRef<'tcx>,
ty::PolyTraitRef<'tcx>,
ty::error::TypeError<'tcx>),
ParameterCountMismatch(ExpectedFound<usize>, Ty<'tcx>, DefId),
TraitNotObjectSafe(DefId),
}

Expand Down
40 changes: 38 additions & 2 deletions src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use super::project;
use super::project::{normalize_with_depth, Normalized};
use super::{PredicateObligation, TraitObligation, ObligationCause};
use super::{ObligationCauseCode, BuiltinDerivedObligation, ImplDerivedObligation};
use super::{SelectionError, Unimplemented, OutputTypeParameterMismatch};
use super::{SelectionError, Unimplemented, ParameterCountMismatch, OutputTypeParameterMismatch};
use super::{ObjectCastObligation, Obligation};
use super::Reveal;
use super::TraitNotObjectSafe;
Expand All @@ -39,6 +39,7 @@ use ty::{self, ToPredicate, ToPolyTraitRef, Ty, TyCtxt, TypeFoldable};
use traits;
use ty::fast_reject;
use ty::relate::TypeRelation;
use ty::error::TypeError::{TupleSize, Sorts};
use middle::lang_items;

use rustc_data_structures::bitvec::BitVector;
Expand Down Expand Up @@ -2425,7 +2426,42 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
expected_trait_ref.clone(),
obligation_trait_ref.clone())
.map(|InferOk { obligations, .. }| self.inferred_obligations.extend(obligations))
.map_err(|e| OutputTypeParameterMismatch(expected_trait_ref, obligation_trait_ref, e))
.map_err(|e| {
let self_ty = expected_trait_ref.self_ty();
match (&self_ty.sty, &e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please just compare the # of parameters in the fn sig vs. the # of parameters in the obligation (that's obligation.predicate.skip_binder().subst_at(1))?

(&ty::TyClosure(def_id, ..), &TupleSize(expected_found)) |
(&ty::TyFnDef(def_id, ..), &TupleSize(expected_found)) => {
// Expected `fn(x)`/`|x| { }`, found `fn(x, y)`/`|x, y| { }`
ParameterCountMismatch(expected_found, self_ty, def_id)
}
(&ty::TyClosure(def_id, ..), &Sorts(expected_found)) |
(&ty::TyFnDef(def_id, ..), &Sorts(expected_found)) => {
// Expected `|| { }`, found `|x, y| { }`
// Expected `fn(x)`, found `|| { }`
let expected = if let ty::TyTuple(tys, _) = expected_found.expected.sty {
tys.len()
} else {
1
};
let found = if let ty::TyTuple(tys, _) = expected_found.found.sty {
tys.len()
} else {
1
};

if expected != found {
let expected_found = ty::error::ExpectedFound {
expected: expected,
found: found,
};
ParameterCountMismatch(expected_found, self_ty, def_id)
} else {
OutputTypeParameterMismatch(expected_trait_ref, obligation_trait_ref, e)
}
}
_ => OutputTypeParameterMismatch(expected_trait_ref, obligation_trait_ref, e),
}
})
}

fn confirm_builtin_unsize_candidate(&mut self,
Expand Down
5 changes: 5 additions & 0 deletions src/librustc/traits/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,11 @@ impl<'a, 'tcx> Lift<'tcx> for traits::SelectionError<'a> {
fn lift_to_tcx<'b, 'gcx>(&self, tcx: TyCtxt<'b, 'gcx, 'tcx>) -> Option<Self::Lifted> {
match *self {
super::Unimplemented => Some(super::Unimplemented),
super::ParameterCountMismatch(expected_found, ref ty, def_id) => {
tcx.lift(ty).map(|ty| {
super::ParameterCountMismatch(expected_found, ty, def_id)
})
}
super::OutputTypeParameterMismatch(a, b, ref err) => {
tcx.lift(&(a, b)).and_then(|(a, b)| {
tcx.lift(err).map(|err| {
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ use syntax_pos::Span;

use hir;

#[derive(Clone, Copy, Debug)]
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
pub struct ExpectedFound<T> {
pub expected: T,
pub found: T,
}

// Data structures used in type unification
#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub enum TypeError<'tcx> {
Mismatch,
UnsafetyMismatch(ExpectedFound<hir::Unsafety>),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ pub struct ClosureUpvar<'tcx> {
pub ty: Ty<'tcx>,
}

#[derive(Clone, Copy, PartialEq)]
#[derive(Clone, Copy, PartialEq, Eq, Hash)]
pub enum IntVarValue {
IntType(ast::IntTy),
UintType(ast::UintTy),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_data_structures/obligation_forest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ impl<O: ForestObligation> ObligationForest<O> {
/// Convert all remaining obligations to the given error.
///
/// This cannot be done during a snapshot.
pub fn to_errors<E: Clone>(&mut self, error: E) -> Vec<Error<O, E>> {
pub fn to_errors<E: Clone + Debug>(&mut self, error: E) -> Vec<Error<O, E>> {
assert!(!self.in_snapshot());
let mut errors = vec![];
for index in 0..self.nodes.len() {
Expand Down
1 change: 0 additions & 1 deletion src/test/compile-fail/extern-wrong-value-type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,4 @@ fn main() {
let _x: extern "C" fn() = f; // OK
is_fn(f);
//~^ ERROR `extern "C" fn() {f}: std::ops::Fn<()>` is not satisfied
//~| ERROR `extern "C" fn() {f}: std::ops::FnOnce<()>` is not satisfied
}
1 change: 0 additions & 1 deletion src/test/compile-fail/fn-trait-formatting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,4 @@ fn main() {

needs_fn(1);
//~^ ERROR : std::ops::Fn<(isize,)>`
//~| ERROR : std::ops::FnOnce<(isize,)>`
}
1 change: 0 additions & 1 deletion src/test/compile-fail/issue-22034.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,5 @@ fn main() {
let _: &mut Fn() = unsafe {
&mut *(ptr as *mut Fn())
//~^ ERROR `(): std::ops::Fn<()>` is not satisfied
//~| ERROR `(): std::ops::FnOnce<()>` is not satisfied
};
}
1 change: 0 additions & 1 deletion src/test/compile-fail/issue-23966.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,4 @@
fn main() {
"".chars().fold(|_, _| (), ());
//~^ ERROR E0277
//~| ERROR E0277
}
2 changes: 0 additions & 2 deletions src/test/compile-fail/kindck-impl-type-params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,12 @@ fn f<T>(val: T) {
let t: S<T> = S(marker::PhantomData);
let a = &t as &Gettable<T>;
//~^ ERROR : std::marker::Send` is not satisfied
//~^^ ERROR : std::marker::Copy` is not satisfied
}

fn g<T>(val: T) {
let t: S<T> = S(marker::PhantomData);
let a: &Gettable<T> = &t;
//~^ ERROR : std::marker::Send` is not satisfied
//~^^ ERROR : std::marker::Copy` is not satisfied
}

fn foo<'a>() {
Expand Down
1 change: 0 additions & 1 deletion src/test/compile-fail/not-panic-safe-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,4 @@ fn assert<T: UnwindSafe + ?Sized>() {}
fn main() {
assert::<Rc<RefCell<i32>>>();
//~^ ERROR `std::cell::UnsafeCell<i32>: std::panic::RefUnwindSafe` is not satisfied
//~^^ ERROR `std::cell::UnsafeCell<usize>: std::panic::RefUnwindSafe` is not satisfied
}
1 change: 0 additions & 1 deletion src/test/compile-fail/not-panic-safe-3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,4 @@ fn assert<T: UnwindSafe + ?Sized>() {}
fn main() {
assert::<Arc<RefCell<i32>>>();
//~^ ERROR `std::cell::UnsafeCell<i32>: std::panic::RefUnwindSafe` is not satisfied
//~^^ ERROR `std::cell::UnsafeCell<usize>: std::panic::RefUnwindSafe` is not satisfied
}
1 change: 0 additions & 1 deletion src/test/compile-fail/not-panic-safe-4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,4 @@ fn assert<T: UnwindSafe + ?Sized>() {}
fn main() {
assert::<&RefCell<i32>>();
//~^ ERROR `std::cell::UnsafeCell<i32>: std::panic::RefUnwindSafe` is not satisfied
//~^^ ERROR `std::cell::UnsafeCell<usize>: std::panic::RefUnwindSafe` is not satisfied
}
1 change: 0 additions & 1 deletion src/test/compile-fail/not-panic-safe-6.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,4 @@ fn assert<T: UnwindSafe + ?Sized>() {}
fn main() {
assert::<*mut RefCell<i32>>();
//~^ ERROR `std::cell::UnsafeCell<i32>: std::panic::RefUnwindSafe` is not satisfied
//~^^ ERROR `std::cell::UnsafeCell<usize>: std::panic::RefUnwindSafe` is not satisfied
}
Loading