Rework diagnostics for wrong number of generic args (fixes #66228 and #71924) by Patryk27 · Pull Request #77524 · rust-lang/rust

Patryk27 · 2020-10-04T11:27:11Z

This PR reworks the wrong number of {} arguments message, so that it provides more details and contextual hints.

rust-highfive · 2020-10-04T11:27:15Z

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

Patryk27 · 2020-10-08T20:57:42Z

Ping, ping, just remindin' - @matthewjasper :-)

bors · 2020-10-14T05:27:54Z

☔ 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

Patryk27 · 2020-10-17T15:34:51Z

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

Patryk27 · 2020-10-17T20:49:19Z

also, re-pinging @matthewjasper :-)

bors · 2020-10-20T05:48:50Z

☔ The latest upstream changes (presumably #78127) 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

Patryk27 · 2020-10-21T19:32:49Z

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

bors · 2020-10-26T23:39:41Z

☔ The latest upstream changes (presumably #77187) 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

Patryk27 · 2020-10-27T17:34:18Z

r? @estebank

Patryk27 · 2020-11-08T20:12:55Z

ping ping @estebank

bors · 2020-11-15T15:53:05Z

☔ The latest upstream changes (presumably #79070) 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

Patryk27 · 2020-11-16T19:33:13Z

ping ping @estebank, @jonas-schievink? (sorry, I'm not quite sure who to ping or how to proceed from here 😅)

jonas-schievink · 2020-11-16T20:01:53Z

The triage working group would pick this up if it doesn't get updated in a while, but every time you comment that time gets reset

estebank

Apologies for the delay in reviewing. I'll finish the review shortly, but in order to avoid keeping you waiting any further giving the length of the PR, I am pushing out some of the things I've found so far.

estebank · 2020-11-25T01:29:36Z

estebank · 2020-11-25T02:08:38Z

I'm wondering if making this
Suggested change
err.span_suggestion(span,&msg,String::new(),Applicability::MaybeIncorrect);
err.span_suggestion_verbose(span,&msg,String::new(),Applicability::MaybeIncorrect);
will look better.

Seems like span_suggestion_verbose() prints message with the suggestion already applied, yielding pearls such as:
@@ -19,7 +19,12 @@ LL | fn foo<const X: usize, const Y: usize>() -> usize { | ------------------------------------------------- defined here, with 2 const parameters: `X`, `Y` ... LL | foo::<0, 0, 0>(); - | ^^^ --- help: remove this const argument+ | ^^^+ |+help: remove this const argument+ |+LL | foo::<0, 0>();+ | --
or
@@ -25,7 +30,12 @@ LL | struct S; | --------- defined here, with 0 const parameters ... LL | S::<0>; - | ^ - help: remove this const argument+ | ^+ |+help: remove this const argument+ |+LL | S::<>;+ | --
IMHO the inline (current) version with s/consider replacing/replace is way more legible.

rust-log-analyzer · 2021-01-02T20:12:48Z

The job mingw-check failed! Check out the build log: (web)(plain)

Click to see the possible cause of the failure (guessed by this bot)

configure: rust.channel := nightly
configure: rust.debug-assertions := True
configure: llvm.assertions := True
configure: dist.missing-tools := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure: configure: run `python /checkout/x.py --help`
configure: ---
Checking which error codes lack tests...
Found 435 error codes
Found 0 error codes with no tests
Done!
tidy error: /checkout/src/test/ui/async-await/issues/issue-65159.rs:5: line longer than 100 chars
tidy error: /checkout/src/test/ui/traits/trait-test-2.rs: ignoring line length unnecessarily
tidy error: /checkout/src/test/ui/seq-args.rs:4: line longer than 100 chars
tidy error: /checkout/src/test/ui/seq-args.rs:7: line longer than 100 chars
tidy error: /checkout/src/test/ui/const-generics/incorrect-number-of-const-args.rs:11: line longer than 100 chars
tidy error: /checkout/src/test/ui/const-generics/incorrect-number-of-const-args.rs:12: line longer than 100 chars
tidy error: /checkout/src/test/ui/const-generics/invalid-const-arg-for-type-param.rs:6: line longer than 100 chars
tidy error: /checkout/src/test/ui/methods/method-call-lifetime-args-fail.rs:17: line longer than 100 chars
tidy error: /checkout/src/test/ui/methods/method-call-lifetime-args-fail.rs:19: line longer than 100 chars
tidy error: /checkout/src/test/ui/methods/method-call-lifetime-args-fail.rs:64: line longer than 100 chars
tidy error: /checkout/src/test/ui/methods/method-call-lifetime-args-fail.rs:66: line longer than 100 chars
tidy error: /checkout/src/test/ui/issues/issue-18423.rs:4: line longer than 100 chars
tidy error: /checkout/src/test/ui/issues/issue-53251.rs:12: line longer than 100 chars
tidy error: /checkout/src/test/ui/issues/issue-53251.rs:13: line longer than 100 chars
tidy error: /checkout/src/test/ui/structs/structure-constructor-type-mismatch.rs:48: line longer than 100 chars
tidy error: /checkout/src/test/ui/structs/structure-constructor-type-mismatch.rs:54: line longer than 100 chars
some tidy checks failed
command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "/checkout/obj/build"
expected success, got: exit code: 1

estebank · 2021-01-08T01:20:48Z

Ideally we would give the error for non-Fn trait using parenthetical notation higher priority and emit it instead of these two, but that should be done in a different PR.

estebank · 2021-01-08T01:22:12Z

Because of the (incorrect) use of the parenthetical notation here, the suggestion is incorrect. That being said it might be worth it to tackle it at another time.

estebank · 2021-01-08T01:28:04Z

I see here that we don't try to match and differentiate between type, lifetime and const params, but that is fine for this PR, I'd like to land it sooner rather than later. We'll have to take a look at refining that later.

Actually, the first version of this message did distinguish between type, lifetime and const parameters; it was changed some time ago on the upstream:
rust/compiler/rustc_typeck/src/astconv/generics.rs
Line 502 in 34628e5
"generic"
... and I just went with it.

estebank · 2021-01-08T01:29:55Z

For lifetimes we would ideally look for available lifetimes (like we do in other cases) and if none use 'static, but again, that should be follow up work. (I'm just marking TODO notes for later right now.)

Parallel idea: for lifetime & type generics in expression position (e.g. .collect::<Vec>()) we could suggest using placeholders (i.e. suggest Vec<_> instead of Vec<T>).

estebank · 2021-01-08T01:34:03Z

Ideally we would tackle these two with the same error, suggesting to remove the entirety of the arguments at once, but again, this can be done later, it is not a regression.

estebank · 2021-01-13T18:56:39Z

+error[E0107]: this trait takes 0 lifetime arguments but 1 lifetime argument was supplied
+ --> $DIR/typeck-builtin-bound-type-parameters.rs:13:15
 |
 LL | fn foo2<'a, T:Copy<'a, U>, U>(x: T) {}
- | ^^ unexpected lifetime argument
+ | ^^^^ ---- help: remove this lifetime argument
+ | |
+ | expected 0 lifetime arguments
+ |
+note: trait defined here, with 0 lifetime parameters
+ --> $SRC_DIR/core/src/marker.rs:LL:COL
+ |
+LL | pub trait Copy: Clone {
+ | ^^^^

-error[E0107]: wrong number of type arguments: expected 0, found 1
- --> $DIR/typeck-builtin-bound-type-parameters.rs:14:24
+error[E0107]: this trait takes 0 type arguments but 1 type argument was supplied
+ --> $DIR/typeck-builtin-bound-type-parameters.rs:13:15
 |
 LL | fn foo2<'a, T:Copy<'a, U>, U>(x: T) {}
- | ^ unexpected type argument
+ | ^^^^ --- help: remove this type argument
+ | |
+ | expected 0 type arguments
+ |
+note: trait defined here, with 0 type parameters
+ --> $SRC_DIR/core/src/marker.rs:LL:COL
+ |
+LL | pub trait Copy: Clone {
+ | ^^^^


This case is somewhat unfortunate (because we would need to look at the whole thing and suggest removing both lifetimes and type params in one go, but we don't need to tackle it now.

I was thinking about a dedicated message for when generics were provided where none were expected, something in the lines of:
error[E0107]: this trait is not generic --> $DIR/typeck-builtin-bound-type-parameters.rs:13:15 | LL | fn foo2<'a, T:Copy<'a, U>, U>(x: T) {} | ^^^^------- help: remove these generics | note: trait defined here, with no generics --> $SRC_DIR/core/src/marker.rs:LL:COL | LL | pub trait Copy: Clone { | ^^^^
I'll try tackling it in another PR, after this one gets merged :-)

estebank · 2021-01-13T18:58:01Z

- |
-note: tuple variant defined here
- --> $DIR/enum-variant-priority-higher-than-other-inherent.rs:5:5
- |
-LL | V(u8)
- | ^^^^^


Somewhat surprised by this change 👀

Yes, it's a bit unfortunate side effect of looking up identifiers via a new query - def_ident_span - that I haven't been able to re-implement for tuple variants. Technically reviving this one should be as easy as improving that query.

estebank · 2021-01-13T19:01:31Z

@bors r+ rollup=never

bors · 2021-01-13T19:01:34Z

📌 Commit d2f8e39 has been approved by estebank

bors · 2021-01-13T20:36:05Z

⌛ Testing commit d2f8e39 with merge a62a760...

bors · 2021-01-13T23:24:24Z

☀️ Test successful - checks-actions
Approved by: estebank
Pushing a62a760 to master...

Rework diagnostics for wrong number of generic args (fixesrust-lang#66228 and rust-lang#71924) This PR reworks the `wrong number of {} arguments` message, so that it provides more details and contextual hints.

rust-highfive assigned matthewjasper Oct 4, 2020

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

Patryk27 force-pushed the fixes/66228 branch 6 times, most recently from 17ca2ab to ea0a68dCompare October 4, 2020 17:40

Patryk27 force-pushed the fixes/66228 branch from ea0a68d to 124d0f2Compare October 17, 2020 15:34

Patryk27 force-pushed the fixes/66228 branch from 124d0f2 to 3ca04daCompare October 21, 2020 19:31

Patryk27 force-pushed the fixes/66228 branch from 3ca04da to ec2b6eaCompare October 27, 2020 17:33

rust-highfive assigned estebank and unassigned matthewjasper Oct 27, 2020

Patryk27 force-pushed the fixes/66228 branch from ec2b6ea to 1028d4fCompare November 16, 2020 19:30

estebank reviewed Nov 25, 2020
View reviewed changes

Comment threadsrc/test/ui/const-generics/invalid-enum.stderr Outdated

estebank reviewed Nov 25, 2020
View reviewed changes

Comment threadsrc/test/ui/const-generics/invalid-enum.stderr Outdated

Patryk27 force-pushed the fixes/66228 branch from ac1853d to 509cffdCompare January 2, 2021 20:40

estebank reviewed Jan 8, 2021
View reviewed changes

Comment threadcompiler/rustc_hir/src/hir.rs Outdated

estebank reviewed Jan 8, 2021
View reviewed changes

Comment threadsrc/test/ui/generics/generic-arg-mismatch-recover.stderr Outdated

estebank reviewed Jan 8, 2021
View reviewed changes

Rework diagnostics for wrong number of generic args
d2f8e39

Patryk27 force-pushed the fixes/66228 branch from 509cffd to d2f8e39Compare January 10, 2021 12:08

estebank reviewed Jan 13, 2021
View reviewed changes

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 Jan 13, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 13, 2021

bors merged commit a62a760 into rust-lang:masterJan 13, 2021

rustbot added this to the 1.51.0 milestone Jan 13, 2021

This was referenced Jan 13, 2021
Better method call error messages #71827
Closed
Refactor Binder to track bound vars #76814
Merged
Generic associated types in trait paths #79554
Merged

Patryk27 deleted the fixes/66228 branch January 14, 2021 06:01

jackh726 mentioned this pull request Feb 5, 2021
Bad diagnostics for GATs in trait paths #81801
Closed

lcnr added the A-const-generics Area: const generics (parameters and arguments) label Aug 26, 2021

lcnr mentioned this pull request Dec 11, 2021
Tracking issue for const generics (RFC 2000) #44580
Closed
16 tasks

	err.span_suggestion(span,&msg,String::new(),Applicability::MaybeIncorrect);
	err.span_suggestion_verbose(span,&msg,String::new(),Applicability::MaybeIncorrect);

Uh oh!

Uh oh!

Conversation

Patryk27 commented Oct 4, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Oct 4, 2020

Uh oh!

Patryk27 commented Oct 8, 2020

Uh oh!

bors commented Oct 14, 2020

Uh oh!

Patryk27 commented Oct 17, 2020

Uh oh!

Patryk27 commented Oct 17, 2020

Uh oh!

bors commented Oct 20, 2020

Uh oh!

Patryk27 commented Oct 21, 2020

Uh oh!

bors commented Oct 26, 2020

Uh oh!

Patryk27 commented Oct 27, 2020

Uh oh!

Patryk27 commented Nov 8, 2020

Uh oh!

bors commented Nov 15, 2020

Uh oh!

Patryk27 commented Nov 16, 2020

Uh oh!

jonas-schievink commented Nov 16, 2020

Uh oh!

estebank left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rust-log-analyzer commented Jan 2, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Patryk27Jan 10, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Patryk27Jan 13, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

estebank commented Jan 13, 2021

Uh oh!

Patryk27 commented Oct 4, 2020•
edited
Loading

Patryk27Jan 10, 2021•
edited
Loading

Patryk27Jan 13, 2021•
edited
Loading