have simpler diagnostic when passing arg to closure and missing borrow

[matthiaskrgr]

playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=9f1e00046f513d4d9da8790042e52ca9

Code:

struct MyStruct {
    age: u32
}

fn run<F: Fn(&MyStruct) -> bool>(func: F, data: &MyStruct) -> bool {
  func(&data)
} 

fn main() {
    let m = MyStruct { age: 4};
    let x = run(|x:MyStruct| x.age > 3, &m);
    println!("{}", x);
}

The error I got was

   Compiling playground v0.0.1 (/playground)
error[E0631]: type mismatch in closure arguments
  --> src/main.rs:12:13
   |
6  | fn run<F: Fn(&MyStruct) -> bool>(func: F, data: &MyStruct) -> bool {
   |    ---    --------------------- required by this bound in `run`
...
12 |     let x = run(|x:MyStruct| x.age > 3, &m);
   |             ^^^ ---------------------- found signature of `fn(MyStruct) -> _`
   |             |
   |             expected signature of `for<'r> fn(&'r MyStruct) -> _`

which is quite convoluted in my opinion.
It turned out I was only missing a borrow:
let x = run(|x:MyStruct| x.age > 3, &m);
=>
let x = run(|x:&MyStruct| x.age > 3, &m);

A simple "hint: add missing borrow: "&MyStruct"" with corresponding span would have been a lot more helpful here in my opinion.

[estebank]

The following function needs to be changed to inspect and compare the expected and found argument and return types and modify err to be more targeted.

rust/src/librustc/traits/error_reporting.rs

Lines 1333 to 1382 in 22bc9e1

	fn report_closure_arg_mismatch(
	&self,
	span: Span,
	found_span: Option<Span>,
	expected_ref: ty::PolyTraitRef<'tcx>,
	found: ty::PolyTraitRef<'tcx>,
	) -> DiagnosticBuilder<'tcx> {
	fn build_fn_sig_string<'tcx>(tcx: TyCtxt<'tcx>, trait_ref: &ty::TraitRef<'tcx>) -> String {
	let inputs = trait_ref.substs.type_at(1);
	let sig = if let ty::Tuple(inputs) = inputs.kind {
	tcx.mk_fn_sig(
	inputs.iter().map(|k| k.expect_ty()),
	tcx.mk_ty_infer(ty::TyVar(ty::TyVid { index: 0 })),
	false,
	hir::Unsafety::Normal,
	::rustc_target::spec::abi::Abi::Rust
	)
	} else {
	tcx.mk_fn_sig(
	::std::iter::once(inputs),
	tcx.mk_ty_infer(ty::TyVar(ty::TyVid { index: 0 })),
	false,
	hir::Unsafety::Normal,
	::rustc_target::spec::abi::Abi::Rust
	)
	};
	ty::Binder::bind(sig).to_string()
	}
	let argument_is_closure = expected_ref.skip_binder().substs.type_at(0).is_closure();
	let mut err = struct_span_err!(self.tcx.sess, span, E0631,
	"type mismatch in {} arguments",
	if argument_is_closure { "closure" } else { "function" });
	let found_str = format!(
	"expected signature of `{}`",
	build_fn_sig_string(self.tcx, found.skip_binder())
	);
	err.span_label(span, found_str);
	let found_span = found_span.unwrap_or(span);
	let expected_str = format!(
	"found signature of `{}`",
	build_fn_sig_string(self.tcx, expected_ref.skip_binder())
	);
	err.span_label(found_span, expected_str);
	err
	}
	}

Sadly at that point you don't have access to the closure expression, but you get it using self.tcx.hir().get_if_local(found_did.unwrap()) here

rust/src/librustc/traits/error_reporting.rs

Lines 878 to 882 in 22bc9e1

	let found_did = match found_trait_ty.kind {
	ty::Closure(did, _) | ty::Foreign(did) | ty::FnDef(did, _) => Some(did),
	ty::Adt(def, _) => Some(def.did),
	_ => None,
	};

With that then you could extract the appropriate span to suggest a code change using err.span_suggestion.

[workingjubilee]

I would be happy to work on implementing this, as it seems like a wonderful opportunity to get into the span code.

[estebank]

Feel free to reach out!

[Quantumplation]

@rustbot claim

It's been a while since @workingjubilee expressed interest, so I'm going to take a crack at this!

@estebank Trying to understand the function you pointed out, and the variable naming has me all twisted around. Are the variable names for found_str and expected_str swapped, or is there some subtlety I'm missing?

[estebank]

@Quantumplation I believe they are just incorrectly swapped.

[workingjubilee]

Ah, I didn't realize I should have formally claimed this mechanically.

I had been delayed a bit, but I have been trying to get into things the past few nights.

[Quantumplation]

@workingjubilee ah! feel free to claim it from me then! Also happy to collaborate, if you want to find me on Discord!

[workingjubilee]

Shiver me timbers, it's piracy on the Rust Sea!!!

@rustbot claim

[JohnTitor]

Triage: I'm going to release assignment due to inactivity.
@workingjubilee If you're still interested in this, feel free to re-claim.
@rustbot release-assignment

[krupitskas]

Working on it.
@rustbot claim

[krupitskas]

Still can't find a free dedicated time to sit, read docs and finish it. Some hints can be found in closed pull requests.

@rustbot release-assignment

[Akida31]

@rustbot claim