Skip to content

Commit

Permalink
Merge #8443 #8446
Browse files Browse the repository at this point in the history
8443: Rewrite `#[derive]` removal code to be based on AST r=jonas-schievink a=jonas-schievink

We now remove any `#[derive]` before and including the one we want to expand, in the `macro_arg` query.

The same infra will be needed by attribute macros (except we only remove the attribute we're expanding, not any preceding ones).

Part of #8434 (doesn't implement the cfg-expansion yet, because that's more difficult)

8446: Undo path resolution hack for extern prelude r=jonas-schievink a=jonas-schievink

Reverts the change made in #7959

We don't populate the extern prelude for block DefMaps anymore,
so this is unnecessary

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
  • Loading branch information
bors[bot] and jonas-schievink authored Apr 9, 2021
3 parents 972e1f4 + d6187de + 4ea5f69 commit 3b1692c
Show file tree
Hide file tree
Showing 10 changed files with 149 additions and 143 deletions.
37 changes: 20 additions & 17 deletions crates/hir_def/src/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::{
use base_db::CrateId;
use cfg::{CfgExpr, CfgOptions};
use either::Either;
use hir_expand::{hygiene::Hygiene, name::AsName, AstId, InFile};
use hir_expand::{hygiene::Hygiene, name::AsName, AstId, AttrId, InFile};
use itertools::Itertools;
use la_arena::ArenaMap;
use mbe::ast_to_token_tree;
Expand Down Expand Up @@ -98,13 +98,16 @@ impl RawAttrs {
pub(crate) fn new(owner: &dyn ast::AttrsOwner, hygiene: &Hygiene) -> Self {
let entries = collect_attrs(owner)
.enumerate()
.flat_map(|(i, attr)| match attr {
Either::Left(attr) => Attr::from_src(attr, hygiene, i as u32),
Either::Right(comment) => comment.doc_comment().map(|doc| Attr {
index: i as u32,
input: Some(AttrInput::Literal(SmolStr::new(doc))),
path: Interned::new(ModPath::from(hir_expand::name!(doc))),
}),
.flat_map(|(i, attr)| {
let index = AttrId(i as u32);
match attr {
Either::Left(attr) => Attr::from_src(attr, hygiene, index),
Either::Right(comment) => comment.doc_comment().map(|doc| Attr {
id: index,
input: Some(AttrInput::Literal(SmolStr::new(doc))),
path: Interned::new(ModPath::from(hir_expand::name!(doc))),
}),
}
})
.collect::<Arc<_>>();

Expand Down Expand Up @@ -161,7 +164,7 @@ impl RawAttrs {
let cfg = parts.next().unwrap();
let cfg = Subtree { delimiter: subtree.delimiter, token_trees: cfg.to_vec() };
let cfg = CfgExpr::parse(&cfg);
let index = attr.index;
let index = attr.id;
let attrs = parts.filter(|a| !a.is_empty()).filter_map(|attr| {
let tree = Subtree { delimiter: None, token_trees: attr.to_vec() };
let attr = ast::Attr::parse(&format!("#[{}]", tree)).ok()?;
Expand Down Expand Up @@ -468,7 +471,7 @@ impl AttrsWithOwner {
) -> Option<(Documentation, DocsRangeMap)> {
// FIXME: code duplication in `docs` above
let docs = self.by_key("doc").attrs().flat_map(|attr| match attr.input.as_ref()? {
AttrInput::Literal(s) => Some((s, attr.index)),
AttrInput::Literal(s) => Some((s, attr.id)),
AttrInput::TokenTree(_) => None,
});
let indent = docs
Expand Down Expand Up @@ -560,8 +563,8 @@ impl AttrSourceMap {
/// the attribute represented by `Attr`.
pub fn source_of(&self, attr: &Attr) -> InFile<&Either<ast::Attr, ast::Comment>> {
self.attrs
.get(attr.index as usize)
.unwrap_or_else(|| panic!("cannot find `Attr` at index {}", attr.index))
.get(attr.id.0 as usize)
.unwrap_or_else(|| panic!("cannot find `Attr` at index {:?}", attr.id))
.as_ref()
}
}
Expand All @@ -572,7 +575,7 @@ pub struct DocsRangeMap {
// (docstring-line-range, attr_index, attr-string-range)
// a mapping from the text range of a line of the [`Documentation`] to the attribute index and
// the original (untrimmed) syntax doc line
mapping: Vec<(TextRange, u32, TextRange)>,
mapping: Vec<(TextRange, AttrId, TextRange)>,
}

impl DocsRangeMap {
Expand All @@ -585,7 +588,7 @@ impl DocsRangeMap {

let relative_range = range - line_docs_range.start();

let &InFile { file_id, value: ref source } = &self.source[idx as usize];
let &InFile { file_id, value: ref source } = &self.source[idx.0 as usize];
match source {
Either::Left(_) => None, // FIXME, figure out a nice way to handle doc attributes here
// as well as for whats done in syntax highlight doc injection
Expand All @@ -606,7 +609,7 @@ impl DocsRangeMap {

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Attr {
index: u32,
pub(crate) id: AttrId,
pub(crate) path: Interned<ModPath>,
pub(crate) input: Option<AttrInput>,
}
Expand All @@ -620,7 +623,7 @@ pub enum AttrInput {
}

impl Attr {
fn from_src(ast: ast::Attr, hygiene: &Hygiene, index: u32) -> Option<Attr> {
fn from_src(ast: ast::Attr, hygiene: &Hygiene, id: AttrId) -> Option<Attr> {
let path = Interned::new(ModPath::from_src(ast.path()?, hygiene)?);
let input = if let Some(ast::Expr::Literal(lit)) = ast.expr() {
let value = match lit.kind() {
Expand All @@ -633,7 +636,7 @@ impl Attr {
} else {
None
};
Some(Attr { index, path, input })
Some(Attr { id, path, input })
}

/// Parses this attribute as a `#[derive]`, returns an iterator that yields all contained paths
Expand Down
4 changes: 3 additions & 1 deletion crates/hir_def/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ use hir_expand::{
ast_id_map::FileAstId,
eager::{expand_eager_macro, ErrorEmitted, ErrorSink},
hygiene::Hygiene,
AstId, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
AstId, AttrId, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
};
use la_arena::Idx;
use nameres::DefMap;
Expand Down Expand Up @@ -699,6 +699,7 @@ fn macro_call_as_call_id(

fn derive_macro_as_call_id(
item_attr: &AstIdWithPath<ast::Item>,
derive_attr: AttrId,
db: &dyn db::DefDatabase,
krate: CrateId,
resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
Expand All @@ -712,6 +713,7 @@ fn derive_macro_as_call_id(
MacroCallKind::Derive {
ast_id: item_attr.ast_id,
derive_name: last_segment.to_string(),
derive_attr,
},
)
.into();
Expand Down
2 changes: 1 addition & 1 deletion crates/hir_def/src/nameres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ mod diagnostics {
let node = ast_id.to_node(db.upcast());
(ast_id.file_id, SyntaxNodePtr::from(AstPtr::new(&node)), None)
}
MacroCallKind::Derive { ast_id, derive_name } => {
MacroCallKind::Derive { ast_id, derive_name, .. } => {
let node = ast_id.to_node(db.upcast());

// Compute the precise location of the macro name's token in the derive
Expand Down
18 changes: 11 additions & 7 deletions crates/hir_def/src/nameres/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use hir_expand::{
builtin_macro::find_builtin_macro,
name::{AsName, Name},
proc_macro::ProcMacroExpander,
HirFileId, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
AttrId, HirFileId, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
};
use hir_expand::{InFile, MacroCallLoc};
use rustc_hash::{FxHashMap, FxHashSet};
Expand Down Expand Up @@ -216,7 +216,7 @@ struct MacroDirective {
#[derive(Clone, Debug, Eq, PartialEq)]
enum MacroDirectiveKind {
FnLike { ast_id: AstIdWithPath<ast::MacroCall> },
Derive { ast_id: AstIdWithPath<ast::Item> },
Derive { ast_id: AstIdWithPath<ast::Item>, derive_attr: AttrId },
}

struct DefData<'a> {
Expand Down Expand Up @@ -831,10 +831,14 @@ impl DefCollector<'_> {
Err(UnresolvedMacro) | Ok(Err(_)) => {}
}
}
MacroDirectiveKind::Derive { ast_id } => {
match derive_macro_as_call_id(ast_id, self.db, self.def_map.krate, |path| {
self.resolve_derive_macro(directive.module_id, &path)
}) {
MacroDirectiveKind::Derive { ast_id, derive_attr } => {
match derive_macro_as_call_id(
ast_id,
*derive_attr,
self.db,
self.def_map.krate,
|path| self.resolve_derive_macro(directive.module_id, &path),
) {
Ok(call_id) => {
resolved.push((directive.module_id, call_id, directive.depth));
res = ReachedFixedPoint::No;
Expand Down Expand Up @@ -1368,7 +1372,7 @@ impl ModCollector<'_, '_> {
self.def_collector.unexpanded_macros.push(MacroDirective {
module_id: self.module_id,
depth: self.macro_depth + 1,
kind: MacroDirectiveKind::Derive { ast_id },
kind: MacroDirectiveKind::Derive { ast_id, derive_attr: derive.id },
});
}
}
Expand Down
13 changes: 4 additions & 9 deletions crates/hir_def/src/nameres/path_resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,15 +384,10 @@ impl DefMap {
}
}
};
// Give precedence to names in outer `DefMap`s over the extern prelude; only check prelude
// from the crate DefMap.
let from_extern_prelude = match self.block {
Some(_) => PerNs::none(),
None => self
.extern_prelude
.get(name)
.map_or(PerNs::none(), |&it| PerNs::types(it, Visibility::Public)),
};
let from_extern_prelude = self
.extern_prelude
.get(name)
.map_or(PerNs::none(), |&it| PerNs::types(it, Visibility::Public));

let from_prelude = self.resolve_in_prelude(db, name);

Expand Down
8 changes: 6 additions & 2 deletions crates/hir_expand/src/builtin_derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ mod tests {
use expect_test::{expect, Expect};
use name::AsName;

use crate::{test_db::TestDB, AstId, MacroCallId, MacroCallKind, MacroCallLoc};
use crate::{test_db::TestDB, AstId, AttrId, MacroCallId, MacroCallKind, MacroCallLoc};

use super::*;

Expand Down Expand Up @@ -317,7 +317,11 @@ $0
local_inner: false,
},
krate: CrateId(0),
kind: MacroCallKind::Derive { ast_id, derive_name: name.to_string() },
kind: MacroCallKind::Derive {
ast_id,
derive_name: name.to_string(),
derive_attr: AttrId(0),
},
};

let id: MacroCallId = db.intern_macro(loc).into();
Expand Down
7 changes: 4 additions & 3 deletions crates/hir_expand/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ use syntax::{
};

use crate::{
ast_id_map::AstIdMap, hygiene::HygieneFrame, BuiltinDeriveExpander, BuiltinFnLikeExpander,
EagerCallLoc, EagerMacroId, HirFileId, HirFileIdRepr, LazyMacroId, MacroCallId, MacroCallLoc,
MacroDefId, MacroDefKind, MacroFile, ProcMacroExpander,
ast_id_map::AstIdMap, hygiene::HygieneFrame, input::process_macro_input, BuiltinDeriveExpander,
BuiltinFnLikeExpander, EagerCallLoc, EagerMacroId, HirFileId, HirFileIdRepr, LazyMacroId,
MacroCallId, MacroCallLoc, MacroDefId, MacroDefKind, MacroFile, ProcMacroExpander,
};

/// Total limit on the number of tokens produced by any macro invocation.
Expand Down Expand Up @@ -191,6 +191,7 @@ fn macro_arg_text(db: &dyn AstDatabase, id: MacroCallId) -> Option<GreenNode> {
};
let loc = db.lookup_intern_macro(id);
let arg = loc.kind.arg(db)?;
let arg = process_macro_input(db, arg, id);
Some(arg.green())
}

Expand Down
95 changes: 95 additions & 0 deletions crates/hir_expand/src/input.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
//! Macro input conditioning.
use syntax::{
ast::{self, AttrsOwner},
AstNode, SyntaxNode,
};

use crate::{
db::AstDatabase,
name::{name, AsName},
AttrId, LazyMacroId, MacroCallKind, MacroCallLoc,
};

pub(crate) fn process_macro_input(
db: &dyn AstDatabase,
node: SyntaxNode,
id: LazyMacroId,
) -> SyntaxNode {
let loc: MacroCallLoc = db.lookup_intern_macro(id);

match loc.kind {
MacroCallKind::FnLike { .. } => node,
MacroCallKind::Derive { derive_attr, .. } => {
let item = match ast::Item::cast(node.clone()) {
Some(item) => item,
None => return node,
};

remove_derives_up_to(item, derive_attr).syntax().clone()
}
}
}

/// Removes `#[derive]` attributes from `item`, up to `attr`.
fn remove_derives_up_to(item: ast::Item, attr: AttrId) -> ast::Item {
let item = item.clone_for_update();
let idx = attr.0 as usize;
for attr in item.attrs().take(idx + 1) {
if let Some(name) =
attr.path().and_then(|path| path.as_single_segment()).and_then(|seg| seg.name_ref())
{
if name.as_name() == name![derive] {
attr.syntax().detach();
}
}
}
item
}

#[cfg(test)]
mod tests {
use base_db::fixture::WithFixture;
use base_db::SourceDatabase;
use expect_test::{expect, Expect};

use crate::test_db::TestDB;

use super::*;

fn test_remove_derives_up_to(attr: AttrId, ra_fixture: &str, expect: Expect) {
let (db, file_id) = TestDB::with_single_file(&ra_fixture);
let parsed = db.parse(file_id);

let mut items: Vec<_> =
parsed.syntax_node().descendants().filter_map(ast::Item::cast).collect();
assert_eq!(items.len(), 1);

let item = remove_derives_up_to(items.pop().unwrap(), attr);
expect.assert_eq(&item.to_string());
}

#[test]
fn remove_derive() {
test_remove_derives_up_to(
AttrId(2),
r#"
#[allow(unused)]
#[derive(Copy)]
#[derive(Hello)]
#[derive(Clone)]
struct A {
bar: u32
}
"#,
expect![[r#"
#[allow(unused)]
#[derive(Clone)]
struct A {
bar: u32
}"#]],
);
}
}
6 changes: 5 additions & 1 deletion crates/hir_expand/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub mod builtin_macro;
pub mod proc_macro;
pub mod quote;
pub mod eager;
mod input;

use either::Either;
pub use mbe::{ExpandError, ExpandResult};
Expand Down Expand Up @@ -291,9 +292,12 @@ pub struct MacroCallLoc {
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum MacroCallKind {
FnLike { ast_id: AstId<ast::MacroCall> },
Derive { ast_id: AstId<ast::Item>, derive_name: String },
Derive { ast_id: AstId<ast::Item>, derive_name: String, derive_attr: AttrId },
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub struct AttrId(pub u32);

impl MacroCallKind {
fn file_id(&self) -> HirFileId {
match self {
Expand Down
Loading

0 comments on commit 3b1692c

Please sign in to comment.