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

rustc: Improve error messages for resolve failures. #14086

Merged
merged 1 commit into from
May 14, 2014

Conversation

Ryman
Copy link
Contributor

@Ryman Ryman commented May 10, 2014

Provides better help for the resolve failures inside an impl if the name matches:

  • a field on the self type
  • a method on the self type
  • a method on the current trait ref (in a trait impl)

Not handling trait method suggestions if in a regular impl (as you can see on line 69 of the test), I believe it is possible though.

Also, provides a better message when self fails to resolve due to being a static method.

It's using some unsafe pointers to skip copying the larger structures (which are only used in error conditions); it's likely possible to get it working with lifetimes (all the useful refs should outlive the visitor calls) but I haven't really figured that out for this case. (can switch to copying code if wanted)

Closes #2356.

@kud1ing
Copy link

kud1ing commented May 10, 2014

Very nice. I'd prefer to replace Did you mean with something neutral though. In the spirit of "don't blame the user".

@Ryman
Copy link
Contributor Author

Ryman commented May 10, 2014

@kud1ing If we can achieve consensus on this, sure, otherwise I'd prefer to leave that for a future PR, the current string is consistent with how rustc already behaves.

@alexcrichton
Copy link
Member

This looks pretty awesome, nice work! The primary thing which needs to get done here is to remove the usage of unsafe pointers. Other than that, just some minor cleanups here and there, and otherwise I think this is good to go!

@Ryman
Copy link
Contributor Author

Ryman commented May 14, 2014

@alexcrichton Check latest commit, I'll squash the commits if those changes are OK for you.

@alexcrichton
Copy link
Member

Looks good to me, with a rebase I think this is good to go.

@alexcrichton
Copy link
Member

Erm, squash, not rebase.

@Ryman
Copy link
Contributor Author

Ryman commented May 14, 2014

@alexcrichton Done.

bors added a commit that referenced this pull request May 14, 2014
…ichton

Provides better help for the resolve failures inside an `impl` if the name matches:
- a field on the self type
- a method on the self type
- a method on the current trait ref (in a trait impl)

Not handling trait method suggestions if in a regular `impl` (as you can see on line 69 of the test), I believe it is possible though.

Also, provides a better message when `self` fails to resolve due to being a static method.

It's using some unsafe pointers to skip copying the larger structures (which are only used in error conditions); it's likely possible to get it working with lifetimes (all the useful refs should outlive the visitor calls) but I haven't really figured that out for this case. (can switch to copying code if wanted)

Closes #2356.
@bors bors closed this May 14, 2014
@bors bors merged commit 595e291 into rust-lang:master May 14, 2014
@emberian
Copy link
Member

This is fantastic. Thank you.

flip1995 pushed a commit to flip1995/rust that referenced this pull request Feb 20, 2025
`manual_flatten` should respect MSRV.

changelog: [`manual_flatten`]: add MSRV check
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.

Un-xfail test of error message for "missing 'self'"
5 participants