Encode disabled proc-macros via boolean flag, not special Expander

8 files changed, 90 insertions, 147 deletions

diff --git a/crates/hir-def/src/data.rs b/crates/hir-def/src/data.rs
index d6aab11afd5..f506864902c 100644
--- a/crates/hir-def/src/data.rs
+++ b/crates/hir-def/src/data.rs
@@ -653,7 +653,8 @@ impl<'a> AssocItemCollector<'a> {
                                 ));
 
                                 continue 'attrs;
-                            } else if exp.is_disabled() {
+                            }
+                            if exp.is_disabled() {
                                 continue 'attrs;
                             }
                         }
diff --git a/crates/hir-def/src/macro_expansion_tests/mod.rs b/crates/hir-def/src/macro_expansion_tests/mod.rs
index fc5a6e80a42..23b10cfd8e6 100644
--- a/crates/hir-def/src/macro_expansion_tests/mod.rs
+++ b/crates/hir-def/src/macro_expansion_tests/mod.rs
@@ -58,6 +58,7 @@ pub fn identity_when_valid(_attr: TokenStream, item: TokenStream) -> TokenStream
             name: "identity_when_valid".into(),
             kind: ProcMacroKind::Attr,
             expander: sync::Arc::new(IdentityWhenValidProcMacroExpander),
+            disabled: false,
         },
     )];
     let db = TestDB::with_files_extra_proc_macros(ra_fixture, extra_proc_macros);
diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs
index 7a957e86942..8f64df280c1 100644
--- a/crates/hir-def/src/nameres/collector.rs
+++ b/crates/hir-def/src/nameres/collector.rs
@@ -11,7 +11,7 @@ use either::Either;
 use hir_expand::{
     ast_id_map::FileAstId,
     attrs::{Attr, AttrId},
-    builtin_attr_macro::find_builtin_attr,
+    builtin_attr_macro::{find_builtin_attr, BuiltinAttrExpander},
     builtin_derive_macro::find_builtin_derive,
     builtin_fn_macro::find_builtin_macro,
     name::{name, AsName, Name},
@@ -98,12 +98,12 @@ pub(super) fn collect_defs(db: &dyn DefDatabase, def_map: DefMap, tree_id: TreeI
                         };
                         (
                             name.as_name(),
-                            if it.expander.should_expand() {
+                            if it.disabled {
+                                CustomProcMacroExpander::disabled()
+                            } else {
                                 CustomProcMacroExpander::new(hir_expand::proc_macro::ProcMacroId(
                                     idx as u32,
                                 ))
-                            } else {
-                                CustomProcMacroExpander::disabled()
                             },
                         )
                     })
@@ -608,9 +608,6 @@ impl DefCollector<'_> {
         id: ItemTreeId<item_tree::Function>,
         fn_id: FunctionId,
     ) {
-        if self.def_map.block.is_some() {
-            return;
-        }
         let kind = def.kind.to_basedb_kind();
         let (expander, kind) =
             match self.proc_macros.as_ref().map(|it| it.iter().find(|(n, _)| n == &def.name)) {
@@ -1124,9 +1121,16 @@ impl DefCollector<'_> {
         let mut push_resolved = |directive: &MacroDirective, call_id| {
             resolved.push((directive.module_id, directive.depth, directive.container, call_id));
         };
+
+        #[derive(PartialEq, Eq)]
+        enum Resolved {
+            Yes,
+            No,
+        }
+
         let mut res = ReachedFixedPoint::Yes;
         // Retain unresolved macros after this round of resolution.
-        macros.retain(|directive| {
+        let mut retain = |directive: &MacroDirective| {
             let subns = match &directive.kind {
                 MacroDirectiveKind::FnLike { .. } => MacroSubNs::Bang,
                 MacroDirectiveKind::Attr { .. } | MacroDirectiveKind::Derive { .. } => {
@@ -1161,34 +1165,10 @@ impl DefCollector<'_> {
                             .scope
                             .add_macro_invoc(ast_id.ast_id, call_id);
 
-                        let loc: MacroCallLoc = self.db.lookup_intern_macro_call(call_id);
-
-                        if let MacroDefKind::ProcMacro(expander, _, _) = loc.def.kind {
-                            if expander.is_dummy() {
-                                // If there's no expander for the proc macro (e.g.
-                                // because proc macros are disabled, or building the
-                                // proc macro crate failed), report this and skip
-                                // expansion like we would if it was disabled
-                                self.def_map.diagnostics.push(
-                                    DefDiagnostic::unresolved_proc_macro(
-                                        directive.module_id,
-                                        loc.kind,
-                                        loc.def.krate,
-                                    ),
-                                );
-
-                                res = ReachedFixedPoint::No;
-                                return false;
-                            } else if expander.is_disabled() {
-                                res = ReachedFixedPoint::No;
-                                return false;
-                            }
-                        }
-
                         push_resolved(directive, call_id);
 
                         res = ReachedFixedPoint::No;
-                        return false;
+                        return Resolved::Yes;
                     }
                 }
                 MacroDirectiveKind::Derive { ast_id, derive_attr, derive_pos, call_site } => {
@@ -1227,7 +1207,7 @@ impl DefCollector<'_> {
 
                         push_resolved(directive, call_id);
                         res = ReachedFixedPoint::No;
-                        return false;
+                        return Resolved::Yes;
                     }
                 }
                 MacroDirectiveKind::Attr { ast_id: file_ast_id, mod_item, attr, tree } => {
@@ -1250,7 +1230,7 @@ impl DefCollector<'_> {
                         }
                         .collect(&[*mod_item], directive.container);
                         res = ReachedFixedPoint::No;
-                        false
+                        Resolved::Yes
                     };
 
                     if let Some(ident) = path.as_ident() {
@@ -1266,13 +1246,18 @@ impl DefCollector<'_> {
 
                     let def = match resolver_def_id(path.clone()) {
                         Some(def) if def.is_attribute() => def,
-                        _ => return true,
+                        _ => return Resolved::No,
                     };
-                    if matches!(
-                        def,
-                        MacroDefId { kind: MacroDefKind::BuiltInAttr(expander, _),.. }
-                        if expander.is_derive()
-                    ) {
+
+                    if let MacroDefId {
+                        kind:
+                            MacroDefKind::BuiltInAttr(
+                                BuiltinAttrExpander::Derive | BuiltinAttrExpander::DeriveConst,
+                                _,
+                            ),
+                        ..
+                    } = def
+                    {
                         // Resolved to `#[derive]`, we don't actually expand this attribute like
                         // normal (as that would just be an identity expansion with extra output)
                         // Instead we treat derive attributes special and apply them separately.
@@ -1345,16 +1330,6 @@ impl DefCollector<'_> {
                     let call_id =
                         attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def);
 
-                    // If proc attribute macro expansion is disabled, skip expanding it here
-                    if !self.db.expand_proc_attr_macros() {
-                        self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro(
-                            directive.module_id,
-                            self.db.lookup_intern_macro_call(call_id).kind,
-                            def.krate,
-                        ));
-                        return recollect_without(self);
-                    }
-
                     // Skip #[test]/#[bench] expansion, which would merely result in more memory usage
                     // due to duplicating functions into macro expansions
                     if matches!(
@@ -1366,19 +1341,29 @@ impl DefCollector<'_> {
                     }
 
                     if let MacroDefKind::ProcMacro(exp, ..) = def.kind {
-                        if exp.is_dummy() {
-                            // If there's no expander for the proc macro (e.g.
-                            // because proc macros are disabled, or building the
-                            // proc macro crate failed), report this and skip
-                            // expansion like we would if it was disabled
+                        // If proc attribute macro expansion is disabled, skip expanding it here
+                        if !self.db.expand_proc_attr_macros() {
                             self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro(
                                 directive.module_id,
                                 self.db.lookup_intern_macro_call(call_id).kind,
                                 def.krate,
                             ));
+                            return recollect_without(self);
+                        }
 
+                        // If there's no expander for the proc macro (e.g.
+                        // because proc macros are disabled, or building the
+                        // proc macro crate failed), report this and skip
+                        // expansion like we would if it was disabled
+                        if exp.is_dummy() {
+                            self.def_map.diagnostics.push(DefDiagnostic::unresolved_proc_macro(
+                                directive.module_id,
+                                self.db.lookup_intern_macro_call(call_id).kind,
+                                def.krate,
+                            ));
                             return recollect_without(self);
-                        } else if exp.is_disabled() {
+                        }
+                        if exp.is_disabled() {
                             return recollect_without(self);
                         }
                     }
@@ -1389,12 +1374,13 @@ impl DefCollector<'_> {
 
                     push_resolved(directive, call_id);
                     res = ReachedFixedPoint::No;
-                    return false;
+                    return Resolved::Yes;
                 }
             }
 
-            true
-        });
+            Resolved::No
+        };
+        macros.retain(|it| retain(it) == Resolved::No);
         // Attribute resolution can add unresolved macro invocations, so concatenate the lists.
         macros.extend(mem::take(&mut self.unresolved_macros));
         self.unresolved_macros = macros;
@@ -1704,7 +1690,11 @@ impl ModCollector<'_, '_> {
                         FunctionLoc { container, id: ItemTreeId::new(self.tree_id, id) }.intern(db);
 
                     let vis = resolve_vis(def_map, &self.item_tree[it.visibility]);
-                    if self.def_collector.is_proc_macro && self.module_id == DefMap::ROOT {
+
+                    if self.def_collector.def_map.block.is_none()
+                        && self.def_collector.is_proc_macro
+                        && self.module_id == DefMap::ROOT
+                    {
                         if let Some(proc_macro) = attrs.parse_proc_macro_decl(&it.name) {
                             self.def_collector.export_proc_macro(
                                 proc_macro,
diff --git a/crates/hir-def/src/nameres/diagnostics.rs b/crates/hir-def/src/nameres/diagnostics.rs
index 0a3f7bf7ec3..161b2c05990 100644
--- a/crates/hir-def/src/nameres/diagnostics.rs
+++ b/crates/hir-def/src/nameres/diagnostics.rs
@@ -103,6 +103,9 @@ impl DefDiagnostic {
     }
 
     // FIXME: Whats the difference between this and unresolved_macro_call
+    // FIXME: This is used for a lot of things, unresolved proc macros, disabled proc macros, etc
+    // yet the diagnostic handler in ide-diagnostics has to figure out what happened because this
+    // struct loses all that information!
     pub(crate) fn unresolved_proc_macro(
         container: LocalModuleId,
         ast: MacroCallKind,
diff --git a/crates/hir-expand/src/proc_macro.rs b/crates/hir-expand/src/proc_macro.rs
index f745ff23efb..7c4de2f1b87 100644
--- a/crates/hir-expand/src/proc_macro.rs
+++ b/crates/hir-expand/src/proc_macro.rs
@@ -31,16 +31,6 @@ pub trait ProcMacroExpander: fmt::Debug + Send + Sync + RefUnwindSafe {
         call_site: Span,
         mixed_site: Span,
     ) -> Result<tt::Subtree, ProcMacroExpansionError>;
-
-    /// If this returns `false`, expansions via [`expand`](ProcMacroExpander::expand) will always
-    /// return the input subtree or will always return an error.
-    ///
-    /// This is used to skip any additional expansion-related work,
-    /// e.g. to make sure we do not touch the syntax tree in any way
-    /// if a proc macro will never be expanded.
-    fn should_expand(&self) -> bool {
-        true
-    }
 }
 
 #[derive(Debug)]
@@ -59,6 +49,7 @@ pub struct ProcMacro {
     pub name: SmolStr,
     pub kind: ProcMacroKind,
     pub expander: sync::Arc<dyn ProcMacroExpander>,
+    pub disabled: bool,
 }
 
 #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)]
@@ -66,31 +57,35 @@ pub struct CustomProcMacroExpander {
     proc_macro_id: ProcMacroId,
 }
 
-const DUMMY_ID: u32 = !0;
-const DISABLED_ID: u32 = !1;
-
 impl CustomProcMacroExpander {
+    const DUMMY_ID: u32 = !0;
+    const DISABLED_ID: u32 = !1;
+
     pub fn new(proc_macro_id: ProcMacroId) -> Self {
-        assert_ne!(proc_macro_id.0, DUMMY_ID);
+        assert_ne!(proc_macro_id.0, Self::DUMMY_ID);
+        assert_ne!(proc_macro_id.0, Self::DISABLED_ID);
         Self { proc_macro_id }
     }
 
-    pub fn dummy() -> Self {
-        Self { proc_macro_id: ProcMacroId(DUMMY_ID) }
+    /// A dummy expander that always errors. This is used for proc-macros that are missing, usually
+    /// due to them not being built yet.
+    pub const fn dummy() -> Self {
+        Self { proc_macro_id: ProcMacroId(Self::DUMMY_ID) }
     }
 
     /// The macro was not yet resolved.
-    pub fn is_dummy(&self) -> bool {
-        self.proc_macro_id.0 == DUMMY_ID
+    pub const fn is_dummy(&self) -> bool {
+        self.proc_macro_id.0 == Self::DUMMY_ID
     }
 
-    pub fn disabled() -> Self {
-        Self { proc_macro_id: ProcMacroId(DISABLED_ID) }
+    /// A dummy expander that always errors. This expander is used for macros that have been disabled.
+    pub const fn disabled() -> Self {
+        Self { proc_macro_id: ProcMacroId(Self::DISABLED_ID) }
     }
 
     /// The macro is explicitly disabled and cannot be expanded.
-    pub fn is_disabled(&self) -> bool {
-        self.proc_macro_id.0 == DISABLED_ID
+    pub const fn is_disabled(&self) -> bool {
+        self.proc_macro_id.0 == Self::DISABLED_ID
     }
 
     pub fn expand(
@@ -105,11 +100,11 @@ impl CustomProcMacroExpander {
         mixed_site: Span,
     ) -> ExpandResult<tt::Subtree> {
         match self.proc_macro_id {
-            ProcMacroId(DUMMY_ID) => ExpandResult::new(
+            ProcMacroId(Self::DUMMY_ID) => ExpandResult::new(
                 tt::Subtree::empty(tt::DelimSpan { open: call_site, close: call_site }),
                 ExpandError::UnresolvedProcMacro(def_crate),
             ),
-            ProcMacroId(DISABLED_ID) => ExpandResult::new(
+            ProcMacroId(Self::DISABLED_ID) => ExpandResult::new(
                 tt::Subtree::empty(tt::DelimSpan { open: call_site, close: call_site }),
                 ExpandError::MacroDisabled,
             ),
@@ -135,7 +130,7 @@ impl CustomProcMacroExpander {
                         );
                         return ExpandResult::new(
                             tt::Subtree::empty(tt::DelimSpan { open: call_site, close: call_site }),
-                            ExpandError::other("Internal error"),
+                            ExpandError::other("Internal error: proc-macro index out of bounds"),
                         );
                     }
                 };
diff --git a/crates/load-cargo/src/lib.rs b/crates/load-cargo/src/lib.rs
index dc9005f5838..a52cc19650f 100644
--- a/crates/load-cargo/src/lib.rs
+++ b/crates/load-cargo/src/lib.rs
@@ -18,7 +18,6 @@ use itertools::Itertools;
 use proc_macro_api::{MacroDylib, ProcMacroServer};
 use project_model::{CargoConfig, PackageRoot, ProjectManifest, ProjectWorkspace};
 use span::Span;
-use tt::DelimSpan;
 use vfs::{file_set::FileSetConfig, loader::Handle, AbsPath, AbsPathBuf, VfsPath};
 
 pub struct LoadCargoConfig {
@@ -357,16 +356,8 @@ fn expander_to_proc_macro(
         proc_macro_api::ProcMacroKind::FuncLike => ProcMacroKind::FuncLike,
         proc_macro_api::ProcMacroKind::Attr => ProcMacroKind::Attr,
     };
-    let expander: sync::Arc<dyn ProcMacroExpander> =
-        if ignored_macros.iter().any(|replace| &**replace == name) {
-            match kind {
-                ProcMacroKind::Attr => sync::Arc::new(IdentityExpander),
-                _ => sync::Arc::new(EmptyExpander),
-            }
-        } else {
-            sync::Arc::new(Expander(expander))
-        };
-    ProcMacro { name, kind, expander }
+    let disabled = ignored_macros.iter().any(|replace| **replace == name);
+    ProcMacro { name, kind, expander: sync::Arc::new(Expander(expander)), disabled }
 }
 
 #[derive(Debug)]
@@ -391,48 +382,6 @@ impl ProcMacroExpander for Expander {
     }
 }
 
-/// Dummy identity expander, used for attribute proc-macros that are deliberately ignored by the user.
-#[derive(Debug)]
-struct IdentityExpander;
-
-impl ProcMacroExpander for IdentityExpander {
-    fn expand(
-        &self,
-        subtree: &tt::Subtree<Span>,
-        _: Option<&tt::Subtree<Span>>,
-        _: &Env,
-        _: Span,
-        _: Span,
-        _: Span,
-    ) -> Result<tt::Subtree<Span>, ProcMacroExpansionError> {
-        Ok(subtree.clone())
-    }
-    fn should_expand(&self) -> bool {
-        false
-    }
-}
-
-/// Empty expander, used for proc-macros that are deliberately ignored by the user.
-#[derive(Debug)]
-struct EmptyExpander;
-
-impl ProcMacroExpander for EmptyExpander {
-    fn expand(
-        &self,
-        _: &tt::Subtree<Span>,
-        _: Option<&tt::Subtree<Span>>,
-        _: &Env,
-        call_site: Span,
-        _: Span,
-        _: Span,
-    ) -> Result<tt::Subtree<Span>, ProcMacroExpansionError> {
-        Ok(tt::Subtree::empty(DelimSpan { open: call_site, close: call_site }))
-    }
-    fn should_expand(&self) -> bool {
-        false
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use ide_db::base_db::SourceDatabase;
diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs
index 14d622abfe2..3c2ba2f115a 100644
--- a/crates/rust-analyzer/src/reload.rs
+++ b/crates/rust-analyzer/src/reload.rs
@@ -333,13 +333,12 @@ impl GlobalState {
                                         crate_name
                                             .as_deref()
                                             .and_then(|crate_name| {
-                                                ignored_proc_macros.iter().find_map(|c| {
-                                                    if eq_ignore_underscore(&*c.0, crate_name) {
-                                                        Some(&**c.1)
-                                                    } else {
-                                                        None
-                                                    }
-                                                })
+                                                ignored_proc_macros.iter().find_map(
+                                                    |(name, macros)| {
+                                                        eq_ignore_underscore(name, crate_name)
+                                                            .then_some(&**macros)
+                                                    },
+                                                )
                                             })
                                             .unwrap_or_default(),
                                     )
diff --git a/crates/test-fixture/src/lib.rs b/crates/test-fixture/src/lib.rs
index 28e757e81bb..f966013b978 100644
--- a/crates/test-fixture/src/lib.rs
+++ b/crates/test-fixture/src/lib.rs
@@ -374,6 +374,7 @@ pub fn identity(_attr: TokenStream, item: TokenStream) -> TokenStream {
                 name: "identity".into(),
                 kind: ProcMacroKind::Attr,
                 expander: sync::Arc::new(IdentityProcMacroExpander),
+                disabled: false,
             },
         ),
         (
@@ -388,6 +389,7 @@ pub fn derive_identity(item: TokenStream) -> TokenStream {
                 name: "DeriveIdentity".into(),
                 kind: ProcMacroKind::CustomDerive,
                 expander: sync::Arc::new(IdentityProcMacroExpander),
+                disabled: false,
             },
         ),
         (
@@ -402,6 +404,7 @@ pub fn input_replace(attr: TokenStream, _item: TokenStream) -> TokenStream {
                 name: "input_replace".into(),
                 kind: ProcMacroKind::Attr,
                 expander: sync::Arc::new(AttributeInputReplaceProcMacroExpander),
+                disabled: false,
             },
         ),
         (
@@ -416,6 +419,7 @@ pub fn mirror(input: TokenStream) -> TokenStream {
                 name: "mirror".into(),
                 kind: ProcMacroKind::FuncLike,
                 expander: sync::Arc::new(MirrorProcMacroExpander),
+                disabled: false,
             },
         ),
         (
@@ -430,6 +434,7 @@ pub fn shorten(input: TokenStream) -> TokenStream {
                 name: "shorten".into(),
                 kind: ProcMacroKind::FuncLike,
                 expander: sync::Arc::new(ShortenProcMacroExpander),
+                disabled: false,
             },
         ),
     ]