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

Fix/matches span #400

Merged
merged 2 commits into from
Jun 12, 2020
Merged

Fix/matches span #400

merged 2 commits into from
Jun 12, 2020

Conversation

CreepySkeleton
Copy link
Collaborator

Merge #398

In `struct-opt-derive`, a function is generated with a parameter named
`matches`. Since `quote!` is used to generate the function, the
`matches` token will be resolved using `Span::call_site`.

However, the literal identifier `matches` is also used inside several
`quote_spanned!` expressions. Such a `matches` identifier will be
resolved using the `Span` passed to `quote_spanned!`, which may not be
the same as `Span::call_site`.

Currently, this is difficult to observe in practice, due to
rust-lang/rust#43081 . However, once PR rust-lang/rust#73084
is merged, proc macros will see properly spanned tokens in more cases,
which will cause these incorrect uses of `quote_spanned!` to break.

This PR uses `quote! { matches }` to generate a correctly spanned
`matches` token, which is then include in the `quote_spanned!`
expressions using `#matches`.
@CreepySkeleton CreepySkeleton requested a review from TeXitoi June 12, 2020 10:12
@CreepySkeleton
Copy link
Collaborator Author

I'm merging this without approval because it's just a bugfix and a pretty straightforward one.

//
// Given that this ident is used in several places and
// that the branches are located inside of a loop, it is possible that
// this ident will be given _different_ spans in different places, and
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that a plain quote { } or format_ident will always use Span::call_site() for hygiene, so the generated matches token will always be 'the same' from the perspective of the compiler's name resolution. However, this change is still a good idea, since it makes it clear that matches is logically independent of the loop.

@CreepySkeleton CreepySkeleton deleted the fix/matches-span branch June 13, 2020 08:04
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.

2 participants