Skip to content

cleanup some more things in proc_macro::bridge#152166

Open
cyrgani wants to merge 6 commits intorust-lang:mainfrom
cyrgani:questionable-pm-cleanups
Open

cleanup some more things in proc_macro::bridge#152166
cyrgani wants to merge 6 commits intorust-lang:mainfrom
cyrgani:questionable-pm-cleanups

Conversation

@cyrgani
Copy link
Contributor

@cyrgani cyrgani commented Feb 5, 2026

Each commit should be reviewable on its own.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 5, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 5, 2026

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

) -> Result<S::TokenStream, PanicMessage>
where
S: Server,
S::TokenStream: Default,
Copy link
Member

Choose a reason for hiding this comment

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

Why does run_server actually return an Option<TokenStream> in the first place? Can't it return a TokenStream directly and if necessary create an empty TokenStream internally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's needed because the crate-level TokenStream wraps the contained TokenStream handle in an Option to avoid using RPC for an empty TokenStream. I tried to remove it here and got use-after-free in proc_macro handle ICEs.

@rust-cloud-vms rust-cloud-vms bot force-pushed the questionable-pm-cleanups branch from c1be145 to 74d2616 Compare February 6, 2026 21:27
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