From 632fa8ef4a3c9e7440b79e04a9f7dd9bd23a4de4 Mon Sep 17 00:00:00 2001 From: Kevin Mehall Date: Sat, 6 Mar 2021 09:36:22 -0700 Subject: [PATCH 1/4] Fix TokenStream::from_str for input consisting of a single Group TokenStream holds a `tt::Subtree` but assumes its `delimiter` is always `None`. In particular, the iterator implementation iterates over the inner `token_trees` and ignores the `delimiter`. However, `TokenStream::from_str` violated this assumption when the input consists of a single Group by producing a Subtree with an outer delimiter, which was ignored as seen by a procedural macro. In this case, wrap an extra level of Subtree around it. Fixes #7810 Fixes #7875 --- crates/proc_macro_srv/src/rustc_server.rs | 36 +++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/crates/proc_macro_srv/src/rustc_server.rs b/crates/proc_macro_srv/src/rustc_server.rs index 14c853c7752..f02b0af603d 100644 --- a/crates/proc_macro_srv/src/rustc_server.rs +++ b/crates/proc_macro_srv/src/rustc_server.rs @@ -34,7 +34,16 @@ pub fn new() -> Self { } pub fn with_subtree(subtree: tt::Subtree) -> Self { - TokenStream { subtree } + if subtree.delimiter.is_some() { + TokenStream { + subtree: tt::Subtree { + token_trees: vec![TokenTree::Subtree(subtree)], + delimiter: None, + }, + } + } else { + TokenStream { subtree } + } } pub fn is_empty(&self) -> bool { @@ -185,7 +194,7 @@ fn from_str(src: &str) -> Result { mbe::parse_to_token_tree(src).ok_or("Failed to parse from mbe")?; let subtree = subtree_replace_token_ids_with_unspecified(subtree); - Ok(TokenStream { subtree }) + Ok(TokenStream::with_subtree(subtree)) } } @@ -779,4 +788,27 @@ fn test_rustc_server_to_string() { assert_eq!(s.to_string(), "struct T {}"); } + + #[test] + fn test_rustc_server_from_str() { + use std::str::FromStr; + let subtree_paren_a = tt::TokenTree::Subtree(tt::Subtree { + delimiter: Some(tt::Delimiter { + id: tt::TokenId::unspecified(), + kind: tt::DelimiterKind::Parenthesis, + }), + token_trees: vec![tt::TokenTree::Leaf(tt::Leaf::Ident(tt::Ident { + text: "a".into(), + id: tt::TokenId::unspecified(), + }))], + }); + + let t1 = TokenStream::from_str("(a)").unwrap(); + assert_eq!(t1.subtree.token_trees.len(), 1); + assert_eq!(t1.subtree.token_trees[0], subtree_paren_a); + + let t2 = TokenStream::from_str("(a);").unwrap(); + assert_eq!(t2.subtree.token_trees.len(), 2); + assert_eq!(t2.subtree.token_trees[0], subtree_paren_a); + } } From 62f594b390e5f648a32b5b08863a6413b4271d19 Mon Sep 17 00:00:00 2001 From: Kevin Mehall Date: Sat, 6 Mar 2021 09:46:32 -0700 Subject: [PATCH 2/4] Refactor TokenStream to hold Vec instead of tt::Subtree `TokenStream` assumes that its subtree's delimeter is `None`, and this should be encoded in the type system instead of having a delimiter field that is mostly ignored. `tt::Subtree` is just `pub delimiter: Option, pub token_trees: Vec`, so a Subtree that is statically guaranteed not to have a delimiter is just Vec. --- crates/proc_macro_srv/src/dylib.rs | 6 +- crates/proc_macro_srv/src/rustc_server.rs | 113 ++++++++++------------ crates/proc_macro_srv/src/tests/utils.rs | 2 +- 3 files changed, 56 insertions(+), 65 deletions(-) diff --git a/crates/proc_macro_srv/src/dylib.rs b/crates/proc_macro_srv/src/dylib.rs index 28a6ee54710..baf10fea937 100644 --- a/crates/proc_macro_srv/src/dylib.rs +++ b/crates/proc_macro_srv/src/dylib.rs @@ -138,7 +138,7 @@ pub fn expand( parsed_body, false, ); - return res.map(|it| it.subtree); + return res.map(|it| it.into_subtree()); } bridge::client::ProcMacro::Bang { name, client } if *name == macro_name => { let res = client.run( @@ -147,7 +147,7 @@ pub fn expand( parsed_body, false, ); - return res.map(|it| it.subtree); + return res.map(|it| it.into_subtree()); } bridge::client::ProcMacro::Attr { name, client } if *name == macro_name => { let res = client.run( @@ -157,7 +157,7 @@ pub fn expand( parsed_body, false, ); - return res.map(|it| it.subtree); + return res.map(|it| it.into_subtree()); } _ => continue, } diff --git a/crates/proc_macro_srv/src/rustc_server.rs b/crates/proc_macro_srv/src/rustc_server.rs index f02b0af603d..2798dbf0d7f 100644 --- a/crates/proc_macro_srv/src/rustc_server.rs +++ b/crates/proc_macro_srv/src/rustc_server.rs @@ -25,36 +25,35 @@ #[derive(Debug, Clone)] pub struct TokenStream { - pub subtree: tt::Subtree, + pub token_trees: Vec, } impl TokenStream { pub fn new() -> Self { - TokenStream { subtree: Default::default() } + TokenStream { token_trees: Default::default() } } pub fn with_subtree(subtree: tt::Subtree) -> Self { if subtree.delimiter.is_some() { - TokenStream { - subtree: tt::Subtree { - token_trees: vec![TokenTree::Subtree(subtree)], - delimiter: None, - }, - } + TokenStream { token_trees: vec![TokenTree::Subtree(subtree)] } } else { - TokenStream { subtree } + TokenStream { token_trees: subtree.token_trees } } } + pub fn into_subtree(self) -> tt::Subtree { + tt::Subtree { delimiter: None, token_trees: self.token_trees } + } + pub fn is_empty(&self) -> bool { - self.subtree.token_trees.is_empty() + self.token_trees.is_empty() } } /// Creates a token stream containing a single token tree. impl From for TokenStream { fn from(tree: TokenTree) -> TokenStream { - TokenStream { subtree: tt::Subtree { delimiter: None, token_trees: vec![tree] } } + TokenStream { token_trees: vec![tree] } } } @@ -87,10 +86,10 @@ fn extend>(&mut self, streams: I) { for tkn in item { match tkn { tt::TokenTree::Subtree(subtree) if subtree.delimiter.is_none() => { - self.subtree.token_trees.extend(subtree.token_trees); + self.token_trees.extend(subtree.token_trees); } _ => { - self.subtree.token_trees.push(tkn); + self.token_trees.push(tkn); } } } @@ -173,7 +172,7 @@ impl IntoIterator for TokenStream { type IntoIter = super::IntoIter; fn into_iter(self) -> Self::IntoIter { - self.subtree.token_trees.into_iter() + self.token_trees.into_iter() } } @@ -200,32 +199,32 @@ fn from_str(src: &str) -> Result { impl ToString for TokenStream { fn to_string(&self) -> String { - let tt = self.subtree.clone().into(); - to_text(&tt) + tokentrees_to_text(&self.token_trees[..]) } } - fn to_text(tkn: &tt::TokenTree) -> String { + fn tokentrees_to_text(tkns: &[tt::TokenTree]) -> String { + tkns.iter() + .fold((String::new(), true), |(last, last_to_joint), tkn| { + let s = [last, tokentree_to_text(tkn)].join(if last_to_joint { "" } else { " " }); + let mut is_joint = false; + if let tt::TokenTree::Leaf(tt::Leaf::Punct(punct)) = tkn { + if punct.spacing == tt::Spacing::Joint { + is_joint = true; + } + } + (s, is_joint) + }) + .0 + } + + fn tokentree_to_text(tkn: &tt::TokenTree) -> String { match tkn { tt::TokenTree::Leaf(tt::Leaf::Ident(ident)) => ident.text.clone().into(), tt::TokenTree::Leaf(tt::Leaf::Literal(literal)) => literal.text.clone().into(), tt::TokenTree::Leaf(tt::Leaf::Punct(punct)) => format!("{}", punct.char), tt::TokenTree::Subtree(subtree) => { - let content = subtree - .token_trees - .iter() - .fold((String::new(), true), |(last, last_to_joint), tkn| { - let s = [last, to_text(tkn)].join(if last_to_joint { "" } else { " " }); - let mut is_joint = false; - if let tt::TokenTree::Leaf(tt::Leaf::Punct(punct)) = tkn { - if punct.spacing == tt::Spacing::Joint { - is_joint = true; - } - } - (s, is_joint) - }) - .0; - + let content = tokentrees_to_text(&subtree.token_trees); let (open, close) = match subtree.delimiter.map(|it| it.kind) { None => ("", ""), Some(tt::DelimiterKind::Brace) => ("{", "}"), @@ -442,10 +441,7 @@ fn spacing_to_external(spacing: Spacing) -> bridge::Spacing { impl server::Group for Rustc { fn new(&mut self, delimiter: bridge::Delimiter, stream: Self::TokenStream) -> Self::Group { - Self::Group { - delimiter: delim_to_internal(delimiter), - token_trees: stream.subtree.token_trees, - } + Self::Group { delimiter: delim_to_internal(delimiter), token_trees: stream.token_trees } } fn delimiter(&mut self, group: &Self::Group) -> bridge::Delimiter { delim_to_external(group.delimiter) @@ -453,9 +449,7 @@ fn delimiter(&mut self, group: &Self::Group) -> bridge::Delimiter { // NOTE: Return value of do not include delimiter fn stream(&mut self, group: &Self::Group) -> Self::TokenStream { - TokenStream { - subtree: tt::Subtree { delimiter: None, token_trees: group.token_trees.clone() }, - } + TokenStream { token_trees: group.token_trees.clone() } } fn span(&mut self, group: &Self::Group) -> Self::Span { @@ -764,26 +758,23 @@ fn test_rustc_server_literals() { #[test] fn test_rustc_server_to_string() { let s = TokenStream { - subtree: tt::Subtree { - delimiter: None, - token_trees: vec![ - tt::TokenTree::Leaf(tt::Leaf::Ident(tt::Ident { - text: "struct".into(), + token_trees: vec![ + tt::TokenTree::Leaf(tt::Leaf::Ident(tt::Ident { + text: "struct".into(), + id: tt::TokenId::unspecified(), + })), + tt::TokenTree::Leaf(tt::Leaf::Ident(tt::Ident { + text: "T".into(), + id: tt::TokenId::unspecified(), + })), + tt::TokenTree::Subtree(tt::Subtree { + delimiter: Some(tt::Delimiter { id: tt::TokenId::unspecified(), - })), - tt::TokenTree::Leaf(tt::Leaf::Ident(tt::Ident { - text: "T".into(), - id: tt::TokenId::unspecified(), - })), - tt::TokenTree::Subtree(tt::Subtree { - delimiter: Some(tt::Delimiter { - id: tt::TokenId::unspecified(), - kind: tt::DelimiterKind::Brace, - }), - token_trees: vec![], + kind: tt::DelimiterKind::Brace, }), - ], - }, + token_trees: vec![], + }), + ], }; assert_eq!(s.to_string(), "struct T {}"); @@ -804,11 +795,11 @@ fn test_rustc_server_from_str() { }); let t1 = TokenStream::from_str("(a)").unwrap(); - assert_eq!(t1.subtree.token_trees.len(), 1); - assert_eq!(t1.subtree.token_trees[0], subtree_paren_a); + assert_eq!(t1.token_trees.len(), 1); + assert_eq!(t1.token_trees[0], subtree_paren_a); let t2 = TokenStream::from_str("(a);").unwrap(); - assert_eq!(t2.subtree.token_trees.len(), 2); - assert_eq!(t2.subtree.token_trees[0], subtree_paren_a); + assert_eq!(t2.token_trees.len(), 2); + assert_eq!(t2.token_trees[0], subtree_paren_a); } } diff --git a/crates/proc_macro_srv/src/tests/utils.rs b/crates/proc_macro_srv/src/tests/utils.rs index 22813052df3..0484c3af449 100644 --- a/crates/proc_macro_srv/src/tests/utils.rs +++ b/crates/proc_macro_srv/src/tests/utils.rs @@ -52,7 +52,7 @@ pub fn assert_expand( let expander = dylib::Expander::new(&path).unwrap(); let fixture = parse_string(ra_fixture).unwrap(); - let res = expander.expand(macro_name, &fixture.subtree, None).unwrap(); + let res = expander.expand(macro_name, &fixture.into_subtree(), None).unwrap(); assert_eq_text!(&expect.trim(), &format!("{:?}", res)); } From 93c9b346359a1ca70126242f740e6fc0911c535b Mon Sep 17 00:00:00 2001 From: Kevin Mehall Date: Sat, 6 Mar 2021 09:51:13 -0700 Subject: [PATCH 3/4] Make a placeholder panic message explain its purpose --- crates/proc_macro_srv/src/proc_macro/bridge/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/proc_macro_srv/src/proc_macro/bridge/client.rs b/crates/proc_macro_srv/src/proc_macro/bridge/client.rs index ca6749b9bc7..b036d4e2016 100644 --- a/crates/proc_macro_srv/src/proc_macro/bridge/client.rs +++ b/crates/proc_macro_srv/src/proc_macro/bridge/client.rs @@ -238,7 +238,7 @@ macro_rules! define_client_side { $(impl $name { #[allow(unused)] $(pub(crate) fn $method($($arg: $arg_ty),*) $(-> $ret_ty)* { - panic!("hello"); + panic!("crates should be linked against the sysroot version of proc_macro, not this one from rust-analyzer"); // Bridge::with(|bridge| { // let mut b = bridge.cached_buffer.take(); From aea974939064b0f7b83de371a93ee4190c80e544 Mon Sep 17 00:00:00 2001 From: Kevin Mehall Date: Sat, 6 Mar 2021 12:30:43 -0700 Subject: [PATCH 4/4] Move TokenStream::to_string helpers inside the method --- crates/proc_macro_srv/src/rustc_server.rs | 64 ++++++++++++----------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/crates/proc_macro_srv/src/rustc_server.rs b/crates/proc_macro_srv/src/rustc_server.rs index 2798dbf0d7f..ceefd187d25 100644 --- a/crates/proc_macro_srv/src/rustc_server.rs +++ b/crates/proc_macro_srv/src/rustc_server.rs @@ -199,39 +199,43 @@ fn from_str(src: &str) -> Result { impl ToString for TokenStream { fn to_string(&self) -> String { - tokentrees_to_text(&self.token_trees[..]) - } - } + return tokentrees_to_text(&self.token_trees[..]); - fn tokentrees_to_text(tkns: &[tt::TokenTree]) -> String { - tkns.iter() - .fold((String::new(), true), |(last, last_to_joint), tkn| { - let s = [last, tokentree_to_text(tkn)].join(if last_to_joint { "" } else { " " }); - let mut is_joint = false; - if let tt::TokenTree::Leaf(tt::Leaf::Punct(punct)) = tkn { - if punct.spacing == tt::Spacing::Joint { - is_joint = true; + fn tokentrees_to_text(tkns: &[tt::TokenTree]) -> String { + tkns.iter() + .fold((String::new(), true), |(last, last_to_joint), tkn| { + let s = [last, tokentree_to_text(tkn)].join(if last_to_joint { + "" + } else { + " " + }); + let mut is_joint = false; + if let tt::TokenTree::Leaf(tt::Leaf::Punct(punct)) = tkn { + if punct.spacing == tt::Spacing::Joint { + is_joint = true; + } + } + (s, is_joint) + }) + .0 + } + + fn tokentree_to_text(tkn: &tt::TokenTree) -> String { + match tkn { + tt::TokenTree::Leaf(tt::Leaf::Ident(ident)) => ident.text.clone().into(), + tt::TokenTree::Leaf(tt::Leaf::Literal(literal)) => literal.text.clone().into(), + tt::TokenTree::Leaf(tt::Leaf::Punct(punct)) => format!("{}", punct.char), + tt::TokenTree::Subtree(subtree) => { + let content = tokentrees_to_text(&subtree.token_trees); + let (open, close) = match subtree.delimiter.map(|it| it.kind) { + None => ("", ""), + Some(tt::DelimiterKind::Brace) => ("{", "}"), + Some(tt::DelimiterKind::Parenthesis) => ("(", ")"), + Some(tt::DelimiterKind::Bracket) => ("[", "]"), + }; + format!("{}{}{}", open, content, close) } } - (s, is_joint) - }) - .0 - } - - fn tokentree_to_text(tkn: &tt::TokenTree) -> String { - match tkn { - tt::TokenTree::Leaf(tt::Leaf::Ident(ident)) => ident.text.clone().into(), - tt::TokenTree::Leaf(tt::Leaf::Literal(literal)) => literal.text.clone().into(), - tt::TokenTree::Leaf(tt::Leaf::Punct(punct)) => format!("{}", punct.char), - tt::TokenTree::Subtree(subtree) => { - let content = tokentrees_to_text(&subtree.token_trees); - let (open, close) = match subtree.delimiter.map(|it| it.kind) { - None => ("", ""), - Some(tt::DelimiterKind::Brace) => ("{", "}"), - Some(tt::DelimiterKind::Parenthesis) => ("(", ")"), - Some(tt::DelimiterKind::Bracket) => ("[", "]"), - }; - format!("{}{}{}", open, content, close) } } }