Skip to content

Add expansion infrastructure for derive macros #2479

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion crates/ra_hir/src/code_model/src.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,10 @@ impl HasSource for TypeAlias {
impl HasSource for MacroDef {
type Ast = ast::MacroCall;
fn source(self, db: &impl DefDatabase) -> InFile<ast::MacroCall> {
InFile { file_id: self.id.ast_id.file_id, value: self.id.ast_id.to_node(db) }
InFile {
file_id: self.id.ast_id.expect("MacroDef without ast_id").file_id,
value: self.id.ast_id.expect("MacroDef without ast_id").to_node(db),
}
}
}
impl HasSource for ImplBlock {
Expand Down
4 changes: 2 additions & 2 deletions crates/ra_hir/src/from_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,9 @@ impl FromSource for MacroDef {

let module_src = ModuleSource::from_child_node(db, src.as_ref().map(|it| it.syntax()));
let module = Module::from_definition(db, InFile::new(src.file_id, module_src))?;
let krate = module.krate().crate_id();
let krate = Some(module.krate().crate_id());

let ast_id = AstId::new(src.file_id, db.ast_id_map(src.file_id).ast_id(&src.value));
let ast_id = Some(AstId::new(src.file_id, db.ast_id_map(src.file_id).ast_id(&src.value)));

let id: MacroDefId = MacroDefId { krate, ast_id, kind };
Some(MacroDef { id })
Expand Down
5 changes: 3 additions & 2 deletions crates/ra_hir/src/source_binder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ use hir_def::{
AssocItemId, DefWithBodyId,
};
use hir_expand::{
hygiene::Hygiene, name::AsName, AstId, HirFileId, InFile, MacroCallId, MacroFileKind,
hygiene::Hygiene, name::AsName, AstId, HirFileId, InFile, MacroCallId, MacroCallKind,
MacroFileKind,
};
use ra_syntax::{
ast::{self, AstNode},
Expand Down Expand Up @@ -456,7 +457,7 @@ impl SourceAnalyzer {
db.ast_id_map(macro_call.file_id).ast_id(macro_call.value),
);
Some(Expansion {
macro_call_id: def.as_call_id(db, ast_id),
macro_call_id: def.as_call_id(db, MacroCallKind::FnLike(ast_id)),
macro_file_kind: to_macro_file_kind(macro_call.value),
})
}
Expand Down
4 changes: 3 additions & 1 deletion crates/ra_hir_def/src/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ impl Attrs {
AdtId::UnionId(it) => attrs_from_ast(it.lookup_intern(db).ast_id, db),
},
AttrDefId::TraitId(it) => attrs_from_ast(it.lookup_intern(db).ast_id, db),
AttrDefId::MacroDefId(it) => attrs_from_ast(it.ast_id, db),
AttrDefId::MacroDefId(it) => {
it.ast_id.map_or_else(Default::default, |ast_id| attrs_from_ast(ast_id, db))
}
AttrDefId::ImplId(it) => attrs_from_ast(it.lookup_intern(db).ast_id, db),
AttrDefId::ConstId(it) => attrs_from_loc(it.lookup(db), db),
AttrDefId::StaticId(it) => attrs_from_loc(it.lookup(db), db),
Expand Down
6 changes: 4 additions & 2 deletions crates/ra_hir_def/src/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ pub mod scope;
use std::{ops::Index, sync::Arc};

use either::Either;
use hir_expand::{hygiene::Hygiene, AstId, HirFileId, InFile, MacroDefId, MacroFileKind};
use hir_expand::{
hygiene::Hygiene, AstId, HirFileId, InFile, MacroCallKind, MacroDefId, MacroFileKind,
};
use ra_arena::{map::ArenaMap, Arena};
use ra_syntax::{ast, AstNode, AstPtr};
use rustc_hash::FxHashMap;
Expand Down Expand Up @@ -46,7 +48,7 @@ impl Expander {

if let Some(path) = macro_call.path().and_then(|path| self.parse_path(path)) {
if let Some(def) = self.resolve_path_as_macro(db, &path) {
let call_id = def.as_call_id(db, ast_id);
let call_id = def.as_call_id(db, MacroCallKind::FnLike(ast_id));
let file_id = call_id.as_file(MacroFileKind::Expr);
if let Some(node) = db.parse_or_expand(file_id) {
if let Some(expr) = ast::Expr::cast(node) {
Expand Down
2 changes: 1 addition & 1 deletion crates/ra_hir_def/src/docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl Documentation {
docs_from_ast(&src.value[it.local_id])
}
AttrDefId::TraitId(it) => docs_from_ast(&it.source(db).value),
AttrDefId::MacroDefId(it) => docs_from_ast(&it.ast_id.to_node(db)),
AttrDefId::MacroDefId(it) => docs_from_ast(&it.ast_id?.to_node(db)),
AttrDefId::ConstId(it) => docs_from_ast(&it.lookup(db).source(db).value),
AttrDefId::StaticId(it) => docs_from_ast(&it.lookup(db).source(db).value),
AttrDefId::FunctionId(it) => docs_from_ast(&it.lookup(db).source(db).value),
Expand Down
73 changes: 66 additions & 7 deletions crates/ra_hir_def/src/nameres/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
//! resolves imports and expands macros.

use hir_expand::{
builtin_derive::find_builtin_derive,
builtin_macro::find_builtin_macro,
name::{self, AsName, Name},
HirFileId, MacroCallId, MacroDefId, MacroDefKind, MacroFileKind,
HirFileId, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind, MacroFileKind,
};
use ra_cfg::CfgOptions;
use ra_db::{CrateId, FileId};
Expand Down Expand Up @@ -58,6 +59,7 @@ pub(super) fn collect_defs(db: &impl DefDatabase, mut def_map: CrateDefMap) -> C
glob_imports: FxHashMap::default(),
unresolved_imports: Vec::new(),
unexpanded_macros: Vec::new(),
unexpanded_attribute_macros: Vec::new(),
mod_dirs: FxHashMap::default(),
macro_stack_monitor: MacroStackMonitor::default(),
poison_macros: FxHashSet::default(),
Expand Down Expand Up @@ -102,6 +104,7 @@ struct DefCollector<'a, DB> {
glob_imports: FxHashMap<LocalModuleId, Vec<(LocalModuleId, LocalImportId)>>,
unresolved_imports: Vec<(LocalModuleId, LocalImportId, raw::ImportData)>,
unexpanded_macros: Vec<(LocalModuleId, AstId<ast::MacroCall>, Path)>,
unexpanded_attribute_macros: Vec<(LocalModuleId, AstId<ast::ModuleItem>, Path)>,
mod_dirs: FxHashMap<LocalModuleId, ModDir>,

/// Some macro use `$tt:tt which mean we have to handle the macro perfectly
Expand Down Expand Up @@ -470,6 +473,8 @@ where

fn resolve_macros(&mut self) -> ReachedFixedPoint {
let mut macros = std::mem::replace(&mut self.unexpanded_macros, Vec::new());
let mut attribute_macros =
std::mem::replace(&mut self.unexpanded_attribute_macros, Vec::new());
let mut resolved = Vec::new();
let mut res = ReachedFixedPoint::Yes;
macros.retain(|(module_id, ast_id, path)| {
Expand All @@ -482,7 +487,19 @@ where
);

if let Some(def) = resolved_res.resolved_def.take_macros() {
let call_id = def.as_call_id(self.db, *ast_id);
let call_id = def.as_call_id(self.db, MacroCallKind::FnLike(*ast_id));
resolved.push((*module_id, call_id, def));
res = ReachedFixedPoint::No;
return false;
}

true
});
attribute_macros.retain(|(module_id, ast_id, path)| {
let resolved_res = self.resolve_attribute_macro(path);

if let Some(def) = resolved_res {
let call_id = def.as_call_id(self.db, MacroCallKind::Attr(*ast_id));
resolved.push((*module_id, call_id, def));
res = ReachedFixedPoint::No;
return false;
Expand All @@ -492,6 +509,7 @@ where
});

self.unexpanded_macros = macros;
self.unexpanded_attribute_macros = attribute_macros;

for (module_id, macro_call_id, macro_def_id) in resolved {
self.collect_macro_expansion(module_id, macro_call_id, macro_def_id);
Expand All @@ -500,6 +518,20 @@ where
res
}

fn resolve_attribute_macro(&self, path: &Path) -> Option<MacroDefId> {
// FIXME this is currently super hacky, just enough to support the
// built-in derives
if let Some(name) = path.as_ident() {
// FIXME this should actually be handled with the normal name
// resolution; the std lib defines built-in stubs for the derives,
// but these are new-style `macro`s, which we don't support yet
if let Some(def_id) = find_builtin_derive(name) {
return Some(def_id);
}
}
None
}

fn collect_macro_expansion(
&mut self,
module_id: LocalModuleId,
Expand Down Expand Up @@ -587,7 +619,9 @@ where
.def_collector
.unresolved_imports
.push((self.module_id, import_id, self.raw_items[import_id].clone())),
raw::RawItemKind::Def(def) => self.define_def(&self.raw_items[def]),
raw::RawItemKind::Def(def) => {
self.define_def(&self.raw_items[def], &item.attrs)
}
raw::RawItemKind::Macro(mac) => self.collect_macro(&self.raw_items[mac]),
raw::RawItemKind::Impl(imp) => {
let module = ModuleId {
Expand Down Expand Up @@ -682,10 +716,16 @@ where
res
}

fn define_def(&mut self, def: &raw::DefData) {
fn define_def(&mut self, def: &raw::DefData, attrs: &Attrs) {
let module = ModuleId { krate: self.def_collector.def_map.krate, local_id: self.module_id };
let ctx = LocationCtx::new(self.def_collector.db, module, self.file_id);

// FIXME: check attrs to see if this is an attribute macro invocation;
// in which case we don't add the invocation, just a single attribute
// macro invocation

self.collect_derives(attrs, def);

let name = def.name.clone();
let def: PerNs = match def.kind {
raw::DefKind::Function(ast_id) => {
Expand Down Expand Up @@ -736,6 +776,23 @@ where
self.def_collector.update(self.module_id, None, &[(name, resolution)])
}

fn collect_derives(&mut self, attrs: &Attrs, def: &raw::DefData) {
for derive_subtree in attrs.by_key("derive").tt_values() {
// for #[derive(Copy, Clone)], `derive_subtree` is the `(Copy, Clone)` subtree
for tt in &derive_subtree.token_trees {
let ident = match &tt {
tt::TokenTree::Leaf(tt::Leaf::Ident(ident)) => ident,
tt::TokenTree::Leaf(tt::Leaf::Punct(_)) => continue, // , is ok
_ => continue, // anything else would be an error (which we currently ignore)
};
let path = Path::from_tt_ident(ident);

let ast_id = AstId::new(self.file_id, def.kind.ast_id());
self.def_collector.unexpanded_attribute_macros.push((self.module_id, ast_id, path));
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this looks better!


fn collect_macro(&mut self, mac: &raw::MacroData) {
let ast_id = AstId::new(self.file_id, mac.ast_id);

Expand All @@ -759,8 +816,8 @@ where
if is_macro_rules(&mac.path) {
if let Some(name) = &mac.name {
let macro_id = MacroDefId {
ast_id,
krate: self.def_collector.def_map.krate,
ast_id: Some(ast_id),
krate: Some(self.def_collector.def_map.krate),
kind: MacroDefKind::Declarative,
};
self.def_collector.define_macro(self.module_id, name.clone(), macro_id, mac.export);
Expand All @@ -773,7 +830,8 @@ where
if let Some(macro_def) = mac.path.as_ident().and_then(|name| {
self.def_collector.def_map[self.module_id].scope.get_legacy_macro(&name)
}) {
let macro_call_id = macro_def.as_call_id(self.def_collector.db, ast_id);
let macro_call_id =
macro_def.as_call_id(self.def_collector.db, MacroCallKind::FnLike(ast_id));

self.def_collector.collect_macro_expansion(self.module_id, macro_call_id, macro_def);
return;
Expand Down Expand Up @@ -829,6 +887,7 @@ mod tests {
glob_imports: FxHashMap::default(),
unresolved_imports: Vec::new(),
unexpanded_macros: Vec::new(),
unexpanded_attribute_macros: Vec::new(),
mod_dirs: FxHashMap::default(),
macro_stack_monitor: monitor,
poison_macros: FxHashSet::default(),
Expand Down
15 changes: 15 additions & 0 deletions crates/ra_hir_def/src/nameres/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,21 @@ pub(super) enum DefKind {
TypeAlias(FileAstId<ast::TypeAliasDef>),
}

impl DefKind {
pub fn ast_id(&self) -> FileAstId<ast::ModuleItem> {
match self {
DefKind::Function(it) => it.upcast(),
DefKind::Struct(it) => it.upcast(),
DefKind::Union(it) => it.upcast(),
DefKind::Enum(it) => it.upcast(),
DefKind::Const(it) => it.upcast(),
DefKind::Static(it) => it.upcast(),
DefKind::Trait(it) => it.upcast(),
DefKind::TypeAlias(it) => it.upcast(),
}
}
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub(super) struct Macro(RawId);
impl_arena_id!(Macro);
Expand Down
24 changes: 24 additions & 0 deletions crates/ra_hir_def/src/nameres/tests/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -600,3 +600,27 @@ fn macro_dollar_crate_is_correct_in_indirect_deps() {
⋮bar: t v
"###);
}

#[test]
fn expand_derive() {
let map = compute_crate_def_map(
"
//- /main.rs
#[derive(Clone)]
struct Foo;
",
);
assert_eq!(map.modules[map.root].impls.len(), 1);
}

#[test]
fn expand_multiple_derive() {
let map = compute_crate_def_map(
"
//- /main.rs
#[derive(Copy, Clone)]
struct Foo;
",
);
assert_eq!(map.modules[map.root].impls.len(), 2);
}
5 changes: 5 additions & 0 deletions crates/ra_hir_def/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,11 @@ impl Path {
name_ref.as_name().into()
}

/// Converts an `tt::Ident` into a single-identifier `Path`.
pub(crate) fn from_tt_ident(ident: &tt::Ident) -> Path {
ident.as_name().into()
}

/// `true` is this path is a single identifier, like `foo`
pub fn is_ident(&self) -> bool {
self.kind == PathKind::Plain && self.segments.len() == 1
Expand Down
12 changes: 11 additions & 1 deletion crates/ra_hir_expand/src/ast_id_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,16 @@ impl<N: AstNode> Hash for FileAstId<N> {
}
}

impl<N: AstNode> FileAstId<N> {
// Can't make this a From implementation because of coherence
pub fn upcast<M: AstNode>(self) -> FileAstId<M>
where
M: From<N>,
{
FileAstId { raw: self.raw, _ty: PhantomData }
}
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
struct ErasedFileAstId(RawId);
impl_arena_id!(ErasedFileAstId);
Expand All @@ -53,7 +63,7 @@ impl AstIdMap {
pub(crate) fn from_source(node: &SyntaxNode) -> AstIdMap {
assert!(node.parent().is_none());
let mut res = AstIdMap { arena: Arena::default() };
// By walking the tree in bread-first order we make sure that parents
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol :D

// By walking the tree in breadth-first order we make sure that parents
// get lower ids then children. That is, adding a new child does not
// change parent's id. This means that, say, adding a new function to a
// trait does not change ids of top-level items, which helps caching.
Expand Down
Loading