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.

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

hygiene: Eliminate expansion hierarchy in favor of call-site hierarchy #51457

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
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);
Copy link
Contributor

Choose a reason for hiding this comment

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

I was more worried about this causing breaking changes to macro_rules than breaking the unstable macros 2.0. But I also cannot come up with an example that would break due to this change. Maybe we should do a crater run?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that would be interesting since unstable crates are on crates.io too.

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() {}