Merge #10161

10161: Don't dump `DefMap`s to build the panic context r=matklad a=matklad internal: remove accidental code re-use FragmentKind played two roles: * entry point to the parser * syntactic category of a macro call These are different use-cases, and warrant different types. For example, macro can't expand to visibility, but we have such fragment today. This PR introduces `ExpandsTo` enum to separate this two use-cases. I suspect we might further split `FragmentKind` into `$x:specifier` enum specific to MBE, and a general parser entry point, but that's for another PR! bors r+ 🤖 Co-authored-by: Aleksey Kladov <[email protected]>
rust-lang · Sep 5, 2021 · b73b321 · b73b321
2 parents 847d0fa + dbb702c
commit b73b321
Show file tree

Hide file tree

Showing 18 changed files with 172 additions and 139 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/hir_def/src/attr.rs b/crates/hir_def/src/attr.rs
@@ -695,8 +695,7 @@ impl Attr {
         hygiene: &Hygiene,
         id: AttrId,
     ) -> Option<Attr> {
-        let (parse, _) =
-            mbe::token_tree_to_syntax_node(tt, hir_expand::FragmentKind::MetaItem).ok()?;
+        let (parse, _) = mbe::token_tree_to_syntax_node(tt, mbe::FragmentKind::MetaItem).ok()?;
         let ast = ast::Meta::cast(parse.syntax_node())?;
 
         Self::from_src(db, ast, hygiene, id)

diff --git a/crates/hir_def/src/item_tree.rs b/crates/hir_def/src/item_tree.rs
@@ -51,7 +51,7 @@ use hir_expand::{
     ast_id_map::FileAstId,
     hygiene::Hygiene,
     name::{name, AsName, Name},
-    FragmentKind, HirFileId, InFile,
+    ExpandTo, HirFileId, InFile,
 };
 use la_arena::{Arena, Idx, RawIdx};
 use profile::Count;
@@ -739,7 +739,7 @@ pub struct MacroCall {
     /// Path to the called macro.
     pub path: Interned<ModPath>,
     pub ast_id: FileAstId<ast::MacroCall>,
-    pub fragment: FragmentKind,
+    pub expand_to: ExpandTo,
 }
 
 #[derive(Debug, Clone, Eq, PartialEq)]

diff --git a/crates/hir_def/src/item_tree/lower.rs b/crates/hir_def/src/item_tree/lower.rs
@@ -573,8 +573,8 @@ 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 ast_id = self.source_ast_id_map.ast_id(m);
-        let fragment = hir_expand::to_fragment_kind(m);
-        let res = MacroCall { path, ast_id, fragment };
+        let expand_to = hir_expand::ExpandTo::from_call_site(m);
+        let res = MacroCall { path, ast_id, expand_to };
         Some(id(self.data().macro_calls.alloc(res)))
     }
 

diff --git a/crates/hir_def/src/item_tree/pretty.rs b/crates/hir_def/src/item_tree/pretty.rs
@@ -440,7 +440,7 @@ impl<'a> Printer<'a> {
                 }
             }
             ModItem::MacroCall(it) => {
-                let MacroCall { path, ast_id: _, fragment: _ } = &self.tree[it];
+                let MacroCall { path, ast_id: _, expand_to: _ } = &self.tree[it];
                 wln!(self, "{}!(...);", path);
             }
             ModItem::MacroRules(it) => {

diff --git a/crates/hir_def/src/lib.rs b/crates/hir_def/src/lib.rs
@@ -61,7 +61,7 @@ use hir_expand::{
     ast_id_map::FileAstId,
     eager::{expand_eager_macro, ErrorEmitted, ErrorSink},
     hygiene::Hygiene,
-    AstId, FragmentKind, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
+    AstId, ExpandTo, HirFileId, InFile, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
 };
 use la_arena::Idx;
 use nameres::DefMap;
@@ -667,7 +667,7 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
         resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
         mut error_sink: &mut dyn FnMut(mbe::ExpandError),
     ) -> Result<Result<MacroCallId, ErrorEmitted>, UnresolvedMacro> {
-        let fragment = hir_expand::to_fragment_kind(self.value);
+        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));
@@ -683,7 +683,7 @@ impl AsMacroCall for InFile<&ast::MacroCall> {
 
         macro_call_as_call_id(
             &AstIdWithPath::new(ast_id.file_id, ast_id.value, path),
-            fragment,
+            expands_to,
             db,
             krate,
             resolver,
@@ -712,7 +712,7 @@ pub struct UnresolvedMacro {
 
 fn macro_call_as_call_id(
     call: &AstIdWithPath<ast::MacroCall>,
-    fragment: FragmentKind,
+    expand_to: ExpandTo,
     db: &dyn db::DefDatabase,
     krate: CrateId,
     resolver: impl Fn(path::ModPath) -> Option<MacroDefId>,
@@ -738,7 +738,7 @@ fn macro_call_as_call_id(
         Ok(def.as_lazy_macro(
             db.upcast(),
             krate,
-            MacroCallKind::FnLike { ast_id: call.ast_id, fragment },
+            MacroCallKind::FnLike { ast_id: call.ast_id, expand_to },
         ))
     };
     Ok(res)

diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs
@@ -14,7 +14,7 @@ use hir_expand::{
     builtin_macro::find_builtin_macro,
     name::{name, AsName, Name},
     proc_macro::ProcMacroExpander,
-    FragmentKind, HirFileId, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
+    ExpandTo, HirFileId, MacroCallId, MacroCallKind, MacroDefId, MacroDefKind,
 };
 use hir_expand::{InFile, MacroCallLoc};
 use itertools::Itertools;
@@ -223,7 +223,7 @@ struct MacroDirective {
 
 #[derive(Clone, Debug, Eq, PartialEq)]
 enum MacroDirectiveKind {
-    FnLike { ast_id: AstIdWithPath<ast::MacroCall>, fragment: FragmentKind },
+    FnLike { ast_id: AstIdWithPath<ast::MacroCall>, expand_to: ExpandTo },
     Derive { ast_id: AstIdWithPath<ast::Item>, derive_attr: AttrId },
     Attr { ast_id: AstIdWithPath<ast::Item>, attr: Attr, mod_item: ModItem },
 }
@@ -1021,10 +1021,10 @@ impl DefCollector<'_> {
             };
 
             match &directive.kind {
-                MacroDirectiveKind::FnLike { ast_id, fragment } => {
+                MacroDirectiveKind::FnLike { ast_id, expand_to } => {
                     match macro_call_as_call_id(
                         ast_id,
-                        *fragment,
+                        *expand_to,
                         self.db,
                         self.def_map.krate,
                         &resolver,
@@ -1223,32 +1223,34 @@ impl DefCollector<'_> {
 
         for directive in &self.unresolved_macros {
             match &directive.kind {
-                MacroDirectiveKind::FnLike { ast_id, fragment } => match macro_call_as_call_id(
-                    ast_id,
-                    *fragment,
-                    self.db,
-                    self.def_map.krate,
-                    |path| {
-                        let resolved_res = self.def_map.resolve_path_fp_with_macro(
-                            self.db,
-                            ResolveMode::Other,
-                            directive.module_id,
-                            &path,
-                            BuiltinShadowMode::Module,
-                        );
-                        resolved_res.resolved_def.take_macros()
-                    },
-                    &mut |_| (),
-                ) {
-                    Ok(_) => (),
-                    Err(UnresolvedMacro { path }) => {
-                        self.def_map.diagnostics.push(DefDiagnostic::unresolved_macro_call(
-                            directive.module_id,
-                            ast_id.ast_id,
-                            path,
-                        ));
+                MacroDirectiveKind::FnLike { ast_id, expand_to } => {
+                    match macro_call_as_call_id(
+                        ast_id,
+                        *expand_to,
+                        self.db,
+                        self.def_map.krate,
+                        |path| {
+                            let resolved_res = self.def_map.resolve_path_fp_with_macro(
+                                self.db,
+                                ResolveMode::Other,
+                                directive.module_id,
+                                &path,
+                                BuiltinShadowMode::Module,
+                            );
+                            resolved_res.resolved_def.take_macros()
+                        },
+                        &mut |_| (),
+                    ) {
+                        Ok(_) => (),
+                        Err(UnresolvedMacro { path }) => {
+                            self.def_map.diagnostics.push(DefDiagnostic::unresolved_macro_call(
+                                directive.module_id,
+                                ast_id.ast_id,
+                                path,
+                            ));
+                        }
                     }
-                },
+                }
                 MacroDirectiveKind::Derive { .. } | MacroDirectiveKind::Attr { .. } => {
                     // FIXME: we might want to diagnose this too
                 }
@@ -1899,7 +1901,7 @@ impl ModCollector<'_, '_> {
         let mut error = None;
         match macro_call_as_call_id(
             &ast_id,
-            mac.fragment,
+            mac.expand_to,
             self.def_collector.db,
             self.def_collector.def_map.krate,
             |path| {
@@ -1930,12 +1932,11 @@ impl ModCollector<'_, '_> {
                 // Built-in macro failed eager expansion.
 
                 // FIXME: don't parse the file here
-                let fragment = hir_expand::to_fragment_kind(
-                    &ast_id.ast_id.to_node(self.def_collector.db.upcast()),
-                );
+                let macro_call = ast_id.ast_id.to_node(self.def_collector.db.upcast());
+                let expand_to = hir_expand::ExpandTo::from_call_site(&macro_call);
                 self.def_collector.def_map.diagnostics.push(DefDiagnostic::macro_error(
                     self.module_id,
-                    MacroCallKind::FnLike { ast_id: ast_id.ast_id, fragment },
+                    MacroCallKind::FnLike { ast_id: ast_id.ast_id, expand_to },
                     error.unwrap().to_string(),
                 ));
                 return;
@@ -1947,7 +1948,7 @@ impl ModCollector<'_, '_> {
         self.def_collector.unresolved_macros.push(MacroDirective {
             module_id: self.module_id,
             depth: self.macro_depth + 1,
-            kind: MacroDirectiveKind::FnLike { ast_id, fragment: mac.fragment },
+            kind: MacroDirectiveKind::FnLike { ast_id, expand_to: mac.expand_to },
         });
     }
 

diff --git a/crates/hir_expand/Cargo.toml b/crates/hir_expand/Cargo.toml
@@ -19,7 +19,6 @@ itertools = "0.10.0"
 base_db = { path = "../base_db", version = "0.0.0" }
 cfg = { path = "../cfg", version = "0.0.0" }
 syntax = { path = "../syntax", version = "0.0.0" }
-parser = { path = "../parser", version = "0.0.0" }
 profile = { path = "../profile", version = "0.0.0" }
 tt = { path = "../tt", version = "0.0.0" }
 mbe = { path = "../mbe", version = "0.0.0" }

diff --git a/crates/hir_expand/src/builtin_derive.rs b/crates/hir_expand/src/builtin_derive.rs
@@ -3,7 +3,6 @@
 use tracing::debug;
 
 use mbe::ExpandResult;
-use parser::FragmentKind;
 use syntax::{
     ast::{self, AstNode, GenericParamsOwner, ModuleItemOwner, NameOwner},
     match_ast,
@@ -73,7 +72,7 @@ struct BasicAdtInfo {
 }
 
 fn parse_adt(tt: &tt::Subtree) -> Result<BasicAdtInfo, mbe::ExpandError> {
-    let (parsed, token_map) = mbe::token_tree_to_syntax_node(tt, FragmentKind::Items)?; // FragmentKind::Items doesn't parse attrs?
+    let (parsed, token_map) = mbe::token_tree_to_syntax_node(tt, mbe::FragmentKind::Items)?; // FragmentKind::Items doesn't parse attrs?
     let macro_items = ast::MacroItems::cast(parsed.syntax_node()).ok_or_else(|| {
         debug!("derive node didn't parse");
         mbe::ExpandError::UnexpectedToken

diff --git a/crates/hir_expand/src/builtin_macro.rs b/crates/hir_expand/src/builtin_macro.rs
@@ -554,17 +554,19 @@ fn option_env_expand(
 
 #[cfg(test)]
 mod tests {
-    use super::*;
-    use crate::{
-        name::AsName, test_db::TestDB, AstNode, EagerCallInfo, MacroCallId, MacroCallKind,
-        MacroCallLoc,
-    };
+    use std::sync::Arc;
+
     use base_db::{fixture::WithFixture, SourceDatabase};
     use expect_test::{expect, Expect};
-    use parser::FragmentKind;
-    use std::sync::Arc;
     use syntax::ast::NameOwner;
 
+    use crate::{
+        name::AsName, test_db::TestDB, AstNode, EagerCallInfo, ExpandTo, MacroCallId,
+        MacroCallKind, MacroCallLoc,
+    };
+
+    use super::*;
+
     fn expand_builtin_macro(ra_fixture: &str) -> String {
         let (db, file_id) = TestDB::with_single_file(ra_fixture);
         let parsed = db.parse(file_id);
@@ -599,7 +601,7 @@ mod tests {
                     eager: None,
                     kind: MacroCallKind::FnLike {
                         ast_id: AstId::new(file_id.into(), ast_id_map.ast_id(&macro_call)),
-                        fragment: FragmentKind::Expr,
+                        expand_to: ExpandTo::Expr,
                     },
                 };
 
@@ -614,7 +616,6 @@ mod tests {
                     local_inner: false,
                 };
 
-                let fragment = crate::to_fragment_kind(&macro_call);
                 let args = macro_call.token_tree().unwrap();
                 let parsed_args = mbe::syntax_node_to_token_tree(args.syntax()).0;
                 let call_id = AstId::new(file_id.into(), ast_id_map.ast_id(&macro_call));
@@ -626,18 +627,19 @@ mod tests {
                         arg_or_expansion: Arc::new(parsed_args.clone()),
                         included_file: None,
                     }),
-                    kind: MacroCallKind::FnLike { ast_id: call_id, fragment: FragmentKind::Expr },
+                    kind: MacroCallKind::FnLike { ast_id: call_id, expand_to: ExpandTo::Expr },
                 });
 
                 let expanded = expander.expand(&db, arg_id, &parsed_args).value.unwrap();
+                let expand_to = crate::ExpandTo::from_call_site(&macro_call);
                 let loc = MacroCallLoc {
                     def,
                     krate,
                     eager: Some(EagerCallInfo {
                         arg_or_expansion: Arc::new(expanded.subtree),
                         included_file: expanded.included_file,
                     }),
-                    kind: MacroCallKind::FnLike { ast_id: call_id, fragment },
+                    kind: MacroCallKind::FnLike { ast_id: call_id, expand_to },
                 };
 
                 let id: MacroCallId = db.intern_macro(loc);

diff --git a/crates/hir_expand/src/db.rs b/crates/hir_expand/src/db.rs
@@ -6,17 +6,16 @@ use base_db::{salsa, SourceDatabase};
 use itertools::Itertools;
 use limit::Limit;
 use mbe::{ExpandError, ExpandResult};
-use parser::{FragmentKind, T};
 use syntax::{
     algo::diff,
     ast::{self, AttrsOwner, NameOwner},
-    AstNode, GreenNode, Parse, SyntaxNode, SyntaxToken, TextRange,
+    AstNode, GreenNode, Parse, SyntaxNode, SyntaxToken, TextRange, T,
 };
 
 use crate::{
     ast_id_map::AstIdMap, hygiene::HygieneFrame, BuiltinAttrExpander, BuiltinDeriveExpander,
-    BuiltinFnLikeExpander, HirFileId, HirFileIdRepr, MacroCallId, MacroCallKind, MacroCallLoc,
-    MacroDefId, MacroDefKind, MacroFile, ProcMacroExpander,
+    BuiltinFnLikeExpander, ExpandTo, HirFileId, HirFileIdRepr, MacroCallId, MacroCallKind,
+    MacroCallLoc, MacroDefId, MacroDefKind, MacroFile, ProcMacroExpander,
 };
 
 /// Total limit on the number of tokens produced by any macro invocation.
@@ -157,10 +156,9 @@ pub fn expand_speculative(
 
     let speculative_expansion = macro_def.expand(db, actual_macro_call, &tt);
 
-    let fragment_kind = macro_fragment_kind(db, actual_macro_call);
+    let expand_to = macro_expand_to(db, actual_macro_call);
 
-    let (node, tmap_2) =
-        mbe::token_tree_to_syntax_node(&speculative_expansion.value, fragment_kind).ok()?;
+    let (node, tmap_2) = token_tree_to_syntax_node(&speculative_expansion.value, expand_to).ok()?;
 
     let token_id = macro_def.map_id_down(token_id);
     let range = tmap_2.first_range_by_token(token_id, token_to_map.kind())?;
@@ -215,17 +213,17 @@ fn parse_macro_expansion(
         None => return ExpandResult { value: None, err: result.err },
     };
 
-    let fragment_kind = macro_fragment_kind(db, macro_file.macro_call_id);
+    let expand_to = macro_expand_to(db, macro_file.macro_call_id);
 
     tracing::debug!("expanded = {}", tt.as_debug_string());
-    tracing::debug!("kind = {:?}", fragment_kind);
+    tracing::debug!("kind = {:?}", expand_to);
 
-    let (parse, rev_token_map) = match mbe::token_tree_to_syntax_node(&tt, fragment_kind) {
+    let (parse, rev_token_map) = match token_tree_to_syntax_node(&tt, expand_to) {
         Ok(it) => it,
         Err(err) => {
             tracing::debug!(
                 "failed to parse expansion to {:?} = {}",
-                fragment_kind,
+                expand_to,
                 tt.as_debug_string()
             );
             return ExpandResult::only_err(err);
@@ -437,7 +435,21 @@ fn hygiene_frame(db: &dyn AstDatabase, file_id: HirFileId) -> Arc<HygieneFrame>
     Arc::new(HygieneFrame::new(db, file_id))
 }
 
-fn macro_fragment_kind(db: &dyn AstDatabase, id: MacroCallId) -> FragmentKind {
+fn macro_expand_to(db: &dyn AstDatabase, id: MacroCallId) -> ExpandTo {
     let loc: MacroCallLoc = db.lookup_intern_macro(id);
-    loc.kind.fragment_kind()
+    loc.kind.expand_to()
+}
+
+fn token_tree_to_syntax_node(
+    tt: &tt::Subtree,
+    expand_to: ExpandTo,
+) -> Result<(Parse<SyntaxNode>, mbe::TokenMap), ExpandError> {
+    let fragment = match expand_to {
+        ExpandTo::Statements => mbe::FragmentKind::Statements,
+        ExpandTo::Items => mbe::FragmentKind::Items,
+        ExpandTo::Pattern => mbe::FragmentKind::Pattern,
+        ExpandTo::Type => mbe::FragmentKind::Type,
+        ExpandTo::Expr => mbe::FragmentKind::Expr,
+    };
+    mbe::token_tree_to_syntax_node(tt, fragment)
 }