Skip to content

proc_macro::bridge: simplify and remove ExecutionStrategy and DispatcherTrait#152531

Open
cyrgani wants to merge 5 commits intorust-lang:mainfrom
cyrgani:pm-yet-another-cleanup
Open

proc_macro::bridge: simplify and remove ExecutionStrategy and DispatcherTrait#152531
cyrgani wants to merge 5 commits intorust-lang:mainfrom
cyrgani:pm-yet-another-cleanup

Conversation

@cyrgani
Copy link
Contributor

@cyrgani cyrgani commented Feb 12, 2026

Also includes another tiny cleanup (functions can only have one return type).

@rustbot
Copy link
Collaborator

rustbot commented Feb 12, 2026

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. labels Feb 12, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 12, 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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @petrochenkov

fn run_bridge_and_client<S: Server>(
&self,
dispatcher: &mut impl DispatcherTrait,
dispatcher: &mut Dispatcher<S>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
dispatcher: &mut Dispatcher<S>,
dispatcher: &mut Dispatcher<impl Server>,

(But this depends on the taste.)

/// A message pipe used for communicating between server and client threads.
pub trait MessagePipe<T>: Sized {
struct MessagePipe<T> {
tx: std::sync::mpsc::SyncSender<T>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
tx: std::sync::mpsc::SyncSender<T>,
tx: mpsc::SyncSender<T>,

Here and in 3 places below.

@petrochenkov
Copy link
Contributor

r=me after addressing comments.
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 12, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 12, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

pub enum ExecutionStrategy {
CrossThread,
SameThread,
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is going to work if we want to support wasm proc macros. The ExecutionStrategy implementation for that has to be in rustc, not libproc_macro. The latter can't use arbitrary dependencies for providing a wasm runtime, nor should we want to link a wasm runtime in every proc macro.

MaybeCrossThread { cross_thread }
}
}
pub struct MaybeCrossThread(pub bool);
Copy link
Member

Choose a reason for hiding this comment

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

I think it is a bit clearer to keep the field name.

Suggested change
pub struct MaybeCrossThread(pub bool);
pub struct MaybeCrossThread {
pub cross_thread: bool,
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants