Skip to content

Commit

Permalink
Auto merge of #17341 - Veykril:inert-attr, r=Veykril
Browse files Browse the repository at this point in the history
internal: Cleanup some inert attribute stuff
  • Loading branch information
bors committed Jun 4, 2024
2 parents 6bae8e3 + 6f0207d commit f28f15a
Show file tree
Hide file tree
Showing 17 changed files with 155 additions and 162 deletions.
57 changes: 52 additions & 5 deletions crates/hir-def/src/attr.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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()),
);
}
}
48 changes: 0 additions & 48 deletions crates/hir-def/src/attr/tests.rs

This file was deleted.

2 changes: 1 addition & 1 deletion crates/hir-def/src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions crates/hir-def/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
};

Expand Down Expand Up @@ -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,
Expand Down
11 changes: 7 additions & 4 deletions crates/hir-def/src/nameres/attr_resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
52 changes: 22 additions & 30 deletions crates/hir-def/src/nameres/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use cfg::{CfgExpr, CfgOptions};
use either::Either;
use hir_expand::{
attrs::{Attr, AttrId},
builtin_attr_macro::{find_builtin_attr, BuiltinAttrExpander},
builtin_attr_macro::find_builtin_attr,
builtin_derive_macro::find_builtin_derive,
builtin_fn_macro::find_builtin_macro,
name::{name, AsName, Name},
Expand Down Expand Up @@ -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>,
}

Expand Down Expand Up @@ -1255,17 +1256,23 @@ impl DefCollector<'_> {
_ => return Resolved::No,
};

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,
_,
),
..
} = 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 matches!(def,
MacroDefId { kind: MacroDefKind::BuiltInAttr(_, exp), .. }
if exp.is_derive()
) {
// 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.
Expand All @@ -1290,6 +1297,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);
Expand All @@ -1312,13 +1320,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);
Expand All @@ -1336,17 +1337,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(
Expand Down
2 changes: 0 additions & 2 deletions crates/hir-expand/src/builtin_attr_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ impl BuiltinAttrExpander {

register_builtin! {
(bench, Bench) => dummy_attr_expand,
(cfg, Cfg) => dummy_attr_expand,
(cfg_attr, CfgAttr) => dummy_attr_expand,
(cfg_accessible, CfgAccessible) => dummy_attr_expand,
(cfg_eval, CfgEval) => dummy_attr_expand,
(derive, Derive) => derive_expand,
Expand Down
4 changes: 2 additions & 2 deletions crates/hir-expand/src/cfg_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit f28f15a

Please sign in to comment.