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

Encode ident rawness and literal kind separately in tt::Leaf #17559

Merged
merged 3 commits into from
Jul 15, 2024

Conversation

Veykril
Copy link
Member

@Veykril Veykril commented Jul 7, 2024

No description provided.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 7, 2024
@Veykril Veykril changed the title Model TokenTree slightly closer to rustc/proc_macro Model TokenTree closer to rustc/proc_macro Jul 7, 2024
@Veykril Veykril changed the title Model TokenTree closer to rustc/proc_macro Model TokenTree closer to rustc/proc_macro's token tree/stream Jul 7, 2024
@Veykril Veykril force-pushed the tokentree branch 3 times, most recently from f931616 to 0be9e2a Compare July 8, 2024 09:37
@Veykril Veykril changed the title Model TokenTree closer to rustc/proc_macro's token tree/stream Encode ident rawness and literal kind separately in tt::Leaf Jul 8, 2024
@Veykril Veykril force-pushed the tokentree branch 2 times, most recently from a860709 to 98e15d4 Compare July 10, 2024 14:30
@Veykril
Copy link
Member Author

Veykril commented Jul 10, 2024

@lnicola this PR is now blocked on us syncing with rust-lang/rust as this touches the proc-macro stuff causing CI to run on nightly 😬 so would be nice if we could do a sync by next week, the PR isn't finished yet and once it is I will only merge it at the start of the week. Just letting you know :)

@lnicola
Copy link
Member

lnicola commented Jul 10, 2024

Yup, will do one soon!

@bors
Copy link
Collaborator

bors commented Jul 15, 2024

☔ The latest upstream changes (presumably #17584) made this pull request unmergeable. Please resolve the merge conflicts.

@Veykril Veykril marked this pull request as ready for review July 15, 2024 10:08

pub const CURRENT_API_VERSION: u32 = RUST_ANALYZER_SPAN_SUPPORT;
pub const CURRENT_API_VERSION: u32 = EXTENDED_LEAF_DATA;
Copy link
Member Author

Choose a reason for hiding this comment

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

We should be able to start dropping support for some proc-macro server API versions soon should the code here become too complicated.

@Veykril
Copy link
Member Author

Veykril commented Jul 15, 2024

I reckon this PR will break stuff, will install the modified proc-macro server and use that for the week to see if something is breaking there as well

@Veykril
Copy link
Member Author

Veykril commented Jul 15, 2024

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 15, 2024

📌 Commit dcfda55 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jul 15, 2024

⌛ Testing commit dcfda55 with merge f913901...

@bors
Copy link
Collaborator

bors commented Jul 15, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing f913901 to master...

@bors bors merged commit f913901 into rust-lang:master Jul 15, 2024
11 checks passed
@Veykril Veykril deleted the tokentree branch July 15, 2024 11:49
bors added a commit that referenced this pull request Jul 15, 2024
Fix incorrect encoding of literals in the proc-macro-api on version 4

Quick follow up on #17559 breaking things
Comment on lines +22 to +23
/// Whether literals encode their kind as an additional u32 field and idents their rawness as a u32 field
pub const EXTENDED_LEAF_DATA: u32 = 5;
Copy link
Member Author

Choose a reason for hiding this comment

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

Headsup @vlad20012, this PR and #17601 make changes to the proc-macro server API adding a new version, adding two more u32s to literals on the wire, and one more u32 to idents. I have not really given this much thought tbf so please comment if you think we can do better than that. Its not set in stone until the next sync to rustc which will update the proc-macro server then.

Copy link
Member

Choose a reason for hiding this comment

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

Note that syncs are currently on hold until we figure out the new rustc_pattern_analysis story.

Copy link
Member Author

Choose a reason for hiding this comment

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

that's fine, probably better even as it gives me more time to test the proc-macro server :D

Copy link
Member

@vlad20012 vlad20012 Jul 16, 2024

Choose a reason for hiding this comment

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

@Veykril Thanks for mentioning! The changes looks good to me

/// pretty-printing of `TokenStream`'s produced by other means (i.e. parsed
/// source code, internally constructed token streams, and token streams
/// produced by declarative macros).
JointHidden,
Copy link
Member

Choose a reason for hiding this comment

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

The changes in Spacing seems unrelated to the wire changes 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes that is not yet used, and iirc it won't ever make it to the proc-macro server either so you should be safe to ignore that. For you only the changes in proc-macro-api ought to be really relevant.

@lnicola
Copy link
Member

lnicola commented Jul 17, 2024

@Veykril did you mean to break cargo xtask install? You now need to do cargo xtask install --server --client.

@Veykril
Copy link
Member Author

Veykril commented Jul 18, 2024

Ah no I didn't

bors added a commit that referenced this pull request Jul 19, 2024
lnicola pushed a commit to lnicola/rust that referenced this pull request Jul 28, 2024
Fix incorrect encoding of literals in the proc-macro-api on version 4

Quick follow up on rust-lang/rust-analyzer#17559 breaking things
lnicola pushed a commit to lnicola/rust that referenced this pull request Jul 28, 2024
RalfJung pushed a commit to RalfJung/rust that referenced this pull request Aug 1, 2024
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.

5 participants