Skip to content

Conversation

Dylan-DPC-zz
Copy link

Successful merges:

Failed merges:

r? @ghost

Aaron1011 and others added 12 commits May 17, 2020 19:04
Fixes rust-lang#66898

When we are unable to resolve a reference to `self`, we current assume
that the containing function doesn't have a `self` parameter, and
emit an error message accordingly.

However, if the reference to `self` was created by a macro invocation,
then resolution will correctly fail, due to hygiene. In this case, we
don't want to tell the user that the containing fuction doesn't have a
'self' paramter if it actually has one.

This PR checks for the precense of a 'self' parameter, and adjusts the
error message we emit accordingly.

TODO: The exact error message we emit could probably be improved. Should
we explicitly mention hygiene?
exhaustively check `ty::Kind` during structural match checking

This was prone to errors as we may forget new kinds in the future.

I am also not yet sure about some kinds.

`ty::GeneratorWitness(..) | ty::Infer(_) | ty::Placeholder(_) | ty::UnnormalizedProjection(..)  | ty::Bound(..)` might be unreachable here.

We may want to forbid `ty::Projection`, similar to `ty::Param`.

`ty::Opaque` seems fine afaict, should not be possible in a match atm.

I believe `ty::Foreign` should not be structurally match, as I don't even know what
that would actually mean.

r? @pnkfelix cc @eddyb
…, r=matthewjasper

Emit a better diagnostic when function actually has a 'self' parameter

Fixes rust-lang#66898

When we are unable to resolve a reference to `self`, we current assume
that the containing function doesn't have a `self` parameter, and
emit an error message accordingly.

However, if the reference to `self` was created by a macro invocation,
then resolution will correctly fail, due to hygiene. In this case, we
don't want to tell the user that the containing fuction doesn't have a
'self' paramter if it actually has one.

This PR checks for the precense of a 'self' parameter, and adjusts the
error message we emit accordingly.

TODO: The exact error message we emit could probably be improved. Should
we explicitly mention hygiene?
Enable `glacier` command via triagebot

I noticed this was required by rust-lang#72554 (comment).
@Dylan-DPC-zz
Copy link
Author

@bors r+ rollup=never p=4

@bors
Copy link
Collaborator

bors commented May 25, 2020

📌 Commit aa8d64b has been approved by Dylan-DPC

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 25, 2020
@bors
Copy link
Collaborator

bors commented May 25, 2020

⌛ Testing commit aa8d64b with merge f93bb2a...

@bors
Copy link
Collaborator

bors commented May 25, 2020

☀️ Test successful - checks-azure
Approved by: Dylan-DPC
Pushing f93bb2a to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 25, 2020
@bors bors merged commit f93bb2a into rust-lang:master May 25, 2020
@JohnTitor JohnTitor added the rollup A PR which is a rollup label May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants