Auto merge of #14636 - Veykril:macros, r=Veykril

internal: Remove unnecessary is_derive field from MacroCallKind::Attr

7 files changed, 74 insertions, 78 deletions

diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs
index 58becd603b2..7c6c089d65b 100644
--- a/crates/hir-def/src/data.rs
+++ b/crates/hir-def/src/data.rs
@@ -623,7 +623,6 @@ impl<'a> AssocItemCollector<'a> {
                                 ast_id,
                                 attr_args: Arc::new((tt::Subtree::empty(), Default::default())),
                                 invoc_attr_index: attr.id,
-                                is_derive: false,
                             },
                             attr.path().clone(),
                         ));
diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs
index b7f0e229eef..8e69a64e7e9 100644
--- a/crates/hir-def/src/lib.rs
+++ b/crates/hir-def/src/lib.rs
@@ -984,7 +984,6 @@ fn attr_macro_as_call_id(
     macro_attr: &Attr,
     krate: CrateId,
     def: MacroDefId,
-    is_derive: bool,
 ) -> MacroCallId {
     let arg = match macro_attr.input.as_deref() {
         Some(AttrInput::TokenTree(tt, map)) => (
@@ -1005,7 +1004,6 @@ fn attr_macro_as_call_id(
             ast_id: item_attr.ast_id,
             attr_args: Arc::new(arg),
             invoc_attr_index: macro_attr.id,
-            is_derive,
         },
     )
 }
diff --git a/crates/hir-def/src/nameres/attr_resolution.rs b/crates/hir-def/src/nameres/attr_resolution.rs
index 6fad01ec2f7..84271ef51d8 100644
--- a/crates/hir-def/src/nameres/attr_resolution.rs
+++ b/crates/hir-def/src/nameres/attr_resolution.rs
@@ -61,7 +61,6 @@ impl DefMap {
             attr,
             self.krate,
             macro_id_to_def_id(db, def),
-            false,
         )))
     }
 
diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs
index ba93b2c70ef..51879dd5553 100644
--- a/crates/hir-def/src/nameres/collector.rs
+++ b/crates/hir-def/src/nameres/collector.rs
@@ -481,7 +481,6 @@ impl DefCollector<'_> {
                                 Default::default(),
                             )),
                             invoc_attr_index: attr.id,
-                            is_derive: false,
                         },
                         attr.path().clone(),
                     ));
@@ -1273,7 +1272,6 @@ impl DefCollector<'_> {
                                     attr,
                                     self.def_map.krate,
                                     def,
-                                    true,
                                 );
                                 self.def_map.modules[directive.module_id]
                                     .scope
@@ -1293,14 +1291,8 @@ impl DefCollector<'_> {
                     }
 
                     // Not resolved to a derive helper or the derive attribute, so try to treat as a normal attribute.
-                    let call_id = attr_macro_as_call_id(
-                        self.db,
-                        file_ast_id,
-                        attr,
-                        self.def_map.krate,
-                        def,
-                        false,
-                    );
+                    let call_id =
+                        attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def);
                     let loc: MacroCallLoc = self.db.lookup_intern_macro_call(call_id);
 
                     // If proc attribute macro expansion is disabled, skip expanding it here
diff --git a/crates/hir-expand/src/builtin_attr_macro.rs b/crates/hir-expand/src/builtin_attr_macro.rs
index 277ecd93994..80695bc0656 100644
--- a/crates/hir-expand/src/builtin_attr_macro.rs
+++ b/crates/hir-expand/src/builtin_attr_macro.rs
@@ -96,7 +96,7 @@ fn derive_attr_expand(
 ) -> ExpandResult<tt::Subtree> {
     let loc = db.lookup_intern_macro_call(id);
     let derives = match &loc.kind {
-        MacroCallKind::Attr { attr_args, is_derive: true, .. } => &attr_args.0,
+        MacroCallKind::Attr { attr_args, .. } if loc.def.is_attribute_derive() => &attr_args.0,
         _ => return ExpandResult::ok(tt::Subtree::empty()),
     };
     pseudo_derive_attr_expansion(tt, derives)
diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs
index bed04b3a343..c8de6954ee7 100644
--- a/crates/hir-expand/src/db.rs
+++ b/crates/hir-expand/src/db.rs
@@ -172,8 +172,8 @@ pub fn expand_speculative(
     );
 
     let (attr_arg, token_id) = match loc.kind {
-        MacroCallKind::Attr { invoc_attr_index, is_derive, .. } => {
-            let attr = if is_derive {
+        MacroCallKind::Attr { invoc_attr_index, .. } => {
+            let attr = if loc.def.is_attribute_derive() {
                 // for pseudo-derive expansion we actually pass the attribute itself only
                 ast::Attr::cast(speculative_args.clone())
             } else {
@@ -285,8 +285,8 @@ fn parse_macro_expansion(
             // Note:
             // The final goal we would like to make all parse_macro success,
             // such that the following log will not call anyway.
-            let loc: MacroCallLoc = db.lookup_intern_macro_call(macro_file.macro_call_id);
-            let node = loc.kind.to_node(db);
+            let loc = db.lookup_intern_macro_call(macro_file.macro_call_id);
+            let node = loc.to_node(db);
 
             // collect parent information for warning log
             let parents = std::iter::successors(loc.kind.file_id().call_node(db), |it| {
@@ -360,7 +360,7 @@ fn censor_for_macro_input(loc: &MacroCallLoc, node: &SyntaxNode) -> FxHashSet<Sy
                     .map(|it| it.syntax().clone())
                     .collect()
             }
-            MacroCallKind::Attr { is_derive: true, .. } => return None,
+            MacroCallKind::Attr { .. } if loc.def.is_attribute_derive() => return None,
             MacroCallKind::Attr { invoc_attr_index, .. } => {
                 cov_mark::hit!(attribute_macro_attr_censoring);
                 ast::Item::cast(node.clone())?
@@ -442,7 +442,7 @@ fn macro_def(
 
 fn macro_expand(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult<Arc<tt::Subtree>> {
     let _p = profile::span("macro_expand");
-    let loc: MacroCallLoc = db.lookup_intern_macro_call(id);
+    let loc = db.lookup_intern_macro_call(id);
     if let Some(eager) = &loc.eager {
         return ExpandResult { value: eager.arg_or_expansion.clone(), err: eager.error.clone() };
     }
@@ -511,7 +511,7 @@ fn parse_macro_expansion_error(
 }
 
 fn expand_proc_macro(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult<tt::Subtree> {
-    let loc: MacroCallLoc = db.lookup_intern_macro_call(id);
+    let loc = db.lookup_intern_macro_call(id);
     let Some(macro_arg) = db.macro_arg(id) else {
         return ExpandResult {
             value: tt::Subtree {
@@ -547,8 +547,7 @@ fn hygiene_frame(db: &dyn ExpandDatabase, file_id: HirFileId) -> Arc<HygieneFram
 }
 
 fn macro_expand_to(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandTo {
-    let loc: MacroCallLoc = db.lookup_intern_macro_call(id);
-    loc.kind.expand_to()
+    db.lookup_intern_macro_call(id).expand_to()
 }
 
 fn token_tree_to_syntax_node(
diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs
index cba448df2d9..ce0006d8422 100644
--- a/crates/hir-expand/src/lib.rs
+++ b/crates/hir-expand/src/lib.rs
@@ -168,8 +168,6 @@ pub enum MacroCallKind {
         /// Outer attributes are counted first, then inner attributes. This does not support
         /// out-of-line modules, which may have attributes spread across 2 files!
         invoc_attr_index: AttrId,
-        /// Whether this attribute is the `#[derive]` attribute.
-        is_derive: bool,
     },
 }
 
@@ -232,18 +230,17 @@ impl HirFileId {
     pub fn call_node(self, db: &dyn db::ExpandDatabase) -> Option<InFile<SyntaxNode>> {
         let macro_file = self.macro_file()?;
         let loc: MacroCallLoc = db.lookup_intern_macro_call(macro_file.macro_call_id);
-        Some(loc.kind.to_node(db))
+        Some(loc.to_node(db))
     }
 
     /// If this is a macro call, returns the syntax node of the very first macro call this file resides in.
     pub fn original_call_node(self, db: &dyn db::ExpandDatabase) -> Option<(FileId, SyntaxNode)> {
-        let mut call =
-            db.lookup_intern_macro_call(self.macro_file()?.macro_call_id).kind.to_node(db);
+        let mut call = db.lookup_intern_macro_call(self.macro_file()?.macro_call_id).to_node(db);
         loop {
             match call.file_id.repr() {
                 HirFileIdRepr::FileId(file_id) => break Some((file_id, call.value)),
                 HirFileIdRepr::MacroFile(MacroFile { macro_call_id }) => {
-                    call = db.lookup_intern_macro_call(macro_call_id).kind.to_node(db);
+                    call = db.lookup_intern_macro_call(macro_call_id).to_node(db);
                 }
             }
         }
@@ -306,7 +303,7 @@ impl HirFileId {
         let macro_file = self.macro_file()?;
         let loc: MacroCallLoc = db.lookup_intern_macro_call(macro_file.macro_call_id);
         let attr = match loc.def.kind {
-            MacroDefKind::BuiltInDerive(..) => loc.kind.to_node(db),
+            MacroDefKind::BuiltInDerive(..) => loc.to_node(db),
             _ => return None,
         };
         Some(attr.with_value(ast::Attr::cast(attr.value.clone())?))
@@ -350,7 +347,7 @@ impl HirFileId {
         match self.macro_file() {
             Some(macro_file) => {
                 let loc: MacroCallLoc = db.lookup_intern_macro_call(macro_file.macro_call_id);
-                matches!(loc.kind, MacroCallKind::Attr { is_derive: true, .. })
+                loc.def.is_attribute_derive()
             }
             None => false,
         }
@@ -421,22 +418,15 @@ impl MacroDefId {
             MacroDefKind::BuiltInAttr(..) | MacroDefKind::ProcMacro(_, ProcMacroKind::Attr, _)
         )
     }
-}
-
-// FIXME: attribute indices do not account for nested `cfg_attr`
 
-impl MacroCallKind {
-    /// Returns the file containing the macro invocation.
-    fn file_id(&self) -> HirFileId {
-        match *self {
-            MacroCallKind::FnLike { ast_id: InFile { file_id, .. }, .. }
-            | MacroCallKind::Derive { ast_id: InFile { file_id, .. }, .. }
-            | MacroCallKind::Attr { ast_id: InFile { file_id, .. }, .. } => file_id,
-        }
+    pub fn is_attribute_derive(&self) -> bool {
+        matches!(self.kind, MacroDefKind::BuiltInAttr(expander, ..) if expander.is_derive())
     }
+}
 
+impl MacroCallLoc {
     pub fn to_node(&self, db: &dyn db::ExpandDatabase) -> InFile<SyntaxNode> {
-        match self {
+        match self.kind {
             MacroCallKind::FnLike { ast_id, .. } => {
                 ast_id.with_value(ast_id.to_node(db).syntax().clone())
             }
@@ -452,23 +442,49 @@ impl MacroCallKind {
                         .unwrap_or_else(|| it.syntax().clone())
                 })
             }
-            MacroCallKind::Attr { ast_id, is_derive: true, invoc_attr_index, .. } => {
-                // FIXME: handle `cfg_attr`
-                ast_id.with_value(ast_id.to_node(db)).map(|it| {
-                    it.doc_comments_and_attrs()
-                        .nth(invoc_attr_index.ast_index())
-                        .and_then(|it| match it {
-                            Either::Left(attr) => Some(attr.syntax().clone()),
-                            Either::Right(_) => None,
-                        })
-                        .unwrap_or_else(|| it.syntax().clone())
-                })
+            MacroCallKind::Attr { ast_id, invoc_attr_index, .. } => {
+                if self.def.is_attribute_derive() {
+                    // FIXME: handle `cfg_attr`
+                    ast_id.with_value(ast_id.to_node(db)).map(|it| {
+                        it.doc_comments_and_attrs()
+                            .nth(invoc_attr_index.ast_index())
+                            .and_then(|it| match it {
+                                Either::Left(attr) => Some(attr.syntax().clone()),
+                                Either::Right(_) => None,
+                            })
+                            .unwrap_or_else(|| it.syntax().clone())
+                    })
+                } else {
+                    ast_id.with_value(ast_id.to_node(db).syntax().clone())
+                }
             }
-            MacroCallKind::Attr { ast_id, .. } => {
-                ast_id.with_value(ast_id.to_node(db).syntax().clone())
+        }
+    }
+
+    fn expand_to(&self) -> ExpandTo {
+        match self.kind {
+            MacroCallKind::FnLike { expand_to, .. } => expand_to,
+            MacroCallKind::Derive { .. } => ExpandTo::Items,
+            MacroCallKind::Attr { .. } if self.def.is_attribute_derive() => ExpandTo::Statements,
+            MacroCallKind::Attr { .. } => {
+                // is this always correct?
+                ExpandTo::Items
             }
         }
     }
+}
+
+// FIXME: attribute indices do not account for nested `cfg_attr`
+
+impl MacroCallKind {
+    /// Returns the file containing the macro invocation.
+    fn file_id(&self) -> HirFileId {
+        match *self {
+            MacroCallKind::FnLike { ast_id: InFile { file_id, .. }, .. }
+            | MacroCallKind::Derive { ast_id: InFile { file_id, .. }, .. }
+            | MacroCallKind::Attr { ast_id: InFile { file_id, .. }, .. } => file_id,
+        }
+    }
 
     /// Returns the original file range that best describes the location of this macro call.
     ///
@@ -546,15 +562,6 @@ impl MacroCallKind {
             MacroCallKind::Attr { ast_id, .. } => Some(ast_id.to_node(db).syntax().clone()),
         }
     }
-
-    fn expand_to(&self) -> ExpandTo {
-        match self {
-            MacroCallKind::FnLike { expand_to, .. } => *expand_to,
-            MacroCallKind::Derive { .. } => ExpandTo::Items,
-            MacroCallKind::Attr { is_derive: true, .. } => ExpandTo::Statements,
-            MacroCallKind::Attr { .. } => ExpandTo::Items, // is this always correct?
-        }
-    }
 }
 
 impl MacroCallId {
@@ -618,7 +625,7 @@ impl ExpansionInfo {
 
             let token_range = token.value.text_range();
             match &loc.kind {
-                MacroCallKind::Attr { attr_args, invoc_attr_index, is_derive, .. } => {
+                MacroCallKind::Attr { attr_args, invoc_attr_index, .. } => {
                     // FIXME: handle `cfg_attr`
                     let attr = item
                         .doc_comments_and_attrs()
@@ -634,7 +641,8 @@ impl ExpansionInfo {
                                 token.value.text_range().checked_sub(attr_input_start)?;
                             // shift by the item's tree's max id
                             let token_id = attr_args.1.token_by_range(relative_range)?;
-                            let token_id = if *is_derive {
+
+                            let token_id = if loc.def.is_attribute_derive() {
                                 // we do not shift for `#[derive]`, as we only need to downmap the derive attribute tokens
                                 token_id
                             } else {
@@ -697,18 +705,19 @@ impl ExpansionInfo {
 
         // Attributes are a bit special for us, they have two inputs, the input tokentree and the annotated item.
         let (token_map, tt) = match &loc.kind {
-            MacroCallKind::Attr { attr_args, is_derive: true, .. } => {
-                (&attr_args.1, self.attr_input_or_mac_def.clone()?.syntax().cloned())
-            }
             MacroCallKind::Attr { attr_args, .. } => {
-                // try unshifting the token id, if unshifting fails, the token resides in the non-item attribute input
-                // note that the `TokenExpander::map_id_up` earlier only unshifts for declarative macros, so we don't double unshift with this
-                match self.macro_arg_shift.unshift(token_id) {
-                    Some(unshifted) => {
-                        token_id = unshifted;
-                        (&attr_args.1, self.attr_input_or_mac_def.clone()?.syntax().cloned())
+                if loc.def.is_attribute_derive() {
+                    (&attr_args.1, self.attr_input_or_mac_def.clone()?.syntax().cloned())
+                } else {
+                    // try unshifting the token id, if unshifting fails, the token resides in the non-item attribute input
+                    // note that the `TokenExpander::map_id_up` earlier only unshifts for declarative macros, so we don't double unshift with this
+                    match self.macro_arg_shift.unshift(token_id) {
+                        Some(unshifted) => {
+                            token_id = unshifted;
+                            (&attr_args.1, self.attr_input_or_mac_def.clone()?.syntax().cloned())
+                        }
+                        None => (&self.macro_arg.1, self.arg.clone()),
                     }
-                    None => (&self.macro_arg.1, self.arg.clone()),
                 }
             }
             _ => match origin {