Skip to content

Commit

Permalink
Auto merge of #51457 - petrochenkov:hygnoparent, r=<try>
Browse files Browse the repository at this point in the history
hygiene: Eliminate expansion hierarchy in favor of call-site hierarchy

"Expansion hierarchy" and "call-site hierarchy" are two slightly different ways to nest macro invocations and answer the question "who is the parent" for a given invocation.

For example, here both hierarchies coincide
```rust
macro inner() { ... }
macro outer() { inner!() }

outer!();
// expansions: root -> outer -> inner
// call-sites: root -> outer -> inner
```
but here they are different
```rust
macro inner() { ... }
macro outer($stuff: stuff) { $stuff }

// expansions: root -> outer -> inner (`inner` is expanded as a part of `outer`'s output)
// call-sites: root -> outer; root -> inner
outer!(inner!())
```

All our talk about hygiene was in terms of "def-site" and "call-site" name resolution so far and the "expansion hierarchy" is an internal detail, but it was actually used in few places in the way affecting resolution results. For example, in the attached test case the structure `S` itself was previously resolved succesfully, but resolution of its field `field` failed due to use of "expansion hierarchy".
This PR eliminates expansion hierarchy from hygiene algorithm and uses call-site hierarchy instead.

This is also a nice simplification of the model.
Instead of growing in *three* dimensions in similar but subtly different ways (def-site, call-site, expansion) hygiene hierarchies now grow in *two* dimensions in similar but subtly different ways (def-site, call-site).
  • Loading branch information
bors committed Jun 11, 2018
2 parents 0d76317 + ef001ae commit 50b5f1d
Show file tree
Hide file tree
Showing 11 changed files with 77 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/libproc_macro/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ impl FromStr for TokenStream {
let expn_info = mark.expn_info().unwrap();
let call_site = expn_info.call_site;
// notify the expansion info that it is unhygienic
let mark = Mark::fresh(mark);
let mark = Mark::fresh();
mark.set_expn_info(expn_info);
let span = call_site.with_ctxt(SyntaxContext::empty().apply_mark(mark));
let stream = parse::parse_stream_from_source_str(name, src, sess, Some(span));
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ impl<'a> LoweringContext<'a> {
}

fn allow_internal_unstable(&self, reason: CompilerDesugaringKind, span: Span) -> Span {
let mark = Mark::fresh(Mark::root());
let mark = Mark::fresh();
mark.set_expn_info(codemap::ExpnInfo {
call_site: span,
callee: codemap::NameAndSpan {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_allocator/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl<'a> Folder for ExpandAllocatorDirectives<'a> {
}
self.found = true;

let mark = Mark::fresh(Mark::root());
let mark = Mark::fresh();
mark.set_expn_info(ExpnInfo {
call_site: DUMMY_SP,
callee: NameAndSpan {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ impl<'a> base::Resolver for Resolver<'a> {
}

fn get_module_scope(&mut self, id: ast::NodeId) -> Mark {
let mark = Mark::fresh(Mark::root());
let mark = Mark::fresh();
let module = self.module_map[&self.definitions.local_def_id(id)];
self.invocations.insert(mark, self.arenas.alloc_invocation_data(InvocationData {
module: Cell::new(module),
Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
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);
let mark = Mark::fresh();
derives.push(mark);
let item = match self.cx.resolver.resolve_macro(
Mark::root(), path, MacroKind::Derive, false) {
Expand Down Expand Up @@ -999,7 +999,7 @@ struct InvocationCollector<'a, 'b: 'a> {

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.invocations.push(Invocation {
kind,
expansion_kind,
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/std_inject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use tokenstream::TokenStream;
/// call to codemap's `is_internal` check.
/// The expanded code uses the unstable `#[prelude_import]` attribute.
fn ignored_span(sp: Span) -> Span {
let mark = Mark::fresh(Mark::root());
let mark = Mark::fresh();
mark.set_expn_info(ExpnInfo {
call_site: DUMMY_SP,
callee: NameAndSpan {
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ fn generate_test_harness(sess: &ParseSess,
let mut cleaner = EntryPointCleaner { depth: 0 };
let krate = cleaner.fold_crate(krate);

let mark = Mark::fresh(Mark::root());
let mark = Mark::fresh();

let mut econfig = ExpansionConfig::default("test".to_string());
econfig.features = Some(features);
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax_ext/deriving/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ fn call_intrinsic(cx: &ExtCtxt,
} else { // Avoid instability errors with user defined curstom derives, cc #36316
let mut info = cx.current_expansion.mark.expn_info().unwrap();
info.callee.allow_internal_unstable = true;
let mark = Mark::fresh(Mark::root());
let mark = Mark::fresh();
mark.set_expn_info(info);
span = span.with_ctxt(SyntaxContext::empty().apply_mark(mark));
}
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax_ext/proc_macro_registrar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ fn mk_registrar(cx: &mut ExtCtxt,
custom_derives: &[ProcMacroDerive],
custom_attrs: &[ProcMacroDef],
custom_macros: &[ProcMacroDef]) -> P<ast::Item> {
let mark = Mark::fresh(Mark::root());
let mark = Mark::fresh();
mark.set_expn_info(ExpnInfo {
call_site: DUMMY_SP,
callee: NameAndSpan {
Expand Down
26 changes: 15 additions & 11 deletions src/libsyntax_pos/hygiene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ pub struct SyntaxContextData {
pub struct Mark(u32);

struct MarkData {
parent: Mark,
kind: MarkKind,
expn_info: Option<ExpnInfo>,
}
Expand All @@ -54,9 +53,9 @@ pub enum MarkKind {
}

impl Mark {
pub fn fresh(parent: Mark) -> Self {
pub fn fresh() -> Self {
HygieneData::with(|data| {
data.marks.push(MarkData { parent: parent, kind: MarkKind::Legacy, expn_info: None });
data.marks.push(MarkData { kind: MarkKind::Legacy, expn_info: None });
Mark(data.marks.len() as u32 - 1)
})
}
Expand Down Expand Up @@ -93,7 +92,7 @@ impl Mark {
if self == Mark::root() || data.marks[self.0 as usize].kind == MarkKind::Modern {
return self;
}
self = data.marks[self.0 as usize].parent;
self = self.call_site_mark(data);
}
})
}
Expand All @@ -114,7 +113,7 @@ impl Mark {
if self == Mark::root() {
return false;
}
self = data.marks[self.0 as usize].parent;
self = self.call_site_mark(data);
}
true
})
Expand All @@ -134,17 +133,24 @@ impl Mark {
let mut a_path = FxHashSet::<Mark>();
while a != Mark::root() {
a_path.insert(a);
a = data.marks[a.0 as usize].parent;
a = a.call_site_mark(data);
}

// While the path from b to the root hasn't intersected, move up the tree
while !a_path.contains(&b) {
b = data.marks[b.0 as usize].parent;
b = b.call_site_mark(data);
}

b
})
}

/// Private helpers not acquiring a lock around global data
fn call_site_mark(self, data: &HygieneData) -> Mark {
data.marks[self.0 as usize].expn_info.as_ref()
.map(|einfo| data.syntax_contexts[einfo.call_site.ctxt().0 as usize].outer_mark)
.unwrap_or(Mark::root())
}
}

pub struct HygieneData {
Expand All @@ -159,7 +165,6 @@ impl HygieneData {
pub fn new() -> Self {
HygieneData {
marks: vec![MarkData {
parent: Mark::root(),
kind: MarkKind::Builtin,
expn_info: None,
}],
Expand Down Expand Up @@ -198,14 +203,13 @@ impl SyntaxContext {

// Allocate a new SyntaxContext with the given ExpnInfo. This is used when
// deserializing Spans from the incr. comp. cache.
// FIXME(mw): This method does not restore MarkData::parent or
// SyntaxContextData::prev_ctxt or SyntaxContextData::modern. These things
// FIXME(mw): This method does not restore SyntaxContextData::prev_ctxt or
// SyntaxContextData::modern. These things
// don't seem to be used after HIR lowering, so everything should be fine
// as long as incremental compilation does not kick in before that.
pub fn allocate_directly(expansion_info: ExpnInfo) -> Self {
HygieneData::with(|data| {
data.marks.push(MarkData {
parent: Mark::root(),
kind: MarkKind::Legacy,
expn_info: Some(expansion_info)
});
Expand Down
52 changes: 52 additions & 0 deletions src/test/ui/hygiene/call-site-parent-vs-expansion-parent.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// compile-pass

#![feature(decl_macro)]

macro_rules! define_field {
() => {
struct S { field: u8 }
};
($field: ident) => {
struct Z { $field: u8 }
};
}

mod modern {
macro use_field($define_field1: item, $define_field2: item) {
$define_field1
$define_field2

// OK, both struct name `S` and field `name` resolve to definitions
// produced by `define_field` and living in the "root" context
// that is in scope at `use_field`'s def-site.
fn f() { S { field: 0 }; }
fn g() { Z { field: 0 }; }
}

use_field!(define_field!{}, define_field!{ field });
}

mod legacy {
macro_rules! use_field {($define_field1: item, $define_field2: item) => {
$define_field1
$define_field2

// OK, everything is at the same call-site.
fn f() { S { field: 0 }; }
fn g() { Z { field: 0 }; }
}}

use_field!(define_field!{}, define_field!{ field });
}

fn main() {}

0 comments on commit 50b5f1d

Please sign in to comment.