Cleanup some inert attribute stuff

Author: Veykril

crates/hir-def/src/attr.rs

@@ -1,10 +1,5 @@
 //! A higher level attributes based on TokenTree, with also some shortcuts.
 
-pub mod builtin;
-
-#[cfg(test)]
-mod tests;
-
 use std::{borrow::Cow, hash::Hash, ops, slice::Iter as SliceIter};
 
 use base_db::CrateId;
@@ -646,3 +641,55 @@ pub(crate) fn fields_attrs_source_map(
 
     Arc::new(res)
 }
+
+#[cfg(test)]
+mod tests {
+    //! This module contains tests for doc-expression parsing.
+    //! Currently, it tests `#[doc(hidden)]` and `#[doc(alias)]`.
+
+    use triomphe::Arc;
+
+    use base_db::FileId;
+    use hir_expand::span_map::{RealSpanMap, SpanMap};
+    use mbe::{syntax_node_to_token_tree, DocCommentDesugarMode};
+    use syntax::{ast, AstNode, TextRange};
+
+    use crate::attr::{DocAtom, DocExpr};
+
+    fn assert_parse_result(input: &str, expected: DocExpr) {
+        let source_file = ast::SourceFile::parse(input, span::Edition::CURRENT).ok().unwrap();
+        let tt = source_file.syntax().descendants().find_map(ast::TokenTree::cast).unwrap();
+        let map = SpanMap::RealSpanMap(Arc::new(RealSpanMap::absolute(FileId::from_raw(0))));
+        let tt = syntax_node_to_token_tree(
+            tt.syntax(),
+            map.as_ref(),
+            map.span_for_range(TextRange::empty(0.into())),
+            DocCommentDesugarMode::ProcMacro,
+        );
+        let cfg = DocExpr::parse(&tt);
+        assert_eq!(cfg, expected);
+    }
+
+    #[test]
+    fn test_doc_expr_parser() {
+        assert_parse_result("#![doc(hidden)]", DocAtom::Flag("hidden".into()).into());
+
+        assert_parse_result(
+            r#"#![doc(alias = "foo")]"#,
+            DocAtom::KeyValue { key: "alias".into(), value: "foo".into() }.into(),
+        );
+
+        assert_parse_result(r#"#![doc(alias("foo"))]"#, DocExpr::Alias(["foo".into()].into()));
+        assert_parse_result(
+            r#"#![doc(alias("foo", "bar", "baz"))]"#,
+            DocExpr::Alias(["foo".into(), "bar".into(), "baz".into()].into()),
+        );
+
+        assert_parse_result(
+            r#"
+        #[doc(alias("Bar", "Qux"))]
+        struct Foo;"#,
+            DocExpr::Alias(["Bar".into(), "Qux".into()].into()),
+        );
+    }
+}

crates/hir-def/src/attr/tests.rs

@@ -642,7 +642,7 @@ impl<'a> AssocItemCollector<'a> {
                             continue 'attrs;
                         }
                         let loc = self.db.lookup_intern_macro_call(call_id);
-                        if let MacroDefKind::ProcMacro(exp, ..) = loc.def.kind {
+                        if let MacroDefKind::ProcMacro(_, exp, _) = loc.def.kind {
                             // If there's no expander for the proc macro (e.g. the
                             // proc macro is ignored, or building the proc macro
                             // crate failed), skip expansion like we would if it was

crates/hir-def/src/data.rs

@@ -294,10 +294,10 @@ fn macro_def(db: &dyn DefDatabase, id: MacroId) -> MacroDefId {
         let in_file = InFile::new(file_id, m);
         match expander {
             MacroExpander::Declarative => MacroDefKind::Declarative(in_file),
-            MacroExpander::BuiltIn(it) => MacroDefKind::BuiltIn(it, in_file),
-            MacroExpander::BuiltInAttr(it) => MacroDefKind::BuiltInAttr(it, in_file),
-            MacroExpander::BuiltInDerive(it) => MacroDefKind::BuiltInDerive(it, in_file),
-            MacroExpander::BuiltInEager(it) => MacroDefKind::BuiltInEager(it, in_file),
+            MacroExpander::BuiltIn(it) => MacroDefKind::BuiltIn(in_file, it),
+            MacroExpander::BuiltInAttr(it) => MacroDefKind::BuiltInAttr(in_file, it),
+            MacroExpander::BuiltInDerive(it) => MacroDefKind::BuiltInDerive(in_file, it),
+            MacroExpander::BuiltInEager(it) => MacroDefKind::BuiltInEager(in_file, it),
         }
     };
 
@@ -338,9 +338,9 @@ fn macro_def(db: &dyn DefDatabase, id: MacroId) -> MacroDefId {
             MacroDefId {
                 krate: loc.container.krate,
                 kind: MacroDefKind::ProcMacro(
+                    InFile::new(loc.id.file_id(), makro.ast_id),
                     loc.expander,
                     loc.kind,
-                    InFile::new(loc.id.file_id(), makro.ast_id),
                 ),
                 local_inner: false,
                 allow_internal_unsafe: false,

crates/hir-def/src/db.rs

@@ -3,14 +3,14 @@
 use base_db::CrateId;
 use hir_expand::{
     attrs::{Attr, AttrId, AttrInput},
+    inert_attr_macro::find_builtin_attr_idx,
     MacroCallId, MacroCallKind, MacroDefId,
 };
 use span::SyntaxContextId;
 use syntax::{ast, SmolStr};
 use triomphe::Arc;
 
 use crate::{
-    attr::builtin::find_builtin_attr_idx,
     db::DefDatabase,
     item_scope::BuiltinShadowMode,
     nameres::path_resolution::ResolveMode,
@@ -89,9 +89,12 @@ impl DefMap {
             }
 
             if segments.len() == 1 {
-                let mut registered = self.data.registered_attrs.iter().map(SmolStr::as_str);
-                let is_inert = find_builtin_attr_idx(&name).is_some() || registered.any(pred);
-                return is_inert;
+                if find_builtin_attr_idx(&name).is_some() {
+                    return true;
+                }
+                if self.data.registered_attrs.iter().map(SmolStr::as_str).any(pred) {
+                    return true;
+                }
             }
         }
         false

crates/hir-def/src/nameres/attr_resolution.rs

@@ -270,6 +270,7 @@ struct DefCollector<'a> {
     ///
     /// This also stores the attributes to skip when we resolve derive helpers and non-macro
     /// non-builtin attributes in general.
+    // FIXME: There has to be a better way to do this
     skip_attrs: FxHashMap<InFile<ModItem>, AttrId>,
 }
 
@@ -1255,13 +1256,24 @@ impl DefCollector<'_> {
                         _ => return Resolved::No,
                     };
 
-                    let call_id =
-                        attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def);
+                    // Skip #[test]/#[bench] expansion, which would merely result in more memory usage
+                    // due to duplicating functions into macro expansions
+                    if matches!(
+                        def.kind,
+                        MacroDefKind::BuiltInAttr(_, expander)
+                        if expander.is_test() || expander.is_bench()
+                    ) {
+                        return recollect_without(self);
+                    }
+
+                    let call_id = || {
+                        attr_macro_as_call_id(self.db, file_ast_id, attr, self.def_map.krate, def)
+                    };
                     if let MacroDefId {
                         kind:
                             MacroDefKind::BuiltInAttr(
-                                BuiltinAttrExpander::Derive | BuiltinAttrExpander::DeriveConst,
                                 _,
+                                BuiltinAttrExpander::Derive | BuiltinAttrExpander::DeriveConst,
                             ),
                         ..
                     } = def
@@ -1290,6 +1302,7 @@ impl DefCollector<'_> {
 
                         match attr.parse_path_comma_token_tree(self.db.upcast()) {
                             Some(derive_macros) => {
+                                let call_id = call_id();
                                 let mut len = 0;
                                 for (idx, (path, call_site)) in derive_macros.enumerate() {
                                     let ast_id = AstIdWithPath::new(file_id, ast_id.value, path);
@@ -1312,13 +1325,6 @@ impl DefCollector<'_> {
                                 // This is just a trick to be able to resolve the input to derives
                                 // as proper paths in `Semantics`.
                                 // Check the comment in [`builtin_attr_macro`].
-                                let call_id = attr_macro_as_call_id(
-                                    self.db,
-                                    file_ast_id,
-                                    attr,
-                                    self.def_map.krate,
-                                    def,
-                                );
                                 self.def_map.modules[directive.module_id]
                                     .scope
                                     .init_derive_attribute(ast_id, attr.id, call_id, len + 1);
@@ -1336,17 +1342,8 @@ impl DefCollector<'_> {
                         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!(
-                        def.kind,
-                        MacroDefKind::BuiltInAttr(expander, _)
-                        if expander.is_test() || expander.is_bench()
-                    ) {
-                        return recollect_without(self);
-                    }
-
-                    if let MacroDefKind::ProcMacro(exp, ..) = def.kind {
+                    let call_id = call_id();
+                    if let MacroDefKind::ProcMacro(_, exp, _) = def.kind {
                         // 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(

crates/hir-def/src/nameres/collector.rs

@@ -189,8 +189,8 @@ pub(crate) fn process_cfg_attrs(
     // FIXME: #[cfg_eval] is not implemented. But it is not stable yet
     let is_derive = match loc.def.kind {
         MacroDefKind::BuiltInDerive(..)
-        | MacroDefKind::ProcMacro(_, ProcMacroKind::CustomDerive, _) => true,
-        MacroDefKind::BuiltInAttr(expander, _) => expander.is_derive(),
+        | MacroDefKind::ProcMacro(_, _, ProcMacroKind::CustomDerive) => true,
+        MacroDefKind::BuiltInAttr(_, expander) => expander.is_derive(),
         _ => false,
     };
     if !is_derive {

crates/hir-expand/src/cfg_process.rs

@@ -252,7 +252,7 @@ pub fn expand_speculative(
     // Otherwise the expand query will fetch the non speculative attribute args and pass those instead.
     let mut speculative_expansion =
         match loc.def.kind {
-            MacroDefKind::ProcMacro(expander, _, ast) => {
+            MacroDefKind::ProcMacro(ast, expander, _) => {
                 let span = db.proc_macro_span(ast);
                 tt.delimiter = tt::Delimiter::invisible_spanned(span);
                 expander.expand(
@@ -266,22 +266,22 @@ pub fn expand_speculative(
                     span_with_mixed_site_ctxt(db, span, actual_macro_call),
                 )
             }
-            MacroDefKind::BuiltInAttr(BuiltinAttrExpander::Derive, _) => {
+            MacroDefKind::BuiltInAttr(_, it) if it.is_derive() => {
                 pseudo_derive_attr_expansion(&tt, attr_arg.as_ref()?, span)
             }
             MacroDefKind::Declarative(it) => db
                 .decl_macro_expander(loc.krate, it)
                 .expand_unhygienic(db, tt, loc.def.krate, span, loc.def.edition),
-            MacroDefKind::BuiltIn(it, _) => {
+            MacroDefKind::BuiltIn(_, it) => {
                 it.expand(db, actual_macro_call, &tt, span).map_err(Into::into)
             }
-            MacroDefKind::BuiltInDerive(it, ..) => {
+            MacroDefKind::BuiltInDerive(_, it) => {
                 it.expand(db, actual_macro_call, &tt, span).map_err(Into::into)
             }
-            MacroDefKind::BuiltInEager(it, _) => {
+            MacroDefKind::BuiltInEager(_, it) => {
                 it.expand(db, actual_macro_call, &tt, span).map_err(Into::into)
             }
-            MacroDefKind::BuiltInAttr(it, _) => it.expand(db, actual_macro_call, &tt, span),
+            MacroDefKind::BuiltInAttr(_, it) => it.expand(db, actual_macro_call, &tt, span),
         };
 
     let expand_to = loc.expand_to();
@@ -493,7 +493,7 @@ fn macro_arg(db: &dyn ExpandDatabase, id: MacroCallId) -> MacroArgResult {
                     .map_or_else(|| node.syntax().text_range(), |it| it.syntax().text_range()),
             );
             // If derive attribute we need to censor the derive input
-            if matches!(loc.def.kind, MacroDefKind::BuiltInAttr(expander, ..) if expander.is_derive())
+            if matches!(loc.def.kind, MacroDefKind::BuiltInAttr(_, expander) if expander.is_derive())
                 && ast::Adt::can_cast(node.syntax().kind())
             {
                 let adt = ast::Adt::cast(node.syntax().clone()).unwrap();
@@ -569,11 +569,11 @@ impl TokenExpander {
             MacroDefKind::Declarative(ast_id) => {
                 TokenExpander::DeclarativeMacro(db.decl_macro_expander(id.krate, ast_id))
             }
-            MacroDefKind::BuiltIn(expander, _) => TokenExpander::BuiltIn(expander),
-            MacroDefKind::BuiltInAttr(expander, _) => TokenExpander::BuiltInAttr(expander),
-            MacroDefKind::BuiltInDerive(expander, _) => TokenExpander::BuiltInDerive(expander),
-            MacroDefKind::BuiltInEager(expander, ..) => TokenExpander::BuiltInEager(expander),
-            MacroDefKind::ProcMacro(expander, ..) => TokenExpander::ProcMacro(expander),
+            MacroDefKind::BuiltIn(_, expander) => TokenExpander::BuiltIn(expander),
+            MacroDefKind::BuiltInAttr(_, expander) => TokenExpander::BuiltInAttr(expander),
+            MacroDefKind::BuiltInDerive(_, expander) => TokenExpander::BuiltInDerive(expander),
+            MacroDefKind::BuiltInEager(_, expander) => TokenExpander::BuiltInEager(expander),
+            MacroDefKind::ProcMacro(_, expander, _) => TokenExpander::ProcMacro(expander),
         }
     }
 }
@@ -604,13 +604,13 @@ fn macro_expand(
                     MacroDefKind::Declarative(id) => db
                         .decl_macro_expander(loc.def.krate, id)
                         .expand(db, arg.clone(), macro_call_id, span),
-                    MacroDefKind::BuiltIn(it, _) => {
+                    MacroDefKind::BuiltIn(_, it) => {
                         it.expand(db, macro_call_id, arg, span).map_err(Into::into).zip_val(None)
                     }
-                    MacroDefKind::BuiltInDerive(it, _) => {
+                    MacroDefKind::BuiltInDerive(_, it) => {
                         it.expand(db, macro_call_id, arg, span).map_err(Into::into).zip_val(None)
                     }
-                    MacroDefKind::BuiltInEager(it, _) => {
+                    MacroDefKind::BuiltInEager(_, it) => {
                         // This might look a bit odd, but we do not expand the inputs to eager macros here.
                         // Eager macros inputs are expanded, well, eagerly when we collect the macro calls.
                         // That kind of expansion uses the ast id map of an eager macros input though which goes through
@@ -634,12 +634,12 @@ fn macro_expand(
                         }
                         res.zip_val(None)
                     }
-                    MacroDefKind::BuiltInAttr(it, _) => {
+                    MacroDefKind::BuiltInAttr(_, it) => {
                         let mut res = it.expand(db, macro_call_id, arg, span);
                         fixup::reverse_fixups(&mut res.value, &undo_info);
                         res.zip_val(None)
                     }
-                    _ => unreachable!(),
+                    MacroDefKind::ProcMacro(_, _, _) => unreachable!(),
                 };
             (ExpandResult { value: res.value, err: res.err }, span)
         }
@@ -678,8 +678,8 @@ fn expand_proc_macro(db: &dyn ExpandDatabase, id: MacroCallId) -> ExpandResult<A
     let loc = db.lookup_intern_macro_call(id);
     let (macro_arg, undo_info, span) = db.macro_arg_considering_derives(id, &loc.kind);
 
-    let (expander, ast) = match loc.def.kind {
-        MacroDefKind::ProcMacro(expander, _, ast) => (expander, ast),
+    let (ast, expander) = match loc.def.kind {
+        MacroDefKind::ProcMacro(ast, expander, _) => (ast, expander),
         _ => unreachable!(),
     };
 

crates/hir-expand/src/db.rs

@@ -36,11 +36,6 @@ pub fn find_builtin_attr_idx(name: &str) -> Option<usize> {
         .copied()
 }
 
-// impl AttributeTemplate {
-//     const DEFAULT: AttributeTemplate =
-//         AttributeTemplate { word: false, list: None, name_value_str: None };
-// }
-
 /// A convenience macro for constructing attribute templates.
 /// E.g., `template!(Word, List: "description")` means that the attribute
 /// supports forms `#[attr]` and `#[attr(description)]`.