Merge #8443 #8446

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 https://github.com/rust-analyzer/rust-analyzer/issues/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 https://github.com/rust-analyzer/rust-analyzer/pull/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>

crates/hir_def/src/attr.rs

@@ -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;
@@ -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<_>>();
 
@@ -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()?;
@@ -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
@@ -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()
     }
 }
@@ -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 {
@@ -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
@@ -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>,
 }
@@ -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() {
@@ -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

crates/hir_def/src/lib.rs

@@ -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;
@@ -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>,
@@ -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();

crates/hir_def/src/nameres.rs

@@ -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

crates/hir_def/src/nameres/collector.rs

@@ -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};
@@ -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> {
@@ -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;
@@ -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 },
                         });
                     }
                 }

crates/hir_def/src/nameres/path_resolution.rs

@@ -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);
 

crates/hir_expand/src/builtin_derive.rs

@@ -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::*;
 
@@ -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();

crates/hir_expand/src/db.rs

@@ -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.
@@ -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())
 }
 

crates/hir_expand/src/input.rs

@@ -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
+}"#]],
+        );
+    }
+}

crates/hir_expand/src/lib.rs

@@ -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};
@@ -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 {

crates/hir_expand/src/proc_macro.rs

@@ -2,7 +2,6 @@
 
 use crate::db::AstDatabase;
 use base_db::{CrateId, ProcMacroId};
-use tt::buffer::{Cursor, TokenBuffer};
 
 #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)]
 pub struct ProcMacroExpander {
@@ -44,9 +43,6 @@ impl ProcMacroExpander {
                     .clone()
                     .ok_or_else(|| err!("No derive macro found."))?;
 
-                let tt = remove_derive_attrs(tt)
-                    .ok_or_else(|| err!("Fail to remove derive for custom derive"))?;
-
                 // Proc macros have access to the environment variables of the invoking crate.
                 let env = &krate_graph[calling_crate].env;
 
@@ -56,101 +52,3 @@ impl ProcMacroExpander {
         }
     }
 }
-
-fn eat_punct(cursor: &mut Cursor, c: char) -> bool {
-    if let Some(tt::buffer::TokenTreeRef::Leaf(tt::Leaf::Punct(punct), _)) = cursor.token_tree() {
-        if punct.char == c {
-            *cursor = cursor.bump();
-            return true;
-        }
-    }
-    false
-}
-
-fn eat_subtree(cursor: &mut Cursor, kind: tt::DelimiterKind) -> bool {
-    if let Some(tt::buffer::TokenTreeRef::Subtree(subtree, _)) = cursor.token_tree() {
-        if Some(kind) == subtree.delimiter_kind() {
-            *cursor = cursor.bump_subtree();
-            return true;
-        }
-    }
-    false
-}
-
-fn eat_ident(cursor: &mut Cursor, t: &str) -> bool {
-    if let Some(tt::buffer::TokenTreeRef::Leaf(tt::Leaf::Ident(ident), _)) = cursor.token_tree() {
-        if t == ident.text.as_str() {
-            *cursor = cursor.bump();
-            return true;
-        }
-    }
-    false
-}
-
-fn remove_derive_attrs(tt: &tt::Subtree) -> Option<tt::Subtree> {
-    let buffer = TokenBuffer::from_tokens(&tt.token_trees);
-    let mut p = buffer.begin();
-    let mut result = tt::Subtree::default();
-
-    while !p.eof() {
-        let curr = p;
-
-        if eat_punct(&mut p, '#') {
-            eat_punct(&mut p, '!');
-            let parent = p;
-            if eat_subtree(&mut p, tt::DelimiterKind::Bracket) {
-                if eat_ident(&mut p, "derive") {
-                    p = parent.bump();
-                    continue;
-                }
-            }
-        }
-
-        result.token_trees.push(curr.token_tree()?.cloned());
-        p = curr.bump();
-    }
-
-    Some(result)
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use test_utils::assert_eq_text;
-
-    #[test]
-    fn test_remove_derive_attrs() {
-        let tt = mbe::parse_to_token_tree(
-            r#"
-    #[allow(unused)]
-    #[derive(Copy)]
-    #[derive(Hello)]
-    struct A {
-        bar: u32
-    }
-"#,
-        )
-        .unwrap()
-        .0;
-        let result = format!("{:#?}", remove_derive_attrs(&tt).unwrap());
-
-        assert_eq_text!(
-            r#"
-SUBTREE $
-  PUNCH   # [alone] 0
-  SUBTREE [] 1
-    IDENT   allow 2
-    SUBTREE () 3
-      IDENT   unused 4
-  IDENT   struct 15
-  IDENT   A 16
-  SUBTREE {} 17
-    IDENT   bar 18
-    PUNCH   : [alone] 19
-    IDENT   u32 20
-"#
-            .trim(),
-            &result
-        );
-    }
-}