Skip to content

Proper handling $crate and local_inner_macros #7133

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 2 commits into from
Jan 2, 2021
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
2 changes: 1 addition & 1 deletion crates/hir_def/src/path/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ pub(super) fn lower_path(mut path: ast::Path, hygiene: &Hygiene) -> Option<Path>
// We follow what it did anyway :)
if segments.len() == 1 && kind == PathKind::Plain {
if let Some(_macro_call) = path.syntax().parent().and_then(ast::MacroCall::cast) {
if let Some(crate_id) = hygiene.local_inner_macros() {
if let Some(crate_id) = hygiene.local_inner_macros(path) {
kind = PathKind::DollarCrate(crate_id);
}
}
Expand Down
151 changes: 126 additions & 25 deletions crates/hir_expand/src/hygiene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,94 @@
//!
//! Specifically, `ast` + `Hygiene` allows you to create a `Name`. Note that, at
//! this moment, this is horribly incomplete and handles only `$crate`.
use std::sync::Arc;

use arena::{Arena, Idx};
use base_db::CrateId;
use either::Either;
use syntax::ast;
use mbe::Origin;
use syntax::{ast, AstNode};

use crate::{
db::AstDatabase,
name::{AsName, Name},
HirFileId, HirFileIdRepr, MacroCallId, MacroDefKind,
ExpansionInfo, HirFileId, HirFileIdRepr, MacroCallId, MacroDefKind,
};

#[derive(Clone, Debug)]
pub struct Hygiene {
// This is what `$crate` expands to
def_crate: Option<CrateId>,
frames: Option<Arc<HygieneFrames>>,
}

impl Hygiene {
pub fn new(db: &dyn AstDatabase, file_id: HirFileId) -> Hygiene {
Hygiene { frames: Some(Arc::new(HygieneFrames::new(db, file_id.clone()))) }
}

pub fn new_unhygienic() -> Hygiene {
Hygiene { frames: None }
}

// FIXME: this should just return name
pub fn name_ref_to_name(&self, name_ref: ast::NameRef) -> Either<Name, CrateId> {
if let Some(frames) = &self.frames {
if name_ref.text() == "$crate" {
if let Some(krate) = frames.root_crate(&name_ref) {
return Either::Right(krate);
}
}
}

Either::Left(name_ref.as_name())
}

pub fn local_inner_macros(&self, path: ast::Path) -> Option<CrateId> {
let frames = self.frames.as_ref()?;

let mut token = path.syntax().first_token()?;
let mut current = frames.first();

while let Some((frame, data)) =
current.and_then(|it| Some((it, it.expansion.as_ref()?.map_token_up(&token)?)))
{
let (mapped, origin) = data;
if origin == Origin::Def {
return if frame.local_inner { frame.krate } else { None };
}
current = Some(&frames.0[frame.call_site?]);
token = mapped.value;
}
None
}
}

#[derive(Default, Debug)]
struct HygieneFrames(Arena<HygieneFrame>);

#[derive(Clone, Debug)]
struct HygieneFrame {
expansion: Option<ExpansionInfo>,

// Indicate this is a local inner macro
local_inner: bool,
krate: Option<CrateId>,

call_site: Option<Idx<HygieneFrame>>,
def_site: Option<Idx<HygieneFrame>>,
}

impl Hygiene {
pub fn new(db: &dyn AstDatabase, file_id: HirFileId) -> Hygiene {
let (def_crate, local_inner) = match file_id.0 {
impl HygieneFrames {
fn new(db: &dyn AstDatabase, file_id: HirFileId) -> Self {
let mut frames = HygieneFrames::default();
frames.add(db, file_id);
frames
}

fn add(&mut self, db: &dyn AstDatabase, file_id: HirFileId) -> Option<Idx<HygieneFrame>> {
let (krate, local_inner) = match file_id.0 {
HirFileIdRepr::FileId(_) => (None, false),
HirFileIdRepr::MacroFile(macro_file) => match macro_file.macro_call_id {
MacroCallId::EagerMacro(_id) => (None, false),
MacroCallId::LazyMacro(id) => {
let loc = db.lookup_intern_macro(id);
match loc.def.kind {
Expand All @@ -36,31 +100,68 @@ impl Hygiene {
MacroDefKind::ProcMacro(_) => (None, false),
}
}
MacroCallId::EagerMacro(_id) => (None, false),
},
};
Hygiene { def_crate, local_inner }
}

pub fn new_unhygienic() -> Hygiene {
Hygiene { def_crate: None, local_inner: false }
let expansion = file_id.expansion_info(db);
let expansion = match expansion {
None => {
return Some(self.0.alloc(HygieneFrame {
expansion: None,
local_inner,
krate,
call_site: None,
def_site: None,
}));
}
Some(it) => it,
};

let def_site = expansion.def.clone();
let call_site = expansion.arg.file_id;
let idx = self.0.alloc(HygieneFrame {
expansion: Some(expansion),
local_inner,
krate,
call_site: None,
def_site: None,
});

self.0[idx].call_site = self.add(db, call_site);
self.0[idx].def_site = def_site.and_then(|it| self.add(db, it.file_id));

Some(idx)
}

// FIXME: this should just return name
pub fn name_ref_to_name(&self, name_ref: ast::NameRef) -> Either<Name, CrateId> {
if let Some(def_crate) = self.def_crate {
if name_ref.text() == "$crate" {
return Either::Right(def_crate);
}
}
Either::Left(name_ref.as_name())
fn first(&self) -> Option<&HygieneFrame> {
self.0.iter().next().map(|it| it.1)
}

pub fn local_inner_macros(&self) -> Option<CrateId> {
if self.local_inner {
self.def_crate
} else {
None
fn root_crate(&self, name_ref: &ast::NameRef) -> Option<CrateId> {
let mut token = name_ref.syntax().first_token()?;
let first = self.first()?;
let mut result = first.krate;
let mut current = Some(first);

while let Some((frame, (mapped, origin))) =
current.and_then(|it| Some((it, it.expansion.as_ref()?.map_token_up(&token)?)))
{
result = frame.krate;

let site = match origin {
Origin::Def => frame.def_site,
Origin::Call => frame.call_site,
};

let site = match site {
None => break,
Some(it) => it,
};

current = Some(&self.0[site]);
token = mapped.value;
}

result
}
}
11 changes: 4 additions & 7 deletions crates/hir_expand/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,11 +340,8 @@ impl ExpansionInfo {
Some(self.expanded.with_value(token))
}

pub fn map_token_up(
&self,
token: InFile<&SyntaxToken>,
) -> Option<(InFile<SyntaxToken>, Origin)> {
let token_id = self.exp_map.token_by_range(token.value.text_range())?;
pub fn map_token_up(&self, token: &SyntaxToken) -> Option<(InFile<SyntaxToken>, Origin)> {
let token_id = self.exp_map.token_by_range(token.text_range())?;

let (token_id, origin) = self.macro_def.0.map_id_up(token_id);
let (token_map, tt) = match origin {
Expand All @@ -359,7 +356,7 @@ impl ExpansionInfo {
),
};

let range = token_map.range_by_token(token_id)?.by_kind(token.value.kind())?;
let range = token_map.range_by_token(token_id)?.by_kind(token.kind())?;
let token = algo::find_covering_element(&tt.value, range + tt.value.text_range().start())
.into_token()?;
Some((tt.with_value(token), origin))
Expand Down Expand Up @@ -495,7 +492,7 @@ fn ascend_call_token(
expansion: &ExpansionInfo,
token: InFile<SyntaxToken>,
) -> Option<InFile<SyntaxToken>> {
let (mapped, origin) = expansion.map_token_up(token.as_ref())?;
let (mapped, origin) = expansion.map_token_up(&token.value)?;
if origin != Origin::Call {
return None;
}
Expand Down
31 changes: 31 additions & 0 deletions crates/hir_ty/src/tests/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,37 @@ expand!();
);
}

#[test]
fn infer_macro_with_dollar_crate_in_def_site() {
check_types(
r#"
//- /main.rs crate:main deps:foo
use foo::expand;

macro_rules! list {
($($tt:tt)*) => { $($tt)* }
}

fn test() {
let r = expand!();
r;
//^ u128
}

//- /lib.rs crate:foo
#[macro_export]
macro_rules! expand {
() => { list!($crate::m!()) };
}

#[macro_export]
macro_rules! m {
() => { 0u128 };
}
"#,
);
}

#[test]
fn infer_type_value_non_legacy_macro_use_as() {
check_infer(
Expand Down
2 changes: 1 addition & 1 deletion crates/mbe/src/mbe_expander/matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ fn match_subtree(
res.add_err(err!("leftover tokens"));
}
}
Op::Var { name, kind } => {
Op::Var { name, kind, .. } => {
let kind = match kind {
Some(k) => k,
None => {
Expand Down
20 changes: 6 additions & 14 deletions crates/mbe/src/mbe_expander/transcriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ fn expand_subtree(
err = err.or(e);
arena.push(tt.into());
}
Op::Var { name, .. } => {
let ExpandResult { value: fragment, err: e } = expand_var(ctx, &name);
Op::Var { name, id, .. } => {
let ExpandResult { value: fragment, err: e } = expand_var(ctx, &name, *id);
err = err.or(e);
push_fragment(arena, fragment);
}
Expand All @@ -118,12 +118,10 @@ fn expand_subtree(
ExpandResult { value: tt::Subtree { delimiter: template.delimiter, token_trees: tts }, err }
}

fn expand_var(ctx: &mut ExpandCtx, v: &SmolStr) -> ExpandResult<Fragment> {
fn expand_var(ctx: &mut ExpandCtx, v: &SmolStr, id: tt::TokenId) -> ExpandResult<Fragment> {
if v == "crate" {
// We simply produce identifier `$crate` here. And it will be resolved when lowering ast to Path.
let tt =
tt::Leaf::from(tt::Ident { text: "$crate".into(), id: tt::TokenId::unspecified() })
.into();
let tt = tt::Leaf::from(tt::Ident { text: "$crate".into(), id }).into();
ExpandResult::ok(Fragment::Tokens(tt))
} else if !ctx.bindings.contains(v) {
// Note that it is possible to have a `$var` inside a macro which is not bound.
Expand All @@ -142,14 +140,8 @@ fn expand_var(ctx: &mut ExpandCtx, v: &SmolStr) -> ExpandResult<Fragment> {
let tt = tt::Subtree {
delimiter: None,
token_trees: vec![
tt::Leaf::from(tt::Punct {
char: '$',
spacing: tt::Spacing::Alone,
id: tt::TokenId::unspecified(),
})
.into(),
tt::Leaf::from(tt::Ident { text: v.clone(), id: tt::TokenId::unspecified() })
.into(),
tt::Leaf::from(tt::Punct { char: '$', spacing: tt::Spacing::Alone, id }).into(),
tt::Leaf::from(tt::Ident { text: v.clone(), id }).into(),
],
}
.into();
Expand Down
11 changes: 7 additions & 4 deletions crates/mbe/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{tt_iter::TtIter, ExpandError, MetaTemplate};

#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) enum Op {
Var { name: SmolStr, kind: Option<SmolStr> },
Var { name: SmolStr, kind: Option<SmolStr>, id: tt::TokenId },
Repeat { subtree: MetaTemplate, kind: RepeatKind, separator: Option<Separator> },
Leaf(tt::Leaf),
Subtree(MetaTemplate),
Expand Down Expand Up @@ -106,18 +106,21 @@ fn next_op<'a>(first: &tt::TokenTree, src: &mut TtIter<'a>, mode: Mode) -> Resul
}
let name = UNDERSCORE.clone();
let kind = eat_fragment_kind(src, mode)?;
Op::Var { name, kind }
let id = punct.id;
Op::Var { name, kind, id }
}
tt::Leaf::Ident(ident) => {
let name = ident.text.clone();
let kind = eat_fragment_kind(src, mode)?;
Op::Var { name, kind }
let id = ident.id;
Op::Var { name, kind, id }
}
tt::Leaf::Literal(lit) => {
if is_boolean_literal(&lit) {
let name = lit.text.clone();
let kind = eat_fragment_kind(src, mode)?;
Op::Var { name, kind }
let id = lit.id;
Op::Var { name, kind, id }
} else {
bail!("bad var 2");
}
Expand Down