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

Make E0243/E0244 message consistent with E0107 #36615

Merged
merged 2 commits into from
Nov 11, 2016
Merged

Conversation

sinkuu
Copy link
Contributor

@sinkuu sinkuu commented Sep 21, 2016

E0243/E0233 prints expected {}, found {} on the span note, while E0107 prints it on the first line. This is confusing when both error occur simultaneously.

This PR makes E0243/E0233 print expected {}, found {} on the first line.

Code:

struct Foo<'a, 'b> {
    s: &'a str,
    t: &'b str,
}

type Bar<T, U> = Foo<T, U>;

rustc output (before):

error[E0107]: wrong number of lifetime parameters: expected 2, found 0
 --> test.rs:6:18
  |
6 | type Bar<T, U> = Foo<T, U>;
  |                  ^^^^^^^^^ expected 2 lifetime parameters

error[E0244]: wrong number of type arguments
 --> test.rs:6:18
  |
6 | type Bar<T, U> = Foo<T, U>;
  |                  ^^^^^^^^^ expected no type arguments, found 2

rustc output (after):

error[E0107]: wrong number of lifetime parameters: expected 2, found 0
 --> /tmp/test.rs:6:18
  |
6 | type Bar<T, U> = Foo<T, U>;
  |                  ^^^^^^^^^ expected 2 lifetime parameters

error[E0244]: wrong number of type arguments: expected 0, found 2
 --> /tmp/test.rs:6:18
  |
6 | type Bar<T, U> = Foo<T, U>;
  |                  ^^^^^^^^^ expected no type arguments

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nrc (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@petrochenkov
Copy link
Contributor

There's one more inconsistency!
E0107 says "parameters" and E0244 says "arguments" while they both should be "arguments".

@sinkuu sinkuu changed the title Make E0243/E0233 message consistent with E0107 Make E0243/E0244 message consistent with E0107 Sep 21, 2016
@sinkuu
Copy link
Contributor Author

sinkuu commented Sep 21, 2016

Currently documentations and compiler messages don't tell lifetime arguments/parameters apart (they seem to be called "parameters" everywhere).

@GuillaumeGomez
Copy link
Member

cc @jonathandturner

@bors
Copy link
Contributor

bors commented Oct 2, 2016

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

@GuillaumeGomez
Copy link
Member

cc @jonathandturner

@sophiajt
Copy link
Contributor

sophiajt commented Oct 9, 2016

This looks close, but as @petrochenkov points out both should be arguments since they are calls.

Also this is confusing:

error[E0107]: wrong number of lifetime parameters: expected 2, found 0
 --> /tmp/test.rs:6:18
  |
6 | type Bar<T, U> = Foo<T, U>;
  |                  ^^^^^^^^^ expected 2 lifetime parameters

You're underlining 2 arguments and saying that's what you expect. How is this an error?

@sinkuu
Copy link
Contributor Author

sinkuu commented Oct 9, 2016

T and U are not lifetime arguments but type arguments.

@sophiajt
Copy link
Contributor

sophiajt commented Oct 9, 2016

@sinkuu do you have an updated example of what it will look like?

@sophiajt
Copy link
Contributor

sophiajt commented Oct 9, 2016

Ah in that case perhaps it should note the difference. I totally overlooked it

@sinkuu
Copy link
Contributor Author

sinkuu commented Oct 9, 2016

I'll try to do that.

@bors
Copy link
Contributor

bors commented Oct 19, 2016

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

@sinkuu sinkuu force-pushed the e0243_0244 branch 2 times, most recently from 3cf465d to 94a896f Compare October 21, 2016 15:13
@sinkuu
Copy link
Contributor Author

sinkuu commented Oct 22, 2016

Converted "lifetime parameters" to "lifetime arguments".

For noting differences between lifetime args and type args, I'm not sure what the note should say. When types are mixed-up with lifetimes, the compiler will put both E0107 and E0244 anyway.gi

@nikomatsakis
Copy link
Contributor

Hmm I'm of two minds about whether to say "arguments" or "parameters". A quick google search suggests that most people say "generic type parameters" (C#, Java), though there are some who say "arguments" (TypeScript). Definitely searching for "generic type parameters" gives a lot more results (1.6mil) than "generic type arguments (670k, the top few of which are actually saying "parameters"). I definitely agree consistency is better though. =)

@nrc
Copy link
Member

nrc commented Oct 25, 2016

+1 for parameters

@brson
Copy link
Contributor

brson commented Nov 9, 2016

The debate about "arguments" vs "parameters" seems to transcend this PR. Can we just press on as-is?

@nikomatsakis
Copy link
Contributor

@brson agreed, this PR just keeps existing language it seems

@alexcrichton
Copy link
Member

In that sense, ping r? @nrc for helping to move forward

@nrc
Copy link
Member

nrc commented Nov 11, 2016

@bors: r+

@bors
Copy link
Contributor

bors commented Nov 11, 2016

📌 Commit 12f0b6f has been approved by nrc

@bors
Copy link
Contributor

bors commented Nov 11, 2016

⌛ Testing commit 12f0b6f with merge 280362a...

bors added a commit that referenced this pull request Nov 11, 2016
Make E0243/E0244 message consistent with E0107

E0243/E0233 prints `expected {}, found {}` on the span note, while E0107 prints it on the first line. This is confusing when both error occur simultaneously.

This PR makes E0243/E0233 print `expected {}, found {}` on the first line.

Code:

``` rust
struct Foo<'a, 'b> {
    s: &'a str,
    t: &'b str,
}

type Bar<T, U> = Foo<T, U>;
```

rustc output (before):

```
error[E0107]: wrong number of lifetime parameters: expected 2, found 0
 --> test.rs:6:18
  |
6 | type Bar<T, U> = Foo<T, U>;
  |                  ^^^^^^^^^ expected 2 lifetime parameters

error[E0244]: wrong number of type arguments
 --> test.rs:6:18
  |
6 | type Bar<T, U> = Foo<T, U>;
  |                  ^^^^^^^^^ expected no type arguments, found 2
```

rustc output (after):

```
error[E0107]: wrong number of lifetime parameters: expected 2, found 0
 --> /tmp/test.rs:6:18
  |
6 | type Bar<T, U> = Foo<T, U>;
  |                  ^^^^^^^^^ expected 2 lifetime parameters

error[E0244]: wrong number of type arguments: expected 0, found 2
 --> /tmp/test.rs:6:18
  |
6 | type Bar<T, U> = Foo<T, U>;
  |                  ^^^^^^^^^ expected no type arguments
```
@bors bors merged commit 12f0b6f into rust-lang:master Nov 11, 2016
@sinkuu sinkuu deleted the e0243_0244 branch November 11, 2016 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants