Skip to content

Commit b70493d

Browse files
bors[bot]Veykril
bors[bot]
and
Veykril
authored
Merge #11225
11225: internal: Cleanup doc and attribute handling r=Veykril a=Veykril (very vague PR title but as I tried to fix the mentioned issue I ran into more and more subtle things that were interwoven) Fixes #11215 Co-authored-by: Lukas Wirth <[email protected]>
2 parents c09504b + 87735e5 commit b70493d

33 files changed

+277
-168
lines changed

‎crates/hir/src/lib.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -649,8 +649,12 @@ impl Module {
649649
let node = ast_id.to_node(db.upcast());
650650
ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&node)))
651651
}
652-
MacroCallKind::Derive { ast_id, .. }
653-
| MacroCallKind::Attr { ast_id, .. } => {
652+
MacroCallKind::Derive { ast_id, .. } => {
653+
// FIXME: point to the attribute instead, this creates very large diagnostics
654+
let node = ast_id.to_node(db.upcast());
655+
ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&node)))
656+
}
657+
MacroCallKind::Attr { ast_id, .. } => {
654658
// FIXME: point to the attribute instead, this creates very large diagnostics
655659
let node = ast_id.to_node(db.upcast());
656660
ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&node)))

‎crates/hir/src/semantics.rs

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use rustc_hash::{FxHashMap, FxHashSet};
1818
use smallvec::{smallvec, SmallVec};
1919
use syntax::{
2020
algo::skip_trivia_token,
21-
ast::{self, HasAttrs, HasGenericParams, HasLoopBody},
21+
ast::{self, HasAttrs as _, HasGenericParams, HasLoopBody},
2222
match_ast, AstNode, AstToken, Direction, SyntaxElement, SyntaxNode, SyntaxNodePtr, SyntaxToken,
2323
TextSize, T,
2424
};
@@ -27,9 +27,9 @@ use crate::{
2727
db::HirDatabase,
2828
semantics::source_to_def::{ChildContainer, SourceToDefCache, SourceToDefCtx},
2929
source_analyzer::{resolve_hir_path, SourceAnalyzer},
30-
Access, AssocItem, BuiltinAttr, Callable, ConstParam, Crate, Field, Function, HasSource,
31-
HirFileId, Impl, InFile, Label, LifetimeParam, Local, MacroDef, Module, ModuleDef, Name, Path,
32-
ScopeDef, ToolModule, Trait, Type, TypeAlias, TypeParam, VariantDef,
30+
Access, AssocItem, BuiltinAttr, Callable, ConstParam, Crate, Field, Function, HasAttrs as _,
31+
HasSource, HirFileId, Impl, InFile, Label, LifetimeParam, Local, MacroDef, Module, ModuleDef,
32+
Name, Path, ScopeDef, ToolModule, Trait, Type, TypeAlias, TypeParam, VariantDef,
3333
};
3434

3535
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -486,7 +486,7 @@ impl<'db> SemanticsImpl<'db> {
486486
let adt = InFile::new(file_id, &adt);
487487
let src = InFile::new(file_id, attr.clone());
488488
self.with_ctx(|ctx| {
489-
let res = ctx.attr_to_derive_macro_call(adt, src)?;
489+
let (_, res) = ctx.attr_to_derive_macro_call(adt, src)?;
490490
Some(res.to_vec())
491491
})
492492
}
@@ -917,15 +917,14 @@ impl<'db> SemanticsImpl<'db> {
917917
let tt = derive.token_tree()?;
918918
let file = self.find_file(derive.syntax());
919919
let adt = derive.syntax().parent().and_then(ast::Adt::cast)?;
920-
920+
let adt_def = ToDef::to_def(self, file.with_value(adt.clone()))?;
921921
let res = self.with_ctx(|ctx| {
922-
let attr_def = ctx.attr_to_def(file.with_value(derive.clone()))?;
923-
let derives = ctx.attr_to_derive_macro_call(
922+
let (attr_id, derives) = ctx.attr_to_derive_macro_call(
924923
file.with_value(&adt),
925924
file.with_value(derive.clone()),
926925
)?;
927-
928-
let mut derive_paths = attr_def.parse_path_comma_token_tree()?;
926+
let attrs = adt_def.attrs(self.db);
927+
let mut derive_paths = attrs[attr_id].parse_path_comma_token_tree()?;
929928

930929
let derive_idx = tt
931930
.syntax()
@@ -1225,7 +1224,6 @@ to_def_impls![
12251224
(crate::Local, ast::SelfParam, self_param_to_def),
12261225
(crate::Label, ast::Label, label_to_def),
12271226
(crate::Adt, ast::Adt, adt_to_def),
1228-
(crate::Attr, ast::Attr, attr_to_def),
12291227
];
12301228

12311229
fn find_root(node: &SyntaxNode) -> SyntaxNode {

‎crates/hir/src/semantics/source_to_def.rs

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@
8787
8888
use base_db::FileId;
8989
use hir_def::{
90+
attr::AttrId,
9091
child_by_source::ChildBySource,
9192
dyn_map::DynMap,
9293
expr::{LabelId, PatId},
@@ -210,19 +211,6 @@ impl SourceToDefCtx<'_, '_> {
210211
ast::Adt::Union(it) => self.union_to_def(InFile::new(file_id, it)).map(AdtId::UnionId),
211212
}
212213
}
213-
pub(super) fn attr_to_def(
214-
&mut self,
215-
InFile { file_id, value }: InFile<ast::Attr>,
216-
) -> Option<crate::Attr> {
217-
// FIXME: Use dynmap?
218-
let adt = value.syntax().parent().and_then(ast::Adt::cast)?;
219-
let attr_pos = ast::HasAttrs::attrs(&adt).position(|it| it == value)?;
220-
let attrs = {
221-
let def = self.adt_to_def(InFile::new(file_id, adt))?;
222-
self.db.attrs(def.into())
223-
};
224-
attrs.get(attr_pos).cloned()
225-
}
226214
pub(super) fn bind_pat_to_def(
227215
&mut self,
228216
src: InFile<ast::IdentPat>,
@@ -254,16 +242,16 @@ impl SourceToDefCtx<'_, '_> {
254242

255243
pub(super) fn item_to_macro_call(&mut self, src: InFile<ast::Item>) -> Option<MacroCallId> {
256244
let map = self.dyn_map(src.as_ref())?;
257-
map[keys::ATTR_MACRO].get(&src).copied()
245+
map[keys::ATTR_MACRO_CALL].get(&src).copied()
258246
}
259247

260248
pub(super) fn attr_to_derive_macro_call(
261249
&mut self,
262250
item: InFile<&ast::Adt>,
263251
src: InFile<ast::Attr>,
264-
) -> Option<&[Option<MacroCallId>]> {
252+
) -> Option<(AttrId, &[Option<MacroCallId>])> {
265253
let map = self.dyn_map(item)?;
266-
map[keys::DERIVE_MACRO].get(&src).map(AsRef::as_ref)
254+
map[keys::DERIVE_MACRO_CALL].get(&src).map(|(id, ids)| (*id, &**ids))
267255
}
268256

269257
fn to_def<Ast: AstNode + 'static, ID: Copy + 'static>(
@@ -328,7 +316,8 @@ impl SourceToDefCtx<'_, '_> {
328316
}
329317

330318
pub(super) fn macro_to_def(&mut self, src: InFile<ast::Macro>) -> Option<MacroDefId> {
331-
let makro = self.dyn_map(src.as_ref()).and_then(|it| it[keys::MACRO].get(&src).copied());
319+
let makro =
320+
self.dyn_map(src.as_ref()).and_then(|it| it[keys::MACRO_CALL].get(&src).copied());
332321
if let res @ Some(_) = makro {
333322
return res;
334323
}

‎crates/hir_def/src/attr.rs

Lines changed: 38 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::{fmt, hash::Hash, ops, sync::Arc};
55
use base_db::CrateId;
66
use cfg::{CfgExpr, CfgOptions};
77
use either::Either;
8-
use hir_expand::{hygiene::Hygiene, name::AsName, AstId, InFile};
8+
use hir_expand::{hygiene::Hygiene, name::AsName, AstId, HirFileId, InFile};
99
use itertools::Itertools;
1010
use la_arena::ArenaMap;
1111
use mbe::{syntax_node_to_token_tree, DelimiterKind, Punct};
@@ -84,6 +84,14 @@ impl ops::Deref for Attrs {
8484
}
8585
}
8686

87+
impl ops::Index<AttrId> for Attrs {
88+
type Output = Attr;
89+
90+
fn index(&self, AttrId { ast_index, .. }: AttrId) -> &Self::Output {
91+
&(**self)[ast_index as usize]
92+
}
93+
}
94+
8795
impl ops::Deref for AttrsWithOwner {
8896
type Target = Attrs;
8997

@@ -509,23 +517,23 @@ fn inner_attributes(
509517
) -> Option<(impl Iterator<Item = ast::Attr>, impl Iterator<Item = ast::Comment>)> {
510518
let (attrs, docs) = match_ast! {
511519
match syntax {
512-
ast::SourceFile(it) => (it.attrs(), ast::CommentIter::from_syntax_node(it.syntax())),
520+
ast::SourceFile(it) => (it.attrs(), ast::DocCommentIter::from_syntax_node(it.syntax())),
513521
ast::ExternBlock(it) => {
514522
let extern_item_list = it.extern_item_list()?;
515-
(extern_item_list.attrs(), ast::CommentIter::from_syntax_node(extern_item_list.syntax()))
523+
(extern_item_list.attrs(), ast::DocCommentIter::from_syntax_node(extern_item_list.syntax()))
516524
},
517525
ast::Fn(it) => {
518526
let body = it.body()?;
519527
let stmt_list = body.stmt_list()?;
520-
(stmt_list.attrs(), ast::CommentIter::from_syntax_node(body.syntax()))
528+
(stmt_list.attrs(), ast::DocCommentIter::from_syntax_node(body.syntax()))
521529
},
522530
ast::Impl(it) => {
523531
let assoc_item_list = it.assoc_item_list()?;
524-
(assoc_item_list.attrs(), ast::CommentIter::from_syntax_node(assoc_item_list.syntax()))
532+
(assoc_item_list.attrs(), ast::DocCommentIter::from_syntax_node(assoc_item_list.syntax()))
525533
},
526534
ast::Module(it) => {
527535
let item_list = it.item_list()?;
528-
(item_list.attrs(), ast::CommentIter::from_syntax_node(item_list.syntax()))
536+
(item_list.attrs(), ast::DocCommentIter::from_syntax_node(item_list.syntax()))
529537
},
530538
// FIXME: BlockExpr's only accept inner attributes in specific cases
531539
// Excerpt from the reference:
@@ -542,27 +550,20 @@ fn inner_attributes(
542550

543551
#[derive(Debug)]
544552
pub struct AttrSourceMap {
545-
attrs: Vec<InFile<ast::Attr>>,
546-
doc_comments: Vec<InFile<ast::Comment>>,
553+
source: Vec<Either<ast::Attr, ast::Comment>>,
554+
file_id: HirFileId,
547555
}
548556

549557
impl AttrSourceMap {
550558
fn new(owner: InFile<&dyn ast::HasAttrs>) -> Self {
551-
let mut attrs = Vec::new();
552-
let mut doc_comments = Vec::new();
553-
for (_, attr) in collect_attrs(owner.value) {
554-
match attr {
555-
Either::Left(attr) => attrs.push(owner.with_value(attr)),
556-
Either::Right(comment) => doc_comments.push(owner.with_value(comment)),
557-
}
559+
Self {
560+
source: collect_attrs(owner.value).map(|(_, it)| it).collect(),
561+
file_id: owner.file_id,
558562
}
559-
560-
Self { attrs, doc_comments }
561563
}
562564

563565
fn merge(&mut self, other: Self) {
564-
self.attrs.extend(other.attrs);
565-
self.doc_comments.extend(other.doc_comments);
566+
self.source.extend(other.source);
566567
}
567568

568569
/// Maps the lowered `Attr` back to its original syntax node.
@@ -571,24 +572,15 @@ impl AttrSourceMap {
571572
///
572573
/// Note that the returned syntax node might be a `#[cfg_attr]`, or a doc comment, instead of
573574
/// the attribute represented by `Attr`.
574-
pub fn source_of(&self, attr: &Attr) -> InFile<Either<ast::Attr, ast::Comment>> {
575+
pub fn source_of(&self, attr: &Attr) -> InFile<&Either<ast::Attr, ast::Comment>> {
575576
self.source_of_id(attr.id)
576577
}
577578

578-
fn source_of_id(&self, id: AttrId) -> InFile<Either<ast::Attr, ast::Comment>> {
579-
if id.is_doc_comment {
580-
self.doc_comments
581-
.get(id.ast_index as usize)
582-
.unwrap_or_else(|| panic!("cannot find doc comment at index {:?}", id))
583-
.clone()
584-
.map(Either::Right)
585-
} else {
586-
self.attrs
587-
.get(id.ast_index as usize)
588-
.unwrap_or_else(|| panic!("cannot find `Attr` at index {:?}", id))
589-
.clone()
590-
.map(Either::Left)
591-
}
579+
fn source_of_id(&self, id: AttrId) -> InFile<&Either<ast::Attr, ast::Comment>> {
580+
self.source
581+
.get(id.ast_index as usize)
582+
.map(|it| InFile::new(self.file_id, it))
583+
.unwrap_or_else(|| panic!("cannot find attr at index {:?}", id))
592584
}
593585
}
594586

@@ -656,8 +648,7 @@ fn get_doc_string_in_attr(it: &ast::Attr) -> Option<ast::String> {
656648
}
657649

658650
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
659-
pub(crate) struct AttrId {
660-
is_doc_comment: bool,
651+
pub struct AttrId {
661652
pub(crate) ast_index: u32,
662653
}
663654

@@ -816,27 +807,20 @@ fn collect_attrs(
816807
.map_or((None, None), |(attrs, docs)| (Some(attrs), Some(docs)));
817808

818809
let outer_attrs = owner.attrs().filter(|attr| attr.kind().is_outer());
819-
let attrs =
820-
outer_attrs.chain(inner_attrs.into_iter().flatten()).enumerate().map(|(idx, attr)| {
821-
(
822-
AttrId { ast_index: idx as u32, is_doc_comment: false },
823-
attr.syntax().text_range().start(),
824-
Either::Left(attr),
825-
)
826-
});
810+
let attrs = outer_attrs
811+
.chain(inner_attrs.into_iter().flatten())
812+
.map(|attr| (attr.syntax().text_range().start(), Either::Left(attr)));
827813

828814
let outer_docs =
829-
ast::CommentIter::from_syntax_node(owner.syntax()).filter(ast::Comment::is_outer);
830-
let docs =
831-
outer_docs.chain(inner_docs.into_iter().flatten()).enumerate().map(|(idx, docs_text)| {
832-
(
833-
AttrId { ast_index: idx as u32, is_doc_comment: true },
834-
docs_text.syntax().text_range().start(),
835-
Either::Right(docs_text),
836-
)
837-
});
815+
ast::DocCommentIter::from_syntax_node(owner.syntax()).filter(ast::Comment::is_outer);
816+
let docs = outer_docs
817+
.chain(inner_docs.into_iter().flatten())
818+
.map(|docs_text| (docs_text.syntax().text_range().start(), Either::Right(docs_text)));
838819
// sort here by syntax node offset because the source can have doc attributes and doc strings be interleaved
839-
docs.chain(attrs).sorted_by_key(|&(_, offset, _)| offset).map(|(id, _, attr)| (id, attr))
820+
docs.chain(attrs)
821+
.sorted_by_key(|&(offset, _)| offset)
822+
.enumerate()
823+
.map(|(id, (_, attr))| (AttrId { ast_index: id as u32 }, attr))
840824
}
841825

842826
pub(crate) fn variants_attrs_source_map(

‎crates/hir_def/src/child_by_source.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
77
use either::Either;
88
use hir_expand::HirFileId;
9-
use syntax::ast::HasAttrs;
9+
use syntax::ast::HasDocComments;
1010

1111
use crate::{
1212
db::DefDatabase,
@@ -110,7 +110,7 @@ impl ChildBySource for ItemScope {
110110
// FIXME: Do we need to add proc-macros into a PROCMACRO dynmap here?
111111
Either::Right(_fn) => return,
112112
};
113-
res[keys::MACRO].insert(src, makro);
113+
res[keys::MACRO_CALL].insert(src, makro);
114114
}
115115
});
116116
self.unnamed_consts().for_each(|konst| {
@@ -120,13 +120,16 @@ impl ChildBySource for ItemScope {
120120
self.impls().for_each(|imp| add_impl(db, file_id, res, imp));
121121
self.attr_macro_invocs().for_each(|(ast_id, call_id)| {
122122
let item = ast_id.with_value(ast_id.to_node(db.upcast()));
123-
res[keys::ATTR_MACRO].insert(item, call_id);
123+
res[keys::ATTR_MACRO_CALL].insert(item, call_id);
124124
});
125125
self.derive_macro_invocs().for_each(|(ast_id, calls)| {
126-
let item = ast_id.to_node(db.upcast());
126+
let adt = ast_id.to_node(db.upcast());
127127
for (attr_id, calls) in calls {
128-
if let Some(attr) = item.attrs().nth(attr_id.ast_index as usize) {
129-
res[keys::DERIVE_MACRO].insert(ast_id.with_value(attr), calls.into());
128+
if let Some(Either::Right(attr)) =
129+
adt.doc_comments_and_attrs().nth(attr_id.ast_index as usize)
130+
{
131+
res[keys::DERIVE_MACRO_CALL]
132+
.insert(ast_id.with_value(attr), (attr_id, calls.into()));
130133
}
131134
}
132135
});

‎crates/hir_def/src/item_scope.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ pub struct ItemScope {
6767
/// The derive macro invocations in this scope, keyed by the owner item over the actual derive attributes
6868
/// paired with the derive macro invocations for the specific attribute.
6969
derive_macros:
70-
FxHashMap<AstId<ast::Item>, SmallVec<[(AttrId, SmallVec<[Option<MacroCallId>; 1]>); 1]>>,
70+
FxHashMap<AstId<ast::Adt>, SmallVec<[(AttrId, SmallVec<[Option<MacroCallId>; 1]>); 1]>>,
7171
}
7272

7373
pub(crate) static BUILTIN_SCOPE: Lazy<FxHashMap<Name, PerNs>> = Lazy::new(|| {
@@ -204,12 +204,12 @@ impl ItemScope {
204204

205205
pub(crate) fn set_derive_macro_invoc(
206206
&mut self,
207-
item: AstId<ast::Item>,
207+
adt: AstId<ast::Adt>,
208208
call: MacroCallId,
209209
attr_id: AttrId,
210210
idx: usize,
211211
) {
212-
if let Some(derives) = self.derive_macros.get_mut(&item) {
212+
if let Some(derives) = self.derive_macros.get_mut(&adt) {
213213
if let Some((_, invocs)) = derives.iter_mut().find(|&&mut (id, _)| id == attr_id) {
214214
invocs[idx] = Some(call);
215215
}
@@ -221,17 +221,17 @@ impl ItemScope {
221221
/// independent of their indices.
222222
pub(crate) fn init_derive_attribute(
223223
&mut self,
224-
item: AstId<ast::Item>,
224+
adt: AstId<ast::Adt>,
225225
attr_id: AttrId,
226226
len: usize,
227227
) {
228-
self.derive_macros.entry(item).or_default().push((attr_id, smallvec![None; len]));
228+
self.derive_macros.entry(adt).or_default().push((attr_id, smallvec![None; len]));
229229
}
230230

231231
pub(crate) fn derive_macro_invocs(
232232
&self,
233233
) -> impl Iterator<
234-
Item = (AstId<ast::Item>, impl Iterator<Item = (AttrId, &[Option<MacroCallId>])>),
234+
Item = (AstId<ast::Adt>, impl Iterator<Item = (AttrId, &[Option<MacroCallId>])>),
235235
> + '_ {
236236
self.derive_macros
237237
.iter()

0 commit comments

Comments
 (0)