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

Suggest solutions for fn foo(&foo: Foo) #38605

Merged
merged 5 commits into from
Jan 12, 2017
Merged
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
31 changes: 29 additions & 2 deletions src/librustc_typeck/check/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,17 @@ use syntax_pos::Span;

impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
pub fn check_pat(&self, pat: &'gcx hir::Pat, expected: Ty<'tcx>) {
self.check_pat_arg(pat, expected, false);
}

/// The `is_arg` argument indicates whether this pattern is the
/// *outermost* pattern in an argument (e.g., in `fn foo(&x:
/// &u32)`, it is true for the `&x` pattern but not `x`). This is
/// used to tailor error reporting.
pub fn check_pat_arg(&self, pat: &'gcx hir::Pat, expected: Ty<'tcx>, is_arg: bool) {
let tcx = self.tcx;

debug!("check_pat(pat={:?},expected={:?})", pat, expected);
debug!("check_pat(pat={:?},expected={:?},is_arg={})", pat, expected, is_arg);

let ty = match pat.node {
PatKind::Wild => {
Expand Down Expand Up @@ -202,6 +210,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
// can, to avoid creating needless variables. This
// also helps with the bad interactions of the given
// hack detailed in (*) below.
debug!("check_pat_arg: expected={:?}", expected);
let (rptr_ty, inner_ty) = match expected.sty {
ty::TyRef(_, mt) if mt.mutbl == mutbl => {
(expected, mt.ty)
Expand All @@ -212,7 +221,25 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
let mt = ty::TypeAndMut { ty: inner_ty, mutbl: mutbl };
let region = self.next_region_var(infer::PatternRegion(pat.span));
let rptr_ty = tcx.mk_ref(region, mt);
self.demand_eqtype(pat.span, expected, rptr_ty);
debug!("check_pat_arg: demanding {:?} = {:?}", expected, rptr_ty);
let err = self.demand_eqtype_diag(pat.span, expected, rptr_ty);

// Look for a case like `fn foo(&foo: u32)` and suggest
// `fn foo(foo: &u32)`
if let Some(mut err) = err {
if is_arg {
if let PatKind::Binding(..) = inner.node {
if let Ok(snippet) = self.sess().codemap()
.span_to_snippet(pat.span)
{
err.help(&format!("did you mean `{}: &{}`?",
&snippet[1..],
expected));
}
}
}
err.emit();
}
(rptr_ty, inner_ty)
}
};
Expand Down
18 changes: 14 additions & 4 deletions src/librustc_typeck/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use syntax_pos::{self, Span};
use rustc::hir;
use rustc::hir::def::Def;
use rustc::ty::{self, AssociatedItem};
use errors::DiagnosticBuilder;

use super::method::probe;

Expand All @@ -38,20 +39,29 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
}

pub fn demand_eqtype(&self, sp: Span, expected: Ty<'tcx>, actual: Ty<'tcx>) {
self.demand_eqtype_with_origin(&self.misc(sp), expected, actual);
if let Some(mut err) = self.demand_eqtype_diag(sp, expected, actual) {
err.emit();
}
}

pub fn demand_eqtype_diag(&self,
sp: Span,
expected: Ty<'tcx>,
actual: Ty<'tcx>) -> Option<DiagnosticBuilder<'tcx>> {
self.demand_eqtype_with_origin(&self.misc(sp), expected, actual)
}

pub fn demand_eqtype_with_origin(&self,
cause: &ObligationCause<'tcx>,
expected: Ty<'tcx>,
actual: Ty<'tcx>)
{
actual: Ty<'tcx>) -> Option<DiagnosticBuilder<'tcx>> {
match self.eq_types(false, cause, actual, expected) {
Ok(InferOk { obligations, value: () }) => {
self.register_predicates(obligations);
None
},
Err(e) => {
self.report_mismatched_types(cause, expected, actual, e).emit();
Some(self.report_mismatched_types(cause, expected, actual, e))
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ fn check_fn<'a, 'gcx, 'tcx>(inherited: &'a Inherited<'a, 'gcx, 'tcx>,
fcx.register_old_wf_obligation(arg_ty, arg.pat.span, traits::MiscObligation);

// Check the pattern.
fcx.check_pat(&arg.pat, arg_ty);
fcx.check_pat_arg(&arg.pat, arg_ty, true);
fcx.write_ty(arg.id, arg_ty);
}

Expand Down
4 changes: 3 additions & 1 deletion src/librustc_typeck/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,9 @@ impl<'ccx, 'gcx> CheckTypeWellFormedVisitor<'ccx, 'gcx> {
debug!("check_method_receiver: receiver ty = {:?}", rcvr_ty);

let cause = fcx.cause(span, ObligationCauseCode::MethodReceiver);
fcx.demand_eqtype_with_origin(&cause, rcvr_ty, self_arg_ty);
if let Some(mut err) = fcx.demand_eqtype_with_origin(&cause, rcvr_ty, self_arg_ty) {
err.emit();
}
}

fn check_variances_for_type_defn(&self,
Expand Down
37 changes: 37 additions & 0 deletions src/test/ui/mismatched_types/issue-38371.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![feature(slice_patterns)]


struct Foo {
}

fn foo(&foo: Foo) {
}

fn bar(foo: Foo) {
}

fn qux(foo: &Foo) {
}

fn zar(&foo: &Foo) {
}

fn agh(&&bar: &u32) {
}

fn bgh(&&bar: u32) {
}

fn ugh(&[bar]: &u32) {
}

fn main() {}
36 changes: 36 additions & 0 deletions src/test/ui/mismatched_types/issue-38371.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
error[E0308]: mismatched types
--> $DIR/issue-38371.rs:16:8
|
16 | fn foo(&foo: Foo) {
| ^^^^ expected struct `Foo`, found reference
|
= note: expected type `Foo`
= note: found type `&_`
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we have to fix this here necessarily, but I feel like these note: messages are kind of distracting from the help, which is more likely to be of use. But I'm not sure how to fix without doing potentially more harm than good, so let's leave as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For a while I've been thinking wether it would make sense to make the expected/found notes a first class comment, so that these errors would look closer to

error[E0308]: mismatched types
  --> ../../src/test/ui/mismatched_types/issue-38371.rs:14:8
   |
14 | fn foo(&foo: Foo) {
   |        ^^^^ expected struct `Foo`, found reference
   |
   = expected type `Foo`
   =    found type `&_`
   = help: did you mean `foo: &Foo`?

error[E0308]: mismatched types
  --> ../../src/test/ui/mismatched_types/issue-38371.rs:26:9
   |
26 | fn agh(&&bar: &u32) {
   |         ^^^^ expected u32, found reference
   |
   = expected type `u32`
   =    found type `&_`

If you feel there'd be value on that, I could create an issue and start a PR for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that looks better

Copy link
Contributor Author

@estebank estebank Jan 7, 2017

Choose a reason for hiding this comment

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

Opened issue #38901 and PR #38902 for this.

= help: did you mean `foo: &Foo`?

error[E0308]: mismatched types
--> $DIR/issue-38371.rs:28:9
|
28 | fn agh(&&bar: &u32) {
| ^^^^ expected u32, found reference
|
= note: expected type `u32`
= note: found type `&_`

error[E0308]: mismatched types
--> $DIR/issue-38371.rs:31:8
|
31 | fn bgh(&&bar: u32) {
| ^^^^^ expected u32, found reference
|
= note: expected type `u32`
= note: found type `&_`

error[E0529]: expected an array or slice, found `u32`
--> $DIR/issue-38371.rs:34:9
|
34 | fn ugh(&[bar]: &u32) {
| ^^^^^ pattern cannot match with input type `u32`

error: aborting due to 4 previous errors