1055: store macro def inside macro id r=matklad a=matklad

This makes macro expansion resilient to changes to code inside the macro

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
This commit is contained in:
bors[bot] 2019-03-26 11:19:38 +00:00
commit 8254244e49
8 changed files with 105 additions and 144 deletions

View File

@ -76,7 +76,7 @@ impl Module {
import: ImportId,
) -> TreeArc<ast::PathSegment> {
let (file_id, source) = self.definition_source(db);
let (_, source_map) = db.raw_items_with_source_map(file_id.original_file(db));
let (_, source_map) = db.raw_items_with_source_map(file_id);
source_map.get(&source, import)
}

View File

@ -1,10 +1,10 @@
use std::sync::Arc;
use ra_syntax::{SyntaxNode, TreeArc, SourceFile};
use ra_db::{SourceDatabase, salsa, FileId};
use ra_db::{SourceDatabase, salsa};
use crate::{
HirFileId, SourceFileItems, SourceItemId, Crate, Module, HirInterner,
HirFileId, MacroDefId, SourceFileItems, SourceItemId, Crate, Module, HirInterner,
Function, FnSignature, ExprScopes, TypeAlias,
Struct, Enum, StructField,
Const, ConstSignature, Static,
@ -19,6 +19,9 @@ use crate::{
#[salsa::query_group(DefDatabaseStorage)]
pub trait DefDatabase: SourceDatabase + AsRef<HirInterner> {
#[salsa::invoke(crate::ids::macro_def_query)]
fn macro_def(&self, macro_id: MacroDefId) -> Option<Arc<mbe::MacroRules>>;
#[salsa::invoke(HirFileId::hir_parse)]
fn hir_parse(&self, file_id: HirFileId) -> TreeArc<SourceFile>;
@ -38,10 +41,13 @@ pub trait DefDatabase: SourceDatabase + AsRef<HirInterner> {
fn file_item(&self, source_item_id: SourceItemId) -> TreeArc<SyntaxNode>;
#[salsa::invoke(RawItems::raw_items_query)]
fn raw_items(&self, file_id: FileId) -> Arc<RawItems>;
fn raw_items(&self, file_id: HirFileId) -> Arc<RawItems>;
#[salsa::invoke(RawItems::raw_items_with_source_map_query)]
fn raw_items_with_source_map(&self, file_id: FileId) -> (Arc<RawItems>, Arc<ImportSourceMap>);
fn raw_items_with_source_map(
&self,
file_id: HirFileId,
) -> (Arc<RawItems>, Arc<ImportSourceMap>);
#[salsa::invoke(CrateDefMap::crate_def_map_query)]
fn crate_def_map(&self, krate: Crate) -> Arc<CrateDefMap>;

View File

@ -7,6 +7,7 @@ use std::{
use ra_db::{LocationInterner, FileId};
use ra_syntax::{TreeArc, SyntaxNode, SourceFile, AstNode, SyntaxNodePtr, ast};
use ra_arena::{Arena, RawId, ArenaId, impl_arena_id};
use mbe::MacroRules;
use crate::{
Module,
@ -100,10 +101,7 @@ fn parse_macro(db: &impl DefDatabase, macro_call_id: MacroCallId) -> Option<Tree
let macro_call = ast::MacroCall::cast(&syntax).unwrap();
let (macro_arg, _) = macro_call.token_tree().and_then(mbe::ast_to_token_tree)?;
let def_map = db.crate_def_map(loc.module.krate);
let (krate, macro_id) = def_map.resolve_macro(macro_call_id)?;
let def_map = db.crate_def_map(krate);
let macro_rules = &def_map[macro_id];
let macro_rules = db.macro_def(loc.def)?;
let tt = macro_rules.expand(&macro_arg).ok()?;
Some(mbe::token_tree_to_ast_item_list(&tt))
}
@ -126,6 +124,22 @@ impl From<MacroCallId> for HirFileId {
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum MacroDefId {
MacroByExample { source_item_id: SourceItemId },
}
pub(crate) fn macro_def_query(db: &impl DefDatabase, id: MacroDefId) -> Option<Arc<MacroRules>> {
let syntax_node = match id {
MacroDefId::MacroByExample { source_item_id } => db.file_item(source_item_id),
};
let macro_call = ast::MacroCall::cast(&syntax_node).unwrap();
let arg = macro_call.token_tree()?;
let (tt, _) = mbe::ast_to_token_tree(arg)?;
let rules = MacroRules::parse(&tt).ok()?;
Some(Arc::new(rules))
}
/// `MacroCallId` identifies a particular macro invocation, like
/// `println!("Hello, {}", world)`.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@ -134,7 +148,7 @@ impl_arena_id!(MacroCallId);
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct MacroCallLoc {
pub(crate) module: Module,
pub(crate) def: MacroDefId,
pub(crate) source_item_id: SourceItemId,
}

View File

@ -53,7 +53,7 @@ use crate::{
pub use self::{
path::{Path, PathKind},
name::Name,
ids::{HirFileId, MacroCallId, MacroCallLoc, HirInterner},
ids::{HirFileId, MacroDefId, MacroCallId, MacroCallLoc, HirInterner},
nameres::{PerNs, Namespace},
ty::{Ty, ApplicationTy, TypeCtor, Substs, display::HirDisplay},
impl_block::{ImplBlock, ImplItem},

View File

@ -63,7 +63,7 @@ use test_utils::tested_by;
use crate::{
ModuleDef, Name, Crate, Module,
DefDatabase, Path, PathKind, HirFileId, Trait,
ids::{SourceItemId, SourceFileItemId, MacroCallId},
ids::{SourceItemId, SourceFileItemId, MacroCallId, MacroDefId},
diagnostics::DiagnosticSink,
nameres::diagnostics::DefDiagnostic,
};
@ -85,7 +85,7 @@ pub struct CrateDefMap {
root: CrateModuleId,
modules: Arena<CrateModuleId, ModuleData>,
macros: Arena<CrateMacroId, mbe::MacroRules>,
public_macros: FxHashMap<Name, CrateMacroId>,
public_macros: FxHashMap<Name, MacroDefId>,
macro_resolutions: FxHashMap<MacroCallId, (Crate, CrateMacroId)>,
diagnostics: Vec<DefDiagnostic>,
}
@ -238,13 +238,6 @@ impl CrateDefMap {
self.diagnostics.iter().for_each(|it| it.add_to(db, module, sink))
}
pub(crate) fn resolve_macro(
&self,
macro_call_id: MacroCallId,
) -> Option<(Crate, CrateMacroId)> {
self.macro_resolutions.get(&macro_call_id).map(|&it| it)
}
pub(crate) fn find_module_by_source(
&self,
file_id: HirFileId,

View File

@ -6,15 +6,15 @@ use ra_db::FileId;
use crate::{
Function, Module, Struct, Enum, Const, Static, Trait, TypeAlias,
DefDatabase, HirFileId, Name, Path, Crate,
DefDatabase, HirFileId, Name, Path,
KnownName,
nameres::{
Resolution, PerNs, ModuleDef, ReachedFixedPoint, ResolveMode,
CrateDefMap, CrateModuleId, ModuleData, CrateMacroId,
CrateDefMap, CrateModuleId, ModuleData,
diagnostics::DefDiagnostic,
raw,
},
ids::{AstItemDef, LocationCtx, MacroCallLoc, SourceItemId, MacroCallId},
ids::{AstItemDef, LocationCtx, MacroCallLoc, SourceItemId, MacroCallId, MacroDefId},
};
pub(super) fn collect_defs(db: &impl DefDatabase, mut def_map: CrateDefMap) -> CrateDefMap {
@ -51,8 +51,8 @@ struct DefCollector<DB> {
def_map: CrateDefMap,
glob_imports: FxHashMap<CrateModuleId, Vec<(CrateModuleId, raw::ImportId)>>,
unresolved_imports: Vec<(CrateModuleId, raw::ImportId, raw::ImportData)>,
unexpanded_macros: Vec<(CrateModuleId, MacroCallId, Path, tt::Subtree)>,
global_macro_scope: FxHashMap<Name, CrateMacroId>,
unexpanded_macros: Vec<(CrateModuleId, SourceItemId, Path)>,
global_macro_scope: FxHashMap<Name, MacroDefId>,
}
impl<'a, DB> DefCollector<&'a DB>
@ -62,7 +62,7 @@ where
fn collect(&mut self) {
let crate_graph = self.db.crate_graph();
let file_id = crate_graph.crate_root(self.def_map.krate.crate_id());
let raw_items = self.db.raw_items(file_id);
let raw_items = self.db.raw_items(file_id.into());
let module_id = self.def_map.root;
self.def_map.modules[module_id].definition = Some(file_id);
ModCollector {
@ -93,14 +93,11 @@ where
}
}
fn define_macro(&mut self, name: Name, tt: &tt::Subtree, export: bool) {
if let Ok(rules) = mbe::MacroRules::parse(tt) {
let macro_id = self.def_map.macros.alloc(rules);
if export {
self.def_map.public_macros.insert(name.clone(), macro_id);
}
self.global_macro_scope.insert(name, macro_id);
fn define_macro(&mut self, name: Name, macro_id: MacroDefId, export: bool) {
if export {
self.def_map.public_macros.insert(name.clone(), macro_id);
}
self.global_macro_scope.insert(name, macro_id);
}
fn resolve_imports(&mut self) -> ReachedFixedPoint {
@ -296,7 +293,7 @@ where
let mut macros = std::mem::replace(&mut self.unexpanded_macros, Vec::new());
let mut resolved = Vec::new();
let mut res = ReachedFixedPoint::Yes;
macros.retain(|(module_id, call_id, path, tt)| {
macros.retain(|(module_id, source_item_id, path)| {
if path.segments.len() != 2 {
return true;
}
@ -312,47 +309,24 @@ where
res = ReachedFixedPoint::No;
let def_map = self.db.crate_def_map(krate);
if let Some(macro_id) = def_map.public_macros.get(&path.segments[1].name).cloned() {
resolved.push((*module_id, *call_id, (krate, macro_id), tt.clone()));
let call_id =
MacroCallLoc { def: macro_id, source_item_id: *source_item_id }.id(self.db);
resolved.push((*module_id, call_id));
}
false
});
for (module_id, macro_call_id, macro_def_id, arg) in resolved {
self.collect_macro_expansion(module_id, macro_call_id, macro_def_id, arg);
for (module_id, macro_call_id) in resolved {
self.collect_macro_expansion(module_id, macro_call_id);
}
res
}
fn collect_macro_expansion(
&mut self,
module_id: CrateModuleId,
macro_call_id: MacroCallId,
macro_def_id: (Crate, CrateMacroId),
macro_arg: tt::Subtree,
) {
let (macro_krate, macro_id) = macro_def_id;
let dm;
let rules = if macro_krate == self.def_map.krate {
&self.def_map[macro_id]
} else {
dm = self.db.crate_def_map(macro_krate);
&dm[macro_id]
};
if let Ok(expansion) = rules.expand(&macro_arg) {
self.def_map.macro_resolutions.insert(macro_call_id, macro_def_id);
// XXX: this **does not** go through a database, because we can't
// identify macro_call without adding the whole state of name resolution
// as a parameter to the query.
//
// So, we run the queries "manually" and we must ensure that
// `db.hir_parse(macro_call_id)` returns the same source_file.
let file_id: HirFileId = macro_call_id.into();
let source_file = mbe::token_tree_to_ast_item_list(&expansion);
let raw_items = raw::RawItems::from_source_file(&source_file, file_id);
ModCollector { def_collector: &mut *self, file_id, module_id, raw_items: &raw_items }
.collect(raw_items.items())
}
fn collect_macro_expansion(&mut self, module_id: CrateModuleId, macro_call_id: MacroCallId) {
let file_id: HirFileId = macro_call_id.into();
let raw_items = self.db.raw_items(file_id);
ModCollector { def_collector: &mut *self, file_id, module_id, raw_items: &raw_items }
.collect(raw_items.items())
}
fn finish(self) -> CrateDefMap {
@ -412,7 +386,7 @@ where
Ok(file_id) => {
let module_id =
self.push_child_module(name.clone(), source_item_id, Some(file_id));
let raw_items = self.def_collector.db.raw_items(file_id);
let raw_items = self.def_collector.db.raw_items(file_id.into());
ModCollector {
def_collector: &mut *self.def_collector,
module_id,
@ -484,38 +458,33 @@ where
// Case 1: macro rules, define a macro in crate-global mutable scope
if is_macro_rules(&mac.path) {
if let Some(name) = &mac.name {
self.def_collector.define_macro(name.clone(), &mac.arg, mac.export)
let macro_id = MacroDefId::MacroByExample {
source_item_id: mac.source_item_id.with_file_id(self.file_id),
};
self.def_collector.define_macro(name.clone(), macro_id, mac.export)
}
return;
}
let source_item_id = SourceItemId { file_id: self.file_id, item_id: mac.source_item_id };
let macro_call_id = MacroCallLoc {
module: Module { krate: self.def_collector.def_map.krate, module_id: self.module_id },
source_item_id,
}
.id(self.def_collector.db);
// Case 2: try to expand macro_rules from this crate, triggering
// recursive item collection.
if let Some(&macro_id) =
mac.path.as_ident().and_then(|name| self.def_collector.global_macro_scope.get(name))
{
self.def_collector.collect_macro_expansion(
self.module_id,
macro_call_id,
(self.def_collector.def_map.krate, macro_id),
mac.arg.clone(),
);
let macro_call_id =
MacroCallLoc { def: macro_id, source_item_id }.id(self.def_collector.db);
self.def_collector.collect_macro_expansion(self.module_id, macro_call_id);
return;
}
// Case 3: path to a macro from another crate, expand during name resolution
self.def_collector.unexpanded_macros.push((
self.module_id,
macro_call_id,
source_item_id,
mac.path.clone(),
mac.arg.clone(),
))
}
}

View File

@ -4,7 +4,6 @@ use std::{
};
use test_utils::tested_by;
use ra_db::FileId;
use ra_arena::{Arena, impl_arena_id, RawId, map::ArenaMap};
use ra_syntax::{
AstNode, SourceFile, AstPtr, TreeArc,
@ -16,6 +15,10 @@ use crate::{
ids::{SourceFileItemId, SourceFileItems},
};
/// `RawItems` is a set of top-level items in a file (except for impls).
///
/// It is the input to name resolution algorithm. `RawItems` are not invalidated
/// on most edits.
#[derive(Debug, Default, PartialEq, Eq)]
pub struct RawItems {
modules: Arena<Module, ModuleData>,
@ -32,11 +35,11 @@ pub struct ImportSourceMap {
}
impl ImportSourceMap {
pub(crate) fn insert(&mut self, import: ImportId, segment: &ast::PathSegment) {
fn insert(&mut self, import: ImportId, segment: &ast::PathSegment) {
self.map.insert(import, AstPtr::new(segment))
}
pub fn get(&self, source: &ModuleSource, import: ImportId) -> TreeArc<ast::PathSegment> {
pub(crate) fn get(&self, source: &ModuleSource, import: ImportId) -> TreeArc<ast::PathSegment> {
let file = match source {
ModuleSource::SourceFile(file) => &*file,
ModuleSource::Module(m) => m.syntax().ancestors().find_map(SourceFile::cast).unwrap(),
@ -47,40 +50,27 @@ impl ImportSourceMap {
}
impl RawItems {
pub(crate) fn raw_items_query(db: &impl DefDatabase, file_id: FileId) -> Arc<RawItems> {
pub(crate) fn raw_items_query(db: &impl DefDatabase, file_id: HirFileId) -> Arc<RawItems> {
db.raw_items_with_source_map(file_id).0
}
pub(crate) fn raw_items_with_source_map_query(
db: &impl DefDatabase,
file_id: FileId,
file_id: HirFileId,
) -> (Arc<RawItems>, Arc<ImportSourceMap>) {
let mut collector = RawItemsCollector {
raw_items: RawItems::default(),
source_file_items: db.file_items(file_id.into()),
source_map: ImportSourceMap::default(),
};
let source_file = db.parse(file_id);
let source_file = db.hir_parse(file_id);
collector.process_module(None, &*source_file);
(Arc::new(collector.raw_items), Arc::new(collector.source_map))
}
pub(crate) fn items(&self) -> &[RawItem] {
pub(super) fn items(&self) -> &[RawItem] {
&self.items
}
// We can't use queries during name resolution for fear of cycles, so this
// is a query-less variant of the above function.
pub(crate) fn from_source_file(source_file: &SourceFile, file_id: HirFileId) -> RawItems {
let source_file_items = SourceFileItems::from_source_file(source_file, file_id);
let mut collector = RawItemsCollector {
raw_items: RawItems::default(),
source_file_items: Arc::new(source_file_items),
source_map: ImportSourceMap::default(),
};
collector.process_module(None, &*source_file);
collector.raw_items
}
}
impl Index<Module> for RawItems {
@ -112,7 +102,7 @@ impl Index<Macro> for RawItems {
}
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
pub(crate) enum RawItem {
pub(super) enum RawItem {
Module(Module),
Import(ImportId),
Def(Def),
@ -120,11 +110,11 @@ pub(crate) enum RawItem {
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub(crate) struct Module(RawId);
pub(super) struct Module(RawId);
impl_arena_id!(Module);
#[derive(Debug, PartialEq, Eq)]
pub(crate) enum ModuleData {
pub(super) enum ModuleData {
Declaration { name: Name, source_item_id: SourceFileItemId },
Definition { name: Name, source_item_id: SourceFileItemId, items: Vec<RawItem> },
}
@ -135,26 +125,26 @@ impl_arena_id!(ImportId);
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ImportData {
pub(crate) path: Path,
pub(crate) alias: Option<Name>,
pub(crate) is_glob: bool,
pub(crate) is_prelude: bool,
pub(crate) is_extern_crate: bool,
pub(super) path: Path,
pub(super) alias: Option<Name>,
pub(super) is_glob: bool,
pub(super) is_prelude: bool,
pub(super) is_extern_crate: bool,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub(crate) struct Def(RawId);
pub(super) struct Def(RawId);
impl_arena_id!(Def);
#[derive(Debug, PartialEq, Eq)]
pub(crate) struct DefData {
pub(crate) source_item_id: SourceFileItemId,
pub(crate) name: Name,
pub(crate) kind: DefKind,
pub(super) struct DefData {
pub(super) source_item_id: SourceFileItemId,
pub(super) name: Name,
pub(super) kind: DefKind,
}
#[derive(Debug, PartialEq, Eq, Clone, Copy)]
pub(crate) enum DefKind {
pub(super) enum DefKind {
Function,
Struct,
Enum,
@ -165,16 +155,15 @@ pub(crate) enum DefKind {
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub(crate) struct Macro(RawId);
pub(super) struct Macro(RawId);
impl_arena_id!(Macro);
#[derive(Debug, PartialEq, Eq)]
pub(crate) struct MacroData {
pub(crate) source_item_id: SourceFileItemId,
pub(crate) path: Path,
pub(crate) name: Option<Name>,
pub(crate) arg: tt::Subtree,
pub(crate) export: bool,
pub(super) struct MacroData {
pub(super) source_item_id: SourceFileItemId,
pub(super) path: Path,
pub(super) name: Option<Name>,
pub(super) export: bool,
}
struct RawItemsCollector {
@ -291,18 +280,15 @@ impl RawItemsCollector {
}
fn add_macro(&mut self, current_module: Option<Module>, m: &ast::MacroCall) {
let (path, arg) = match (
m.path().and_then(Path::from_ast),
m.token_tree().and_then(mbe::ast_to_token_tree),
) {
(Some(path), Some((token_tree, _token_map))) => (path, token_tree),
let path = match m.path().and_then(Path::from_ast) {
Some(it) => it,
_ => return,
};
let name = m.name().map(|it| it.as_name());
let source_item_id = self.source_file_items.id_of_unchecked(m.syntax());
let export = m.has_atom_attr("macro_export");
let m = self.raw_items.macros.alloc(MacroData { source_item_id, path, arg, name, export });
let m = self.raw_items.macros.alloc(MacroData { source_item_id, path, name, export });
self.push_item(current_module, RawItem::Macro(m));
}

View File

@ -90,34 +90,27 @@ fn adding_inner_items_should_not_invalidate_def_map() {
);
}
// It would be awesome to make this work, but it's unclear how
#[test]
#[ignore]
fn typing_inside_a_function_inside_a_macro_should_not_invalidate_def_map() {
fn typing_inside_a_macro_should_not_invalidate_def_map() {
check_def_map_is_not_recomputed(
"
//- /lib.rs
macro_rules! m {
($ident:ident) => {
struct Foo;
}
}
mod foo;
use crate::foo::bar::Baz;
//- /foo/mod.rs
pub mod bar;
//- /foo/bar.rs
<|>
salsa::query_group! {
trait Baz {
fn foo() -> i32 { 1 + 1 }
}
}
m!(X);
",
"
salsa::query_group! {
trait Baz {
fn foo() -> i32 { 92 }
}
}
m!(Y);
",
);
}