Skip to content

Commit

Permalink
expand: Simplify expansion of derives
Browse files Browse the repository at this point in the history
And make it more uniform with other macros.
By merging placeholders for future derives' outputs into the derive container's output fragment early.
  • Loading branch information
petrochenkov committed Oct 10, 2019
1 parent e8af4c1 commit 1270140
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 41 deletions.
2 changes: 1 addition & 1 deletion src/librustc/hir/map/def_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl<'a> DefCollector<'a> {
}
}

pub fn visit_macro_invoc(&mut self, id: NodeId) {
fn visit_macro_invoc(&mut self, id: NodeId) {
self.definitions.set_invocation_parent(id.placeholder_to_expn_id(), self.parent_def);
}
}
Expand Down
10 changes: 0 additions & 10 deletions src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,25 +161,15 @@ impl<'a> Resolver<'a> {
Some(ext)
}

// FIXME: `extra_placeholders` should be included into the `fragment` as regular placeholders.
crate fn build_reduced_graph(
&mut self,
fragment: &AstFragment,
extra_placeholders: &[NodeId],
parent_scope: ParentScope<'a>,
) -> LegacyScope<'a> {
let mut def_collector = DefCollector::new(&mut self.definitions, parent_scope.expansion);
fragment.visit_with(&mut def_collector);
for placeholder in extra_placeholders {
def_collector.visit_macro_invoc(*placeholder);
}

let mut visitor = BuildReducedGraphVisitor { r: self, parent_scope };
fragment.visit_with(&mut visitor);
for placeholder in extra_placeholders {
visitor.parent_scope.legacy = visitor.visit_invoc(*placeholder);
}

visitor.parent_scope.legacy
}

Expand Down
8 changes: 2 additions & 6 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,11 @@ impl<'a> base::Resolver for Resolver<'a> {
});
}

// FIXME: `extra_placeholders` should be included into the `fragment` as regular placeholders.
fn visit_ast_fragment_with_placeholders(
&mut self, expansion: ExpnId, fragment: &AstFragment, extra_placeholders: &[NodeId]
) {
fn visit_ast_fragment_with_placeholders(&mut self, expansion: ExpnId, fragment: &AstFragment) {
// Integrate the new AST fragment into all the definition and module structures.
// We are inside the `expansion` now, but other parent scope components are still the same.
let parent_scope = ParentScope { expansion, ..self.invocation_parent_scopes[&expansion] };
let output_legacy_scope =
self.build_reduced_graph(fragment, extra_placeholders, parent_scope);
let output_legacy_scope = self.build_reduced_graph(fragment, parent_scope);
self.output_legacy_scopes.insert(expansion, output_legacy_scope);

parent_scope.module.unexpanded_invocations.borrow_mut().remove(&expansion);
Expand Down
3 changes: 1 addition & 2 deletions src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -849,8 +849,7 @@ pub trait Resolver {
fn next_node_id(&mut self) -> NodeId;

fn resolve_dollar_crates(&mut self);
fn visit_ast_fragment_with_placeholders(&mut self, expn_id: ExpnId, fragment: &AstFragment,
extra_placeholders: &[NodeId]);
fn visit_ast_fragment_with_placeholders(&mut self, expn_id: ExpnId, fragment: &AstFragment);
fn register_builtin_macro(&mut self, ident: ast::Ident, ext: SyntaxExtension);

fn expansion_for_ast_pass(
Expand Down
33 changes: 22 additions & 11 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use errors::{Applicability, FatalError};
use smallvec::{smallvec, SmallVec};
use syntax_pos::{Span, DUMMY_SP, FileName};

use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::sync::Lrc;
use std::io::ErrorKind;
use std::{iter, mem, slice};
Expand Down Expand Up @@ -72,6 +71,22 @@ macro_rules! ast_fragments {
}

impl AstFragment {
pub fn add_placeholders(&mut self, placeholders: &[NodeId]) {
if placeholders.is_empty() {
return;
}
match self {
$($(AstFragment::$Kind(ast) => ast.extend(placeholders.iter().flat_map(|id| {
// We are repeating through arguments with `many`, to do that we have to
// mention some macro variable from those arguments even if it's not used.
#[cfg_attr(bootstrap, allow(unused_macros))]
macro _repeating($flat_map_ast_elt) {}
placeholder(AstFragmentKind::$Kind, *id).$make_ast()
})),)?)*
_ => panic!("unexpected AST fragment kind")
}
}

pub fn make_opt_expr(self) -> Option<P<ast::Expr>> {
match self {
AstFragment::OptExpr(expr) => expr,
Expand Down Expand Up @@ -339,7 +354,6 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
// Unresolved macros produce dummy outputs as a recovery measure.
invocations.reverse();
let mut expanded_fragments = Vec::new();
let mut all_derive_placeholders: FxHashMap<ExpnId, Vec<_>> = FxHashMap::default();
let mut undetermined_invocations = Vec::new();
let (mut progress, mut force) = (false, !self.monotonic);
loop {
Expand Down Expand Up @@ -416,9 +430,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
self.cx.resolver.add_derives(invoc.expansion_data.id, SpecialDerives::COPY);
}

let derive_placeholders =
all_derive_placeholders.entry(invoc.expansion_data.id).or_default();
derive_placeholders.reserve(derives.len());
let mut derive_placeholders = Vec::with_capacity(derives.len());
invocations.reserve(derives.len());
for path in derives {
let expn_id = ExpnId::fresh(None);
Expand All @@ -434,7 +446,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
}
let fragment = invoc.fragment_kind
.expect_from_annotatables(::std::iter::once(item));
self.collect_invocations(fragment, derive_placeholders)
self.collect_invocations(fragment, &derive_placeholders)
}
};

Expand All @@ -453,10 +465,8 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
let mut placeholder_expander = PlaceholderExpander::new(self.cx, self.monotonic);
while let Some(expanded_fragments) = expanded_fragments.pop() {
for (expn_id, expanded_fragment) in expanded_fragments.into_iter().rev() {
let derive_placeholders =
all_derive_placeholders.remove(&expn_id).unwrap_or_else(Vec::new);
placeholder_expander.add(NodeId::placeholder_from_expn_id(expn_id),
expanded_fragment, derive_placeholders);
expanded_fragment);
}
}
fragment_with_placeholders.mut_visit_with(&mut placeholder_expander);
Expand Down Expand Up @@ -489,13 +499,14 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
monotonic: self.monotonic,
};
fragment.mut_visit_with(&mut collector);
fragment.add_placeholders(extra_placeholders);
collector.invocations
};

// FIXME: Merge `extra_placeholders` into the `fragment` as regular placeholders.
if self.monotonic {
self.cx.resolver.visit_ast_fragment_with_placeholders(
self.cx.current_expansion.id, &fragment, extra_placeholders);
self.cx.current_expansion.id, &fragment
);
}

(fragment, invocations)
Expand Down
13 changes: 2 additions & 11 deletions src/libsyntax/ext/placeholders.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::ast::{self, NodeId};
use crate::ast;
use crate::source_map::{DUMMY_SP, dummy_spanned};
use crate::ext::base::ExtCtxt;
use crate::ext::expand::{AstFragment, AstFragmentKind};
Expand Down Expand Up @@ -170,17 +170,8 @@ impl<'a, 'b> PlaceholderExpander<'a, 'b> {
}
}

pub fn add(&mut self, id: ast::NodeId, mut fragment: AstFragment, placeholders: Vec<NodeId>) {
pub fn add(&mut self, id: ast::NodeId, mut fragment: AstFragment) {
fragment.mut_visit_with(self);
if let AstFragment::Items(mut items) = fragment {
for placeholder in placeholders {
match self.remove(placeholder) {
AstFragment::Items(derived_items) => items.extend(derived_items),
_ => unreachable!(),
}
}
fragment = AstFragment::Items(items);
}
self.expanded_fragments.insert(id, fragment);
}

Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#![feature(const_fn)]
#![feature(const_transmute)]
#![feature(crate_visibility_modifier)]
#![feature(decl_macro)]
#![feature(label_break_value)]
#![feature(mem_take)]
#![feature(nll)]
Expand Down

0 comments on commit 1270140

Please sign in to comment.