Skip to content

Improve spans when splitting multi-char operator tokens for proc macros. #102639

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Oct 4, 2022

Conversation

nnethercote
Copy link
Contributor

When a two-char (or three-char) operator token is split into single-char operator tokens before being passed to a proc macro, the single-char tokens are given the original span of length two (or three). This PR gives them more accurate spans.

r? @Aaron1011
cc @petrochenkov

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 3, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 3, 2022
@nnethercote nnethercote force-pushed the improve-spans-splitting branch from 3c05485 to 88dab8d Compare October 3, 2022 22:08
let hi = lo + BytePos::from_usize(1);
span.with_lo(lo).with_hi(hi)
} else {
span
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a test that exercises this (if there isn't one already)? You should be able to use quote! to get tokens with a weird enough span.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are already tests that cover this. (That's how I found out about such spans.) If I remove the length check, the following ui tests fail:

failures:
    [ui] src/test/ui/proc-macro/doc-comment-preserved.rs
    [ui] src/test/ui/proc-macro/inner-attr-non-inline-mod.rs
    [ui] src/test/ui/proc-macro/issue-78675-captured-inner-attrs.rs
    [ui] src/test/ui/proc-macro/issue-81007-item-attrs.rs
    [ui] src/test/ui/proc-macro/meta-macro-hygiene.rs

For example, in src/test/ui/proc-macro/inner-attr-non-inline-mod.rs every token passed to the proc macro has a span of $DIR/inner-attr-non-inline-mod.rs:14:1: 14:23.

@Aaron1011
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 3, 2022

📌 Commit 88dab8d has been approved by Aaron1011

It is now in the queue for this repository.

@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 Oct 3, 2022
@nnethercote
Copy link
Contributor Author

Thank you for the fast review.

@bors rollup=always

bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 4, 2022
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#102441 (Suggest unwrap_or_else when a closure is given)
 - rust-lang#102547 (Migrate CSS theme for search results)
 - rust-lang#102567 (Delay evaluating lint primary message until after it would be suppressed)
 - rust-lang#102624 (rustdoc: remove font family CSS on `.rustdoc-toggle summary::before`)
 - rust-lang#102628 (Change the parameter name of From::from to `value`)
 - rust-lang#102637 (Ignore fuchsia on two compiler tests)
 - rust-lang#102639 (Improve spans when splitting multi-char operator tokens for proc macros.)

Failed merges:

 - rust-lang#102496 (Suggest `.into()` when all other coercion suggestions fail)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 185ca0f into rust-lang:master Oct 4, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 4, 2022
@nnethercote nnethercote deleted the improve-spans-splitting branch October 5, 2022 03:46
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants