Encode one level of cfg_attr in attr_id

Veykril · Veykril · commit 621e96bd6ad0 · 2023-01-09T21:06:13.000+01:00
diff --git a/crates/hir-def/src/attr.rs b/crates/hir-def/src/attr.rs
@@ -546,7 +546,7 @@ impl AttrSourceMap {
     }
 
     fn source_of_id(&self, id: AttrId) -> InFile<&Either<ast::Attr, ast::Comment>> {
-        let ast_idx = id.ast_index as usize;
+        let ast_idx = id.ast_index();
         let file_id = match self.mod_def_site_file_id {
             Some((file_id, def_site_cut)) if def_site_cut <= ast_idx => file_id,
             _ => self.file_id,
diff --git a/crates/hir-def/src/child_by_source.rs b/crates/hir-def/src/child_by_source.rs
@@ -117,7 +117,7 @@ impl ChildBySource for ItemScope {
                 let adt = ast_id.to_node(db.upcast());
                 calls.for_each(|(attr_id, call_id, calls)| {
                     if let Some(Either::Left(attr)) =
-                        adt.doc_comments_and_attrs().nth(attr_id.ast_index as usize)
+                        adt.doc_comments_and_attrs().nth(attr_id.ast_index())
                     {
                         res[keys::DERIVE_MACRO_CALL].insert(attr, (attr_id, call_id, calls.into()));
                     }
diff --git a/crates/hir-def/src/lib.rs b/crates/hir-def/src/lib.rs
@@ -941,7 +941,7 @@ pub fn macro_id_to_def_id(db: &dyn db::DefDatabase, id: MacroId) -> MacroDefId {
 fn derive_macro_as_call_id(
     db: &dyn db::DefDatabase,
     item_attr: &AstIdWithPath<ast::Adt>,
-    derive_attr: AttrId,
+    derive_attr_index: AttrId,
     derive_pos: u32,
     krate: CrateId,
     resolver: impl Fn(path::ModPath) -> Option<(MacroId, MacroDefId)>,
@@ -954,7 +954,7 @@ fn derive_macro_as_call_id(
         MacroCallKind::Derive {
             ast_id: item_attr.ast_id,
             derive_index: derive_pos,
-            derive_attr_index: derive_attr.ast_index,
+            derive_attr_index,
         },
     );
     Ok((macro_id, def_id, call_id))
@@ -982,7 +982,7 @@ fn attr_macro_as_call_id(
         MacroCallKind::Attr {
             ast_id: item_attr.ast_id,
             attr_args: Arc::new(arg),
-            invoc_attr_index: macro_attr.id.ast_index,
+            invoc_attr_index: macro_attr.id,
             is_derive,
         },
     );
diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs
@@ -452,7 +452,7 @@ impl DefCollector<'_> {
                         MacroCallKind::Attr {
                             ast_id: ast_id.ast_id,
                             attr_args: Default::default(),
-                            invoc_attr_index: attr.id.ast_index,
+                            invoc_attr_index: attr.id,
                             is_derive: false,
                         },
                         attr.path().clone(),
@@ -1407,7 +1407,7 @@ impl DefCollector<'_> {
                         directive.module_id,
                         MacroCallKind::Derive {
                             ast_id: ast_id.ast_id,
-                            derive_attr_index: derive_attr.ast_index,
+                            derive_attr_index: *derive_attr,
                             derive_index: *derive_pos as u32,
                         },
                         ast_id.path.clone(),
diff --git a/crates/hir-def/src/nameres/diagnostics.rs b/crates/hir-def/src/nameres/diagnostics.rs
@@ -31,9 +31,9 @@ pub enum DefDiagnosticKind {
 
     UnimplementedBuiltinMacro { ast: AstId<ast::Macro> },
 
-    InvalidDeriveTarget { ast: AstId<ast::Item>, id: u32 },
+    InvalidDeriveTarget { ast: AstId<ast::Item>, id: usize },
 
-    MalformedDerive { ast: AstId<ast::Adt>, id: u32 },
+    MalformedDerive { ast: AstId<ast::Adt>, id: usize },
 }
 
 #[derive(Debug, PartialEq, Eq)]
@@ -119,7 +119,7 @@ impl DefDiagnostic {
     ) -> Self {
         Self {
             in_module: container,
-            kind: DefDiagnosticKind::InvalidDeriveTarget { ast, id: id.ast_index },
+            kind: DefDiagnosticKind::InvalidDeriveTarget { ast, id: id.ast_index() },
         }
     }
 
@@ -130,7 +130,7 @@ impl DefDiagnostic {
     ) -> Self {
         Self {
             in_module: container,
-            kind: DefDiagnosticKind::MalformedDerive { ast, id: id.ast_index },
+            kind: DefDiagnosticKind::MalformedDerive { ast, id: id.ast_index() },
         }
     }
 }
diff --git a/crates/hir-expand/src/attrs.rs b/crates/hir-expand/src/attrs.rs
@@ -1,3 +1,4 @@
+//! A higher level attributes based on TokenTree, with also some shortcuts.
 use std::{fmt, ops, sync::Arc};
 
 use base_db::CrateId;
@@ -65,14 +66,16 @@ impl RawAttrs {
             (None, entries @ Some(_)) => Self { entries },
             (Some(entries), None) => Self { entries: Some(entries.clone()) },
             (Some(a), Some(b)) => {
-                let last_ast_index = a.last().map_or(0, |it| it.id.ast_index + 1);
+                let last_ast_index = a.last().map_or(0, |it| it.id.ast_index() + 1) as u32;
                 Self {
                     entries: Some(
                         a.iter()
                             .cloned()
                             .chain(b.iter().map(|it| {
                                 let mut it = it.clone();
-                                it.id.ast_index += last_ast_index;
+                                it.id.id = it.id.ast_index() as u32 + last_ast_index
+                                    | (it.id.cfg_attr_index().unwrap_or(0) as u32)
+                                        << AttrId::AST_INDEX_BITS;
                                 it
                             }))
                             .collect(),
@@ -83,6 +86,7 @@ impl RawAttrs {
     }
 
     /// Processes `cfg_attr`s, returning the resulting semantic `Attrs`.
+    // FIXME: This should return a different type
     pub fn filter(self, db: &dyn AstDatabase, krate: CrateId) -> RawAttrs {
         let has_cfg_attrs = self
             .iter()
@@ -106,27 +110,22 @@ impl RawAttrs {
                     _ => return smallvec![attr.clone()],
                 };
 
-                // Input subtree is: `(cfg, $(attr),+)`
-                // Split it up into a `cfg` subtree and the `attr` subtrees.
-                // FIXME: There should be a common API for this.
-                let mut parts = subtree.token_trees.split(|tt| {
-                    matches!(tt, tt::TokenTree::Leaf(tt::Leaf::Punct(Punct { char: ',', .. })))
-                });
-                let cfg = match parts.next() {
+                let (cfg, parts) = match parse_cfg_attr_input(subtree) {
                     Some(it) => it,
-                    None => return smallvec![],
+                    None => return smallvec![attr.clone()],
                 };
-                let cfg = Subtree { delimiter: subtree.delimiter, token_trees: cfg.to_vec() };
-                let cfg = CfgExpr::parse(&cfg);
                 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() };
-                    // FIXME hygiene
-                    let hygiene = Hygiene::new_unhygienic();
-                    Attr::from_tt(db, &tree, &hygiene, index)
-                });
+                let attrs =
+                    parts.enumerate().take(1 << AttrId::CFG_ATTR_BITS).filter_map(|(idx, attr)| {
+                        let tree = Subtree { delimiter: None, token_trees: attr.to_vec() };
+                        // FIXME hygiene
+                        let hygiene = Hygiene::new_unhygienic();
+                        Attr::from_tt(db, &tree, &hygiene, index.with_cfg_attr(idx))
+                    });
 
                 let cfg_options = &crate_graph[krate].cfg_options;
+                let cfg = Subtree { delimiter: subtree.delimiter, token_trees: cfg.to_vec() };
+                let cfg = CfgExpr::parse(&cfg);
                 if cfg_options.check(&cfg) == Some(false) {
                     smallvec![]
                 } else {
@@ -143,7 +142,32 @@ impl RawAttrs {
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub struct AttrId {
-    pub ast_index: u32,
+    id: u32,
+}
+
+// FIXME: This only handles a single level of cfg_attr nesting
+// that is `#[cfg_attr(all(), cfg_attr(all(), cfg(any())))]` breaks again
+impl AttrId {
+    const CFG_ATTR_BITS: usize = 7;
+    const AST_INDEX_MASK: usize = 0x00FF_FFFF;
+    const AST_INDEX_BITS: usize = Self::AST_INDEX_MASK.count_ones() as usize;
+    const CFG_ATTR_SET_BITS: u32 = 1 << 31;
+
+    pub fn ast_index(&self) -> usize {
+        self.id as usize & Self::AST_INDEX_MASK
+    }
+
+    pub fn cfg_attr_index(&self) -> Option<usize> {
+        if self.id & Self::CFG_ATTR_SET_BITS == 0 {
+            None
+        } else {
+            Some(self.id as usize >> Self::AST_INDEX_BITS)
+        }
+    }
+
+    pub fn with_cfg_attr(self, idx: usize) -> AttrId {
+        AttrId { id: self.id | (idx as u32) << Self::AST_INDEX_BITS | Self::CFG_ATTR_SET_BITS }
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -272,10 +296,7 @@ pub fn collect_attrs(
             Either::Left(attr) => attr.kind().is_outer(),
             Either::Right(comment) => comment.is_outer(),
         });
-    outer_attrs
-        .chain(inner_attrs)
-        .enumerate()
-        .map(|(id, attr)| (AttrId { ast_index: id as u32 }, attr))
+    outer_attrs.chain(inner_attrs).enumerate().map(|(id, attr)| (AttrId { id: id as u32 }, attr))
 }
 
 fn inner_attributes(
@@ -311,3 +332,15 @@ fn inner_attributes(
     });
     Some(attrs)
 }
+
+// Input subtree is: `(cfg, $(attr),+)`
+// Split it up into a `cfg` subtree and the `attr` subtrees.
+pub fn parse_cfg_attr_input(
+    subtree: &Subtree,
+) -> Option<(&[tt::TokenTree], impl Iterator<Item = &[tt::TokenTree]>)> {
+    let mut parts = subtree
+        .token_trees
+        .split(|tt| matches!(tt, tt::TokenTree::Leaf(tt::Leaf::Punct(Punct { char: ',', .. }))));
+    let cfg = parts.next()?;
+    Some((cfg, parts.filter(|it| !it.is_empty())))
+}
diff --git a/crates/hir-expand/src/db.rs b/crates/hir-expand/src/db.rs
@@ -168,7 +168,9 @@ pub fn expand_speculative(
                 // Attributes may have an input token tree, build the subtree and map for this as well
                 // then try finding a token id for our token if it is inside this input subtree.
                 let item = ast::Item::cast(speculative_args.clone())?;
-                item.doc_comments_and_attrs().nth(invoc_attr_index as usize).and_then(Either::left)
+                item.doc_comments_and_attrs()
+                    .nth(invoc_attr_index.ast_index())
+                    .and_then(Either::left)
             }?;
             match attr.token_tree() {
                 Some(token_tree) => {
@@ -321,14 +323,15 @@ fn macro_arg(
 }
 
 fn censor_for_macro_input(loc: &MacroCallLoc, node: &SyntaxNode) -> FxHashSet<SyntaxNode> {
+    // FIXME: handle `cfg_attr`
     (|| {
         let censor = match loc.kind {
             MacroCallKind::FnLike { .. } => return None,
             MacroCallKind::Derive { derive_attr_index, .. } => {
                 cov_mark::hit!(derive_censoring);
                 ast::Item::cast(node.clone())?
                     .attrs()
-                    .take(derive_attr_index as usize + 1)
+                    .take(derive_attr_index.ast_index() + 1)
                     // FIXME, this resolution should not be done syntactically
                     // derive is a proper macro now, no longer builtin
                     // But we do not have resolution at this stage, this means
@@ -343,7 +346,7 @@ fn censor_for_macro_input(loc: &MacroCallLoc, node: &SyntaxNode) -> FxHashSet<Sy
                 cov_mark::hit!(attribute_macro_attr_censoring);
                 ast::Item::cast(node.clone())?
                     .doc_comments_and_attrs()
-                    .nth(invoc_attr_index as usize)
+                    .nth(invoc_attr_index.ast_index())
                     .and_then(Either::left)
                     .map(|attr| attr.syntax().clone())
                     .into_iter()
diff --git a/crates/hir-expand/src/hygiene.rs b/crates/hir-expand/src/hygiene.rs
@@ -191,7 +191,7 @@ fn make_hygiene_info(
             let tt = ast_id
                 .to_node(db)
                 .doc_comments_and_attrs()
-                .nth(invoc_attr_index as usize)
+                .nth(invoc_attr_index.ast_index())
                 .and_then(Either::left)?
                 .token_tree()?;
             Some(InFile::new(ast_id.file_id, tt))
diff --git a/crates/hir-expand/src/lib.rs b/crates/hir-expand/src/lib.rs
@@ -38,6 +38,7 @@ use syntax::{
 
 use crate::{
     ast_id_map::FileAstId,
+    attrs::AttrId,
     builtin_attr_macro::BuiltinAttrExpander,
     builtin_derive_macro::BuiltinDeriveExpander,
     builtin_fn_macro::{BuiltinFnLikeExpander, EagerExpander},
@@ -146,7 +147,7 @@ pub enum MacroCallKind {
         ///
         /// Outer attributes are counted first, then inner attributes. This does not support
         /// out-of-line modules, which may have attributes spread across 2 files!
-        derive_attr_index: u32,
+        derive_attr_index: AttrId,
         /// Index of the derive macro in the derive attribute
         derive_index: u32,
     },
@@ -157,7 +158,7 @@ pub enum MacroCallKind {
         ///
         /// Outer attributes are counted first, then inner attributes. This does not support
         /// out-of-line modules, which may have attributes spread across 2 files!
-        invoc_attr_index: u32,
+        invoc_attr_index: AttrId,
         /// Whether this attribute is the `#[derive]` attribute.
         is_derive: bool,
     },
@@ -262,10 +263,11 @@ impl HirFileId {
         });
         let attr_input_or_mac_def = def.or_else(|| match loc.kind {
             MacroCallKind::Attr { ast_id, invoc_attr_index, .. } => {
+                // FIXME: handle `cfg_attr`
                 let tt = ast_id
                     .to_node(db)
                     .doc_comments_and_attrs()
-                    .nth(invoc_attr_index as usize)
+                    .nth(invoc_attr_index.ast_index())
                     .and_then(Either::left)?
                     .token_tree()?;
                 Some(InFile::new(ast_id.file_id, tt))
@@ -398,8 +400,7 @@ impl MacroDefId {
     }
 }
 
-// FIXME: attribute indices do not account for `cfg_attr`, which means that we'll strip the whole
-// `cfg_attr` instead of just one of the attributes it expands to
+// FIXME: attribute indices do not account for nested `cfg_attr`
 
 impl MacroCallKind {
     /// Returns the file containing the macro invocation.
@@ -420,7 +421,7 @@ impl MacroCallKind {
                 // FIXME: handle `cfg_attr`
                 ast_id.with_value(ast_id.to_node(db)).map(|it| {
                     it.doc_comments_and_attrs()
-                        .nth(*derive_attr_index as usize)
+                        .nth(derive_attr_index.ast_index())
                         .and_then(|it| match it {
                             Either::Left(attr) => Some(attr.syntax().clone()),
                             Either::Right(_) => None,
@@ -432,7 +433,7 @@ impl MacroCallKind {
                 // FIXME: handle `cfg_attr`
                 ast_id.with_value(ast_id.to_node(db)).map(|it| {
                     it.doc_comments_and_attrs()
-                        .nth(*invoc_attr_index as usize)
+                        .nth(invoc_attr_index.ast_index())
                         .and_then(|it| match it {
                             Either::Left(attr) => Some(attr.syntax().clone()),
                             Either::Right(_) => None,
@@ -489,19 +490,21 @@ impl MacroCallKind {
             MacroCallKind::FnLike { ast_id, .. } => ast_id.to_node(db).syntax().text_range(),
             MacroCallKind::Derive { ast_id, derive_attr_index, .. } => {
                 // FIXME: should be the range of the macro name, not the whole derive
+                // FIXME: handle `cfg_attr`
                 ast_id
                     .to_node(db)
                     .doc_comments_and_attrs()
-                    .nth(derive_attr_index as usize)
+                    .nth(derive_attr_index.ast_index())
                     .expect("missing derive")
                     .expect_left("derive is a doc comment?")
                     .syntax()
                     .text_range()
             }
+            // FIXME: handle `cfg_attr`
             MacroCallKind::Attr { ast_id, invoc_attr_index, .. } => ast_id
                 .to_node(db)
                 .doc_comments_and_attrs()
-                .nth(invoc_attr_index as usize)
+                .nth(invoc_attr_index.ast_index())
                 .expect("missing attribute")
                 .expect_left("attribute macro is a doc comment?")
                 .syntax()
@@ -593,9 +596,10 @@ impl ExpansionInfo {
             let token_range = token.value.text_range();
             match &loc.kind {
                 MacroCallKind::Attr { attr_args, invoc_attr_index, is_derive, .. } => {
+                    // FIXME: handle `cfg_attr`
                     let attr = item
                         .doc_comments_and_attrs()
-                        .nth(*invoc_attr_index as usize)
+                        .nth(invoc_attr_index.ast_index())
                         .and_then(Either::left)?;
                     match attr.token_tree() {
                         Some(token_tree)
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
@@ -785,7 +785,7 @@ fn precise_macro_call_location(
             let token = (|| {
                 let derive_attr = node
                     .doc_comments_and_attrs()
-                    .nth(*derive_attr_index as usize)
+                    .nth(derive_attr_index.ast_index())
                     .and_then(Either::left)?;
                 let token_tree = derive_attr.meta()?.token_tree()?;
                 let group_by = token_tree
@@ -813,9 +813,11 @@ fn precise_macro_call_location(
             let node = ast_id.to_node(db.upcast());
             let attr = node
                 .doc_comments_and_attrs()
-                .nth((*invoc_attr_index) as usize)
+                .nth(invoc_attr_index.ast_index())
                 .and_then(Either::left)
-                .unwrap_or_else(|| panic!("cannot find attribute #{invoc_attr_index}"));
+                .unwrap_or_else(|| {
+                    panic!("cannot find attribute #{}", invoc_attr_index.ast_index())
+                });
 
             (
                 ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&attr))),
diff --git a/crates/intern/Cargo.toml b/crates/intern/Cargo.toml

Original file line number	Diff line number	Diff line change
`@@ -546,7 +546,7 @@ impl AttrSourceMap {`
`546`	`546`	`}`
`547`	`547`
`548`	`548`	`fn source_of_id(&self, id: AttrId) -> InFile<&Either<ast::Attr, ast::Comment>> {`
`549`		`- let ast_idx = id.ast_index as usize;`
	`549`	`+ let ast_idx = id.ast_index();`
`550`	`550`	`let file_id = match self.mod_def_site_file_id {`
`551`	`551`	`Some((file_id, def_site_cut)) if def_site_cut <= ast_idx => file_id,`
`552`	`552`	`_ => self.file_id,`
Original file line number	Diff line number	Diff line change
`@@ -117,7 +117,7 @@ impl ChildBySource for ItemScope {`
`117`	`117`	`let adt = ast_id.to_node(db.upcast());`
`118`	`118`	`calls.for_each(\|(attr_id, call_id, calls)\| {`
`119`	`119`	`if let Some(Either::Left(attr)) =`
`120`		`- adt.doc_comments_and_attrs().nth(attr_id.ast_index as usize)`
	`120`	`+ adt.doc_comments_and_attrs().nth(attr_id.ast_index())`
`121`	`121`	`{`
`122`	`122`	`res[keys::DERIVE_MACRO_CALL].insert(attr, (attr_id, call_id, calls.into()));`
`123`	`123`	`}`
Original file line number	Diff line number	Diff line change
`@@ -31,9 +31,9 @@ pub enum DefDiagnosticKind {`
`31`	`31`
`32`	`32`	`UnimplementedBuiltinMacro { ast: AstId<ast::Macro> },`
`33`	`33`
`34`		`- InvalidDeriveTarget { ast: AstId<ast::Item>, id: u32 },`
	`34`	`+ InvalidDeriveTarget { ast: AstId<ast::Item>, id: usize },`
`35`	`35`
`36`		`- MalformedDerive { ast: AstId<ast::Adt>, id: u32 },`
	`36`	`+ MalformedDerive { ast: AstId<ast::Adt>, id: usize },`
`37`	`37`	`}`
`38`	`38`
`39`	`39`	`#[derive(Debug, PartialEq, Eq)]`
`@@ -119,7 +119,7 @@ impl DefDiagnostic {`
`119`	`119`	`) -> Self {`
`120`	`120`	`Self {`
`121`	`121`	`in_module: container,`
`122`		`- kind: DefDiagnosticKind::InvalidDeriveTarget { ast, id: id.ast_index },`
	`122`	`+ kind: DefDiagnosticKind::InvalidDeriveTarget { ast, id: id.ast_index() },`
`123`	`123`	`}`
`124`	`124`	`}`
`125`	`125`
`@@ -130,7 +130,7 @@ impl DefDiagnostic {`
`130`	`130`	`) -> Self {`
`131`	`131`	`Self {`
`132`	`132`	`in_module: container,`
`133`		`- kind: DefDiagnosticKind::MalformedDerive { ast, id: id.ast_index },`
	`133`	`+ kind: DefDiagnosticKind::MalformedDerive { ast, id: id.ast_index() },`
`134`	`134`	`}`
`135`	`135`	`}`
`136`	`136`	`}`