-
Notifications
You must be signed in to change notification settings - Fork 13.5k
proc_macro/bridge: stop using a remote object handle for proc_macro Punct and Group #98188
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -182,7 +182,6 @@ define_handles! { | |
Diagnostic, | ||
|
||
'interned: | ||
Punct, | ||
Ident, | ||
Span, | ||
} | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -212,8 +212,8 @@ pub use quote::{quote, quote_span}; | |||||||||||||||
fn tree_to_bridge_tree( | ||||||||||||||||
tree: TokenTree, | ||||||||||||||||
) -> bridge::TokenTree< | ||||||||||||||||
bridge::client::Span, | ||||||||||||||||
bridge::client::Group, | ||||||||||||||||
bridge::client::Punct, | ||||||||||||||||
bridge::client::Ident, | ||||||||||||||||
bridge::client::Literal, | ||||||||||||||||
> { | ||||||||||||||||
|
@@ -238,8 +238,8 @@ impl From<TokenTree> for TokenStream { | |||||||||||||||
struct ConcatTreesHelper { | ||||||||||||||||
trees: Vec< | ||||||||||||||||
bridge::TokenTree< | ||||||||||||||||
bridge::client::Span, | ||||||||||||||||
bridge::client::Group, | ||||||||||||||||
bridge::client::Punct, | ||||||||||||||||
bridge::client::Ident, | ||||||||||||||||
bridge::client::Literal, | ||||||||||||||||
>, | ||||||||||||||||
|
@@ -365,8 +365,8 @@ pub mod token_stream { | |||||||||||||||
pub struct IntoIter( | ||||||||||||||||
std::vec::IntoIter< | ||||||||||||||||
bridge::TokenTree< | ||||||||||||||||
bridge::client::Span, | ||||||||||||||||
bridge::client::Group, | ||||||||||||||||
bridge::client::Punct, | ||||||||||||||||
bridge::client::Ident, | ||||||||||||||||
bridge::client::Literal, | ||||||||||||||||
>, | ||||||||||||||||
|
@@ -925,7 +925,7 @@ impl fmt::Debug for Group { | |||||||||||||||
/// forms of `Spacing` returned. | ||||||||||||||||
#[stable(feature = "proc_macro_lib2", since = "1.29.0")] | ||||||||||||||||
#[derive(Clone)] | ||||||||||||||||
pub struct Punct(bridge::client::Punct); | ||||||||||||||||
pub struct Punct(bridge::Punct<bridge::client::Span>); | ||||||||||||||||
|
||||||||||||||||
#[stable(feature = "proc_macro_lib2", since = "1.29.0")] | ||||||||||||||||
impl !Send for Punct {} | ||||||||||||||||
|
@@ -958,13 +958,20 @@ impl Punct { | |||||||||||||||
/// which can be further configured with the `set_span` method below. | ||||||||||||||||
#[stable(feature = "proc_macro_lib2", since = "1.29.0")] | ||||||||||||||||
pub fn new(ch: char, spacing: Spacing) -> Punct { | ||||||||||||||||
Punct(bridge::client::Punct::new(ch, spacing)) | ||||||||||||||||
const LEGAL_CHARS: &[char] = &[ | ||||||||||||||||
'=', '<', '>', '!', '~', '+', '-', '*', '/', '%', '^', '&', '|', '@', '.', ',', ';', | ||||||||||||||||
':', '#', '$', '?', '\'', | ||||||||||||||||
]; | ||||||||||||||||
if !LEGAL_CHARS.contains(&ch) { | ||||||||||||||||
panic!("unsupported character `{:?}`", ch); | ||||||||||||||||
} | ||||||||||||||||
Comment on lines
+965
to
+971
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't something like this be a (Oops, just checked and this is what it takes: https://godbolt.org/z/vP13eKjsW - I guess that's pretty nasty) If you are going to use an array search, can you do an ASCII check first and make it a bytestring literal? So that it at least can hit some kind of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, that's an easy change. The current code is literally verbatim cut-pasted from the code which used to be in rust/compiler/rustc_expand/src/proc_macro_server.rs Lines 301 to 307 in bd2e51a
FWIW the current code appears to actually compile to a somewhat efficient jump table (https://godbolt.org/z/P9d5366YT), and sloppily switching it to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The existing codegen seems like the nicest of the options (I wouldn't be too surprised if it performs better than the memchr variant), so I'm inclined to stick with it. |
||||||||||||||||
Punct(bridge::Punct { ch, joint: spacing == Spacing::Joint, span: Span::call_site().0 }) | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
/// Returns the value of this punctuation character as `char`. | ||||||||||||||||
#[stable(feature = "proc_macro_lib2", since = "1.29.0")] | ||||||||||||||||
pub fn as_char(&self) -> char { | ||||||||||||||||
self.0.as_char() | ||||||||||||||||
self.0.ch | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
/// Returns the spacing of this punctuation character, indicating whether it's immediately | ||||||||||||||||
|
@@ -973,28 +980,19 @@ impl Punct { | |||||||||||||||
/// (`Alone`) so the operator has certainly ended. | ||||||||||||||||
#[stable(feature = "proc_macro_lib2", since = "1.29.0")] | ||||||||||||||||
pub fn spacing(&self) -> Spacing { | ||||||||||||||||
self.0.spacing() | ||||||||||||||||
if self.0.joint { Spacing::Joint } else { Spacing::Alone } | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
/// Returns the span for this punctuation character. | ||||||||||||||||
#[stable(feature = "proc_macro_lib2", since = "1.29.0")] | ||||||||||||||||
pub fn span(&self) -> Span { | ||||||||||||||||
Span(self.0.span()) | ||||||||||||||||
Span(self.0.span) | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
/// Configure the span for this punctuation character. | ||||||||||||||||
#[stable(feature = "proc_macro_lib2", since = "1.29.0")] | ||||||||||||||||
pub fn set_span(&mut self, span: Span) { | ||||||||||||||||
self.0 = self.0.with_span(span.0); | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
// N.B., the bridge only provides `to_string`, implement `fmt::Display` | ||||||||||||||||
// based on it (the reverse of the usual relationship between the two). | ||||||||||||||||
#[stable(feature = "proc_macro_lib", since = "1.15.0")] | ||||||||||||||||
impl ToString for Punct { | ||||||||||||||||
fn to_string(&self) -> String { | ||||||||||||||||
TokenStream::from(TokenTree::from(self.clone())).to_string() | ||||||||||||||||
self.0.span = span.0; | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
|
@@ -1003,7 +1001,7 @@ impl ToString for Punct { | |||||||||||||||
#[stable(feature = "proc_macro_lib2", since = "1.29.0")] | ||||||||||||||||
impl fmt::Display for Punct { | ||||||||||||||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||||||||||||||||
f.write_str(&self.to_string()) | ||||||||||||||||
write!(f, "{}", self.as_char()) | ||||||||||||||||
} | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
|
Uh oh!
There was an error while loading. Please reload this page.