Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Use ordinal number in argument error #125042

Merged
merged 1 commit into from
Jul 18, 2024
Merged

Use ordinal number in argument error #125042

merged 1 commit into from
Jul 18, 2024

Conversation

long-long-float
Copy link
Contributor

@long-long-float long-long-float commented May 12, 2024 •
edited
Loading

Add an ordinal number to two argument errors ("unexpected" and "missing") for ease of understanding error.

error[E0061]: this function takes 3 arguments but 2 arguments were supplied
  --> test.rs:11:5
   |
11 |     f(42, 'a');
   |     ^     --- 2nd argument of type `f32` is missing
   |
(snip)

error[E0061]: this function takes 3 arguments but 4 arguments were supplied
  --> test.rs:12:5
   |
12 |     f(42, 42, 1.0, 'a');
   |     ^   ----
   |         | |
   |         | unexpected 2nd argument of type `{integer}`
   |         help: remove the extra argument

To get an ordinal number, I copied ordinalize from other crate rustc_resolve because I think it is too much to link rustc_resolve for this small function. Please let me know if there is a better way.

@rustbot
Copy link
Collaborator

rustbot commented May 12, 2024

r? @wesleywiser

rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 12, 2024
@Noratrieb Noratrieb mentioned this pull request May 14, 2024
Merged
@rust-log-analyzer

This comment has been minimized.

# to view
@nnethercote
Copy link
Contributor

I think using #n instead of the ordinal number is better, as in #125105 :)

@long-long-float
Copy link
Contributor Author

long-long-float commented May 22, 2024 •
edited
Loading

I think using #n instead of the ordinal number is better, as in #125105 :)

Yes, I will fix to use #n format because I don't have a strong reason to use ordinalize.

@rust-log-analyzer

This comment has been minimized.

# to view
@long-long-float long-long-float force-pushed the suggest-move-arg-outside branch 2 times, most recently from 2d889a9 to c71324b Compare May 26, 2024 02:57
@apiraino
Copy link
Contributor

r? compiler

@rustbot rustbot assigned fmease and unassigned wesleywiser Jun 26, 2024
fmease
fmease requested changes Jul 2, 2024
View reviewed changes
Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

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

One nit (swapping some words), then this is ready to go.

@rustbot rustbot 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 Jul 2, 2024
@long-long-float long-long-float force-pushed the suggest-move-arg-outside branch from c71324b to 5743c3f Compare July 13, 2024 04:36
@fmease fmease 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 Jul 13, 2024
@long-long-float
Use ordinal number in argument error
332b41d 
Fix error message

Fix tests

Format
@long-long-float long-long-float force-pushed the suggest-move-arg-outside branch from 5743c3f to 332b41d Compare July 14, 2024 04:59
@long-long-float
Copy link
Contributor Author

@fmease I fixed the messages. Thank you!

@rustbot ready

@fmease
Copy link
Member

fmease commented Jul 17, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 17, 2024

📌 Commit 332b41d has been approved by fmease

It is now in the queue for this repository.

@bors 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 Jul 17, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 17, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 17, 2024
@bors
Auto merge of rust-lang#127865 - matthiaskrgr:rollup-8m49dlg, r=matth…
c9517f3 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125042 (Use ordinal number in argument error)
 - rust-lang#127229 (rustdoc: click target for sidebar items flush left)
 - rust-lang#127337 (Move a few intrinsics to Rust abi)
 - rust-lang#127472 (MIR building: Stop using `unpack!` for `BlockAnd<()>`)
 - rust-lang#127579 (Solve a error `.clone()` suggestion when moving a mutable reference)
 - rust-lang#127769 (Don't use implicit features in `Cargo.toml` in `compiler/`)
 - rust-lang#127844 (Remove invalid further restricting suggestion for type bound)
 - rust-lang#127855 (Add myself to review rotation)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 17, 2024
@bors
Auto merge of rust-lang#127865 - matthiaskrgr:rollup-8m49dlg, r=matth…
798d1eb 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125042 (Use ordinal number in argument error)
 - rust-lang#127229 (rustdoc: click target for sidebar items flush left)
 - rust-lang#127337 (Move a few intrinsics to Rust abi)
 - rust-lang#127472 (MIR building: Stop using `unpack!` for `BlockAnd<()>`)
 - rust-lang#127579 (Solve a error `.clone()` suggestion when moving a mutable reference)
 - rust-lang#127769 (Don't use implicit features in `Cargo.toml` in `compiler/`)
 - rust-lang#127844 (Remove invalid further restricting suggestion for type bound)
 - rust-lang#127855 (Add myself to review rotation)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 17, 2024
@bors
Auto merge of rust-lang#127865 - matthiaskrgr:rollup-8m49dlg, r=matth…
e35364a 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125042 (Use ordinal number in argument error)
 - rust-lang#127229 (rustdoc: click target for sidebar items flush left)
 - rust-lang#127337 (Move a few intrinsics to Rust abi)
 - rust-lang#127472 (MIR building: Stop using `unpack!` for `BlockAnd<()>`)
 - rust-lang#127579 (Solve a error `.clone()` suggestion when moving a mutable reference)
 - rust-lang#127769 (Don't use implicit features in `Cargo.toml` in `compiler/`)
 - rust-lang#127844 (Remove invalid further restricting suggestion for type bound)
 - rust-lang#127855 (Add myself to review rotation)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2b34490 into rust-lang:master Jul 18, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 18, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 18, 2024
@rust-timer
Unrolled build for rust-lang#125042
53ced6f 
Rollup merge of rust-lang#125042 - long-long-float:suggest-move-arg-outside, r=fmease

Use ordinal number in argument error

Add an ordinal number to two argument errors ("unexpected" and "missing") for ease of understanding error.

```
error[E0061]: this function takes 3 arguments but 2 arguments were supplied
  --> test.rs:11:5
   |
11 |     f(42, 'a');
   |     ^     --- 2nd argument of type `f32` is missing
   |
(snip)

error[E0061]: this function takes 3 arguments but 4 arguments were supplied
  --> test.rs:12:5
   |
12 |     f(42, 42, 1.0, 'a');
   |     ^   ----
   |         | |
   |         | unexpected 2nd argument of type `{integer}`
   |         help: remove the extra argument
```

To get an ordinal number, I copied `ordinalize` from other crate `rustc_resolve` because I think it is too much to link `rustc_resolve` for this small function. Please let me know if there is a better way.
@fmease fmease mentioned this pull request Nov 25, 2024
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@veera-sivarajan veera-sivarajan veera-sivarajan left review comments

@fmease fmease fmease approved these changes

Assignees

@fmease fmease

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.81.0
Development

Successfully merging this pull request may close these issues.
9 participants
@long-long-float @rustbot @rust-log-analyzer @nnethercote @apiraino @fmease @bors @veera-sivarajan @wesleywiser