Skip to content

Commit 1a29934

Browse files
Merge #7145
7145: Proper handling $crate Take 2 [DO NOT MERGE] r=edwin0cheng a=edwin0cheng Similar to previous PR (#7133) , but improved the following things : 1. Instead of storing the whole `ExpansionInfo`, we store a similar but stripped version `HygieneInfo`. 2. Instread of storing the `SyntaxNode` (because every token we are interested are IDENT), we store the `TextRange` only. 3. Because of 2, we now can put it in Salsa. 4. And most important improvement: Instead of computing the whole frames every single time, we compute it recursively through salsa: (Such that in the best scenario, we only need to compute the first layer of frame) ```rust let def_site = db.hygiene_frame(info.def.file_id); let call_site = db.hygiene_frame(info.arg.file_id); HygieneFrame { expansion: Some(info), local_inner, krate, call_site, def_site } ``` The overall speed compared to previous PR is much faster (65s vs 45s) : ``` [WITH old PR] Database loaded 644.86ms, 284mi Crates in this dir: 36 Total modules found: 576 Total declarations: 11153 Total functions: 8715 Item Collection: 15.78s, 91562mi Total expressions: 240721 Expressions of unknown type: 2635 (1%) Expressions of partially unknown type: 2064 (0%) Type mismatches: 865 Inference: 49.84s, 250747mi Total: 65.62s, 342310mi rust-analyzer -q analysis-stats . 66.72s user 0.57s system 99% cpu 1:07.40 total [WITH this PR] Database loaded 665.83ms, 284mi Crates in this dir: 36 Total modules found: 577 Total declarations: 11188 Total functions: 8743 Item Collection: 15.28s, 84919mi Total expressions: 241229 Expressions of unknown type: 2637 (1%) Expressions of partially unknown type: 2064 (0%) Type mismatches: 868 Inference: 30.15s, 135293mi Total: 45.43s, 220213mi rust-analyzer -q analysis-stats . 46.26s user 0.74s system 99% cpu 47.294 total ``` *HOWEVER*, it is still a perf regression (35s vs 45s): ``` [WITHOUT this PR] Database loaded 657.42ms, 284mi Crates in this dir: 36 Total modules found: 577 Total declarations: 11177 Total functions: 8735 Item Collection: 12.87s, 72407mi Total expressions: 239380 Expressions of unknown type: 2643 (1%) Expressions of partially unknown type: 2064 (0%) Type mismatches: 868 Inference: 22.88s, 97889mi Total: 35.74s, 170297mi rust-analyzer -q analysis-stats . 36.71s user 0.63s system 99% cpu 37.498 total ``` Co-authored-by: Edwin Cheng <[email protected]>
2 parents a3e5dcc + 76f2b9d commit 1a29934

File tree

9 files changed

+235
-58
lines changed

9 files changed

+235
-58
lines changed

crates/hir/src/db.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ pub use hir_def::db::{
1010
TypeAliasDataQuery, UnionDataQuery,
1111
};
1212
pub use hir_expand::db::{
13-
AstDatabase, AstDatabaseStorage, AstIdMapQuery, InternEagerExpansionQuery, InternMacroQuery,
14-
MacroArgTextQuery, MacroDefQuery, MacroExpandQuery, ParseMacroExpansionQuery,
13+
AstDatabase, AstDatabaseStorage, AstIdMapQuery, HygieneFrameQuery, InternEagerExpansionQuery,
14+
InternMacroQuery, MacroArgTextQuery, MacroDefQuery, MacroExpandQuery, ParseMacroExpansionQuery,
1515
};
1616
pub use hir_ty::db::*;
1717

crates/hir_def/src/path/lower.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ pub(super) fn lower_path(mut path: ast::Path, hygiene: &Hygiene) -> Option<Path>
123123
// We follow what it did anyway :)
124124
if segments.len() == 1 && kind == PathKind::Plain {
125125
if let Some(_macro_call) = path.syntax().parent().and_then(ast::MacroCall::cast) {
126-
if let Some(crate_id) = hygiene.local_inner_macros() {
126+
if let Some(crate_id) = hygiene.local_inner_macros(path) {
127127
kind = PathKind::DollarCrate(crate_id);
128128
}
129129
}

crates/hir_expand/src/db.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ use parser::FragmentKind;
88
use syntax::{algo::diff, ast::NameOwner, AstNode, GreenNode, Parse, SyntaxKind::*, SyntaxNode};
99

1010
use crate::{
11-
ast_id_map::AstIdMap, BuiltinDeriveExpander, BuiltinFnLikeExpander, EagerCallLoc, EagerMacroId,
12-
HirFileId, HirFileIdRepr, LazyMacroId, MacroCallId, MacroCallLoc, MacroDefId, MacroDefKind,
13-
MacroFile, ProcMacroExpander,
11+
ast_id_map::AstIdMap, hygiene::HygieneFrame, BuiltinDeriveExpander, BuiltinFnLikeExpander,
12+
EagerCallLoc, EagerMacroId, HirFileId, HirFileIdRepr, LazyMacroId, MacroCallId, MacroCallLoc,
13+
MacroDefId, MacroDefKind, MacroFile, ProcMacroExpander,
1414
};
1515

1616
/// Total limit on the number of tokens produced by any macro invocation.
@@ -94,6 +94,8 @@ pub trait AstDatabase: SourceDatabase {
9494
fn intern_eager_expansion(&self, eager: EagerCallLoc) -> EagerMacroId;
9595

9696
fn expand_proc_macro(&self, call: MacroCallId) -> Result<tt::Subtree, mbe::ExpandError>;
97+
98+
fn hygiene_frame(&self, file_id: HirFileId) -> Arc<HygieneFrame>;
9799
}
98100

99101
/// This expands the given macro call, but with different arguments. This is
@@ -369,6 +371,10 @@ fn parse_macro_with_arg(
369371
}
370372
}
371373

374+
fn hygiene_frame(db: &dyn AstDatabase, file_id: HirFileId) -> Arc<HygieneFrame> {
375+
Arc::new(HygieneFrame::new(db, file_id))
376+
}
377+
372378
/// Given a `MacroCallId`, return what `FragmentKind` it belongs to.
373379
/// FIXME: Not completed
374380
fn to_fragment_kind(db: &dyn AstDatabase, id: MacroCallId) -> FragmentKind {

crates/hir_expand/src/hygiene.rs

Lines changed: 177 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2,65 +2,209 @@
22
//!
33
//! Specifically, `ast` + `Hygiene` allows you to create a `Name`. Note that, at
44
//! this moment, this is horribly incomplete and handles only `$crate`.
5+
use std::sync::Arc;
6+
57
use base_db::CrateId;
68
use either::Either;
7-
use syntax::ast;
9+
use mbe::Origin;
10+
use parser::SyntaxKind;
11+
use syntax::{ast, AstNode, SyntaxNode, TextRange, TextSize};
812

913
use crate::{
10-
db::AstDatabase,
14+
db::{self, AstDatabase},
1115
name::{AsName, Name},
12-
HirFileId, HirFileIdRepr, MacroCallId, MacroDefKind,
16+
HirFileId, HirFileIdRepr, InFile, MacroCallId, MacroCallLoc, MacroDefKind, MacroFile,
1317
};
1418

1519
#[derive(Clone, Debug)]
1620
pub struct Hygiene {
17-
// This is what `$crate` expands to
18-
def_crate: Option<CrateId>,
19-
20-
// Indicate this is a local inner macro
21-
local_inner: bool,
21+
frames: Option<HygieneFrames>,
2222
}
2323

2424
impl Hygiene {
2525
pub fn new(db: &dyn AstDatabase, file_id: HirFileId) -> Hygiene {
26-
let (def_crate, local_inner) = match file_id.0 {
27-
HirFileIdRepr::FileId(_) => (None, false),
28-
HirFileIdRepr::MacroFile(macro_file) => match macro_file.macro_call_id {
29-
MacroCallId::LazyMacro(id) => {
30-
let loc = db.lookup_intern_macro(id);
31-
match loc.def.kind {
32-
MacroDefKind::Declarative => (Some(loc.def.krate), loc.def.local_inner),
33-
MacroDefKind::BuiltIn(_) => (Some(loc.def.krate), false),
34-
MacroDefKind::BuiltInDerive(_) => (None, false),
35-
MacroDefKind::BuiltInEager(_) => (None, false),
36-
MacroDefKind::ProcMacro(_) => (None, false),
37-
}
38-
}
39-
MacroCallId::EagerMacro(_id) => (None, false),
40-
},
41-
};
42-
Hygiene { def_crate, local_inner }
26+
Hygiene { frames: Some(HygieneFrames::new(db, file_id.clone())) }
4327
}
4428

4529
pub fn new_unhygienic() -> Hygiene {
46-
Hygiene { def_crate: None, local_inner: false }
30+
Hygiene { frames: None }
4731
}
4832

4933
// FIXME: this should just return name
5034
pub fn name_ref_to_name(&self, name_ref: ast::NameRef) -> Either<Name, CrateId> {
51-
if let Some(def_crate) = self.def_crate {
35+
if let Some(frames) = &self.frames {
5236
if name_ref.text() == "$crate" {
53-
return Either::Right(def_crate);
37+
if let Some(krate) = frames.root_crate(name_ref.syntax()) {
38+
return Either::Right(krate);
39+
}
5440
}
5541
}
42+
5643
Either::Left(name_ref.as_name())
5744
}
5845

59-
pub fn local_inner_macros(&self) -> Option<CrateId> {
60-
if self.local_inner {
61-
self.def_crate
62-
} else {
63-
None
46+
pub fn local_inner_macros(&self, path: ast::Path) -> Option<CrateId> {
47+
let mut token = path.syntax().first_token()?.text_range();
48+
let frames = self.frames.as_ref()?;
49+
let mut current = frames.0.clone();
50+
51+
loop {
52+
let (mapped, origin) = current.expansion.as_ref()?.map_ident_up(token)?;
53+
if origin == Origin::Def {
54+
return if current.local_inner { frames.root_crate(path.syntax()) } else { None };
55+
}
56+
current = current.call_site.as_ref()?.clone();
57+
token = mapped.value;
58+
}
59+
}
60+
}
61+
62+
#[derive(Clone, Debug)]
63+
struct HygieneFrames(Arc<HygieneFrame>);
64+
65+
#[derive(Clone, Debug, Eq, PartialEq)]
66+
pub struct HygieneFrame {
67+
expansion: Option<HygieneInfo>,
68+
69+
// Indicate this is a local inner macro
70+
local_inner: bool,
71+
krate: Option<CrateId>,
72+
73+
call_site: Option<Arc<HygieneFrame>>,
74+
def_site: Option<Arc<HygieneFrame>>,
75+
}
76+
77+
impl HygieneFrames {
78+
fn new(db: &dyn AstDatabase, file_id: HirFileId) -> Self {
79+
HygieneFrames(Arc::new(HygieneFrame::new(db, file_id)))
80+
}
81+
82+
fn root_crate(&self, node: &SyntaxNode) -> Option<CrateId> {
83+
let mut token = node.first_token()?.text_range();
84+
let mut result = self.0.krate;
85+
let mut current = self.0.clone();
86+
87+
while let Some((mapped, origin)) =
88+
current.expansion.as_ref().and_then(|it| it.map_ident_up(token))
89+
{
90+
result = current.krate;
91+
92+
let site = match origin {
93+
Origin::Def => &current.def_site,
94+
Origin::Call => &current.call_site,
95+
};
96+
97+
let site = match site {
98+
None => break,
99+
Some(it) => it,
100+
};
101+
102+
current = site.clone();
103+
token = mapped.value;
64104
}
105+
106+
result
107+
}
108+
}
109+
110+
#[derive(Debug, Clone, PartialEq, Eq)]
111+
struct HygieneInfo {
112+
arg_start: InFile<TextSize>,
113+
/// The `macro_rules!` arguments.
114+
def_start: Option<InFile<TextSize>>,
115+
116+
macro_def: Arc<(db::TokenExpander, mbe::TokenMap)>,
117+
macro_arg: Arc<(tt::Subtree, mbe::TokenMap)>,
118+
exp_map: Arc<mbe::TokenMap>,
119+
}
120+
121+
impl HygieneInfo {
122+
fn map_ident_up(&self, token: TextRange) -> Option<(InFile<TextRange>, Origin)> {
123+
let token_id = self.exp_map.token_by_range(token)?;
124+
125+
let (token_id, origin) = self.macro_def.0.map_id_up(token_id);
126+
let (token_map, tt) = match origin {
127+
mbe::Origin::Call => (&self.macro_arg.1, self.arg_start),
128+
mbe::Origin::Def => (
129+
&self.macro_def.1,
130+
self.def_start
131+
.as_ref()
132+
.expect("`Origin::Def` used with non-`macro_rules!` macro")
133+
.clone(),
134+
),
135+
};
136+
137+
let range = token_map.range_by_token(token_id)?.by_kind(SyntaxKind::IDENT)?;
138+
Some((tt.with_value(range + tt.value), origin))
139+
}
140+
}
141+
142+
fn make_hygiene_info(
143+
db: &dyn AstDatabase,
144+
macro_file: MacroFile,
145+
loc: &MacroCallLoc,
146+
) -> Option<HygieneInfo> {
147+
let arg_tt = loc.kind.arg(db)?;
148+
149+
let def_offset = loc.def.ast_id.and_then(|id| {
150+
let def_tt = match id.to_node(db) {
151+
ast::Macro::MacroRules(mac) => mac.token_tree()?.syntax().text_range().start(),
152+
ast::Macro::MacroDef(_) => return None,
153+
};
154+
Some(InFile::new(id.file_id, def_tt))
155+
});
156+
157+
let macro_def = db.macro_def(loc.def)?;
158+
let (_, exp_map) = db.parse_macro_expansion(macro_file).value?;
159+
let macro_arg = db.macro_arg(macro_file.macro_call_id)?;
160+
161+
Some(HygieneInfo {
162+
arg_start: InFile::new(loc.kind.file_id(), arg_tt.text_range().start()),
163+
def_start: def_offset,
164+
macro_arg,
165+
macro_def,
166+
exp_map,
167+
})
168+
}
169+
170+
impl HygieneFrame {
171+
pub(crate) fn new(db: &dyn AstDatabase, file_id: HirFileId) -> HygieneFrame {
172+
let (info, krate, local_inner) = match file_id.0 {
173+
HirFileIdRepr::FileId(_) => (None, None, false),
174+
HirFileIdRepr::MacroFile(macro_file) => match macro_file.macro_call_id {
175+
MacroCallId::EagerMacro(_id) => (None, None, false),
176+
MacroCallId::LazyMacro(id) => {
177+
let loc = db.lookup_intern_macro(id);
178+
let info = make_hygiene_info(db, macro_file, &loc);
179+
match loc.def.kind {
180+
MacroDefKind::Declarative => {
181+
(info, Some(loc.def.krate), loc.def.local_inner)
182+
}
183+
MacroDefKind::BuiltIn(_) => (info, Some(loc.def.krate), false),
184+
MacroDefKind::BuiltInDerive(_) => (info, None, false),
185+
MacroDefKind::BuiltInEager(_) => (info, None, false),
186+
MacroDefKind::ProcMacro(_) => (info, None, false),
187+
}
188+
}
189+
},
190+
};
191+
192+
let info = match info {
193+
None => {
194+
return HygieneFrame {
195+
expansion: None,
196+
local_inner,
197+
krate,
198+
call_site: None,
199+
def_site: None,
200+
};
201+
}
202+
Some(it) => it,
203+
};
204+
205+
let def_site = info.def_start.map(|it| db.hygiene_frame(it.file_id));
206+
let call_site = Some(db.hygiene_frame(info.arg_start.file_id));
207+
208+
HygieneFrame { expansion: Some(info), local_inner, krate, call_site, def_site }
65209
}
66210
}

crates/hir_ty/src/tests/macros.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,37 @@ expand!();
370370
);
371371
}
372372

373+
#[test]
374+
fn infer_macro_with_dollar_crate_in_def_site() {
375+
check_types(
376+
r#"
377+
//- /main.rs crate:main deps:foo
378+
use foo::expand;
379+
380+
macro_rules! list {
381+
($($tt:tt)*) => { $($tt)* }
382+
}
383+
384+
fn test() {
385+
let r = expand!();
386+
r;
387+
//^ u128
388+
}
389+
390+
//- /lib.rs crate:foo
391+
#[macro_export]
392+
macro_rules! expand {
393+
() => { list!($crate::m!()) };
394+
}
395+
396+
#[macro_export]
397+
macro_rules! m {
398+
() => { 0u128 };
399+
}
400+
"#,
401+
);
402+
}
403+
373404
#[test]
374405
fn infer_type_value_non_legacy_macro_use_as() {
375406
check_infer(

crates/ide_db/src/apply_change.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ impl RootDatabase {
145145
hir::db::MacroDefQuery
146146
hir::db::ParseMacroExpansionQuery
147147
hir::db::MacroExpandQuery
148+
hir::db::HygieneFrameQuery
148149

149150
// DefDatabase
150151
hir::db::ItemTreeQuery

crates/mbe/src/mbe_expander/matcher.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ fn match_subtree(
150150
res.add_err(err!("leftover tokens"));
151151
}
152152
}
153-
Op::Var { name, kind } => {
153+
Op::Var { name, kind, .. } => {
154154
let kind = match kind {
155155
Some(k) => k,
156156
None => {

crates/mbe/src/mbe_expander/transcriber.rs

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@ fn expand_subtree(
100100
err = err.or(e);
101101
arena.push(tt.into());
102102
}
103-
Op::Var { name, .. } => {
104-
let ExpandResult { value: fragment, err: e } = expand_var(ctx, &name);
103+
Op::Var { name, id, .. } => {
104+
let ExpandResult { value: fragment, err: e } = expand_var(ctx, &name, *id);
105105
err = err.or(e);
106106
push_fragment(arena, fragment);
107107
}
@@ -118,12 +118,10 @@ fn expand_subtree(
118118
ExpandResult { value: tt::Subtree { delimiter: template.delimiter, token_trees: tts }, err }
119119
}
120120

121-
fn expand_var(ctx: &mut ExpandCtx, v: &SmolStr) -> ExpandResult<Fragment> {
121+
fn expand_var(ctx: &mut ExpandCtx, v: &SmolStr, id: tt::TokenId) -> ExpandResult<Fragment> {
122122
if v == "crate" {
123123
// We simply produce identifier `$crate` here. And it will be resolved when lowering ast to Path.
124-
let tt =
125-
tt::Leaf::from(tt::Ident { text: "$crate".into(), id: tt::TokenId::unspecified() })
126-
.into();
124+
let tt = tt::Leaf::from(tt::Ident { text: "$crate".into(), id }).into();
127125
ExpandResult::ok(Fragment::Tokens(tt))
128126
} else if !ctx.bindings.contains(v) {
129127
// Note that it is possible to have a `$var` inside a macro which is not bound.
@@ -142,14 +140,8 @@ fn expand_var(ctx: &mut ExpandCtx, v: &SmolStr) -> ExpandResult<Fragment> {
142140
let tt = tt::Subtree {
143141
delimiter: None,
144142
token_trees: vec![
145-
tt::Leaf::from(tt::Punct {
146-
char: '$',
147-
spacing: tt::Spacing::Alone,
148-
id: tt::TokenId::unspecified(),
149-
})
150-
.into(),
151-
tt::Leaf::from(tt::Ident { text: v.clone(), id: tt::TokenId::unspecified() })
152-
.into(),
143+
tt::Leaf::from(tt::Punct { char: '$', spacing: tt::Spacing::Alone, id }).into(),
144+
tt::Leaf::from(tt::Ident { text: v.clone(), id }).into(),
153145
],
154146
}
155147
.into();

0 commit comments

Comments
 (0)