Skip to content

Commit

Permalink
auto merge of #13316 : eddyb/rust/ast-ptr, r=brson
Browse files Browse the repository at this point in the history
Replaces Gc<T> in the AST with a custom owned smart pointer, P<T>. Fixes #7929.

## Benefits
* **Identity** (affinity?): sharing AST nodes is bad for the various analysis passes (e.g. one could bypass borrowck with a shared `ExprAddrOf` node taking a mutable borrow), the only reason we haven't hit any serious issues with it is because of inefficient folding passes which will always deduplicate any such shared nodes. Even if we were to switch to an arena, this would still hold, i.e. we wouldn't just use `&'a T` in the AST, but rather an wrapper (`P<'a, T>`?).

* **Immutability**: `P<T>` disallows mutating its inner `T` (unless that contains an `Unsafe` interior, which won't happen in the AST), unlike `~T`.

* **Efficiency**: folding can reuse allocation space for `P<T>` and `Vec<T>`, the latter even when the input and output types differ (as it would be the case with arenas or an AST with type parameters to toggle macro support). Also, various algorithms have been changed from copying `Gc<T>` to using `&T` and iterators.

* **Maintainability**: there is another reason I didn't just replace `Gc<T>` with `~T`: `P<T>` provides a fixed interface (`Deref`, `and_then` and `map`) which can remain fully functional even if the implementation changes (using a special thread-local heap, for example). Moreover, switching to, e.g. `P<'a, T>` (for a contextual arena) is easy and mostly automated.
  • Loading branch information
bors committed Sep 14, 2014
2 parents 931b115 + 8577343 commit 19311b6
Show file tree
Hide file tree
Showing 126 changed files with 4,912 additions and 5,037 deletions.
2 changes: 1 addition & 1 deletion mk/crates.mk
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ DEPS_graphviz := std
DEPS_green := std native:context_switch
DEPS_rustuv := std native:uv native:uv_support
DEPS_native := std
DEPS_syntax := std term serialize log fmt_macros debug
DEPS_syntax := std term serialize log fmt_macros debug arena
DEPS_rustc := syntax flate arena serialize getopts rbml \
time log graphviz debug rustc_llvm rustc_back
DEPS_rustc_llvm := native:rustllvm libc std
Expand Down
7 changes: 3 additions & 4 deletions src/libfourcc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,9 @@ use syntax::ext::base::{ExtCtxt, MacExpr};
use syntax::ext::build::AstBuilder;
use syntax::parse::token;
use syntax::parse::token::InternedString;
use syntax::ptr::P;
use rustc::plugin::Registry;

use std::gc::Gc;

#[plugin_registrar]
pub fn plugin_registrar(reg: &mut Registry) {
reg.register_macro("fourcc", expand_syntax_ext);
Expand Down Expand Up @@ -135,7 +134,7 @@ struct Ident {
}

fn parse_tts(cx: &ExtCtxt,
tts: &[ast::TokenTree]) -> (Gc<ast::Expr>, Option<Ident>) {
tts: &[ast::TokenTree]) -> (P<ast::Expr>, Option<Ident>) {
let p = &mut cx.new_parser_from_tts(tts);
let ex = p.parse_expr();
let id = if p.token == token::EOF {
Expand All @@ -156,7 +155,7 @@ fn parse_tts(cx: &ExtCtxt,
fn target_endian_little(cx: &ExtCtxt, sp: Span) -> bool {
let meta = cx.meta_name_value(sp, InternedString::new("target_endian"),
ast::LitStr(InternedString::new("little"), ast::CookedStr));
contains(cx.cfg().as_slice(), meta)
contains(cx.cfg().as_slice(), &*meta)
}

// FIXME (10872): This is required to prevent an LLVM assert on Windows
Expand Down
7 changes: 3 additions & 4 deletions src/libhexfloat/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,9 @@ use syntax::ext::base;
use syntax::ext::base::{ExtCtxt, MacExpr};
use syntax::ext::build::AstBuilder;
use syntax::parse::token;
use syntax::ptr::P;
use rustc::plugin::Registry;

use std::gc::Gc;

#[plugin_registrar]
pub fn plugin_registrar(reg: &mut Registry) {
reg.register_macro("hexfloat", expand_syntax_ext);
Expand Down Expand Up @@ -122,7 +121,7 @@ pub fn expand_syntax_ext(cx: &mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])

let s = match expr.node {
// expression is a literal
ast::ExprLit(lit) => match lit.node {
ast::ExprLit(ref lit) => match lit.node {
// string literal
ast::LitStr(ref s, _) => {
s.clone()
Expand Down Expand Up @@ -167,7 +166,7 @@ struct Ident {
}

fn parse_tts(cx: &ExtCtxt,
tts: &[ast::TokenTree]) -> (Gc<ast::Expr>, Option<Ident>) {
tts: &[ast::TokenTree]) -> (P<ast::Expr>, Option<Ident>) {
let p = &mut cx.new_parser_from_tts(tts);
let ex = p.parse_expr();
let id = if p.token == token::EOF {
Expand Down
36 changes: 17 additions & 19 deletions src/libregex_macros/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ extern crate syntax;
extern crate rustc;

use std::rc::Rc;
use std::gc::{Gc, GC};

use syntax::ast;
use syntax::codemap;
Expand All @@ -35,6 +34,7 @@ use syntax::ext::base::{ExtCtxt, MacResult, MacExpr, DummyResult};
use syntax::parse::token;
use syntax::print::pprust;
use syntax::fold::Folder;
use syntax::ptr::P;

use rustc::plugin::Registry;

Expand Down Expand Up @@ -111,7 +111,7 @@ struct NfaGen<'a> {
}

impl<'a> NfaGen<'a> {
fn code(&mut self) -> Gc<ast::Expr> {
fn code(&mut self) -> P<ast::Expr> {
// Most or all of the following things are used in the quasiquoted
// expression returned.
let num_cap_locs = 2 * self.prog.num_captures();
Expand Down Expand Up @@ -332,7 +332,7 @@ fn exec<'t>(which: ::regex::native::MatchKind, input: &'t str,

// Generates code for the `add` method, which is responsible for adding
// zero-width states to the next queue of states to visit.
fn add_insts(&self) -> Gc<ast::Expr> {
fn add_insts(&self) -> P<ast::Expr> {
let arms = self.prog.insts.iter().enumerate().map(|(pc, inst)| {
let nextpc = pc + 1;
let body = match *inst {
Expand Down Expand Up @@ -433,7 +433,7 @@ fn exec<'t>(which: ::regex::native::MatchKind, input: &'t str,

// Generates the code for the `step` method, which processes all states
// in the current queue that consume a single character.
fn step_insts(&self) -> Gc<ast::Expr> {
fn step_insts(&self) -> P<ast::Expr> {
let arms = self.prog.insts.iter().enumerate().map(|(pc, inst)| {
let nextpc = pc + 1;
let body = match *inst {
Expand Down Expand Up @@ -524,17 +524,15 @@ fn exec<'t>(which: ::regex::native::MatchKind, input: &'t str,
// Translates a character class into a match expression.
// This avoids a binary search (and is hopefully replaced by a jump
// table).
fn match_class(&self, casei: bool, ranges: &[(char, char)]) -> Gc<ast::Expr> {
let expr_true = quote_expr!(self.cx, true);

fn match_class(&self, casei: bool, ranges: &[(char, char)]) -> P<ast::Expr> {
let mut arms = ranges.iter().map(|&(mut start, mut end)| {
if casei {
start = start.to_uppercase();
end = end.to_uppercase();
}
let pat = self.cx.pat(self.sp, ast::PatRange(quote_expr!(self.cx, $start),
quote_expr!(self.cx, $end)));
self.cx.arm(self.sp, vec!(pat), expr_true)
self.cx.arm(self.sp, vec!(pat), quote_expr!(self.cx, true))
}).collect::<Vec<ast::Arm>>();

arms.push(self.wild_arm_expr(quote_expr!(self.cx, false)));
Expand All @@ -546,7 +544,7 @@ fn exec<'t>(which: ::regex::native::MatchKind, input: &'t str,
// Generates code for checking a literal prefix of the search string.
// The code is only generated if the regex *has* a literal prefix.
// Otherwise, a no-op is returned.
fn check_prefix(&self) -> Gc<ast::Expr> {
fn check_prefix(&self) -> P<ast::Expr> {
if self.prog.prefix.len() == 0 {
self.empty_block()
} else {
Expand All @@ -570,32 +568,32 @@ fn exec<'t>(which: ::regex::native::MatchKind, input: &'t str,
// A wild-card arm is automatically added that executes a no-op. It will
// never be used, but is added to satisfy the compiler complaining about
// non-exhaustive patterns.
fn match_insts(&self, mut arms: Vec<ast::Arm>) -> Gc<ast::Expr> {
fn match_insts(&self, mut arms: Vec<ast::Arm>) -> P<ast::Expr> {
arms.push(self.wild_arm_expr(self.empty_block()));
self.cx.expr_match(self.sp, quote_expr!(self.cx, pc), arms)
}

fn empty_block(&self) -> Gc<ast::Expr> {
fn empty_block(&self) -> P<ast::Expr> {
quote_expr!(self.cx, {})
}

// Creates a match arm for the instruction at `pc` with the expression
// `body`.
fn arm_inst(&self, pc: uint, body: Gc<ast::Expr>) -> ast::Arm {
fn arm_inst(&self, pc: uint, body: P<ast::Expr>) -> ast::Arm {
let pc_pat = self.cx.pat_lit(self.sp, quote_expr!(self.cx, $pc));

self.cx.arm(self.sp, vec!(pc_pat), body)
}

// Creates a wild-card match arm with the expression `body`.
fn wild_arm_expr(&self, body: Gc<ast::Expr>) -> ast::Arm {
fn wild_arm_expr(&self, body: P<ast::Expr>) -> ast::Arm {
ast::Arm {
attrs: vec!(),
pats: vec!(box(GC) ast::Pat{
pats: vec!(P(ast::Pat{
id: ast::DUMMY_NODE_ID,
span: self.sp,
node: ast::PatWild(ast::PatWildSingle),
}),
})),
guard: None,
body: body,
}
Expand All @@ -605,8 +603,8 @@ fn exec<'t>(which: ::regex::native::MatchKind, input: &'t str,
// Converts `xs` to a `[x1, x2, .., xN]` expression by calling `to_expr`
// on each element in `xs`.
fn vec_expr<T, It: Iterator<T>>(&self, xs: It,
to_expr: |&ExtCtxt, T| -> Gc<ast::Expr>)
-> Gc<ast::Expr> {
to_expr: |&ExtCtxt, T| -> P<ast::Expr>)
-> P<ast::Expr> {
let exprs = xs.map(|x| to_expr(self.cx, x)).collect();
self.cx.expr_vec(self.sp, exprs)
}
Expand All @@ -618,13 +616,13 @@ fn parse(cx: &mut ExtCtxt, tts: &[ast::TokenTree]) -> Option<String> {
let mut parser = cx.new_parser_from_tts(tts);
let entry = cx.expander().fold_expr(parser.parse_expr());
let regex = match entry.node {
ast::ExprLit(lit) => {
ast::ExprLit(ref lit) => {
match lit.node {
ast::LitStr(ref s, _) => s.to_string(),
_ => {
cx.span_err(entry.span, format!(
"expected string literal but got `{}`",
pprust::lit_to_string(&*lit)).as_slice());
pprust::lit_to_string(&**lit)).as_slice());
return None
}
}
Expand Down
Loading

0 comments on commit 19311b6

Please sign in to comment.