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

Improves handling of statement macros. #24155

Merged
merged 5 commits into from
Apr 11, 2015
Merged
Show file tree
Hide file tree
Changes from 3 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
30 changes: 16 additions & 14 deletions src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,11 @@ impl<F> IdentMacroExpander for F
}

// Use a macro because forwarding to a simple function has type system issues
macro_rules! make_stmt_default {
macro_rules! make_stmts_default {
($me:expr) => {
$me.make_expr().map(|e| {
P(codemap::respan(e.span, ast::StmtExpr(e, ast::DUMMY_NODE_ID)))
SmallVector::one(P(codemap::respan(
e.span, ast::StmtExpr(e, ast::DUMMY_NODE_ID))))
})
}
}
Expand All @@ -238,12 +239,12 @@ pub trait MacResult {
None
}

/// Create a statement.
/// Create zero or more statements.
///
/// By default this attempts to create an expression statement,
/// returning None if that fails.
fn make_stmt(self: Box<Self>) -> Option<P<ast::Stmt>> {
make_stmt_default!(self)
fn make_stmts(self: Box<Self>) -> Option<SmallVector<P<ast::Stmt>>> {
make_stmts_default!(self)
}
}

Expand Down Expand Up @@ -276,7 +277,7 @@ make_MacEager! {
pat: P<ast::Pat>,
items: SmallVector<P<ast::Item>>,
impl_items: SmallVector<P<ast::ImplItem>>,
stmt: P<ast::Stmt>,
stmts: SmallVector<P<ast::Stmt>>,
}

impl MacResult for MacEager {
Expand All @@ -292,10 +293,10 @@ impl MacResult for MacEager {
self.impl_items
}

fn make_stmt(self: Box<Self>) -> Option<P<ast::Stmt>> {
match self.stmt {
None => make_stmt_default!(self),
s => s,
fn make_stmts(self: Box<Self>) -> Option<SmallVector<P<ast::Stmt>>> {
match self.stmts.as_ref().map_or(0, |s| s.len()) {
0 => make_stmts_default!(self),
_ => self.stmts,
}
}

Expand Down Expand Up @@ -384,10 +385,11 @@ impl MacResult for DummyResult {
Some(SmallVector::zero())
}
}
fn make_stmt(self: Box<DummyResult>) -> Option<P<ast::Stmt>> {
Some(P(codemap::respan(self.span,
ast::StmtExpr(DummyResult::raw_expr(self.span),
ast::DUMMY_NODE_ID))))
fn make_stmts(self: Box<DummyResult>) -> Option<SmallVector<P<ast::Stmt>>> {
Some(SmallVector::one(P(
codemap::respan(self.span,
ast::StmtExpr(DummyResult::raw_expr(self.span),
ast::DUMMY_NODE_ID)))))
}
}

Expand Down
64 changes: 39 additions & 25 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -745,37 +745,51 @@ pub fn expand_item_mac(it: P<ast::Item>,
}

/// Expand a stmt
fn expand_stmt(s: Stmt, fld: &mut MacroExpander) -> SmallVector<P<Stmt>> {
let (mac, style) = match s.node {
fn expand_stmt(stmt: P<Stmt>, fld: &mut MacroExpander) -> SmallVector<P<Stmt>> {
let stmt = stmt.and_then(|stmt| stmt);
let (mac, style) = match stmt.node {
StmtMac(mac, style) => (mac, style),
_ => return expand_non_macro_stmt(s, fld)
_ => return expand_non_macro_stmt(stmt, fld)
};
let expanded_stmt = match expand_mac_invoc(mac.and_then(|m| m), s.span,
|r| r.make_stmt(),
mark_stmt, fld) {
Some(stmt) => stmt,
None => {
return SmallVector::zero();

let maybe_new_items =
expand_mac_invoc(mac.and_then(|m| m), stmt.span,
|r| r.make_stmts(),
|stmts, mark| stmts.move_map(|m| mark_stmt(m, mark)),
fld);

let mut fully_expanded = match maybe_new_items {
Some(stmts) => {
// Keep going, outside-in.
stmts.into_iter().flat_map(|s| {
fld.fold_stmt(s).into_iter()
}).collect()
}
None => SmallVector::zero()
};

// Keep going, outside-in.
let fully_expanded = fld.fold_stmt(expanded_stmt);
fld.cx.bt_pop();

// If this is a macro invocation with a semicolon, then apply that
// semicolon to the final statement produced by expansion.
if style == MacStmtWithSemicolon {
fully_expanded.into_iter().map(|s| s.map(|Spanned {node, span}| {
Spanned {
node: match node {
StmtExpr(e, stmt_id) => StmtSemi(e, stmt_id),
_ => node /* might already have a semi */
},
span: span
match fully_expanded.pop() {
Copy link
Member

Choose a reason for hiding this comment

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

I think this can be an if let Some(stmt) = fully_expanded.pop() { .. } instead of a match.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes, that would be cleaner.

Some(stmt) => {
let new_stmt = stmt.map(|Spanned {node, span}| {
Spanned {
node: match node {
StmtExpr(e, stmt_id) => StmtSemi(e, stmt_id),
_ => node /* might already have a semi */
},
span: span
}
});
fully_expanded.push(new_stmt);
}
})).collect()
} else {
fully_expanded
None => (),
}
}

fully_expanded
}

// expand a non-macro stmt. this is essentially the fallthrough for
Expand Down Expand Up @@ -1389,7 +1403,7 @@ impl<'a, 'b> Folder for MacroExpander<'a, 'b> {
}

fn fold_stmt(&mut self, stmt: P<ast::Stmt>) -> SmallVector<P<ast::Stmt>> {
stmt.and_then(|stmt| expand_stmt(stmt, self))
expand_stmt(stmt, self)
}

fn fold_block(&mut self, block: P<Block>) -> P<Block> {
Expand Down Expand Up @@ -1541,8 +1555,8 @@ fn mark_pat(pat: P<ast::Pat>, m: Mrk) -> P<ast::Pat> {
}

// apply a given mark to the given stmt. Used following the expansion of a macro.
fn mark_stmt(expr: P<ast::Stmt>, m: Mrk) -> P<ast::Stmt> {
Marker{mark:m}.fold_stmt(expr)
fn mark_stmt(stmt: P<ast::Stmt>, m: Mrk) -> P<ast::Stmt> {
Marker{mark:m}.fold_stmt(stmt)
.expect_one("marking a stmt didn't return exactly one stmt")
}

Expand Down
22 changes: 18 additions & 4 deletions src/libsyntax/ext/tt/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,24 @@ impl<'a> MacResult for ParserAnyMacro<'a> {
Some(ret)
}

fn make_stmt(self: Box<ParserAnyMacro<'a>>) -> Option<P<ast::Stmt>> {
let ret = self.parser.borrow_mut().parse_stmt();
self.ensure_complete_parse(true);
ret
fn make_stmts(self: Box<ParserAnyMacro<'a>>)
-> Option<SmallVector<P<ast::Stmt>>> {
let mut ret = SmallVector::zero();
loop {
let mut parser = self.parser.borrow_mut();
match parser.token {
token::Eof => break,
_ => match parser.parse_stmt_nopanic() {
Ok(maybe_stmt) => match maybe_stmt {
Some(stmt) => ret.push(stmt),
None => (),
},
Err(_) => break,
}
}
}
self.ensure_complete_parse(false);
Some(ret)
}
}

Expand Down
36 changes: 36 additions & 0 deletions src/libsyntax/util/small_vector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,42 @@ impl<T> SmallVector<T> {
}
}

pub fn pop(&mut self) -> Option<T> {
match self.repr {
Zero => None,
One(..) => {
let one = mem::replace(&mut self.repr, Zero);
match one {
One(v1) => Some(v1),
_ => unreachable!()
}
}
Many(..) => {
Copy link
Member

Choose a reason for hiding this comment

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

You can simplify this a bit more since we don't actually need to swap anyore:

Many(ref mut vs) => vs.pop()

Edit: though that may treat the repr as borrowed in the other branches...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, right you are. Updated.

let mut many = mem::replace(&mut self.repr, Zero);
let item =
match many {
Copy link
Member

Choose a reason for hiding this comment

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

We don't necessarily need to worry too much about making sure to convert a length 1 vec to a One or 0 to a Zero. SmallVector is trying to avoid the initial allocation, but if that's already been made, keeping the vec around isn't that big of a deal.

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 wasn't sure whether it was a concern about allocation time or space (I guessed space, from the name). I also went for the safe thing, since I wasn't sure if the rest of the impl was prepared for zero- or one-length vectors, rather than Zero and One enum values.

It would definitely simplify things, if that is an acceptable way to do it.

Many(ref mut vs) if vs.len() == 1 => {
// self.repr is already Zero
vs.pop()
},
Many(ref mut vs) if vs.len() == 2 => {
let item = vs.pop();
mem::replace(&mut self.repr, One(vs.pop().unwrap()));
item
},
Many(ref mut vs) if vs.len() > 2 => {
let item = vs.pop();
let rest = mem::replace(vs, vec!());
mem::replace(&mut self.repr, Many(rest));
item
},
_ => unreachable!()
};
item
}
}
}

pub fn push(&mut self, v: T) {
match self.repr {
Zero => self.repr = One(v),
Expand Down
3 changes: 2 additions & 1 deletion src/test/compile-fail/macro-incomplete-parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ macro_rules! ignored_item {
}

macro_rules! ignored_expr {
() => ( 1, 2 ) //~ ERROR macro expansion ignores token `,`
() => ( 1, //~ ERROR unexpected token: `,`
2 ) //~ ERROR macro expansion ignores token `2`
}

macro_rules! ignored_pat {
Expand Down
32 changes: 32 additions & 0 deletions src/test/run-pass/macro-nested_stmt_macros.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright 2015 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.

macro_rules! foo {
() => {
struct Bar;
struct Baz;
}
}

macro_rules! grault {
() => {
foo!();
struct Xyzzy;
}
}

fn static_assert_exists<T>() { }

fn main() {
grault!();
static_assert_exists::<Bar>();
static_assert_exists::<Baz>();
static_assert_exists::<Xyzzy>();
}
29 changes: 29 additions & 0 deletions src/test/run-pass/macro-stmt_macro_in_expr_macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2015 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.

macro_rules! foo {
() => {
struct Bar;
struct Baz;
}
}

macro_rules! grault {
() => {{
foo!();
struct Xyzzy;
0
}}
}

fn main() {
let x = grault!();
assert_eq!(x, 0);
}