Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand items before their derives #48465

Closed
wants to merge 2 commits into from
Closed

Expand items before their derives #48465

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c6706ea
Expand items before their `#[derive]`s.
jseyfried Apr 2, 2017
0890244
add regression test for #47358
abonander Mar 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions src/librustc_resolve/macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc::hir::def_id::{DefId, BUILTIN_MACROS_CRATE, CRATE_DEF_INDEX, DefIndex,
use rustc::hir::def::{Def, Export};
use rustc::hir::map::{self, DefCollector};
use rustc::{ty, lint};
use syntax::ast::{self, Name, Ident};
use syntax::ast::{self, Name, Ident, Path};
use syntax::attr::{self, HasAttrs};
use syntax::codemap::respan;
use syntax::errors::DiagnosticBuilder;
Expand Down Expand Up @@ -166,16 +166,25 @@ impl<'a> base::Resolver for Resolver<'a> {
self.whitelisted_legacy_custom_derives.contains(&name)
}

fn visit_expansion(&mut self, mark: Mark, expansion: &Expansion, derives: &[Mark]) {
fn visit_expansion(&mut self, mark: Mark, expansion: &Expansion, derives: &[(Mark, Path)]) {
let invocation = self.invocations[&mark];
self.collect_def_ids(mark, invocation, expansion);

self.current_module = invocation.module.get();
self.current_module.unresolved_invocations.borrow_mut().remove(&mark);
self.current_module.unresolved_invocations.borrow_mut().extend(derives);
for &derive in derives {
self.invocations.insert(derive, invocation);

{
let mut unresolved = self.current_module.unresolved_invocations.borrow_mut();
unresolved.remove(&mark);

unresolved.reserve(derives.len());
self.invocations.reserve(derives.len());

for &(derive, _) in derives {
unresolved.insert(derive);
self.invocations.insert(derive, invocation);
}
}

let mut visitor = BuildReducedGraphVisitor {
resolver: self,
legacy_scope: LegacyScope::Invocation(invocation),
Expand Down
8 changes: 5 additions & 3 deletions src/libsyntax/ext/base.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

pub use self::SyntaxExtension::*;

use ast::{self, Attribute, Name, PatKind, MetaItem};
use ast::{self, Attribute, Name, PatKind, MetaItem, Path};
use attr::HasAttrs;
use codemap::{self, CodeMap, Spanned, respan};
use syntax_pos::{Span, MultiSpan, DUMMY_SP};
Expand Down Expand Up @@ -615,7 +615,7 @@ pub trait Resolver {
fn eliminate_crate_var(&mut self, item: P<ast::Item>) -> P<ast::Item>;
fn is_whitelisted_legacy_custom_derive(&self, name: Name) -> bool;

fn visit_expansion(&mut self, mark: Mark, expansion: &Expansion, derives: &[Mark]);
fn visit_expansion(&mut self, mark: Mark, expansion: &Expansion, derives: &[(Mark, Path)]);
fn add_builtin(&mut self, ident: ast::Ident, ext: Lrc<SyntaxExtension>);

fn resolve_imports(&mut self);
Expand All @@ -642,7 +642,9 @@ impl Resolver for DummyResolver {
fn eliminate_crate_var(&mut self, item: P<ast::Item>) -> P<ast::Item> { item }
fn is_whitelisted_legacy_custom_derive(&self, _name: Name) -> bool { false }

fn visit_expansion(&mut self, _invoc: Mark, _expansion: &Expansion, _derives: &[Mark]) {}
fn visit_expansion(&mut self, _invoc: Mark, _expansion: &Expansion,
_derives: &[(Mark, Path)]) {}

fn add_builtin(&mut self, _ident: ast::Ident, _ext: Lrc<SyntaxExtension>) {}

fn resolve_imports(&mut self) {}
Expand Down
170 changes: 114 additions & 56 deletions src/libsyntax/ext/expand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ macro_rules! expansions {
$(.$visit:ident)* $(lift .$visit_elt:ident)*;)*) => {
#[derive(Copy, Clone, PartialEq, Eq)]
pub enum ExpansionKind { OptExpr, $( $kind, )* }
#[derive(Clone)]
pub enum Expansion { OptExpr(Option<P<ast::Expr>>), $( $kind($ty), )* }

impl ExpansionKind {
Expand Down Expand Up @@ -210,12 +211,36 @@ impl Invocation {

pub struct MacroExpander<'a, 'b:'a> {
pub cx: &'a mut ExtCtxt<'b>,
partial_expansions: HashMap<Mark, PartialExpansion>,
full_expansions: HashMap<Mark, FullExpansion>,
monotonic: bool, // c.f. `cx.monotonic_expander()`
}


// jseyfried (10/04/2017):
// Partial expansion are macro expansions that have unexpanded macro invocations in them.
// That is, the macro itself has resolved and been expanded, but it created more macro invocations
// that have yet to be expanded. This is in contrast to "fully expanded" AST,
// which has no macro invocations left.
struct PartialExpansion {
expansion: Expansion,
derives: Vec<(Mark, Path)>,
expansion_kind: ExpansionKind,
expansion_data: ExpansionData,
unexpanded_children: usize,
}

pub struct FullExpansion {
pub expansion: Expansion,
pub derives: Vec<Mark>,
}

impl<'a, 'b> MacroExpander<'a, 'b> {
pub fn new(cx: &'a mut ExtCtxt<'b>, monotonic: bool) -> Self {
MacroExpander { cx: cx, monotonic: monotonic }
MacroExpander {
cx: cx, monotonic: monotonic,
partial_expansions: HashMap::new(), full_expansions: HashMap::new(),
}
}

pub fn expand_crate(&mut self, mut krate: ast::Crate) -> ast::Crate {
Expand Down Expand Up @@ -266,12 +291,12 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
let orig_expansion_data = self.cx.current_expansion.clone();
self.cx.current_expansion.depth = 0;

let (expansion, mut invocations) = self.collect_invocations(expansion, &[]);
let mark = self.cx.current_expansion.mark;
let mut invocations =
self.collect_invocations(mark, expansion, Vec::new(), ExpansionKind::Items);
self.resolve_imports();
invocations.reverse();

let mut expansions = Vec::new();
let mut derives = HashMap::new();
let mut undetermined_invocations = Vec::new();
let (mut progress, mut force) = (false, !self.monotonic);
loop {
Expand All @@ -285,8 +310,8 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
continue
};

let scope =
if self.monotonic { invoc.expansion_data.mark } else { orig_expansion_data.mark };
let mark = invoc.expansion_data.mark;
let scope = if self.monotonic { mark } else { orig_expansion_data.mark };
let ext = match self.cx.resolver.resolve_invoc(&mut invoc, scope, force) {
Ok(ext) => Some(ext),
Err(Determinacy::Determined) => None,
Expand All @@ -297,16 +322,15 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
};

progress = true;
let ExpansionData { depth, mark, .. } = invoc.expansion_data;
self.cx.current_expansion = invoc.expansion_data.clone();

self.cx.current_expansion.mark = scope;
// FIXME(jseyfried): Refactor out the following logic
let (expansion, new_invocations) = if let Some(ext) = ext {
let new_invocations = if let Some(ext) = ext {
if let Some(ext) = ext {
let expansion_kind = invoc.expansion_kind;
let dummy = invoc.expansion_kind.dummy(invoc.span()).unwrap();
let expansion = self.expand_invoc(invoc, &*ext).unwrap_or(dummy);
self.collect_invocations(expansion, &[])
self.collect_invocations(mark, expansion, Vec::new(), expansion_kind)
} else if let InvocationKind::Attr { attr: None, traits, item } = invoc.kind {
if !item.derive_allowed() {
let attr = attr::find_by_name(item.attrs(), "derive")
Expand All @@ -326,60 +350,33 @@ impl<'a, 'b> MacroExpander<'a, 'b> {

let item = self.fully_configure(item)
.map_attrs(|mut attrs| { attrs.retain(|a| a.path != "derive"); attrs });
let item_with_markers =
add_derived_markers(&mut self.cx, item.span(), &traits, item.clone());
let derives = derives.entry(invoc.expansion_data.mark).or_insert_with(Vec::new);

for path in &traits {
let mark = Mark::fresh(self.cx.current_expansion.mark);
derives.push(mark);
let item = match self.cx.resolver.resolve_macro(
Mark::root(), path, MacroKind::Derive, false) {
Ok(ext) => match *ext {
BuiltinDerive(..) => item_with_markers.clone(),
_ => item.clone(),
},
_ => item.clone(),
};
invocations.push(Invocation {
kind: InvocationKind::Derive { path: path.clone(), item: item },
expansion_kind: invoc.expansion_kind,
expansion_data: ExpansionData {
mark,
..invoc.expansion_data.clone()
},
});
}
let item = add_derived_markers(&mut self.cx, item.span(), &traits, item);
let expansion = invoc.expansion_kind
.expect_from_annotatables(::std::iter::once(item_with_markers));
self.collect_invocations(expansion, derives)
.expect_from_annotatables(::std::iter::once(item));
self.collect_invocations(mark, expansion, traits, invoc.expansion_kind)
} else {
unreachable!()
}
} else {
self.collect_invocations(invoc.expansion_kind.dummy(invoc.span()).unwrap(), &[])
let dummy = invoc.expansion_kind.dummy(invoc.span()).unwrap();
self.collect_invocations(mark, dummy, Vec::new(), invoc.expansion_kind)
};

if expansions.len() < depth {
expansions.push(Vec::new());
}
expansions[depth - 1].push((mark, expansion));
if !self.cx.ecfg.single_step {
invocations.extend(new_invocations.into_iter().rev());
}
}

self.cx.current_expansion = orig_expansion_data;
self.placeholder_expander().remove(NodeId::placeholder_from_mark(mark))
}

let mut placeholder_expander = PlaceholderExpander::new(self.cx, self.monotonic);
while let Some(expansions) = expansions.pop() {
for (mark, expansion) in expansions.into_iter().rev() {
let derives = derives.remove(&mark).unwrap_or_else(Vec::new);
placeholder_expander.add(NodeId::placeholder_from_mark(mark), expansion, derives);
}
fn placeholder_expander<'c>(&'c mut self) -> PlaceholderExpander<'c, 'b> {
PlaceholderExpander {
cx: self.cx,
expansions: &mut self.full_expansions,
monotonic: self.monotonic,
}

expansion.fold_with(&mut placeholder_expander)
}

fn resolve_imports(&mut self) {
Expand All @@ -390,30 +387,91 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
}
}

fn collect_invocations(&mut self, expansion: Expansion, derives: &[Mark])
-> (Expansion, Vec<Invocation>) {
let result = {
fn collect_invocations(&mut self,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abonander
Did you figure out what happens here and above in fn expand?
I can't understand it just by reading the code, but it usually becomes more clear after actively doing some refactoring or modifications.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean by "figure out what happens here". It seems to do what it says on the tin; it accumulates bang/attribute macro invocations and then expand expands them. It has to be done in a loop because macro expansions can produce more invocations.

mut mark: Mark,
expansion: Expansion,
traits: Vec<Path>,
expansion_kind: ExpansionKind)
-> Vec<Invocation> {
let (expansion, mut invocations) = {
let mut collector = InvocationCollector {
cfg: StripUnconfigured {
should_test: self.cx.ecfg.should_test,
sess: self.cx.parse_sess,
features: self.cx.ecfg.features,
},
cx: self.cx,
mark: mark,
invocations: Vec::new(),
monotonic: self.monotonic,
};
(expansion.fold_with(&mut collector), collector.invocations)
};

let mark_parent = mark.parent();
let derives: Vec<_> =
traits.into_iter().map(|path| (Mark::fresh(mark_parent), path)).collect();
if derives.len() > 0 {
self.partial_expansions.get_mut(&mark_parent).unwrap().unexpanded_children +=
derives.len();
}

if self.monotonic {
let err_count = self.cx.parse_sess.span_diagnostic.err_count();
let mark = self.cx.current_expansion.mark;
self.cx.resolver.visit_expansion(mark, &result.0, derives);
self.cx.resolver.visit_expansion(mark, &expansion, &derives);
self.cx.resolve_err_count += self.cx.parse_sess.span_diagnostic.err_count() - err_count;
}

result
self.partial_expansions.insert(mark, PartialExpansion {
expansion: expansion, derives: derives, expansion_kind: expansion_kind,
expansion_data: self.cx.current_expansion.clone(),
unexpanded_children: invocations.len(),
});

if !invocations.is_empty() {
return invocations;
}

loop {
let partial_expansion = self.partial_expansions.remove(&mark).unwrap();
let expansion = partial_expansion.expansion.fold_with(&mut self.placeholder_expander());

let PartialExpansion { expansion_kind, ref expansion_data, .. } = partial_expansion;
let derives = partial_expansion.derives.into_iter().map(|(mark, path)| {
let item = match expansion.clone() {
Expansion::Items(mut items) => Annotatable::Item(items.pop().unwrap()),
Expansion::TraitItems(mut items) =>
Annotatable::TraitItem(P(items.pop().unwrap())),
Expansion::ImplItems(mut items) =>
Annotatable::ImplItem(P(items.pop().unwrap())),
_ => panic!("expected item"),
};
invocations.push(Invocation {
kind: InvocationKind::Derive { path: path, item: item },
expansion_kind: expansion_kind,
expansion_data: ExpansionData { mark: mark, ..expansion_data.clone() },
});
mark
}).collect();

self.full_expansions
.insert(mark, FullExpansion { expansion: expansion, derives: derives });

if mark == Mark::root() {
break
}
mark = mark.parent();
if let Some(partial_expansion) = self.partial_expansions.get_mut(&mark) {
partial_expansion.unexpanded_children -= 1;
if partial_expansion.unexpanded_children == 0 {
continue
}
}
break
}

invocations
}

fn fully_configure(&mut self, item: Annotatable) -> Annotatable {
Expand Down Expand Up @@ -793,14 +851,15 @@ impl<'a> Parser<'a> {

struct InvocationCollector<'a, 'b: 'a> {
cx: &'a mut ExtCtxt<'b>,
mark: Mark,
cfg: StripUnconfigured<'a>,
invocations: Vec<Invocation>,
monotonic: bool,
}

impl<'a, 'b> InvocationCollector<'a, 'b> {
fn collect(&mut self, expansion_kind: ExpansionKind, kind: InvocationKind) -> Expansion {
let mark = Mark::fresh(self.cx.current_expansion.mark);
let mark = Mark::fresh(self.mark);
self.invocations.push(Invocation {
kind,
expansion_kind,
Expand Down Expand Up @@ -1180,7 +1239,6 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {

fn new_id(&mut self, id: ast::NodeId) -> ast::NodeId {
if self.monotonic {
assert_eq!(id, ast::DUMMY_NODE_ID);
self.cx.resolver.next_node_id()
} else {
id
Expand Down
Loading