Don't unnecessarily clone the input tt for decl macros

Author: Veykril

crates/hir-expand/src/db.rs

@@ -35,7 +35,7 @@ pub struct DeclarativeMacroExpander {
 }
 
 impl DeclarativeMacroExpander {
-    pub fn expand(&self, tt: &tt::Subtree) -> ExpandResult<tt::Subtree> {
+    pub fn expand(&self, tt: tt::Subtree) -> ExpandResult<tt::Subtree> {
         match self.mac.err() {
             Some(e) => ExpandResult::new(
                 tt::Subtree::empty(),
@@ -44,6 +44,14 @@ impl DeclarativeMacroExpander {
             None => self.mac.expand(tt).map_err(Into::into),
         }
     }
+
+    pub fn map_id_down(&self, token_id: tt::TokenId) -> tt::TokenId {
+        self.mac.map_id_down(token_id)
+    }
+
+    pub fn map_id_up(&self, token_id: tt::TokenId) -> (tt::TokenId, mbe::Origin) {
+        self.mac.map_id_up(token_id)
+    }
 }
 
 #[derive(Debug, Clone, Eq, PartialEq)]
@@ -61,10 +69,11 @@ pub enum TokenExpander {
     ProcMacro(ProcMacroExpander),
 }
 
+// FIXME: Get rid of these methods
 impl TokenExpander {
     pub(crate) fn map_id_down(&self, id: tt::TokenId) -> tt::TokenId {
         match self {
-            TokenExpander::DeclarativeMacro(expander) => expander.mac.map_id_down(id),
+            TokenExpander::DeclarativeMacro(expander) => expander.map_id_down(id),
             TokenExpander::BuiltIn(..)
             | TokenExpander::BuiltInEager(..)
             | TokenExpander::BuiltInAttr(..)
@@ -75,7 +84,7 @@ impl TokenExpander {
 
     pub(crate) fn map_id_up(&self, id: tt::TokenId) -> (tt::TokenId, mbe::Origin) {
         match self {
-            TokenExpander::DeclarativeMacro(expander) => expander.mac.map_id_up(id),
+            TokenExpander::DeclarativeMacro(expander) => expander.map_id_up(id),
             TokenExpander::BuiltIn(..)
             | TokenExpander::BuiltInEager(..)
             | TokenExpander::BuiltInAttr(..)
@@ -167,7 +176,6 @@ pub fn expand_speculative(
     token_to_map: SyntaxToken,
 ) -> Option<(SyntaxNode, SyntaxToken)> {
     let loc = db.lookup_intern_macro_call(actual_macro_call);
-    let macro_def = db.macro_def(loc.def);
     let token_range = token_to_map.text_range();
 
     // Build the subtree and token mapping for the speculative args
@@ -225,7 +233,12 @@ pub fn expand_speculative(
         None => {
             let range = token_range.checked_sub(speculative_args.text_range().start())?;
             let token_id = spec_args_tmap.token_by_range(range)?;
-            macro_def.map_id_down(token_id)
+            match loc.def.kind {
+                MacroDefKind::Declarative(it) => {
+                    db.decl_macro_expander(loc.krate, it).map_id_down(token_id)
+                }
+                _ => token_id,
+            }
         }
     };
 
@@ -244,7 +257,7 @@ pub fn expand_speculative(
             let adt = ast::Adt::cast(speculative_args.clone()).unwrap();
             expander.expand(db, actual_macro_call, &adt, &spec_args_tmap)
         }
-        MacroDefKind::Declarative(it) => db.decl_macro_expander(loc.krate, it).expand(&tt),
+        MacroDefKind::Declarative(it) => db.decl_macro_expander(loc.krate, it).expand(tt),
         MacroDefKind::BuiltIn(it, _) => it.expand(db, actual_macro_call, &tt).map_err(Into::into),
         MacroDefKind::BuiltInEager(it, _) => {
             it.expand(db, actual_macro_call, &tt).map_err(Into::into)
@@ -518,7 +531,7 @@ fn macro_expand(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult<Arc<tt
             let (arg, arg_tm, undo_info) = &*macro_arg;
             let mut res = match loc.def.kind {
                 MacroDefKind::Declarative(id) => {
-                    db.decl_macro_expander(loc.def.krate, id).expand(&arg)
+                    db.decl_macro_expander(loc.def.krate, id).expand(arg.clone())
                 }
                 MacroDefKind::BuiltIn(it, _) => it.expand(db, id, &arg).map_err(Into::into),
                 MacroDefKind::BuiltInEager(it, _) => it.expand(db, id, &arg).map_err(Into::into),

crates/mbe/src/benchmark.rs

@@ -38,7 +38,7 @@ fn benchmark_expand_macro_rules() {
         invocations
             .into_iter()
             .map(|(id, tt)| {
-                let res = rules[&id].expand(&tt);
+                let res = rules[&id].expand(tt);
                 assert!(res.err.is_none());
                 res.value.token_trees.len()
             })
@@ -102,7 +102,7 @@ fn invocation_fixtures(rules: &FxHashMap<String, DeclarativeMacro>) -> Vec<(Stri
                     for op in rule.lhs.iter() {
                         collect_from_op(op, &mut subtree, &mut seed);
                     }
-                    if it.expand(&subtree).err.is_none() {
+                    if it.expand(subtree.clone()).err.is_none() {
                         res.push((name.clone(), subtree));
                         break;
                     }

crates/mbe/src/lib.rs

@@ -311,9 +311,7 @@ impl DeclarativeMacro {
         DeclarativeMacro { rules: rules.into_boxed_slice(), shift: Shift::new(tt), is_2021, err }
     }
 
-    pub fn expand(&self, tt: &tt::Subtree) -> ExpandResult<tt::Subtree> {
-        // apply shift
-        let mut tt = tt.clone();
+    pub fn expand(&self, mut tt: tt::Subtree) -> ExpandResult<tt::Subtree> {
         self.shift.shift_all(&mut tt);
         expander::expand_rules(&self.rules, &tt, self.is_2021)
     }