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

TokenMap -> SpanMap rewrite #15959

Merged
merged 24 commits into from
Dec 4, 2023
Merged

TokenMap -> SpanMap rewrite #15959

merged 24 commits into from
Dec 4, 2023

Conversation

Veykril
Copy link
Member

@Veykril Veykril commented Nov 24, 2023

Opening early so I can have an overview over the full diff more easily, still very unfinished and lots of work to be done.

The gist of what this PR does is move away from assigning IDs to tokens in arguments and expansions and instead gives the subtrees the text ranges they are sourced from (made relative to some item for incrementality). This means we now only have a single map per expension, opposed to map for expansion and arguments.

A few of the things that are not done yet (in arbitrary order):

  • generally clean up the current mess
  • proc-macros, have been completely ignored so far
  • syntax fixups, has been commented out for the time being needs to be rewritten on top of some marker SyntaxContextId
  • macro invocation syntax contexts are not properly passed around yet, so $crate hygiene does not work in all cases (but most)
    • builtin macros do not set spans properly, $crate basically does not work with them rn (which we use)
      - [ ] remove all uses of dummy spans (or if that does not work, change the dummy entries for dummy spans so that tests will not silently pass due to havin a file id for the dummy file)
    • de-queryfy macro_expand, the sole caller of it is parse_macro_expansion, and both of these are lru-cached with the same limit so having it be a query is pointless
  • fix eager macro spans and other stuff
    • simplify include! handling
  • Figure out how to undo the sudden () expression wrapping in expansions / alternatively prioritize getting invisible delimiters working again
  • Simplify InFile stuff and HirFIleId extensions
    - [ ] span crate containing all the file ids, span stuff, ast ids. Then remove the dependency injection generics from tt and mbe

Fixes #10300
Fixes #15685

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 24, 2023
@Veykril Veykril force-pushed the macro-shower3 branch 3 times, most recently from d7fd984 to 8707c13 Compare November 25, 2023 14:28
This was referenced Dec 3, 2023
@Veykril Veykril marked this pull request as ready for review December 3, 2023 17:50
@Veykril
Copy link
Member Author

Veykril commented Dec 3, 2023

Okay this is ready now, so we can merge this after tomorrows release, and depending on the fallout we might want to skip the following stable release, though I don't expect this to be that problematic.

@Veykril Veykril force-pushed the macro-shower3 branch 3 times, most recently from 12fefc3 to 05f4e55 Compare December 3, 2023 19:09
@lnicola
Copy link
Member

lnicola commented Dec 4, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Dec 4, 2023

📋 Looks like this PR is still in progress, ignoring approval.

Hint: Remove [WIP] from this PR's title when it is ready for review.

@lnicola lnicola changed the title [WIP]: TokenMap -> SpanMap rewrite TokenMap -> SpanMap rewrite Dec 4, 2023
@lnicola
Copy link
Member

lnicola commented Dec 4, 2023

@bors retry

@lnicola
Copy link
Member

lnicola commented Dec 4, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Dec 4, 2023

📌 Commit 18f1a3c has been approved by lnicola

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Dec 4, 2023

⌛ Testing commit 18f1a3c with merge e91fdf7...

@bors
Copy link
Contributor

bors commented Dec 4, 2023

☀️ Test successful - checks-actions
Approved by: lnicola
Pushing e91fdf7 to master...

@bors bors merged commit e91fdf7 into rust-lang:master Dec 4, 2023
10 checks passed
@Veykril Veykril deleted the macro-shower3 branch December 4, 2023 20:30
@Veykril
Copy link
Member Author

Veykril commented Dec 8, 2023

This also seemed to have a decent improvement on body lowering time? Not quite sure why though
image
image

resolver,
)
};
let err = parse_err.or(err);
if cfg!(debug) {
Copy link
Member

@lnicola lnicola Dec 11, 2023

Choose a reason for hiding this comment

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

(sorry for the duplicate ping, I'm not sure how well they work in commit comments)

@Veykril is this supposed to be cfg!(debug_assertions) or can we call it unconditionally?

Copy link
Member Author

Choose a reason for hiding this comment

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

it was supposed do be debug_assertions (cause it may reallocate unnecessarily)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

We need more incrementality tests Re-exported concat macro causes "failed to resolve macro" error
4 participants