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

Properly capture trailing 'unglued' token #79978

Merged
merged 1 commit into from
Dec 13, 2020

Conversation

Aaron1011
Copy link
Member

If we try to capture the Vec<u8> in Option<Vec<u8>>, we'll
need to capture a > token which was 'unglued' from a >> token.
The processing of unglueing a token for parsing purposes bypasses the
usual capturing infrastructure, so we currently lose the trailing >.
As a result, we fall back to the reparsed TokenStream, causing us to
lose spans.

This commit makes token capturing keep track of a trailing 'unglued'
token. Note that we don't need to care about unglueing except at the end
of the captured tokens - if we capture both the first and second unglued
tokens, then we'll end up capturing the full 'glued' token, which
already works correctly.

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 12, 2020
@Aaron1011 Aaron1011 force-pushed the fix/capture-broken-token branch from 27fd7e1 to 759403f Compare December 12, 2020 20:56
@petrochenkov
Copy link
Contributor

petrochenkov commented Dec 12, 2020

What a coincidence, I reviewed #79912 today and started thinking - how does token collection works with cases like tuple.0.0 where we break tokens (a float 0.0 in this case) into parts?
I didn't have a chance to test anything yet though.

Could you actually add tuple.0.0 to test cases in addition to >>?

@petrochenkov
Copy link
Contributor

All this stuff will go away once lexer starts producing "fine-grained" tokens, e.g. two tokens for >> and three tokens for 0.0, and parser learns to work with them.

@Aaron1011
Copy link
Member Author

@petrochenkov: We can't run into this kind of issue with tuple.0.0. This issue occurs when we try to capture part of the TokenStream through a macro_rules! matcher. In the case of tuple.0.0, you would need to capture tuple.0 as an expression to trigger the tuple unglueing behavior - however, macro_rules! explicitly prevent you from doing this by disallowing . (and other tokens) after an :expr matcher).

@Aaron1011 Aaron1011 changed the title Properly capture traiing 'unglued' token Properly capture trailing 'unglued' token Dec 12, 2020
If we try to capture the `Vec<u8>` in `Option<Vec<u8>>`, we'll
need to capture a `>` token which was 'unglued' from a `>>` token.
The processing of unglueing a token for parsing purposes bypasses the
usual capturing infrastructure, so we currently lose the trailing `>`.
As a result, we fall back to the reparsed `TokenStream`, causing us to
lose spans.

This commit makes token capturing keep track of a trailing 'unglued'
token. Note that we don't need to care about unglueing except at the end
of the captured tokens - if we capture both the first and second unglued
tokens, then we'll end up capturing the full 'glued' token, which
already works correctly.
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Dec 12, 2020

📌 Commit e6fa633 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 12, 2020
@bors
Copy link
Contributor

bors commented Dec 13, 2020

⌛ Testing commit e6fa633 with merge 803c602...

@bors
Copy link
Contributor

bors commented Dec 13, 2020

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 803c602 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 13, 2020
@bors bors merged commit 803c602 into rust-lang:master Dec 13, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 13, 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. 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.

5 participants