Rollup merge of #86491 - petrochenkov:derefact, r=Aaron1011

expand: Move some more derive logic to rustc_builtin_macros And cleanup some `unwrap`s in `cfg_eval`. Refactorings extracted from #83354 and #86057. r? `@Aaron1011`
rust-lang · Jun 21, 2021 · d449ce1 · d449ce1
2 parents 989dccc + d9fd5ea
commit d449ce1
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 62 deletions.
diff --git a/compiler/rustc_builtin_macros/src/cfg_eval.rs b/compiler/rustc_builtin_macros/src/cfg_eval.rs
@@ -24,61 +24,60 @@ crate fn expand(
     annotatable: Annotatable,
 ) -> Vec<Annotatable> {
     check_builtin_macro_attribute(ecx, meta_item, sym::cfg_eval);
-    cfg_eval(ecx, annotatable)
+    vec![cfg_eval(ecx, annotatable)]
 }
 
-crate fn cfg_eval(ecx: &ExtCtxt<'_>, annotatable: Annotatable) -> Vec<Annotatable> {
-    let mut visitor = CfgEval {
+crate fn cfg_eval(ecx: &ExtCtxt<'_>, annotatable: Annotatable) -> Annotatable {
+    CfgEval {
         cfg: &mut StripUnconfigured {
             sess: ecx.sess,
             features: ecx.ecfg.features,
             config_tokens: true,
         },
-    };
-    let annotatable = visitor.configure_annotatable(annotatable);
-    vec![annotatable]
+    }
+    .configure_annotatable(annotatable)
+    // Since the item itself has already been configured by the `InvocationCollector`,
+    // we know that fold result vector will contain exactly one element.
+    .unwrap()
 }
 
 struct CfgEval<'a, 'b> {
     cfg: &'a mut StripUnconfigured<'b>,
 }
 
-fn flat_map_annotatable(vis: &mut impl MutVisitor, annotatable: Annotatable) -> Annotatable {
-    // Since the item itself has already been configured by the InvocationCollector,
-    // we know that fold result vector will contain exactly one element
+fn flat_map_annotatable(
+    vis: &mut impl MutVisitor,
+    annotatable: Annotatable,
+) -> Option<Annotatable> {
     match annotatable {
-        Annotatable::Item(item) => Annotatable::Item(vis.flat_map_item(item).pop().unwrap()),
+        Annotatable::Item(item) => vis.flat_map_item(item).pop().map(Annotatable::Item),
         Annotatable::TraitItem(item) => {
-            Annotatable::TraitItem(vis.flat_map_trait_item(item).pop().unwrap())
+            vis.flat_map_trait_item(item).pop().map(Annotatable::TraitItem)
         }
         Annotatable::ImplItem(item) => {
-            Annotatable::ImplItem(vis.flat_map_impl_item(item).pop().unwrap())
+            vis.flat_map_impl_item(item).pop().map(Annotatable::ImplItem)
         }
         Annotatable::ForeignItem(item) => {
-            Annotatable::ForeignItem(vis.flat_map_foreign_item(item).pop().unwrap())
+            vis.flat_map_foreign_item(item).pop().map(Annotatable::ForeignItem)
         }
         Annotatable::Stmt(stmt) => {
-            Annotatable::Stmt(stmt.map(|stmt| vis.flat_map_stmt(stmt).pop().unwrap()))
+            vis.flat_map_stmt(stmt.into_inner()).pop().map(P).map(Annotatable::Stmt)
         }
-        Annotatable::Expr(mut expr) => Annotatable::Expr({
+        Annotatable::Expr(mut expr) => {
             vis.visit_expr(&mut expr);
-            expr
-        }),
-        Annotatable::Arm(arm) => Annotatable::Arm(vis.flat_map_arm(arm).pop().unwrap()),
-        Annotatable::ExprField(field) => {
-            Annotatable::ExprField(vis.flat_map_expr_field(field).pop().unwrap())
+            Some(Annotatable::Expr(expr))
         }
-        Annotatable::PatField(fp) => {
-            Annotatable::PatField(vis.flat_map_pat_field(fp).pop().unwrap())
+        Annotatable::Arm(arm) => vis.flat_map_arm(arm).pop().map(Annotatable::Arm),
+        Annotatable::ExprField(field) => {
+            vis.flat_map_expr_field(field).pop().map(Annotatable::ExprField)
         }
+        Annotatable::PatField(fp) => vis.flat_map_pat_field(fp).pop().map(Annotatable::PatField),
         Annotatable::GenericParam(param) => {
-            Annotatable::GenericParam(vis.flat_map_generic_param(param).pop().unwrap())
-        }
-        Annotatable::Param(param) => Annotatable::Param(vis.flat_map_param(param).pop().unwrap()),
-        Annotatable::FieldDef(sf) => {
-            Annotatable::FieldDef(vis.flat_map_field_def(sf).pop().unwrap())
+            vis.flat_map_generic_param(param).pop().map(Annotatable::GenericParam)
         }
-        Annotatable::Variant(v) => Annotatable::Variant(vis.flat_map_variant(v).pop().unwrap()),
+        Annotatable::Param(param) => vis.flat_map_param(param).pop().map(Annotatable::Param),
+        Annotatable::FieldDef(sf) => vis.flat_map_field_def(sf).pop().map(Annotatable::FieldDef),
+        Annotatable::Variant(v) => vis.flat_map_variant(v).pop().map(Annotatable::Variant),
     }
 }
 
@@ -123,11 +122,11 @@ impl CfgEval<'_, '_> {
         self.cfg.configure(node)
     }
 
-    pub fn configure_annotatable(&mut self, mut annotatable: Annotatable) -> Annotatable {
+    fn configure_annotatable(&mut self, mut annotatable: Annotatable) -> Option<Annotatable> {
         // Tokenizing and re-parsing the `Annotatable` can have a significant
         // performance impact, so try to avoid it if possible
         if !CfgFinder::has_cfg_or_cfg_attr(&annotatable) {
-            return annotatable;
+            return Some(annotatable);
         }
 
         // The majority of parsed attribute targets will never need to have early cfg-expansion

diff --git a/compiler/rustc_builtin_macros/src/derive.rs b/compiler/rustc_builtin_macros/src/derive.rs
@@ -26,6 +26,8 @@ impl MultiItemModifier for Expander {
             return ExpandResult::Ready(vec![item]);
         }
 
+        let item = cfg_eval(ecx, item);
+
         let result =
             ecx.resolver.resolve_derives(ecx.current_expansion.id, ecx.force_mode, &|| {
                 let template =
@@ -54,12 +56,12 @@ impl MultiItemModifier for Expander {
                         report_path_args(sess, &meta);
                         meta.path
                     })
-                    .map(|path| (path, None))
+                    .map(|path| (path, item.clone(), None))
                     .collect()
             });
 
         match result {
-            Ok(()) => ExpandResult::Ready(cfg_eval(ecx, item)),
+            Ok(()) => ExpandResult::Ready(vec![item]),
             Err(Indeterminate) => ExpandResult::Retry(item),
         }
     }

diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs
@@ -835,7 +835,7 @@ impl SyntaxExtension {
 /// Error type that denotes indeterminacy.
 pub struct Indeterminate;
 
-pub type DeriveResolutions = Vec<(ast::Path, Option<Lrc<SyntaxExtension>>)>;
+pub type DeriveResolutions = Vec<(ast::Path, Annotatable, Option<Lrc<SyntaxExtension>>)>;
 
 pub trait ResolverExpand {
     fn next_node_id(&mut self) -> NodeId;

diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs
@@ -500,42 +500,16 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
                         .resolver
                         .take_derive_resolutions(expn_id)
                         .map(|derives| {
-                            enum AnnotatableRef<'a> {
-                                Item(&'a P<ast::Item>),
-                                Stmt(&'a ast::Stmt),
-                            }
-                            let item = match &fragment {
-                                AstFragment::Items(items) => match &items[..] {
-                                    [item] => AnnotatableRef::Item(item),
-                                    _ => unreachable!(),
-                                },
-                                AstFragment::Stmts(stmts) => match &stmts[..] {
-                                    [stmt] => AnnotatableRef::Stmt(stmt),
-                                    _ => unreachable!(),
-                                },
-                                _ => unreachable!(),
-                            };
-
                             derive_invocations.reserve(derives.len());
                             derives
                                 .into_iter()
-                                .map(|(path, _exts)| {
+                                .map(|(path, item, _exts)| {
                                     // FIXME: Consider using the derive resolutions (`_exts`)
                                     // instead of enqueuing the derives to be resolved again later.
                                     let expn_id = ExpnId::fresh(None);
                                     derive_invocations.push((
                                         Invocation {
-                                            kind: InvocationKind::Derive {
-                                                path,
-                                                item: match item {
-                                                    AnnotatableRef::Item(item) => {
-                                                        Annotatable::Item(item.clone())
-                                                    }
-                                                    AnnotatableRef::Stmt(stmt) => {
-                                                        Annotatable::Stmt(P(stmt.clone()))
-                                                    }
-                                                },
-                                            },
+                                            kind: InvocationKind::Derive { path, item },
                                             fragment_kind,
                                             expansion_data: ExpansionData {
                                                 id: expn_id,

diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs
@@ -380,7 +380,7 @@ impl<'a> ResolverExpand for Resolver<'a> {
             has_derive_copy: false,
         });
         let parent_scope = self.invocation_parent_scopes[&expn_id];
-        for (i, (path, opt_ext)) in entry.resolutions.iter_mut().enumerate() {
+        for (i, (path, _, opt_ext)) in entry.resolutions.iter_mut().enumerate() {
             if opt_ext.is_none() {
                 *opt_ext = Some(
                     match self.resolve_macro_path(