Merge #11347

11347: fix: Fix resolution of eager macro contents r=jonas-schievink a=jonas-schievink

Eager macros resolve and expand any contained macro invocations before they are expanded. The logic for this was previously pretty broken: any nameres failure would be reported as a generic macro expansion error, so this didn't work correctly with the fixed-point resolution loop. This manifested as spurious errors whenever a non-legacy macro was used in an eager macro (that means *any* path with more than one segment).

After an intense staring contest with the abyss, this PR fixes the basic logic, but some bugs still remain (particularly around `$crate`). As a side-effect, this PR moves `ModPath` into `hir_expand`.

bors r+

Co-authored-by: Jonas Schievink <[email protected]>

Author: bors[bot]

crates/hir_def/src/attr.rs

@@ -713,7 +713,7 @@ impl Attr {
         hygiene: &Hygiene,
         id: AttrId,
     ) -> Option<Attr> {
-        let path = Interned::new(ModPath::from_src(db, ast.path()?, hygiene)?);
+        let path = Interned::new(ModPath::from_src(db.upcast(), ast.path()?, hygiene)?);
         let input = if let Some(ast::Expr::Literal(lit)) = ast.expr() {
             let value = match lit.kind() {
                 ast::LiteralKind::String(string) => string.value()?.into(),

crates/hir_def/src/find_path.rs

@@ -35,7 +35,12 @@ pub fn find_path_prefixed(
 
 const MAX_PATH_LEN: usize = 15;
 
-impl ModPath {
+trait ModPathExt {
+    fn starts_with_std(&self) -> bool;
+    fn can_start_with_std(&self) -> bool;
+}
+
+impl ModPathExt for ModPath {
     fn starts_with_std(&self) -> bool {
         self.segments().first() == Some(&known::std)
     }

crates/hir_def/src/item_tree/lower.rs

@@ -502,7 +502,7 @@ impl<'a> Ctx<'a> {
     }
 
     fn lower_macro_call(&mut self, m: &ast::MacroCall) -> Option<FileItemTreeId<MacroCall>> {
-        let path = Interned::new(ModPath::from_src(self.db, m.path()?, self.hygiene())?);
+        let path = Interned::new(ModPath::from_src(self.db.upcast(), m.path()?, self.hygiene())?);
         let ast_id = self.source_ast_id_map.ast_id(m);
         let expand_to = hir_expand::ExpandTo::from_call_site(m);
         let res = MacroCall { path, ast_id, expand_to };
@@ -769,7 +769,7 @@ impl UseTreeLowering<'_> {
                 // E.g. `use something::{inner}` (prefix is `None`, path is `something`)
                 // or `use something::{path::{inner::{innerer}}}` (prefix is `something::path`, path is `inner`)
                 Some(path) => {
-                    match ModPath::from_src(self.db, path, self.hygiene) {
+                    match ModPath::from_src(self.db.upcast(), path, self.hygiene) {
                         Some(it) => Some(it),
                         None => return None, // FIXME: report errors somewhere
                     }
@@ -788,7 +788,7 @@ impl UseTreeLowering<'_> {
         } else {
             let is_glob = tree.star_token().is_some();
             let path = match tree.path() {
-                Some(path) => Some(ModPath::from_src(self.db, path, self.hygiene)?),
+                Some(path) => Some(ModPath::from_src(self.db.upcast(), path, self.hygiene)?),
                 None => None,
             };
             let alias = tree.rename().map(|a| {

crates/hir_def/src/lib.rs

@@ -64,11 +64,11 @@ use hir_expand::{
     eager::{expand_eager_macro, ErrorEmitted, ErrorSink},
     hygiene::Hygiene,
     AstId, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
+    UnresolvedMacro,
 };
 use item_tree::ExternBlock;
 use la_arena::Idx;
 use nameres::DefMap;
-use path::ModPath;
 use stdx::impl_from;
 use syntax::ast;
 
@@ -677,7 +677,8 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
         let expands_to = hir_expand::ExpandTo::from_call_site(self.value);
         let ast_id = AstId::new(self.file_id, db.ast_id_map(self.file_id).ast_id(self.value));
         let h = Hygiene::new(db.upcast(), self.file_id);
-        let path = self.value.path().and_then(|path| path::ModPath::from_src(db, path, &h));
+        let path =
+            self.value.path().and_then(|path| path::ModPath::from_src(db.upcast(), path, &h));
 
         let path = match error_sink
             .option(path, || mbe::ExpandError::Other("malformed macro invocation".into()))
@@ -712,11 +713,6 @@ impl<T: ast::AstNode> AstIdWithPath<T> {
     }
 }
 
-#[derive(Debug)]
-pub struct UnresolvedMacro {
-    pub path: ModPath,
-}
-
 fn macro_call_as_call_id(
     call: &AstIdWithPath<ast::MacroCall>,
     expand_to: ExpandTo,
@@ -730,16 +726,8 @@ fn macro_call_as_call_id(
 
     let res = if let MacroDefKind::BuiltInEager(..) = def.kind {
         let macro_call = InFile::new(call.ast_id.file_id, call.ast_id.to_node(db.upcast()));
-        let hygiene = Hygiene::new(db.upcast(), call.ast_id.file_id);
 
-        expand_eager_macro(
-            db.upcast(),
-            krate,
-            macro_call,
-            def,
-            &|path: ast::Path| resolver(path::ModPath::from_src(db, path, &hygiene)?),
-            error_sink,
-        )
+        expand_eager_macro(db.upcast(), krate, macro_call, def, &resolver, error_sink)?
     } else {
         Ok(def.as_lazy_macro(
             db.upcast(),

crates/hir_def/src/nameres/tests/macros.rs

@@ -1046,3 +1046,71 @@ m!(
         "#]],
     )
 }
+
+#[test]
+fn eager_macro_correctly_resolves_contents() {
+    // Eager macros resolve any contained macros when expanded. This should work correctly with the
+    // usual name resolution rules, so both of these `include!`s should include the right file.
+
+    check(
+        r#"
+//- /lib.rs
+#[rustc_builtin_macro]
+macro_rules! include { () => {} }
+
+include!(inner_a!());
+include!(crate::inner_b!());
+
+#[macro_export]
+macro_rules! inner_a {
+    () => { "inc_a.rs" };
+}
+#[macro_export]
+macro_rules! inner_b {
+    () => { "inc_b.rs" };
+}
+//- /inc_a.rs
+struct A;
+//- /inc_b.rs
+struct B;
+"#,
+        expect![[r#"
+        crate
+        A: t v
+        B: t v
+        inner_a: m
+        inner_b: m
+    "#]],
+    );
+}
+
+#[test]
+fn eager_macro_correctly_resolves_dollar_crate() {
+    check(
+        r#"
+//- /lib.rs
+#[rustc_builtin_macro]
+macro_rules! include { () => {} }
+
+#[macro_export]
+macro_rules! inner {
+    () => { "inc.rs" };
+}
+
+macro_rules! m {
+    () => { include!($crate::inner!()); };
+}
+
+m!();
+
+//- /inc.rs
+struct A;
+"#,
+        expect![[r#"
+            crate
+            inner: m
+        "#]],
+    );
+
+    // FIXME: This currently fails. The result should contain `A: t v`.
+}

crates/hir_def/src/path.rs

@@ -6,33 +6,13 @@ use std::{
     iter,
 };
 
-use crate::{body::LowerCtx, db::DefDatabase, intern::Interned, type_ref::LifetimeRef};
-use base_db::CrateId;
-use hir_expand::{
-    hygiene::Hygiene,
-    name::{name, Name},
-};
+use crate::{body::LowerCtx, intern::Interned, type_ref::LifetimeRef};
+use hir_expand::name::{name, Name};
 use syntax::ast;
 
 use crate::type_ref::{TypeBound, TypeRef};
 
-#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub struct ModPath {
-    pub kind: PathKind,
-    segments: Vec<Name>,
-}
-
-#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub enum PathKind {
-    Plain,
-    /// `self::` is `Super(0)`
-    Super(u8),
-    Crate,
-    /// Absolute path (::foo)
-    Abs,
-    /// `$crate` from macro expansion
-    DollarCrate(CrateId),
-}
+pub use hir_expand::mod_path::{path, ModPath, PathKind};
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum ImportAlias {
@@ -51,67 +31,6 @@ impl Display for ImportAlias {
     }
 }
 
-impl ModPath {
-    pub fn from_src(db: &dyn DefDatabase, path: ast::Path, hygiene: &Hygiene) -> Option<ModPath> {
-        lower::convert_path(db, None, path, hygiene)
-    }
-
-    pub fn from_segments(kind: PathKind, segments: impl IntoIterator<Item = Name>) -> ModPath {
-        let segments = segments.into_iter().collect::<Vec<_>>();
-        ModPath { kind, segments }
-    }
-
-    /// Creates a `ModPath` from a `PathKind`, with no extra path segments.
-    pub const fn from_kind(kind: PathKind) -> ModPath {
-        ModPath { kind, segments: Vec::new() }
-    }
-
-    pub fn segments(&self) -> &[Name] {
-        &self.segments
-    }
-
-    pub fn push_segment(&mut self, segment: Name) {
-        self.segments.push(segment);
-    }
-
-    pub fn pop_segment(&mut self) -> Option<Name> {
-        self.segments.pop()
-    }
-
-    /// Returns the number of segments in the path (counting special segments like `$crate` and
-    /// `super`).
-    pub fn len(&self) -> usize {
-        self.segments.len()
-            + match self.kind {
-                PathKind::Plain => 0,
-                PathKind::Super(i) => i as usize,
-                PathKind::Crate => 1,
-                PathKind::Abs => 0,
-                PathKind::DollarCrate(_) => 1,
-            }
-    }
-
-    pub fn is_ident(&self) -> bool {
-        self.as_ident().is_some()
-    }
-
-    pub fn is_self(&self) -> bool {
-        self.kind == PathKind::Super(0) && self.segments.is_empty()
-    }
-
-    /// If this path is a single identifier, like `foo`, return its name.
-    pub fn as_ident(&self) -> Option<&Name> {
-        if self.kind != PathKind::Plain {
-            return None;
-        }
-
-        match &*self.segments {
-            [name] => Some(name),
-            _ => None,
-        }
-    }
-}
-
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Path {
     /// Type based path like `<T>::foo`.
@@ -185,10 +104,7 @@ impl Path {
     }
 
     pub fn segments(&self) -> PathSegments<'_> {
-        PathSegments {
-            segments: self.mod_path.segments.as_slice(),
-            generic_args: &self.generic_args,
-        }
+        PathSegments { segments: self.mod_path.segments(), generic_args: &self.generic_args }
     }
 
     pub fn mod_path(&self) -> &ModPath {
@@ -203,7 +119,7 @@ impl Path {
             type_anchor: self.type_anchor.clone(),
             mod_path: Interned::new(ModPath::from_segments(
                 self.mod_path.kind.clone(),
-                self.mod_path.segments[..self.mod_path.segments.len() - 1].iter().cloned(),
+                self.mod_path.segments()[..self.mod_path.segments().len() - 1].iter().cloned(),
             )),
             generic_args: self.generic_args[..self.generic_args.len() - 1].to_vec().into(),
         };
@@ -296,76 +212,3 @@ impl From<Name> for Box<Path> {
         Box::new(Path::from(name))
     }
 }
-
-impl From<Name> for ModPath {
-    fn from(name: Name) -> ModPath {
-        ModPath::from_segments(PathKind::Plain, iter::once(name))
-    }
-}
-
-impl Display for ModPath {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let mut first_segment = true;
-        let mut add_segment = |s| -> fmt::Result {
-            if !first_segment {
-                f.write_str("::")?;
-            }
-            first_segment = false;
-            f.write_str(s)?;
-            Ok(())
-        };
-        match self.kind {
-            PathKind::Plain => {}
-            PathKind::Super(0) => add_segment("self")?,
-            PathKind::Super(n) => {
-                for _ in 0..n {
-                    add_segment("super")?;
-                }
-            }
-            PathKind::Crate => add_segment("crate")?,
-            PathKind::Abs => add_segment("")?,
-            PathKind::DollarCrate(_) => add_segment("$crate")?,
-        }
-        for segment in &self.segments {
-            if !first_segment {
-                f.write_str("::")?;
-            }
-            first_segment = false;
-            write!(f, "{}", segment)?;
-        }
-        Ok(())
-    }
-}
-
-pub use hir_expand::name as __name;
-
-#[macro_export]
-macro_rules! __known_path {
-    (core::iter::IntoIterator) => {};
-    (core::iter::Iterator) => {};
-    (core::result::Result) => {};
-    (core::option::Option) => {};
-    (core::ops::Range) => {};
-    (core::ops::RangeFrom) => {};
-    (core::ops::RangeFull) => {};
-    (core::ops::RangeTo) => {};
-    (core::ops::RangeToInclusive) => {};
-    (core::ops::RangeInclusive) => {};
-    (core::future::Future) => {};
-    (core::ops::Try) => {};
-    ($path:path) => {
-        compile_error!("Please register your known path in the path module")
-    };
-}
-
-#[macro_export]
-macro_rules! __path {
-    ($start:ident $(:: $seg:ident)*) => ({
-        $crate::__known_path!($start $(:: $seg)*);
-        $crate::path::ModPath::from_segments($crate::path::PathKind::Abs, vec![
-            $crate::path::__name![$start], $($crate::path::__name![$seg],)*
-        ])
-    });
-}
-
-pub use crate::__path as path;