Skip to content

Modified E0220 to show error messages for more general cases #34364

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

Merged
merged 1 commit into from
Jun 23, 2016

Conversation

nikhilshagri
Copy link
Contributor

@nikhilshagri nikhilshagri commented Jun 19, 2016

This PR extends E0220's description to explain more cases.
Refer to #34342 for more.
r? @GuillaumeGomez

@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @GuillaumeGomez (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.


trait Trait {
type Bar;
type Baz; // Baz has been defined before being used
Copy link
Member

@GuillaumeGomez GuillaumeGomez Jun 19, 2016

Choose a reason for hiding this comment

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

Hum... This sentence implies that a type must be defined before being used, which is wrong:

trait Foo {
    fn foo(&self, Self::Bar);

    type Bar;
}

What about:

trait Trait {
    type Bar;
    type Baz; // we declare the `Baz` associated type in our trait `Trait`.

    // and now we can use it here:
    fn return_bool(&self, &Self::Bar, &Self::Baz) -> bool;

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, that one seems better. Will change it.

@nikhilshagri
Copy link
Contributor Author

I noticed that the terms 'declared' and 'defined' are being used interchangeably here. Should I change the error message to be more consistent and use one word throughout, or is it ok as it is?

@GuillaumeGomez
Copy link
Member

It didn't hit me so I'd say you can keep it this way.

@nikhilshagri
Copy link
Contributor Author

Fixed the changes.


// or:

trait Trait {
Copy link
Member

Choose a reason for hiding this comment

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

You cannot put the same trait name in this same block, otherwise the test will fail. Either you change the trait's name or you create a new code block.

@GuillaumeGomez
Copy link
Member

Once the last nit fixed, please squash your commits.

@nikhilshagri
Copy link
Contributor Author

nikhilshagri commented Jun 21, 2016

Fixed trait names and squashed commits; all tests should pass now.

@@ -2787,23 +2787,43 @@ You used an associated type which isn't defined in the trait.
Erroneous code example:

```compile_fail
trait Trait {
trait T1 {
type Bar;
}

type Foo = Trait<F=i32>; // error: associated type `F` not found for
Copy link
Member

Choose a reason for hiding this comment

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

Trait isn't defined.

@nikhilshagri
Copy link
Contributor Author

Sorry for that, I have fixed them now. Please take a look.

type Foo = Trait<F=i32>; // error: associated type `F` not found for
// `Trait`
type Foo = T1<F=i32>; // error: associated type `F` not found for
// `T1`
Copy link
Member

Choose a reason for hiding this comment

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

This line isn't aligned with the previous one.

@GuillaumeGomez
Copy link
Member

One last nit! 😛

@nikhilshagri
Copy link
Contributor Author

Fixed.

@GuillaumeGomez
Copy link
Member

Thanks for your patience!

@bors: r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 22, 2016

📌 Commit 09ffe47 has been approved by GuillaumeGomez

@bors
Copy link
Collaborator

bors commented Jun 23, 2016

⌛ Testing commit 09ffe47 with merge bca6868...

bors added a commit that referenced this pull request Jun 23, 2016
Modified E0220 to show error messages for more general cases

This PR extends `E0220`'s description to explain more cases.
Refer to [#34342](#34342) for more.
r? @GuillaumeGomez
@bors bors merged commit 09ffe47 into rust-lang:master Jun 23, 2016
@nikhilshagri nikhilshagri deleted the develop branch July 12, 2016 22:21
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.

4 participants