5 files changed, 64 insertions, 66 deletions

diff --git a/crates/hir_expand/src/ast_id_map.rs b/crates/hir_expand/src/ast_id_map.rs
index 2401b0cc5..16cf29907 100644
--- a/crates/hir_expand/src/ast_id_map.rs
+++ b/crates/hir_expand/src/ast_id_map.rs
@@ -13,7 +13,8 @@ use std::{
 };
 
 use la_arena::{Arena, Idx};
-use syntax::{ast, AstNode, AstPtr, SyntaxNode, SyntaxNodePtr};
+use profile::Count;
+use syntax::{ast, match_ast, AstNode, AstPtr, SyntaxNode, SyntaxNodePtr};
 
 /// `AstId` points to an AST node in a specific file.
 pub struct FileAstId<N: AstNode> {
@@ -62,22 +63,31 @@ type ErasedFileAstId = Idx<SyntaxNodePtr>;
 #[derive(Debug, PartialEq, Eq, Default)]
 pub struct AstIdMap {
     arena: Arena<SyntaxNodePtr>,
+    _c: Count<Self>,
 }
 
 impl AstIdMap {
     pub(crate) fn from_source(node: &SyntaxNode) -> AstIdMap {
         assert!(node.parent().is_none());
-        let mut res = AstIdMap { arena: Arena::default() };
+        let mut res = AstIdMap::default();
         // By walking the tree in breadth-first order we make sure that parents
         // get lower ids then children. That is, adding a new child does not
         // change parent's id. This means that, say, adding a new function to a
         // trait does not change ids of top-level items, which helps caching.
-        bdfs(node, |it| match ast::Item::cast(it) {
-            Some(module_item) => {
-                res.alloc(module_item.syntax());
-                true
+        bdfs(node, |it| {
+            match_ast! {
+                match it {
+                    ast::Item(module_item) => {
+                        res.alloc(module_item.syntax());
+                        true
+                    },
+                    ast::BlockExpr(block) => {
+                        res.alloc(block.syntax());
+                        true
+                    },
+                    _ => false,
+                }
             }
-            None => false,
         });
         res
     }
diff --git a/crates/hir_expand/src/builtin_derive.rs b/crates/hir_expand/src/builtin_derive.rs
index eb257579f..b7f1aae8f 100644
--- a/crates/hir_expand/src/builtin_derive.rs
+++ b/crates/hir_expand/src/builtin_derive.rs
@@ -102,7 +102,7 @@ fn parse_adt(tt: &tt::Subtree) -> Result<BasicAdtInfo, mbe::ExpandError> {
         debug!("name token not found");
         mbe::ExpandError::ConversionError
     })?;
-    let name_token = tt::Ident { id: name_token_id, text: name.text().clone() };
+    let name_token = tt::Ident { id: name_token_id, text: name.text().into() };
     let type_params = params.map_or(0, |type_param_list| type_param_list.type_params().count());
     Ok(BasicAdtInfo { name: name_token, type_params })
 }
diff --git a/crates/hir_expand/src/builtin_macro.rs b/crates/hir_expand/src/builtin_macro.rs
index 80b60d59f..eb57ea7d6 100644
--- a/crates/hir_expand/src/builtin_macro.rs
+++ b/crates/hir_expand/src/builtin_macro.rs
@@ -6,7 +6,7 @@ use crate::{
 
 use base_db::{AnchoredPath, FileId};
 use either::Either;
-use mbe::{parse_to_token_tree, ExpandResult};
+use mbe::{parse_exprs_with_sep, parse_to_token_tree, ExpandResult};
 use parser::FragmentKind;
 use syntax::ast::{self, AstToken};
 
@@ -182,25 +182,10 @@ fn assert_expand(
     // ```,
     // which is wrong but useful.
 
-    let mut args = Vec::new();
-    let mut current = Vec::new();
-    for tt in tt.token_trees.iter().cloned() {
-        match tt {
-            tt::TokenTree::Leaf(tt::Leaf::Punct(p)) if p.char == ',' => {
-                args.push(current);
-                current = Vec::new();
-            }
-            _ => {
-                current.push(tt);
-            }
-        }
-    }
-    if !current.is_empty() {
-        args.push(current);
-    }
+    let args = parse_exprs_with_sep(tt, ',');
 
     let arg_tts = args.into_iter().flat_map(|arg| {
-        quote! { &(##arg), }
+        quote! { &(#arg), }
     }.token_trees).collect::<Vec<_>>();
 
     let expanded = quote! {
@@ -238,35 +223,21 @@ fn format_args_expand(
     // ])
     // ```,
     // which is still not really correct, but close enough for now
-    let mut args = Vec::new();
-    let mut current = Vec::new();
-    for tt in tt.token_trees.iter().cloned() {
-        match tt {
-            tt::TokenTree::Leaf(tt::Leaf::Punct(p)) if p.char == ',' => {
-                args.push(current);
-                current = Vec::new();
-            }
-            _ => {
-                current.push(tt);
-            }
-        }
-    }
-    if !current.is_empty() {
-        args.push(current);
-    }
+    let mut args = parse_exprs_with_sep(tt, ',');
+
     if args.is_empty() {
         return ExpandResult::only_err(mbe::ExpandError::NoMatchingRule);
     }
     for arg in &mut args {
         // Remove `key =`.
-        if matches!(arg.get(1), Some(tt::TokenTree::Leaf(tt::Leaf::Punct(p))) if p.char == '=' && p.spacing != tt::Spacing::Joint)
+        if matches!(arg.token_trees.get(1), Some(tt::TokenTree::Leaf(tt::Leaf::Punct(p))) if p.char == '=' && p.spacing != tt::Spacing::Joint)
         {
-            arg.drain(..2);
+            arg.token_trees.drain(..2);
         }
     }
     let _format_string = args.remove(0);
     let arg_tts = args.into_iter().flat_map(|arg| {
-        quote! { std::fmt::ArgumentV1::new(&(##arg), std::fmt::Display::fmt), }
+        quote! { std::fmt::ArgumentV1::new(&(#arg), std::fmt::Display::fmt), }
     }.token_trees).collect::<Vec<_>>();
     let expanded = quote! {
         std::fmt::Arguments::new_v1(&[], &[##arg_tts])
@@ -327,17 +298,14 @@ fn concat_expand(
                 // concat works with string and char literals, so remove any quotes.
                 // It also works with integer, float and boolean literals, so just use the rest
                 // as-is.
-
-                text += it
-                    .text
-                    .trim_start_matches(|c| match c {
-                        'r' | '#' | '\'' | '"' => true,
-                        _ => false,
-                    })
-                    .trim_end_matches(|c| match c {
-                        '#' | '\'' | '"' => true,
-                        _ => false,
-                    });
+                let component = unquote_str(&it).unwrap_or_else(|| it.text.to_string());
+                text.push_str(&component);
+            }
+            // handle boolean literals
+            tt::TokenTree::Leaf(tt::Leaf::Ident(id))
+                if i % 2 == 0 && (id.text == "true" || id.text == "false") =>
+            {
+                text.push_str(id.text.as_str());
             }
             tt::TokenTree::Leaf(tt::Leaf::Punct(punct)) if i % 2 == 1 && punct.char == ',' => (),
             _ => {
@@ -345,7 +313,6 @@ fn concat_expand(
             }
         }
     }
-
     ExpandResult { value: Some((quote!(#text), FragmentKind::Expr)), err }
 }
 
@@ -724,6 +691,25 @@ mod tests {
     }
 
     #[test]
+    fn test_format_args_expand_with_comma_exprs() {
+        let expanded = expand_builtin_macro(
+            r#"
+            #[rustc_builtin_macro]
+            macro_rules! format_args {
+                ($fmt:expr) => ({ /* compiler built-in */ });
+                ($fmt:expr, $($args:tt)*) => ({ /* compiler built-in */ })
+            }
+            format_args!("{} {:?}", a::<A,B>(), b);
+            "#,
+        );
+
+        assert_eq!(
+            expanded,
+            r#"std::fmt::Arguments::new_v1(&[], &[std::fmt::ArgumentV1::new(&(a::<A,B>()),std::fmt::Display::fmt),std::fmt::ArgumentV1::new(&(b),std::fmt::Display::fmt),])"#
+        );
+    }
+
+    #[test]
     fn test_include_bytes_expand() {
         let expanded = expand_builtin_macro(
             r#"
@@ -745,12 +731,10 @@ mod tests {
             r##"
             #[rustc_builtin_macro]
             macro_rules! concat {}
-            concat!("foo", 0, r#"bar"#);
+            concat!("foo", "r", 0, r#"bar"#, false);
             "##,
         );
 
-        assert_eq!(expanded, r#""foo0bar""#);
-
-        // FIXME: `true`/`false` literals don't work.
+        assert_eq!(expanded, r#""foor0barfalse""#);
     }
 }
diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs
index 467516eb7..cb6e23320 100644
--- a/crates/hir_expand/src/db.rs
+++ b/crates/hir_expand/src/db.rs
@@ -173,7 +173,7 @@ fn macro_arg_text(db: &dyn AstDatabase, id: MacroCallId) -> Option<GreenNode> {
     };
     let loc = db.lookup_intern_macro(id);
     let arg = loc.kind.arg(db)?;
-    Some(arg.green().clone())
+    Some(arg.green().to_owned())
 }
 
 fn macro_arg(db: &dyn AstDatabase, id: MacroCallId) -> Option<Arc<(tt::Subtree, mbe::TokenMap)>> {
diff --git a/crates/hir_expand/src/name.rs b/crates/hir_expand/src/name.rs
index 95d853b6d..c94fb580a 100644
--- a/crates/hir_expand/src/name.rs
+++ b/crates/hir_expand/src/name.rs
@@ -38,7 +38,7 @@ impl Name {
     }
 
     pub fn new_lifetime(lt: &ast::Lifetime) -> Name {
-        Self::new_text(lt.text().clone())
+        Self::new_text(lt.text().into())
     }
 
     /// Shortcut to create inline plain text name
@@ -47,12 +47,12 @@ impl Name {
     }
 
     /// Resolve a name from the text of token.
-    fn resolve(raw_text: &SmolStr) -> Name {
+    fn resolve(raw_text: &str) -> Name {
         let raw_start = "r#";
-        if raw_text.as_str().starts_with(raw_start) {
+        if raw_text.starts_with(raw_start) {
             Name::new_text(SmolStr::new(&raw_text[raw_start.len()..]))
         } else {
-            Name::new_text(raw_text.clone())
+            Name::new_text(raw_text.into())
         }
     }
 
@@ -186,6 +186,10 @@ pub mod known {
         Neg,
         Not,
         Index,
+        // Components of known path (function name)
+        filter_map,
+        next,
+        iter_mut,
         // Builtin macros
         file,
         column,