Merge #1209

1209: Bugs fixes And Improvements of MBE r=matklad a=edwin0cheng

This PR fixed / improve followings things:

* Add `token` `$repeat` separator support: Previously $repeat only support single punct separator. 
* Fixed a bug which expand infinite pattern, see `test_match_group_in_group` 
* Correctly handle +,*,? case of $repeat patterns
* Increase the limit of $repeat patterns (128 => 65536), personally i think we could remove this limit as we seem to fix all major loop bugs
* **Re-enable tt matcher**
* Better meta item parsing.
* Add related tests and add some real world test cases.
* Add more debug information.


Co-authored-by: Edwin Cheng <[email protected]>

9 files changed, 541 insertions, 87 deletions

diff --git a/crates/ra_hir/src/ids.rs b/crates/ra_hir/src/ids.rs
index c7849c995..b0e9b1f9a 100644
--- a/crates/ra_hir/src/ids.rs
+++ b/crates/ra_hir/src/ids.rs
@@ -128,8 +128,14 @@ pub struct MacroDefId(pub(crate) AstId<ast::MacroCall>);
 pub(crate) fn macro_def_query(db: &impl DefDatabase, id: MacroDefId) -> Option<Arc<MacroRules>> {
     let macro_call = id.0.to_node(db);
     let arg = macro_call.token_tree()?;
-    let (tt, _) = mbe::ast_to_token_tree(arg)?;
-    let rules = MacroRules::parse(&tt).ok()?;
+    let (tt, _) = mbe::ast_to_token_tree(arg).or_else(|| {
+        log::warn!("fail on macro_def to token tree: {:#?}", arg);
+        None
+    })?;
+    let rules = MacroRules::parse(&tt).ok().or_else(|| {
+        log::warn!("fail on macro_def parse: {:#?}", tt);
+        None
+    })?;
     Some(Arc::new(rules))
 }
 
diff --git a/crates/ra_mbe/Cargo.toml b/crates/ra_mbe/Cargo.toml
index 1d0c2a340..68f559295 100644
--- a/crates/ra_mbe/Cargo.toml
+++ b/crates/ra_mbe/Cargo.toml
@@ -10,3 +10,5 @@ ra_parser = { path = "../ra_parser" }
 tt = { path = "../ra_tt", package = "ra_tt" }
 itertools = "0.8.0"
 rustc-hash = "1.0.0"
+smallvec = "0.6.9"
+log = "0.4.5"
diff --git a/crates/ra_mbe/src/lib.rs b/crates/ra_mbe/src/lib.rs
index eedc0c5dd..7817232d6 100644
--- a/crates/ra_mbe/src/lib.rs
+++ b/crates/ra_mbe/src/lib.rs
@@ -24,6 +24,7 @@ mod subtree_source;
 mod subtree_parser;
 
 use ra_syntax::SmolStr;
+use smallvec::SmallVec;
 
 pub use tt::{Delimiter, Punct};
 
@@ -99,10 +100,17 @@ pub(crate) struct Subtree {
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
+pub(crate) enum Separator {
+    Literal(tt::Literal),
+    Ident(tt::Ident),
+    Puncts(SmallVec<[tt::Punct; 3]>),
+}
+
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub(crate) struct Repeat {
     pub(crate) subtree: Subtree,
     pub(crate) kind: RepeatKind,
-    pub(crate) separator: Option<char>,
+    pub(crate) separator: Option<Separator>,
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -175,8 +183,8 @@ impl_froms!(TokenTree: Leaf, Subtree);
         let expansion = rules.expand(&invocation_tt).unwrap();
         assert_eq!(
         expansion.to_string(),
-        "impl From < Leaf > for TokenTree {fn from (it : Leaf) -> TokenTree {TokenTree :: Leaf (it)}} \
-         impl From < Subtree > for TokenTree {fn from (it : Subtree) -> TokenTree {TokenTree :: Subtree (it)}}"
+        "impl From <Leaf > for TokenTree {fn from (it : Leaf) -> TokenTree {TokenTree ::Leaf (it)}} \
+         impl From <Subtree > for TokenTree {fn from (it : Subtree) -> TokenTree {TokenTree ::Subtree (it)}}"
     )
     }
 
@@ -384,7 +392,7 @@ impl_froms!(TokenTree: Leaf, Subtree);
 "#,
         );
 
-        assert_expansion(&rules, "foo! { foo, bar }", "fn baz {foo () ; bar () ;}");
+        assert_expansion(&rules, "foo! { foo, bar }", "fn baz {foo () ;bar ()}");
     }
 
     #[test]
@@ -417,6 +425,42 @@ impl_froms!(TokenTree: Leaf, Subtree);
     }
 
     #[test]
+    fn test_match_group_with_multichar_sep() {
+        let rules = create_rules(
+            r#"
+        macro_rules! foo {            
+            (fn $name:ident {$($i:literal)*} ) => ( fn $name() -> bool { $($i)&&*} );            
+        }"#,
+        );
+
+        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(
             "
@@ -597,7 +641,7 @@ MACRO_ITEMS@[0; 40)
         assert_expansion(
             &rules,
             "foo! { bar::<u8>::baz::<u8> }",
-            "fn foo () {let a = bar :: < u8 > :: baz :: < u8 > ;}",
+            "fn foo () {let a = bar ::< u8 >:: baz ::< u8 > ;}",
         );
     }
 
@@ -891,7 +935,7 @@ MACRO_ITEMS@[0; 40)
         }
 "#,
         );
-        assert_expansion(&rules, r#"foo!{'a}"#, r#"struct Ref < 'a > {s : & 'a str}"#);
+        assert_expansion(&rules, r#"foo!{'a}"#, r#"struct Ref <'a > {s : &'a str}"#);
     }
 
     #[test]
@@ -1063,7 +1107,165 @@ macro_rules! int_base {
         );
 
         assert_expansion(&rules, r#" int_base!{Binary for isize as usize -> Binary}"#, 
-        "# [stable (feature = \"rust1\" , since = \"1.0.0\")] impl fmt :: Binary for isize {fn fmt (& self , f : & mut fmt :: Formatter < \'_ >) -> fmt :: Result {Binary . fmt_int (* self as usize , f)}}"
+        "# [stable (feature = \"rust1\" , since = \"1.0.0\")] impl fmt ::Binary for isize {fn fmt (& self , f : & mut fmt :: Formatter < \'_ >) -> fmt :: Result {Binary . fmt_int (* self as usize , f)}}"
+        );
+    }
+
+    #[test]
+    fn test_generate_pattern_iterators() {
+        // from https://github.com/rust-lang/rust/blob/316a391dcb7d66dc25f1f9a4ec9d368ef7615005/src/libcore/str/mod.rs
+        let rules = create_rules(
+            r#"
+macro_rules! generate_pattern_iterators {        
+        { double ended; with $(#[$common_stability_attribute:meta])*,
+                           $forward_iterator:ident,
+                           $reverse_iterator:ident, $iterty:ty
+        } => {
+            fn foo(){}
+        }
+}
+"#,
+        );
+
+        assert_expansion(&rules, r#"generate_pattern_iterators ! ( double ended ; with # [ stable ( feature = "rust1" , since = "1.0.0" ) ] , Split , RSplit , & 'a str )"#, 
+        "fn foo () {}");
+    }
+
+    #[test]
+    fn test_impl_fn_for_zst() {
+        // from https://github.com/rust-lang/rust/blob/5d20ff4d2718c820632b38c1e49d4de648a9810b/src/libcore/internal_macros.rs
+        let rules = create_rules(
+            r#"
+macro_rules! impl_fn_for_zst  {        
+        {  $( $( #[$attr: meta] )*
+        struct $Name: ident impl$( <$( $lifetime : lifetime ),+> )? Fn =
+            |$( $arg: ident: $ArgTy: ty ),*| -> $ReturnTy: ty
+$body: block; )+
+        } => {
+           $(
+            $( #[$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, )*))
+                }
+            }
+        )+
+}
+        }
+}
+"#,
+        );
+
+        assert_expansion(&rules, r#"
+impl_fn_for_zst !   { 
+     # [ derive ( Clone ) ] 
+     struct   CharEscapeDebugContinue   impl   Fn   =   | c :   char |   ->   char :: EscapeDebug   { 
+         c . escape_debug_ext ( false ) 
+     } ; 
+
+     # [ derive ( Clone ) ] 
+     struct   CharEscapeUnicode   impl   Fn   =   | c :   char |   ->   char :: EscapeUnicode   { 
+         c . escape_unicode ( ) 
+     } ; 
+     # [ derive ( Clone ) ] 
+     struct   CharEscapeDefault   impl   Fn   =   | c :   char |   ->   char :: EscapeDefault   { 
+         c . escape_default ( ) 
+     } ; 
+ }
+"#, 
+        "# [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]
+    fn test_impl_nonzero_fmt() {
+        // from https://github.com/rust-lang/rust/blob/316a391dcb7d66dc25f1f9a4ec9d368ef7615005/src/libcore/num/mod.rs#L12
+        let rules = create_rules(
+            r#"
+        macro_rules! impl_nonzero_fmt {
+            ( #[$stability: meta] ( $( $Trait: ident ),+ ) for $Ty: ident ) => {
+                fn foo() {}
+            }
+        }
+"#,
         );
+
+        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 :: * ;)) ,}");
     }
 }
diff --git a/crates/ra_mbe/src/mbe_expander.rs b/crates/ra_mbe/src/mbe_expander.rs
index 00fb09a3b..d5189b537 100644
--- a/crates/ra_mbe/src/mbe_expander.rs
+++ b/crates/ra_mbe/src/mbe_expander.rs
@@ -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.
@@ -179,10 +182,10 @@ fn match_lhs(pattern: &crate::Subtree, input: &mut TtCursor) -> Result<Bindings,
                         // Enable followiing code when everything is fixed
                         // At least we can dogfood itself to not stackoverflow
                         //
-                        // "tt" => {
-                        //     let token = input.eat().ok_or(ExpandError::UnexpectedToken)?.clone();
-                        //     res.inner.insert(text.clone(), Binding::Simple(token.into()));
-                        // }
+                        "tt" => {
+                            let token = input.eat().ok_or(ExpandError::UnexpectedToken)?.clone();
+                            res.inner.insert(text.clone(), Binding::Simple(token.into()));
+                        }
                         "item" => {
                             let item =
                                 input.eat_item().ok_or(ExpandError::UnexpectedToken)?.clone();
@@ -196,6 +199,7 @@ fn match_lhs(pattern: &crate::Subtree, input: &mut TtCursor) -> Result<Bindings,
                         "literal" => {
                             let literal =
                                 input.eat_literal().ok_or(ExpandError::UnexpectedToken)?.clone();
+
                             res.inner.insert(
                                 text.clone(),
                                 Binding::Simple(tt::Leaf::from(literal).into()),
@@ -210,7 +214,7 @@ fn match_lhs(pattern: &crate::Subtree, input: &mut TtCursor) -> Result<Bindings,
                     }
                 }
                 crate::Leaf::Punct(punct) => {
-                    if input.eat_punct() != Some(punct) {
+                    if !input.eat_punct().map(|p| p.char == punct.char).unwrap_or(false) {
                         return Err(ExpandError::UnexpectedToken);
                     }
                 }
@@ -224,20 +228,54 @@ 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;
-                while let Ok(nested) = match_lhs(subtree, input) {
-                    counter += 1;
-                    limit -= 1;
-                    if limit == 0 {
-                        break;
-                    }
-                    res.push_nested(nested)?;
-                    if let Some(separator) = *separator {
-                        if !input.is_eof() {
-                            if input.eat_punct().map(|p| p.char) != Some(separator) {
-                                return Err(ExpandError::UnexpectedToken);
+
+                let mut memento = input.save();
+
+                loop {
+                    match match_lhs(subtree, input) {
+                        Ok(nested) => {
+                            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;
+                            }
+
+                            memento = input.save();
+                            res.push_nested(nested)?;
+                            if counter == 1 {
+                                if let crate::RepeatKind::ZeroOrOne = kind {
+                                    break;
+                                }
                             }
+
+                            if let Some(separator) = separator {
+                                use crate::Separator::*;
+
+                                if !input
+                                    .eat_seperator()
+                                    .map(|sep| match (sep, separator) {
+                                        (Ident(ref a), Ident(ref b)) => a.text == b.text,
+                                        (Literal(ref a), Literal(ref b)) => a.text == b.text,
+                                        (Puncts(ref a), Puncts(ref b)) if a.len() == b.len() => {
+                                            let a_iter = a.iter().map(|a| a.char);
+                                            let b_iter = b.iter().map(|b| b.char);
+                                            a_iter.eq(b_iter)
+                                        }
+                                        _ => false,
+                                    })
+                                    .unwrap_or(false)
+                                {
+                                    input.rollback(memento);
+                                    break;
+                                }
+                            }
+                        }
+                        Err(_) => {
+                            input.rollback(memento);
+                            break;
                         }
                     }
                 }
@@ -246,10 +284,6 @@ fn match_lhs(pattern: &crate::Subtree, input: &mut TtCursor) -> Result<Bindings,
                     crate::RepeatKind::OneOrMore if counter == 0 => {
                         return Err(ExpandError::UnexpectedToken);
                     }
-                    crate::RepeatKind::ZeroOrOne if counter > 1 => {
-                        return Err(ExpandError::UnexpectedToken);
-                    }
-
                     _ => {}
                 }
             }
@@ -273,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 })
@@ -303,43 +343,81 @@ 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 has_sep = false;
+            let mut limit = 65536;
+            let mut has_seps = 0;
+            let mut counter = 0;
 
-            while let Ok(t) = expand_subtree(&repeat.subtree, bindings, nesting) {
+            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;
+
+                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(sep) = repeat.separator {
-                    let punct =
-                        tt::Leaf::from(tt::Punct { char: sep, spacing: tt::Spacing::Alone });
-                    token_trees.push(punct.into());
-                    has_sep = true;
+                if let Some(ref sep) = repeat.separator {
+                    match sep {
+                        crate::Separator::Ident(ident) => {
+                            has_seps = 1;
+                            token_trees.push(tt::Leaf::from(ident.clone()).into());
+                        }
+                        crate::Separator::Literal(lit) => {
+                            has_seps = 1;
+                            token_trees.push(tt::Leaf::from(lit.clone()).into());
+                        }
+
+                        crate::Separator::Puncts(puncts) => {
+                            has_seps = puncts.len();
+                            for punct in puncts {
+                                token_trees.push(tt::Leaf::from(*punct).into());
+                            }
+                        }
+                    }
+                }
+
+                if let crate::RepeatKind::ZeroOrOne = repeat.kind {
+                    break;
                 }
             }
-            nesting.pop().unwrap();
 
-            // Dirty hack for remove the last sep
-            // if it is a "," undo the push
-            if has_sep && repeat.separator.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 })
@@ -356,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)
diff --git a/crates/ra_mbe/src/mbe_parser.rs b/crates/ra_mbe/src/mbe_parser.rs
index 0710062d9..c7ab463e2 100644
--- a/crates/ra_mbe/src/mbe_parser.rs
+++ b/crates/ra_mbe/src/mbe_parser.rs
@@ -74,18 +74,11 @@ fn parse_var(p: &mut TtCursor, transcriber: bool) -> Result<crate::Var, ParseErr
     Ok(crate::Var { text, kind })
 }
 
-fn parse_repeat(p: &mut TtCursor, transcriber: bool) -> Result<crate::Repeat, ParseError> {
-    let subtree = p.eat_subtree().unwrap();
-    let mut subtree = parse_subtree(subtree, transcriber)?;
-    subtree.delimiter = crate::Delimiter::None;
-    let sep = p.eat_punct().ok_or(ParseError::Expected(String::from("separator")))?;
-    let (separator, rep) = match sep.char {
-        '*' | '+' | '?' => (None, sep.char),
-        char => {
-            (Some(char), p.eat_punct().ok_or(ParseError::Expected(String::from("separator")))?.char)
-        }
-    };
-
+fn mk_repeat(
+    rep: char,
+    subtree: crate::Subtree,
+    separator: Option<crate::Separator>,
+) -> Result<crate::Repeat, ParseError> {
     let kind = match rep {
         '*' => crate::RepeatKind::ZeroOrMore,
         '+' => crate::RepeatKind::OneOrMore,
@@ -95,6 +88,27 @@ fn parse_repeat(p: &mut TtCursor, transcriber: bool) -> Result<crate::Repeat, Pa
     Ok(crate::Repeat { subtree, kind, separator })
 }
 
+fn parse_repeat(p: &mut TtCursor, transcriber: bool) -> Result<crate::Repeat, ParseError> {
+    let subtree = p.eat_subtree().unwrap();
+    let mut subtree = parse_subtree(subtree, transcriber)?;
+    subtree.delimiter = crate::Delimiter::None;
+
+    if let Some(rep) = p.at_punct() {
+        match rep.char {
+            '*' | '+' | '?' => {
+                p.bump();
+                return mk_repeat(rep.char, subtree, None);
+            }
+            _ => {}
+        }
+    }
+
+    let sep = p.eat_seperator().ok_or(ParseError::Expected(String::from("separator")))?;
+    let rep = p.eat_punct().ok_or(ParseError::Expected(String::from("repeat")))?;
+
+    mk_repeat(rep.char, subtree, Some(sep))
+}
+
 #[cfg(test)]
 mod tests {
     use ra_syntax::{ast, AstNode};
@@ -109,7 +123,7 @@ mod tests {
         is_valid("($i:ident) => ()");
         expect_err("$i:ident => ()", "subtree");
         expect_err("($i:ident) ()", "`=`");
-        expect_err("($($i:ident)_) => ()", "separator");
+        expect_err("($($i:ident)_) => ()", "repeat");
     }
 
     fn expect_err(macro_body: &str, expected: &str) {
diff --git a/crates/ra_mbe/src/subtree_source.rs b/crates/ra_mbe/src/subtree_source.rs
index 3229cfa8f..6255ea304 100644
--- a/crates/ra_mbe/src/subtree_source.rs
+++ b/crates/ra_mbe/src/subtree_source.rs
@@ -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
@@ -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))
     }
 }
 
@@ -342,7 +343,7 @@ impl<'a> TokenSource for SubtreeTokenSource<'a> {
     }
 }
 
-struct TokenPeek<'a, I>
+pub(crate) struct TokenPeek<'a, I>
 where
     I: Iterator<Item = &'a tt::TokenTree>,
 {
@@ -365,7 +366,7 @@ where
         TokenPeek { iter: itertools::multipeek(iter) }
     }
 
-    fn current_punct2(&mut self, p: &tt::Punct) -> Option<((char, char), bool)> {
+    pub fn current_punct2(&mut self, p: &tt::Punct) -> Option<((char, char), bool)> {
         if p.spacing != tt::Spacing::Joint {
             return None;
         }
@@ -375,7 +376,7 @@ where
         Some(((p.char, p1.char), p1.spacing == tt::Spacing::Joint))
     }
 
-    fn current_punct3(&mut self, p: &tt::Punct) -> Option<((char, char, char), bool)> {
+    pub fn current_punct3(&mut self, p: &tt::Punct) -> Option<((char, char, char), bool)> {
         self.current_punct2(p).and_then(|((p0, p1), last_joint)| {
             if !last_joint {
                 None
@@ -437,12 +438,14 @@ fn convert_delim(d: tt::Delimiter, closing: bool) -> TtToken {
 }
 
 fn convert_literal(l: &tt::Literal) -> TtToken {
-    TtToken {
-        kind: classify_literal(&l.text).unwrap().kind,
-        is_joint_to_next: false,
-        text: l.text.clone(),
-        n_tokens: 1,
-    }
+    let kind =
+        classify_literal(&l.text).map(|tkn| tkn.kind).unwrap_or_else(|| match l.text.as_ref() {
+            "true" => SyntaxKind::TRUE_KW,
+            "false" => SyntaxKind::FALSE_KW,
+            _ => panic!("Fail to convert given literal {:#?}", &l),
+        });
+
+    TtToken { kind, is_joint_to_next: false, text: l.text.clone(), n_tokens: 1 }
 }
 
 fn convert_ident(ident: &tt::Ident) -> TtToken {
diff --git a/crates/ra_mbe/src/syntax_bridge.rs b/crates/ra_mbe/src/syntax_bridge.rs
index 38a481029..e0f228ce9 100644
--- a/crates/ra_mbe/src/syntax_bridge.rs
+++ b/crates/ra_mbe/src/syntax_bridge.rs
@@ -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()) {
@@ -133,7 +138,9 @@ fn convert_tt(
     };
 
     let mut token_trees = Vec::new();
-    for child in tt.children_with_tokens().skip(skip_first as usize) {
+    let mut child_iter = tt.children_with_tokens().skip(skip_first as usize).peekable();
+
+    while let Some(child) = child_iter.next() {
         if (skip_first && (child == first_child || child == last_child)) || child.kind().is_trivia()
         {
             continue;
@@ -152,12 +159,25 @@ fn convert_tt(
                         prev = Some(char)
                     }
                     if let Some(char) = prev {
-                        token_trees.push(
-                            tt::Leaf::from(tt::Punct { char, spacing: tt::Spacing::Alone }).into(),
-                        );
+                        let spacing = match child_iter.peek() {
+                            Some(SyntaxElement::Token(token)) => {
+                                if token.kind().is_punct() {
+                                    tt::Spacing::Joint
+                                } else {
+                                    tt::Spacing::Alone
+                                }
+                            }
+                            _ => tt::Spacing::Alone,
+                        };
+
+                        token_trees.push(tt::Leaf::from(tt::Punct { char, spacing }).into());
                     }
                 } else {
-                    let child: tt::TokenTree = if token.kind().is_keyword()
+                    let child: tt::TokenTree = if token.kind() == SyntaxKind::TRUE_KW
+                        || token.kind() == SyntaxKind::FALSE_KW
+                    {
+                        tt::Leaf::from(tt::Literal { text: token.text().clone() }).into()
+                    } else if token.kind().is_keyword()
                         || token.kind() == IDENT
                         || token.kind() == LIFETIME
                     {
@@ -218,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) {
diff --git a/crates/ra_mbe/src/tt_cursor.rs b/crates/ra_mbe/src/tt_cursor.rs
index 741b5ea1c..eef642a9c 100644
--- a/crates/ra_mbe/src/tt_cursor.rs
+++ b/crates/ra_mbe/src/tt_cursor.rs
@@ -1,12 +1,18 @@
 use crate::ParseError;
 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,
 }
 
+pub(crate) struct TtCursorMemento {
+    pos: usize,
+}
+
 impl<'a> TtCursor<'a> {
     pub(crate) fn new(subtree: &'a tt::Subtree) -> TtCursor<'a> {
         TtCursor { subtree, pos: 0 }
@@ -157,4 +163,102 @@ impl<'a> TtCursor<'a> {
             Err(ParseError::Expected(format!("`{}`", char)))
         }
     }
+
+    fn eat_punct3(&mut self, p: &tt::Punct) -> Option<SmallVec<[tt::Punct; 3]>> {
+        let sec = self.eat_punct()?.clone();
+        let third = self.eat_punct()?.clone();
+        Some(smallvec![p.clone(), sec, third])
+    }
+
+    fn eat_punct2(&mut self, p: &tt::Punct) -> Option<SmallVec<[tt::Punct; 3]>> {
+        let sec = self.eat_punct()?.clone();
+        Some(smallvec![p.clone(), sec])
+    }
+
+    fn eat_multi_char_punct<'b, I>(
+        &mut self,
+        p: &tt::Punct,
+        iter: &mut TokenPeek<'b, I>,
+    ) -> Option<SmallVec<[tt::Punct; 3]>>
+    where
+        I: Iterator<Item = &'b tt::TokenTree>,
+    {
+        if let Some((m, _)) = iter.current_punct3(p) {
+            if let r @ Some(_) = match m {
+                ('<', '<', '=') | ('>', '>', '=') | ('.', '.', '.') | ('.', '.', '=') => {
+                    self.eat_punct3(p)
+                }
+                _ => None,
+            } {
+                return r;
+            }
+        }
+
+        if let Some((m, _)) = iter.current_punct2(p) {
+            if let r @ Some(_) = match m {
+                ('<', '=')
+                | ('>', '=')
+                | ('+', '=')
+                | ('-', '=')
+                | ('|', '=')
+                | ('&', '=')
+                | ('^', '=')
+                | ('/', '=')
+                | ('*', '=')
+                | ('%', '=')
+                | ('&', '&')
+                | ('|', '|')
+                | ('<', '<')
+                | ('>', '>')
+                | ('-', '>')
+                | ('!', '=')
+                | ('=', '>')
+                | ('=', '=')
+                | ('.', '.')
+                | (':', ':') => self.eat_punct2(p),
+
+                _ => None,
+            } {
+                return r;
+            }
+        }
+
+        None
+    }
+
+    pub(crate) fn eat_seperator(&mut self) -> Option<crate::Separator> {
+        match self.eat()? {
+            tt::TokenTree::Leaf(tt::Leaf::Literal(lit)) => {
+                Some(crate::Separator::Literal(lit.clone()))
+            }
+            tt::TokenTree::Leaf(tt::Leaf::Ident(ident)) => {
+                Some(crate::Separator::Ident(ident.clone()))
+            }
+            tt::TokenTree::Leaf(tt::Leaf::Punct(punct)) => {
+                match punct.char {
+                    '*' | '+' | '?' => return None,
+                    _ => {}
+                };
+
+                // FIXME: The parser is only handle some compositeable punct,
+                // But at this phase, some punct still is jointed.
+                // So we by pass that check here.
+                let mut peekable = TokenPeek::new(self.subtree.token_trees[self.pos..].iter());
+                let puncts = self.eat_multi_char_punct(punct, &mut peekable);
+                let puncts = puncts.unwrap_or_else(|| smallvec![punct.clone()]);
+
+                Some(crate::Separator::Puncts(puncts))
+            }
+            _ => None,
+        }
+    }
+
+    #[must_use]
+    pub(crate) fn save(&self) -> TtCursorMemento {
+        TtCursorMemento { pos: self.pos }
+    }
+
+    pub(crate) fn rollback(&mut self, memento: TtCursorMemento) {
+        self.pos = memento.pos;
+    }
 }
diff --git a/crates/ra_parser/src/grammar.rs b/crates/ra_parser/src/grammar.rs
index 67eae749d..a538ec081 100644
--- a/crates/ra_parser/src/grammar.rs
+++ b/crates/ra_parser/src/grammar.rs
@@ -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,
+            }
         }
     }