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

Bugs fixes And Improvements of MBE #1209

Merged
merged 5 commits into from
Apr 25, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/ra_mbe/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ tt = { path = "../ra_tt", package = "ra_tt" }
itertools = "0.8.0"
rustc-hash = "1.0.0"
smallvec = "0.6.9"
log = "0.4.5"
112 changes: 110 additions & 2 deletions crates/ra_mbe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,30 @@ impl_froms!(TokenTree: Leaf, Subtree);
assert_expansion(&rules, "foo! (fn baz {true true} )", "fn baz () -> bool {true &&true}");
}

#[test]
fn test_match_group_zero_match() {
let rules = create_rules(
r#"
macro_rules! foo {
( $($i:ident)* ) => ();
}"#,
);

assert_expansion(&rules, "foo! ()", "");
}

#[test]
fn test_match_group_in_group() {
let rules = create_rules(
r#"
macro_rules! foo {
{ $( ( $($i:ident)* ) )* } => ( $( ( $($i)* ) )* );
}"#,
);

assert_expansion(&rules, "foo! ( (a b) )", "(a b)");
}

#[test]
fn test_expand_to_item_list() {
let rules = create_rules(
Expand Down Expand Up @@ -1118,7 +1142,37 @@ macro_rules! impl_fn_for_zst {
|$( $arg: ident: $ArgTy: ty ),*| -> $ReturnTy: ty
$body: block; )+
} => {
fn foo(){}
$(
$( #[$attr] )*
struct $Name;

impl $( <$( $lifetime ),+> )? Fn<($( $ArgTy, )*)> for $Name {
#[inline]
extern "rust-call" fn call(&self, ($( $arg, )*): ($( $ArgTy, )*)) -> $ReturnTy {
$body
}
}

impl $( <$( $lifetime ),+> )? FnMut<($( $ArgTy, )*)> for $Name {
#[inline]
extern "rust-call" fn call_mut(
&mut self,
($( $arg, )*): ($( $ArgTy, )*)
) -> $ReturnTy {
Fn::call(&*self, ($( $arg, )*))
}
}

impl $( <$( $lifetime ),+> )? FnOnce<($( $ArgTy, )*)> for $Name {
type Output = $ReturnTy;

#[inline]
extern "rust-call" fn call_once(self, ($( $arg, )*): ($( $ArgTy, )*)) -> $ReturnTy {
Fn::call(&self, ($( $arg, )*))
}
}
)+
}
}
}
"#,
Expand All @@ -1141,7 +1195,7 @@ impl_fn_for_zst ! {
} ;
}
"#,
"fn foo () {}");
"# [derive (Clone)] struct CharEscapeDebugContinue ; impl Fn < (char ,) > for CharEscapeDebugContinue {# [inline] extern \"rust-call\" fn call (& self , (c ,) : (char ,)) -> char :: EscapeDebug {{c . escape_debug_ext (false)}}} impl FnMut < (char ,) > for CharEscapeDebugContinue {# [inline] extern \"rust-call\" fn call_mut (& mut self , (c ,) : (char ,)) -> char :: EscapeDebug {Fn :: call (&* self , (c ,))}} impl FnOnce < (char ,) > for CharEscapeDebugContinue {type Output = char :: EscapeDebug ; # [inline] extern \"rust-call\" fn call_once (self , (c ,) : (char ,)) -> char :: EscapeDebug {Fn :: call (& self , (c ,))}} # [derive (Clone)] struct CharEscapeUnicode ; impl Fn < (char ,) > for CharEscapeUnicode {# [inline] extern \"rust-call\" fn call (& self , (c ,) : (char ,)) -> char :: EscapeUnicode {{c . escape_unicode ()}}} impl FnMut < (char ,) > for CharEscapeUnicode {# [inline] extern \"rust-call\" fn call_mut (& mut self , (c ,) : (char ,)) -> char :: EscapeUnicode {Fn :: call (&* self , (c ,))}} impl FnOnce < (char ,) > for CharEscapeUnicode {type Output = char :: EscapeUnicode ; # [inline] extern \"rust-call\" fn call_once (self , (c ,) : (char ,)) -> char :: EscapeUnicode {Fn :: call (& self , (c ,))}} # [derive (Clone)] struct CharEscapeDefault ; impl Fn < (char ,) > for CharEscapeDefault {# [inline] extern \"rust-call\" fn call (& self , (c ,) : (char ,)) -> char :: EscapeDefault {{c . escape_default ()}}} impl FnMut < (char ,) > for CharEscapeDefault {# [inline] extern \"rust-call\" fn call_mut (& mut self , (c ,) : (char ,)) -> char :: EscapeDefault {Fn :: call (&* self , (c ,))}} impl FnOnce < (char ,) > for CharEscapeDefault {type Output = char :: EscapeDefault ; # [inline] extern \"rust-call\" fn call_once (self , (c ,) : (char ,)) -> char :: EscapeDefault {Fn :: call (& self , (c ,))}}");
}

#[test]
Expand All @@ -1160,4 +1214,58 @@ impl_fn_for_zst ! {
assert_expansion(&rules, r#"impl_nonzero_fmt ! { # [ stable ( feature = "nonzero" , since = "1.28.0" ) ] ( Debug , Display , Binary , Octal , LowerHex , UpperHex ) for NonZeroU8 }"#,
"fn foo () {}");
}

#[test]
fn test_cfg_if_items() {
// from https://github.com/rust-lang/rust/blob/33fe1131cadba69d317156847be9a402b89f11bb/src/libstd/macros.rs#L986
let rules = create_rules(
r#"
macro_rules! __cfg_if_items {
(($($not:meta,)*) ; ) => {};
(($($not:meta,)*) ; ( ($($m:meta),*) ($($it:item)*) ), $($rest:tt)*) => {
__cfg_if_items! { ($($not,)* $($m,)*) ; $($rest)* }
}
}
"#,
);

assert_expansion(&rules, r#"__cfg_if_items ! { ( rustdoc , ) ; ( ( ) ( # [ cfg ( any ( target_os = "redox" , unix ) ) ] # [ stable ( feature = "rust1" , since = "1.0.0" ) ] pub use sys :: ext as unix ; # [ cfg ( windows ) ] # [ stable ( feature = "rust1" , since = "1.0.0" ) ] pub use sys :: ext as windows ; # [ cfg ( any ( target_os = "linux" , target_os = "l4re" ) ) ] pub mod linux ; ) ) , }"#,
"__cfg_if_items ! {(rustdoc , ) ; }");
}

#[test]
fn test_cfg_if_main() {
// from https://github.com/rust-lang/rust/blob/3d211248393686e0f73851fc7548f6605220fbe1/src/libpanic_unwind/macros.rs#L9
let rules = create_rules(
r#"
macro_rules! cfg_if {
($(
if #[cfg($($meta:meta),*)] { $($it:item)* }
) else * else {
$($it2:item)*
}) => {
__cfg_if_items! {
() ;
$( ( ($($meta),*) ($($it)*) ), )*
( () ($($it2)*) ),
}
}
}
"#,
);

assert_expansion(&rules, r#"
cfg_if ! {
if # [ cfg ( target_env = "msvc" ) ] {
// no extra unwinder support needed
} else if # [ cfg ( all ( target_arch = "wasm32" , not ( target_os = "emscripten" ) ) ) ] {
// no unwinder on the system!
} else {
mod libunwind ;
pub use libunwind :: * ;
}
}
"#,
"__cfg_if_items ! {() ; (() (mod libunwind ; pub use libunwind :: * ;)) ,}");
}
}
69 changes: 54 additions & 15 deletions crates/ra_mbe/src/mbe_expander.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ fn expand_rule(rule: &crate::Rule, input: &tt::Subtree) -> Result<tt::Subtree, E
if !input.is_eof() {
return Err(ExpandError::UnexpectedToken);
}
expand_subtree(&rule.rhs, &bindings, &mut Vec::new())

let mut ctx = ExpandCtx { bindings: &bindings, nesting: Vec::new(), var_expanded: false };

expand_subtree(&rule.rhs, &mut ctx)
}

/// The actual algorithm for expansion is not too hard, but is pretty tricky.
Expand Down Expand Up @@ -225,7 +228,7 @@ fn match_lhs(pattern: &crate::Subtree, input: &mut TtCursor) -> Result<Bindings,
crate::TokenTree::Repeat(crate::Repeat { subtree, kind, separator }) => {
// Dirty hack to make macro-expansion terminate.
// This should be replaced by a propper macro-by-example implementation
let mut limit = 128;
let mut limit = 65536;
let mut counter = 0;

let mut memento = input.save();
Expand All @@ -236,6 +239,7 @@ fn match_lhs(pattern: &crate::Subtree, input: &mut TtCursor) -> Result<Bindings,
counter += 1;
limit -= 1;
if limit == 0 {
log::warn!("match_lhs excced in repeat pattern exceed limit => {:#?}\n{:#?}\n{:#?}\n{:#?}", subtree, input, kind, separator);
break;
}

Expand Down Expand Up @@ -303,15 +307,21 @@ fn match_lhs(pattern: &crate::Subtree, input: &mut TtCursor) -> Result<Bindings,
Ok(res)
}

#[derive(Debug)]
struct ExpandCtx<'a> {
bindings: &'a Bindings,
nesting: Vec<usize>,
var_expanded: bool,
}

fn expand_subtree(
template: &crate::Subtree,
bindings: &Bindings,
nesting: &mut Vec<usize>,
ctx: &mut ExpandCtx,
) -> Result<tt::Subtree, ExpandError> {
let token_trees = template
.token_trees
.iter()
.map(|it| expand_tt(it, bindings, nesting))
.map(|it| expand_tt(it, ctx))
.collect::<Result<Vec<_>, ExpandError>>()?;

Ok(tt::Subtree { token_trees, delimiter: template.delimiter })
Expand All @@ -333,26 +343,43 @@ fn reduce_single_token(mut subtree: tt::Subtree) -> tt::TokenTree {

fn expand_tt(
template: &crate::TokenTree,
bindings: &Bindings,
nesting: &mut Vec<usize>,
ctx: &mut ExpandCtx,
) -> Result<tt::TokenTree, ExpandError> {
let res: tt::TokenTree = match template {
crate::TokenTree::Subtree(subtree) => expand_subtree(subtree, bindings, nesting)?.into(),
crate::TokenTree::Subtree(subtree) => expand_subtree(subtree, ctx)?.into(),
crate::TokenTree::Repeat(repeat) => {
let mut token_trees: Vec<tt::TokenTree> = Vec::new();
nesting.push(0);
ctx.nesting.push(0);
// Dirty hack to make macro-expansion terminate.
// This should be replaced by a propper macro-by-example implementation
let mut limit = 128;
let mut limit = 65536;
let mut has_seps = 0;
let mut counter = 0;

let mut some_var_expanded = false;
ctx.var_expanded = false;

while let Ok(t) = expand_subtree(&repeat.subtree, ctx) {
// if no var expaned in the child, we count it as a fail
if !ctx.var_expanded {
break;
}
some_var_expanded = true;
ctx.var_expanded = false;

while let Ok(t) = expand_subtree(&repeat.subtree, bindings, nesting) {
counter += 1;
limit -= 1;
if limit == 0 {
log::warn!(
"expand_tt excced in repeat pattern exceed limit => {:#?}\n{:#?}",
template,
ctx
);
break;
}
let idx = nesting.pop().unwrap();
nesting.push(idx + 1);

let idx = ctx.nesting.pop().unwrap();
ctx.nesting.push(idx + 1);
token_trees.push(reduce_single_token(t).into());

if let Some(ref sep) = repeat.separator {
Expand All @@ -374,12 +401,23 @@ fn expand_tt(
}
}
}

if let crate::RepeatKind::ZeroOrOne = repeat.kind {
break;
}
}
nesting.pop().unwrap();

ctx.var_expanded = some_var_expanded;

ctx.nesting.pop().unwrap();
for _ in 0..has_seps {
token_trees.pop();
}

if crate::RepeatKind::OneOrMore == repeat.kind && counter == 0 {
return Err(ExpandError::UnexpectedToken);
}

// Check if it is a singel token subtree without any delimiter
// e.g {Delimiter:None> ['>'] /Delimiter:None>}
reduce_single_token(tt::Subtree { token_trees, delimiter: tt::Delimiter::None })
Expand All @@ -396,7 +434,8 @@ fn expand_tt(
tt::Leaf::from(tt::Ident { text: "$crate".into(), id: TokenId::unspecified() })
.into()
} else {
let tkn = bindings.get(&v.text, nesting)?.clone();
let tkn = ctx.bindings.get(&v.text, &ctx.nesting)?.clone();
ctx.var_expanded = true;

if let tt::TokenTree::Subtree(subtree) = tkn {
reduce_single_token(subtree)
Expand Down
9 changes: 5 additions & 4 deletions crates/ra_mbe/src/subtree_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ impl<'a> SubTreeWalker<'a> {
}

pub(crate) trait Querier {
fn token(&self, uidx: usize) -> (SyntaxKind, SmolStr);
fn token(&self, uidx: usize) -> (SyntaxKind, SmolStr, bool);
}

// A wrapper class for ref cell
Expand Down Expand Up @@ -292,9 +292,10 @@ impl<'a> WalkerOwner<'a> {
}

impl<'a> Querier for WalkerOwner<'a> {
fn token(&self, uidx: usize) -> (SyntaxKind, SmolStr) {
let tkn = self.get(uidx).unwrap();
(tkn.kind, tkn.text)
fn token(&self, uidx: usize) -> (SyntaxKind, SmolStr, bool) {
self.get(uidx)
.map(|tkn| (tkn.kind, tkn.text, tkn.is_joint_to_next))
.unwrap_or_else(|| (SyntaxKind::EOF, "".into(), false))
}
}

Expand Down
16 changes: 15 additions & 1 deletion crates/ra_mbe/src/syntax_bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,11 @@ fn convert_tt(
global_offset: TextUnit,
tt: &SyntaxNode,
) -> Option<tt::Subtree> {
// This tree is empty
if tt.first_child_or_token().is_none() {
return Some(tt::Subtree { token_trees: vec![], delimiter: tt::Delimiter::None });
}

let first_child = tt.first_child_or_token()?;
let last_child = tt.last_child_or_token()?;
let (delimiter, skip_first) = match (first_child.kind(), last_child.kind()) {
Expand Down Expand Up @@ -233,7 +238,16 @@ impl<'a, Q: Querier> TreeSink for TtTreeSink<'a, Q> {
self.text_pos += TextUnit::of_str(&self.buf);
let text = SmolStr::new(self.buf.as_str());
self.buf.clear();
self.inner.token(kind, text)
self.inner.token(kind, text);

// // Add a white space to token
// let (last_kind, _, last_joint_to_next ) = self.src_querier.token(self.token_pos-n_tokens as usize);
// if !last_joint_to_next && last_kind.is_punct() {
// let (cur_kind, _, _ ) = self.src_querier.token(self.token_pos);
// if cur_kind.is_punct() {
// self.inner.token(WHITESPACE, " ".into());
// }
// }
}

fn start_node(&mut self, kind: SyntaxKind) {
Expand Down
2 changes: 1 addition & 1 deletion crates/ra_mbe/src/tt_cursor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::subtree_parser::Parser;
use crate::subtree_source::TokenPeek;
use smallvec::{SmallVec, smallvec};

#[derive(Clone)]
#[derive(Debug, Clone)]
pub(crate) struct TtCursor<'a> {
subtree: &'a tt::Subtree,
pos: usize,
Expand Down
17 changes: 16 additions & 1 deletion crates/ra_parser/src/grammar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,22 @@ pub(crate) fn meta_item(p: &mut Parser) {
items::token_tree(p);
break;
} else {
p.bump();
// https://doc.rust-lang.org/reference/attributes.html
// https://doc.rust-lang.org/reference/paths.html#simple-paths
// The start of an meta must be a simple path
match p.current() {
IDENT | COLONCOLON | SUPER_KW | SELF_KW | CRATE_KW => p.bump(),
EQ => {
p.bump();
match p.current() {
c if c.is_literal() => p.bump(),
TRUE_KW | FALSE_KW => p.bump(),
_ => {}
}
break;
}
_ => break,
}
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a test for it?

Also, this was recently changed in rust-lang/rust#57367, I am not sure if the refercne is up-to-date

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know where to add the test, as this function only called from mbe right now.
I can add that test in mbe if you want.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, right, the parser doesn't need this itself. I guess I am fine with existing tests in mbe then!

}
}

Expand Down