Skip to content

Commit

Permalink
Rollup merge of rust-lang#75023 - euclio:argument-span, r=estebank
Browse files Browse the repository at this point in the history
ensure arguments are included in count mismatch span

The current diagnostic isn't very helpful if the function header spans multiple lines. Lines comprising the function signature may be elided to keep the diagnostic short, but these lines are essential to fixing the error. This is made worse when the function has a body, because the last two lines of the span are then dedicated to showing the end of the body, which is irrelevant.

This PR changes the span to be a multispan made up of the header and the the arguments, ensuring they won't be elided. It also discards the function body from the span.

[Old](https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=f92d9f81a8c9416f0f04e4e09923b6d4):

```
error[E0061]: this function takes 6 arguments but 1 argument was supplied
  --> src/main.rs:18:5
   |
1  | / fn bar(
2  | |     a: i32,
3  | |     b: i32,
4  | |     c: i32,
...  |
14 | |     println!("{}", f);
15 | | }
   | |_- defined here
...
18 |       bar(1);
   |       ^^^ - supplied 1 argument
   |       |
   |       expected 6 arguments
```

New:

```
error[E0061]: this function takes 6 arguments but 1 argument was supplied
  --> $DIR/not-enough-arguments.rs:28:3
   |
LL |   bar(1);
   |   ^^^ - supplied 1 argument
   |   |
   |   expected 6 arguments
   |
note: function defined here
  --> $DIR/not-enough-arguments.rs:9:1
   |
LL | / fn bar(
LL | |     a: i32,
   | |     ^^^^^^^
LL | |     b: i32,
   | |     ^^^^^^^
LL | |     c: i32,
   | |     ^^^^^^^
LL | |     d: i32,
   | |     ^^^^^^^
LL | |     e: i32,
   | |     ^^^^^^^
LL | |     f: i32,
   | |     ^^^^^^^
LL | | ) {
   | |_^
```
  • Loading branch information
Dylan-DPC committed Oct 15, 2020
2 parents 73585b8 + 14b2d16 commit 5e3b4fa
Show file tree
Hide file tree
Showing 21 changed files with 276 additions and 125 deletions.
28 changes: 28 additions & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2742,4 +2742,32 @@ impl<'hir> Node<'hir> {
_ => None,
}
}

pub fn hir_id(&self) -> Option<HirId> {
match self {
Node::Item(Item { hir_id, .. })
| Node::ForeignItem(ForeignItem { hir_id, .. })
| Node::TraitItem(TraitItem { hir_id, .. })
| Node::ImplItem(ImplItem { hir_id, .. })
| Node::Field(StructField { hir_id, .. })
| Node::AnonConst(AnonConst { hir_id, .. })
| Node::Expr(Expr { hir_id, .. })
| Node::Stmt(Stmt { hir_id, .. })
| Node::Ty(Ty { hir_id, .. })
| Node::Binding(Pat { hir_id, .. })
| Node::Pat(Pat { hir_id, .. })
| Node::Arm(Arm { hir_id, .. })
| Node::Block(Block { hir_id, .. })
| Node::Local(Local { hir_id, .. })
| Node::MacroDef(MacroDef { hir_id, .. })
| Node::Lifetime(Lifetime { hir_id, .. })
| Node::Param(Param { hir_id, .. })
| Node::GenericParam(GenericParam { hir_id, .. }) => Some(*hir_id),
Node::TraitRef(TraitRef { hir_ref_id, .. }) => Some(*hir_ref_id),
Node::PathSegment(PathSegment { hir_id, .. }) => *hir_id,
Node::Variant(Variant { id, .. }) => Some(*id),
Node::Ctor(variant) => variant.ctor_hir_id(),
Node::Crate(_) | Node::Visibility(_) => None,
}
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1744,7 +1744,7 @@ impl<'a> Parser<'a> {
}
};

let span = lo.to(self.token.span);
let span = lo.until(self.token.span);

Ok(Param {
attrs: attrs.into(),
Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_typeck/src/check/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,10 +285,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
arg_exprs: &'tcx [hir::Expr<'tcx>],
expected: Expectation<'tcx>,
) -> Ty<'tcx> {
let (fn_sig, def_span) = match *callee_ty.kind() {
ty::FnDef(def_id, _) => {
(callee_ty.fn_sig(self.tcx), self.tcx.hir().span_if_local(def_id))
}
let (fn_sig, def_id) = match *callee_ty.kind() {
ty::FnDef(def_id, _) => (callee_ty.fn_sig(self.tcx), Some(def_id)),
ty::FnPtr(sig) => (sig, None),
ref t => {
let mut unit_variant = None;
Expand Down Expand Up @@ -427,7 +425,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
arg_exprs,
fn_sig.c_variadic,
TupleArgumentsFlag::DontTupleArguments,
def_span,
def_id,
);

fn_sig.output()
Expand Down
27 changes: 22 additions & 5 deletions compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use rustc_middle::ty::fold::TypeFoldable;
use rustc_middle::ty::{self, Ty};
use rustc_session::Session;
use rustc_span::symbol::{sym, Ident};
use rustc_span::{self, Span};
use rustc_span::{self, MultiSpan, Span};
use rustc_trait_selection::traits::{self, ObligationCauseCode};

use std::mem::replace;
Expand Down Expand Up @@ -83,7 +83,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
args_no_rcvr,
method.sig.c_variadic,
tuple_arguments,
self.tcx.hir().span_if_local(method.def_id),
Some(method.def_id),
);
method.sig.output()
}
Expand All @@ -99,7 +99,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
args: &'tcx [hir::Expr<'tcx>],
c_variadic: bool,
tuple_arguments: TupleArgumentsFlag,
def_span: Option<Span>,
def_id: Option<DefId>,
) {
let tcx = self.tcx;
// Grab the argument types, supplying fresh type variables
Expand Down Expand Up @@ -172,9 +172,26 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);
}

if let Some(def_s) = def_span.map(|sp| tcx.sess.source_map().guess_head_span(sp)) {
err.span_label(def_s, "defined here");
if let Some(def_id) = def_id {
if let Some(node) = tcx.hir().get_if_local(def_id) {
let mut spans: MultiSpan = node
.ident()
.map(|ident| ident.span)
.unwrap_or_else(|| tcx.hir().span(node.hir_id().unwrap()))
.into();

if let Some(id) = node.body_id() {
let body = tcx.hir().body(id);
for param in body.params {
spans.push_span_label(param.span, String::new());
}
}

let def_kind = tcx.def_kind(def_id);
err.span_note(spans, &format!("{} defined here", def_kind.descr(def_id)));
}
}

if sugg_unit {
let sugg_span = tcx.sess.source_map().end_point(expr.span);
// remove closing `)` from the span
Expand Down
9 changes: 6 additions & 3 deletions src/test/ui/arg-count-mismatch.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
error[E0061]: this function takes 1 argument but 0 arguments were supplied
--> $DIR/arg-count-mismatch.rs:5:28
|
LL | fn f(x: isize) { }
| -------------- defined here
LL |
LL | fn main() { let i: (); i = f(); }
| ^-- supplied 0 arguments
| |
| expected 1 argument
|
note: function defined here
--> $DIR/arg-count-mismatch.rs:3:4
|
LL | fn f(x: isize) { }
| ^ --------

error: aborting due to previous error

Expand Down
18 changes: 12 additions & 6 deletions src/test/ui/c-variadic/variadic-ffi-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,30 @@ LL | fn printf(_: *const u8, ...);
error[E0060]: this function takes at least 2 arguments but 0 arguments were supplied
--> $DIR/variadic-ffi-1.rs:17:9
|
LL | fn foo(f: isize, x: u8, ...);
| ----------------------------- defined here
...
LL | foo();
| ^^^-- supplied 0 arguments
| |
| expected at least 2 arguments
|
note: function defined here
--> $DIR/variadic-ffi-1.rs:10:8
|
LL | fn foo(f: isize, x: u8, ...);
| ^^^

error[E0060]: this function takes at least 2 arguments but 1 argument was supplied
--> $DIR/variadic-ffi-1.rs:18:9
|
LL | fn foo(f: isize, x: u8, ...);
| ----------------------------- defined here
...
LL | foo(1);
| ^^^ - supplied 1 argument
| |
| expected at least 2 arguments
|
note: function defined here
--> $DIR/variadic-ffi-1.rs:10:8
|
LL | fn foo(f: isize, x: u8, ...);
| ^^^

error[E0308]: mismatched types
--> $DIR/variadic-ffi-1.rs:20:56
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/c-variadic/variadic-ffi-no-fixed-args.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error: C-variadic function must be declared with at least one named argument
--> $DIR/variadic-ffi-no-fixed-args.rs:2:12
|
LL | fn foo(...);
| ^^^^
| ^^^

error: aborting due to previous error

9 changes: 6 additions & 3 deletions src/test/ui/error-codes/E0060.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
error[E0060]: this function takes at least 1 argument but 0 arguments were supplied
--> $DIR/E0060.rs:6:14
|
LL | fn printf(_: *const u8, ...) -> u32;
| ------------------------------------ defined here
...
LL | unsafe { printf(); }
| ^^^^^^-- supplied 0 arguments
| |
| expected at least 1 argument
|
note: function defined here
--> $DIR/E0060.rs:2:8
|
LL | fn printf(_: *const u8, ...) -> u32;
| ^^^^^^

error: aborting due to previous error

Expand Down
18 changes: 12 additions & 6 deletions src/test/ui/error-codes/E0061.stderr
Original file line number Diff line number Diff line change
@@ -1,24 +1,30 @@
error[E0061]: this function takes 2 arguments but 1 argument was supplied
--> $DIR/E0061.rs:6:5
|
LL | fn f(a: u16, b: &str) {}
| --------------------- defined here
...
LL | f(0);
| ^ - supplied 1 argument
| |
| expected 2 arguments
|
note: function defined here
--> $DIR/E0061.rs:1:4
|
LL | fn f(a: u16, b: &str) {}
| ^ ------ -------

error[E0061]: this function takes 1 argument but 0 arguments were supplied
--> $DIR/E0061.rs:10:5
|
LL | fn f2(a: u16) {}
| ------------- defined here
...
LL | f2();
| ^^-- supplied 0 arguments
| |
| expected 1 argument
|
note: function defined here
--> $DIR/E0061.rs:3:4
|
LL | fn f2(a: u16) {}
| ^^ ------

error: aborting due to 2 previous errors

Expand Down
20 changes: 10 additions & 10 deletions src/test/ui/hrtb/issue-58451.stderr
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
error[E0061]: this function takes 1 argument but 0 arguments were supplied
--> $DIR/issue-58451.rs:12:9
|
LL | / fn f<I>(i: I)
LL | | where
LL | | I: IntoIterator,
LL | | I::Item: for<'a> Into<&'a ()>,
| |__________________________________- defined here
...
LL | f(&[f()]);
| ^-- supplied 0 arguments
| |
| expected 1 argument
LL | f(&[f()]);
| ^-- supplied 0 arguments
| |
| expected 1 argument
|
note: function defined here
--> $DIR/issue-58451.rs:5:4
|
LL | fn f<I>(i: I)
| ^ ----

error: aborting due to previous error

Expand Down
9 changes: 6 additions & 3 deletions src/test/ui/issues/issue-18819.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
error[E0061]: this function takes 2 arguments but 1 argument was supplied
--> $DIR/issue-18819.rs:16:5
|
LL | fn print_x(_: &dyn Foo<Item=bool>, extra: &str) {
| ----------------------------------------------- defined here
...
LL | print_x(X);
| ^^^^^^^ - supplied 1 argument
| |
| expected 2 arguments
|
note: function defined here
--> $DIR/issue-18819.rs:11:4
|
LL | fn print_x(_: &dyn Foo<Item=bool>, extra: &str) {
| ^^^^^^^ ---------------------- -----------

error: aborting due to previous error

Expand Down
9 changes: 6 additions & 3 deletions src/test/ui/issues/issue-26094.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@ error[E0061]: this function takes 0 arguments but 1 argument was supplied
LL | $other(None)
| ---- supplied 1 argument
...
LL | fn some_function() {}
| ------------------ defined here
...
LL | some_macro!(some_function);
| ^^^^^^^^^^^^^ expected 0 arguments
|
note: function defined here
--> $DIR/issue-26094.rs:7:4
|
LL | fn some_function() {}
| ^^^^^^^^^^^^^

error: aborting due to previous error

Expand Down
9 changes: 6 additions & 3 deletions src/test/ui/issues/issue-4935.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
error[E0061]: this function takes 1 argument but 2 arguments were supplied
--> $DIR/issue-4935.rs:5:13
|
LL | fn foo(a: usize) {}
| ---------------- defined here
LL |
LL | fn main() { foo(5, 6) }
| ^^^ - - supplied 2 arguments
| |
| expected 1 argument
|
note: function defined here
--> $DIR/issue-4935.rs:3:4
|
LL | fn foo(a: usize) {}
| ^^^ --------

error: aborting due to previous error

Expand Down
Loading

0 comments on commit 5e3b4fa

Please sign in to comment.