ensure arguments are included in count mismatch span #75023

euclio · 2020-08-01T19:03:09Z

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:

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 | | ) {
   | |_^

rust-highfive · 2020-08-01T19:03:12Z

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

eddyb · 2020-08-16T17:28:37Z

r? @estebank

estebank · 2020-08-16T20:04:18Z

src/test/ui/not-enough-arguments.stderr

+note: function defined here
+  --> $DIR/not-enough-arguments.rs:9:1
+   |
+LL | / fn bar(


Can we instead point at the ident instead of the full fn head?

supplied --> $DIR/not-enough-arguments.rs:26:3 | LL | foo(1, 2, 3); | ^^^ - - - supplied 3 arguments | | | expected 4 arguments | note: function defined here --> $DIR/not-enough-arguments.rs:5:4 | LL | fn foo(a: isize, b: isize, c: isize, d:isize) { | ^^^ ^^^^^^^^^ ^^^^^^^^^ ^^^^^^^^^ ^^^^^^^^ 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:4 | LL | fn bar( | ^^^ LL | a: i32, | ^^^^^^^ LL | b: i32, | ^^^^^^^ LL | c: i32, | ^^^^^^^ LL | d: i32, | ^^^^^^^ LL | e: i32, | ^^^^^^^ LL | f: i32, | ^^^^^^^ error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0061`.

Personally, I prefer pointing at the whole head since the missing paren in the multi-line case looks off-balance.

It does a bit, but it makes it less busy (fewer lines going around) and works much better when used by VSCode (overlapping spans look terrible there).

bors · 2020-08-23T10:39:34Z

☔ The latest upstream changes (presumably #75465) made this pull request unmergeable. Please resolve the merge conflicts.

crlf0710 · 2020-09-12T02:54:31Z

There's merge conflicts now

euclio · 2020-09-12T15:40:21Z

Yes, I'll try to get back to this this weekend.

euclio · 2020-09-24T13:58:50Z

@rustbot modify labels: -S-waiting-on-author +S-waiting-on-review

estebank

Minor nitpick

src/test/ui/error-codes/E0061.stderr

compiler/rustc_typeck/src/check/fn_ctxt.rs

bors · 2020-10-14T05:06:44Z

☔ The latest upstream changes (presumably #77926) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

estebank · 2020-10-14T06:56:53Z

r=me fixing the merge conflict (sorry I didn't approve earlier!)

euclio · 2020-10-14T15:51:18Z

Rebased.

euclio · 2020-10-14T15:51:33Z

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

estebank · 2020-10-14T19:19:38Z

@bors r+

bors · 2020-10-14T19:19:40Z

📌 Commit 873cce7130eb1af94b4ca610b09ff929f2969121 has been approved by estebank

bors · 2020-10-14T19:19:41Z

🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened

bors · 2020-10-15T01:43:29Z

⌛ Testing commit 873cce7130eb1af94b4ca610b09ff929f2969121 with merge 9b9c79afb73c9b07aae3e711b5b479bd92466210...

bors · 2020-10-15T02:10:05Z

💔 Test failed - checks-actions

estebank · 2020-10-15T06:35:36Z

compiler/rustc_typeck/src/check/fn_ctxt/checks.rs

+                    let mut spans: MultiSpan = node
+                        .ident()
+                        .map(|ident| ident.span)
+                        .unwrap_or_else(|| tcx.hir().span(node.hir_id().unwrap()))


https://github.com/rust-lang-ci/rust/runs/1256722042#step:24:22154

error[E0599]: no method named `hir_id` found for enum `rustc_hir::Node<'_>` in the current scope --> compiler/rustc_typeck/src/check/fn_ctxt/checks.rs:180:64 | 180 | .unwrap_or_else(|| tcx.hir().span(node.hir_id().unwrap())) | ^^^^^^ method not found in `rustc_hir::Node<'_>`

#77739 removed hir_id(). Rebased and added it back.

estebank · 2020-10-15T16:00:46Z

@bors r+

bors · 2020-10-15T16:00:48Z

📌 Commit 14b2d16 has been approved by estebank

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 | | ) { | |_^ ```

Rollup of 14 pull requests Successful merges: - rust-lang#75023 (ensure arguments are included in count mismatch span) - rust-lang#75265 (Add `str::{Split,RSplit,SplitN,RSplitN,SplitTerminator,RSplitTerminator,SplitInclusive}::as_str` methods) - rust-lang#75675 (mangling: mangle impl params w/ v0 scheme) - rust-lang#76084 (Refactor io/buffered.rs into submodules) - rust-lang#76119 (Stabilize move_ref_pattern) - rust-lang#77493 (ICEs should always print the top of the query stack) - rust-lang#77619 (Use futex-based thread-parker for Wasm32.) - rust-lang#77646 (For backtrace, use StaticMutex instead of a raw sys Mutex.) - rust-lang#77648 (Static mutex is static) - rust-lang#77657 (Cleanup cloudabi mutexes and condvars) - rust-lang#77672 (Simplify doc-cfg rendering based on the current context) - rust-lang#77780 (rustc_parse: fix spans on cast and range exprs with attrs) - rust-lang#77935 (BTreeMap: make PartialCmp/PartialEq explicit and tested) - rust-lang#77980 (Fix intra doc link for needs_drop) Failed merges: r? `@ghost`

rust-highfive assigned eddyb Aug 1, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 1, 2020

rust-highfive assigned estebank and unassigned eddyb Aug 16, 2020

estebank reviewed Aug 16, 2020

View reviewed changes

crlf0710 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 12, 2020

euclio force-pushed the argument-span branch from 173f5b5 to 6bbc711 Compare September 24, 2020 02:44

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 24, 2020

estebank reviewed Sep 24, 2020

View reviewed changes

src/test/ui/error-codes/E0061.stderr Outdated Show resolved Hide resolved

compiler/rustc_typeck/src/check/fn_ctxt.rs Outdated Show resolved Hide resolved

euclio force-pushed the argument-span branch from 6bbc711 to 6b38eaa Compare September 26, 2020 17:21

euclio mentioned this pull request Sep 26, 2020

fix off-by-one in parameter spans #77237

Closed

euclio force-pushed the argument-span branch from 6b38eaa to 286b88f Compare September 26, 2020 17:56

euclio force-pushed the argument-span branch from 286b88f to 873cce7 Compare October 14, 2020 15:03

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 14, 2020

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 15, 2020

estebank reviewed Oct 15, 2020

View reviewed changes

euclio added 2 commits October 15, 2020 09:49

fix off-by-one in parameter spans

95daa06

ensure arguments are included in count mismatch span

14b2d16

euclio force-pushed the argument-span branch from 873cce7 to 14b2d16 Compare October 15, 2020 14:23

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 15, 2020

This was referenced Oct 15, 2020

Rollup of 13 pull requests #78000

Closed

Rollup of 14 pull requests #78001

Merged

bors merged commit 075f2bf into rust-lang:master Oct 16, 2020

rustbot added this to the 1.49.0 milestone Oct 16, 2020

euclio deleted the argument-span branch October 16, 2020 15:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ensure arguments are included in count mismatch span #75023

ensure arguments are included in count mismatch span #75023

euclio commented Aug 1, 2020

rust-highfive commented Aug 1, 2020

eddyb commented Aug 16, 2020

estebank Aug 16, 2020

euclio Aug 20, 2020

estebank Aug 23, 2020

bors commented Aug 23, 2020

crlf0710 commented Sep 12, 2020

euclio commented Sep 12, 2020

euclio commented Sep 24, 2020

estebank left a comment

bors commented Oct 14, 2020

estebank commented Oct 14, 2020

euclio commented Oct 14, 2020

euclio commented Oct 14, 2020

estebank commented Oct 14, 2020

bors commented Oct 14, 2020

bors commented Oct 14, 2020

bors commented Oct 15, 2020

bors commented Oct 15, 2020

estebank Oct 15, 2020 •

edited

Loading

euclio Oct 15, 2020

estebank commented Oct 15, 2020

bors commented Oct 15, 2020

ensure arguments are included in count mismatch span #75023

ensure arguments are included in count mismatch span #75023

Conversation

euclio commented Aug 1, 2020

rust-highfive commented Aug 1, 2020

eddyb commented Aug 16, 2020

estebank Aug 16, 2020

Choose a reason for hiding this comment

euclio Aug 20, 2020

Choose a reason for hiding this comment

estebank Aug 23, 2020

Choose a reason for hiding this comment

bors commented Aug 23, 2020

crlf0710 commented Sep 12, 2020

euclio commented Sep 12, 2020

euclio commented Sep 24, 2020

estebank left a comment

Choose a reason for hiding this comment

bors commented Oct 14, 2020

estebank commented Oct 14, 2020

euclio commented Oct 14, 2020

euclio commented Oct 14, 2020

estebank commented Oct 14, 2020

bors commented Oct 14, 2020

bors commented Oct 14, 2020

bors commented Oct 15, 2020

bors commented Oct 15, 2020

estebank Oct 15, 2020 • edited Loading

Choose a reason for hiding this comment

euclio Oct 15, 2020

Choose a reason for hiding this comment

estebank commented Oct 15, 2020

bors commented Oct 15, 2020

estebank Oct 15, 2020 •

edited

Loading