mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

auto merge of : jbclements/rust/for-loop-hygiene-etc, r=jbclements

Browse Source 
It turns out that bindings introduced by 'for' loops were not treated hygienically. The fix for this is to make the 'for' expansion more like a macro; rather than expanding sub-pieces and then assembling them, we need to rewrite the for and then call expand again on the whole thing.

This PR includes a test and the fix.

It also contains a number of other things:
- unit tests for other forms of hygiene (currently ignored)
- a fix for the isaac.rs macro that (it turned out) was relying on capturing
- other miscellaneous cleanup and comments
This commit is contained in:
bors 2014-06-26 02:21:28 +00:00
commit edb4e599ab
19 changed files with 249 additions and 110 deletions

127
src/librand/isaac.rs

@ -130,37 +130,39 @@ impl IsaacRng {
macro_rules! ind (($x:expr) => {
self.mem[(($x >> 2) as uint & ((RAND_SIZE - 1) as uint))]
});
macro_rules! rngstepp(
($j:expr, $shift:expr) => {{
let base = $j;
let mix = a << $shift as uint;
let x = self.mem[base + mr_offset];
a = (a ^ mix) + self.mem[base + m2_offset];
let y = ind!(x) + a + b;
self.mem[base + mr_offset] = y;
b = ind!(y >> RAND_SIZE_LEN as uint) + x;
self.rsl[base + mr_offset] = b;
}}
);
macro_rules! rngstepn(
($j:expr, $shift:expr) => {{
let base = $j;
let mix = a >> $shift as uint;
let x = self.mem[base + mr_offset];
a = (a ^ mix) + self.mem[base + m2_offset];
let y = ind!(x) + a + b;
self.mem[base + mr_offset] = y;
b = ind!(y >> RAND_SIZE_LEN as uint) + x;
self.rsl[base + mr_offset] = b;
}}
);
let r = [(0, MIDPOINT), (MIDPOINT, 0)];
for &(mr_offset, m2_offset) in r.iter() {
macro_rules! rngstepp(
($j:expr, $shift:expr) => {{
let base = $j;
let mix = a << $shift as uint;
let x = self.mem[base + mr_offset];
a = (a ^ mix) + self.mem[base + m2_offset];
let y = ind!(x) + a + b;
self.mem[base + mr_offset] = y;
b = ind!(y >> RAND_SIZE_LEN as uint) + x;
self.rsl[base + mr_offset] = b;
}}
);
macro_rules! rngstepn(
($j:expr, $shift:expr) => {{
let base = $j;
let mix = a >> $shift as uint;
let x = self.mem[base + mr_offset];
a = (a ^ mix) + self.mem[base + m2_offset];
let y = ind!(x) + a + b;
self.mem[base + mr_offset] = y;
b = ind!(y >> RAND_SIZE_LEN as uint) + x;
self.rsl[base + mr_offset] = b;
}}
);
for i in range_step(0u, MIDPOINT, 4) {
rngstepp!(i + 0, 13);
rngstepn!(i + 1, 6);
@ -349,43 +351,44 @@ impl Isaac64Rng {
*self.mem.unsafe_ref(($x as uint >> 3) & (RAND_SIZE_64 - 1))
}
);
macro_rules! rngstepp(
($j:expr, $shift:expr) => {{
let base = base + $j;
let mix = a ^ (a << $shift as uint);
let mix = if $j == 0 {!mix} else {mix};
unsafe {
let x = *self.mem.unsafe_ref(base + mr_offset);
a = mix + *self.mem.unsafe_ref(base + m2_offset);
let y = ind!(x) + a + b;
self.mem.unsafe_set(base + mr_offset, y);
b = ind!(y >> RAND_SIZE_64_LEN) + x;
self.rsl.unsafe_set(base + mr_offset, b);
}
}}
);
macro_rules! rngstepn(
($j:expr, $shift:expr) => {{
let base = base + $j;
let mix = a ^ (a >> $shift as uint);
let mix = if $j == 0 {!mix} else {mix};
unsafe {
let x = *self.mem.unsafe_ref(base + mr_offset);
a = mix + *self.mem.unsafe_ref(base + m2_offset);
let y = ind!(x) + a + b;
self.mem.unsafe_set(base + mr_offset, y);
b = ind!(y >> RAND_SIZE_64_LEN) + x;
self.rsl.unsafe_set(base + mr_offset, b);
}
}}
);
for &(mr_offset, m2_offset) in MP_VEC.iter() {
for base in range(0, MIDPOINT / 4).map(|i| i * 4) {
macro_rules! rngstepp(
($j:expr, $shift:expr) => {{
let base = base + $j;
let mix = a ^ (a << $shift as uint);
let mix = if $j == 0 {!mix} else {mix};
unsafe {
let x = *self.mem.unsafe_ref(base + mr_offset);
a = mix + *self.mem.unsafe_ref(base + m2_offset);
let y = ind!(x) + a + b;
self.mem.unsafe_set(base + mr_offset, y);
b = ind!(y >> RAND_SIZE_64_LEN) + x;
self.rsl.unsafe_set(base + mr_offset, b);
}
}}
);
macro_rules! rngstepn(
($j:expr, $shift:expr) => {{
let base = base + $j;
let mix = a ^ (a >> $shift as uint);
let mix = if $j == 0 {!mix} else {mix};
unsafe {
let x = *self.mem.unsafe_ref(base + mr_offset);
a = mix + *self.mem.unsafe_ref(base + m2_offset);
let y = ind!(x) + a + b;
self.mem.unsafe_set(base + mr_offset, y);
b = ind!(y >> RAND_SIZE_64_LEN) + x;
self.rsl.unsafe_set(base + mr_offset, b);
}
}}
);
rngstepp!(0, 21);
rngstepn!(1, 5);
rngstepp!(2, 12);

1
src/libsyntax/ast.rs

@ -835,6 +835,7 @@ impl Arg {
}
}
// represents the header (not the body) of a function declaration
#[deriving(Clone, PartialEq, Eq, Encodable, Decodable, Hash)]
pub struct FnDecl {
pub inputs: Vec<Arg>,

138
src/libsyntax/ext/expand.rs

@ -132,8 +132,6 @@ pub fn expand_expr(e: Gc<ast::Expr>, fld: &mut MacroExpander) -> Gc<ast::Expr> {
ast::ExprForLoop(src_pat, src_expr, src_loop_block, opt_ident) => {
// Expand any interior macros etc.
// NB: we don't fold pats yet. Curious.
let src_expr = fld.fold_expr(src_expr).clone();
let (src_loop_block, opt_ident) = expand_loop_block(src_loop_block, opt_ident, fld);
let span = e.span;
@ -143,7 +141,7 @@ pub fn expand_expr(e: Gc<ast::Expr>, fld: &mut MacroExpander) -> Gc<ast::Expr> {
// i => {
// ['<ident>:] loop {
// match i.next() {
// None => break,
// None => break ['<ident>],
// Some(mut value) => {
// let <src_pat> = value;
// <src_loop_block>
@ -156,14 +154,14 @@ pub fn expand_expr(e: Gc<ast::Expr>, fld: &mut MacroExpander) -> Gc<ast::Expr> {
// (The use of the `let` is to give better error messages
// when the pattern is refutable.)
let local_ident = token::gensym_ident("__i"); // FIXME #13573
let local_ident = token::gensym_ident("i");
let next_ident = fld.cx.ident_of("next");
let none_ident = fld.cx.ident_of("None");
let local_path = fld.cx.path_ident(span, local_ident);
let some_path = fld.cx.path_ident(span, fld.cx.ident_of("Some"));
// `None => break ['<ident>];`
// `None => break ['<ident>],`
let none_arm = {
let break_expr = fld.cx.expr(span, ast::ExprBreak(opt_ident));
let none_pat = fld.cx.pat_ident(span, none_ident);
@ -171,7 +169,8 @@ pub fn expand_expr(e: Gc<ast::Expr>, fld: &mut MacroExpander) -> Gc<ast::Expr> {
};
// let <src_pat> = value;
let value_ident = token::gensym_ident("__value");
// use underscore to suppress lint error:
let value_ident = token::gensym_ident("_value");
// this is careful to use src_pat.span so that error
// messages point exact at that.
let local = box(GC) ast::Local {
@ -221,7 +220,9 @@ pub fn expand_expr(e: Gc<ast::Expr>, fld: &mut MacroExpander) -> Gc<ast::Expr> {
let discrim = fld.cx.expr_mut_addr_of(span, src_expr);
let i_pattern = fld.cx.pat_ident(span, local_ident);
let arm = fld.cx.arm(span, vec!(i_pattern), loop_expr);
fld.cx.expr_match(span, discrim, vec!(arm))
// why these clone()'s everywhere? I guess I'll follow the pattern....
let match_expr = fld.cx.expr_match(span, discrim, vec!(arm));
fld.fold_expr(match_expr).clone()
}
ast::ExprLoop(loop_block, opt_ident) => {
@ -658,15 +659,15 @@ fn expand_non_macro_stmt(s: &Stmt, fld: &mut MacroExpander)
}
}
// a visitor that extracts the pat_ident paths
// a visitor that extracts the pat_ident (binding) paths
// from a given thingy and puts them in a mutable
// array (passed in to the traversal)
// array (passed in to the traversal).
#[deriving(Clone)]
pub struct NewNameFinderContext {
struct NameFinderContext {
ident_accumulator: Vec<ast::Ident> ,
}
impl Visitor<()> for NewNameFinderContext {
impl Visitor<()> for NameFinderContext {
fn visit_pat(&mut self, pattern: &ast::Pat, _: ()) {
match *pattern {
// we found a pat_ident!
@ -698,17 +699,13 @@ impl Visitor<()> for NewNameFinderContext {
}
}
fn visit_ty(&mut self, typ: &ast::Ty, _: ()) {
visit::walk_ty(self, typ, ())
}
}
// return a visitor that extracts the pat_ident paths
// from a given thingy and puts them in a mutable
// array (passed in to the traversal)
pub fn new_name_finder(idents: Vec<ast::Ident> ) -> NewNameFinderContext {
NewNameFinderContext {
fn new_name_finder(idents: Vec<ast::Ident> ) -> NameFinderContext {
NameFinderContext {
ident_accumulator: idents,
}
}
@ -1016,7 +1013,7 @@ fn original_span(cx: &ExtCtxt) -> Gc<codemap::ExpnInfo> {
#[cfg(test)]
mod test {
use super::*;
use super::{new_name_finder, expand_crate, contains_macro_escape};
use ast;
use ast::{Attribute_, AttrOuter, MetaWord};
use attr;
@ -1026,7 +1023,7 @@ mod test {
use parse;
use parse::token;
use util::parser_testing::{string_to_parser};
use util::parser_testing::{string_to_pat, strs_to_idents};
use util::parser_testing::{string_to_pat, string_to_crate, strs_to_idents};
use visit;
use visit::Visitor;
@ -1036,11 +1033,11 @@ mod test {
// from a given thingy and puts them in a mutable
// array (passed in to the traversal)
#[deriving(Clone)]
struct NewPathExprFinderContext {
struct PathExprFinderContext {
path_accumulator: Vec<ast::Path> ,
}
impl Visitor<()> for NewPathExprFinderContext {
impl Visitor<()> for PathExprFinderContext {
fn visit_expr(&mut self, expr: &ast::Expr, _: ()) {
match *expr {
@ -1051,18 +1048,13 @@ mod test {
_ => visit::walk_expr(self,expr,())
}
}
fn visit_ty(&mut self, typ: &ast::Ty, _: ()) {
visit::walk_ty(self, typ, ())
}
}
// return a visitor that extracts the paths
// from a given pattern and puts them in a mutable
// from a given thingy and puts them in a mutable
// array (passed in to the traversal)
pub fn new_path_finder(paths: Vec<ast::Path> ) -> NewPathExprFinderContext {
NewPathExprFinderContext {
fn new_path_finder(paths: Vec<ast::Path> ) -> PathExprFinderContext {
PathExprFinderContext {
path_accumulator: paths
}
}
@ -1070,7 +1062,7 @@ mod test {
// these following tests are quite fragile, in that they don't test what
// *kind* of failure occurs.
// make sure that macros can leave scope
// make sure that macros can't escape fns
#[should_fail]
#[test] fn macros_cant_escape_fns_test () {
let src = "fn bogus() {macro_rules! z (() => (3+4))}\
@ -1088,7 +1080,7 @@ mod test {
expand_crate(&sess,cfg,vec!(),vec!(),crate_ast);
}
// make sure that macros can leave scope for modules
// make sure that macros can't escape modules
#[should_fail]
#[test] fn macros_cant_escape_mods_test () {
let src = "mod foo {macro_rules! z (() => (3+4))}\
@ -1105,7 +1097,7 @@ mod test {
expand_crate(&sess,cfg,vec!(),vec!(),crate_ast);
}
// macro_escape modules shouldn't cause macros to leave scope
// macro_escape modules should allow macros to escape
#[test] fn macros_can_escape_flattened_mods_test () {
let src = "#[macro_escape] mod foo {macro_rules! z (() => (3+4))}\
fn inty() -> int { z!() }".to_string();
@ -1114,7 +1106,6 @@ mod test {
"<test>".to_string(),
src,
Vec::new(), &sess);
// should fail:
let cfg = ::syntax::ext::expand::ExpansionConfig {
deriving_hash_type_parameter: false,
crate_id: from_str("test").unwrap(),
@ -1185,10 +1176,16 @@ mod test {
// binding should match the second two varrefs, and the second binding
// should match the first varref.
//
// Put differently; this is a sparse representation of a boolean matrix
// indicating which bindings capture which identifiers.
//
// Note also that this matrix is dependent on the implicit ordering of
// the bindings and the varrefs discovered by the name-finder and the path-finder.
//
// The comparisons are done post-mtwt-resolve, so we're comparing renamed
// names; differences in marks don't matter any more.
//
// oog... I also want tests that check "binding-identifier-=?". That is,
// oog... I also want tests that check "bound-identifier-=?". That is,
// not just "do these have the same name", but "do they have the same
// name *and* the same marks"? Understanding this is really pretty painful.
// in principle, you might want to control this boolean on a per-varref basis,
@ -1217,12 +1214,68 @@ mod test {
("macro_rules! letty(($x:ident) => (let $x = 15;))
macro_rules! user(($x:ident) => ({letty!($x); $x}))
fn main() -> int {user!(z)}",
vec!(vec!(0)), false));
vec!(vec!(0)), false)
);
for (idx,s) in tests.iter().enumerate() {
run_renaming_test(s,idx);
}
}
// no longer a fixme #8062: this test exposes a *potential* bug; our system does
// not behave exactly like MTWT, but a conversation with Matthew Flatt
// suggests that this can only occur in the presence of local-expand, which
// we have no plans to support. ... unless it's needed for item hygiene....
#[ignore]
#[test] fn issue_8062(){
run_renaming_test(
&("fn main() {let hrcoo = 19; macro_rules! getx(()=>(hrcoo)); getx!();}",
vec!(vec!(0)), true), 0)
}
// FIXME #6994:
// the z flows into and out of two macros (g & f) along one path, and one
// (just g) along the other, so the result of the whole thing should
// be "let z_123 = 3; z_123"
#[ignore]
#[test] fn issue_6994(){
run_renaming_test(
&("macro_rules! g (($x:ident) =>
({macro_rules! f(($y:ident)=>({let $y=3;$x}));f!($x)}))
fn a(){g!(z)}",
vec!(vec!(0)),false),
0)
}
// FIXME #9384, match variable hygiene. Should expand into
// fn z() {match 8 {x_1 => {match 9 {x_2 | x_2 => x_2 + x_1}}}}
#[ignore]
#[test] fn issue_9384(){
run_renaming_test(
&("macro_rules! bad_macro (($ex:expr) => ({match 9 {x | x => x + $ex}}))
fn z() {match 8 {x => bad_macro!(_x)}}",
// NB: the third "binding" is the repeat of the second one.
vec!(vec!(1),vec!(0),vec!(0)),
true),
0)
}
// create a really evil test case where a $x appears inside a binding of $x
// but *shouldnt* bind because it was inserted by a different macro....
// can't write this test case until we have macro-generating macros.
// FIXME #9383 : lambda var hygiene
// interesting... can't even write this test, yet, because the name-finder
// only finds pattern vars. Time to upgrade test framework.
/*#[test]
fn issue_9383(){
run_renaming_test(
&("macro_rules! bad_macro (($ex:expr) => ({(|_x| { $ex }) (9) }))
fn takes_x(_x : int) { assert_eq!(bad_macro!(_x),8); }
fn main() { takes_x(8); }",
vec!(vec!()),false),
0)
}*/
// run one of the renaming tests
fn run_renaming_test(t: &RenamingTest, test_idx: uint) {
let invalid_name = token::special_idents::invalid.name;
@ -1358,4 +1411,19 @@ foo_module!()
strs_to_idents(vec!("a","c","b","d")));
}
// test the list of identifier patterns gathered by the visitor. Note that
// 'None' is listed as an identifier pattern because we don't yet know that
// it's the name of a 0-ary variant, and that 'i' appears twice in succession.
#[test]
fn crate_idents(){
let the_crate = string_to_crate("fn main (a : int) -> int {|b| {
match 34 {None => 3, Some(i) | i => j, Foo{k:z,l:y} => \"banana\"}} }".to_string());
let mut idents = new_name_finder(Vec::new());
//visit::walk_crate(&mut idents, &the_crate, ());
idents.visit_mod(&the_crate.module, the_crate.span, ast::CRATE_NODE_ID, ());
assert_eq!(idents.ident_accumulator,
strs_to_idents(vec!("a","b","None","i","i","z","y")));
}
}

26
src/test/bench/core-std.rs

@ -23,23 +23,23 @@ use std::str;
use std::vec;
use std::io::File;
macro_rules! bench (
($argv:expr, $id:ident) => (maybe_run_test($argv.as_slice(),
stringify!($id).to_string(),
$id))
)
fn main() {
let argv = os::args().move_iter().map(|x| x.to_string()).collect::<Vec<String>>();
let _tests = argv.slice(1, argv.len());
bench!(argv, shift_push);
bench!(argv, read_line);
bench!(argv, vec_plus);
bench!(argv, vec_append);
bench!(argv, vec_push_all);
bench!(argv, is_utf8_ascii);
bench!(argv, is_utf8_multibyte);
macro_rules! bench (
($id:ident) =>
(maybe_run_test(argv.as_slice(),
stringify!($id).to_string(),
$id)))
bench!(shift_push);
bench!(read_line);
bench!(vec_plus);
bench!(vec_append);
bench!(vec_push_all);
bench!(is_utf8_ascii);
bench!(is_utf8_multibyte);
}
fn maybe_run_test(argv: &[String], name: String, test: ||) {

2
src/test/bench/msgsend-ring-mutex-arcs.rs

@ -15,6 +15,8 @@
// This also serves as a pipes test, because Arcs are implemented with pipes.
// ignore-pretty FIXME #15189
extern crate time;
use std::sync::{Arc, Future, Mutex};

2
src/test/bench/msgsend-ring-rw-arcs.rs

@ -15,6 +15,8 @@
// This also serves as a pipes test, because Arcs are implemented with pipes.
// ignore-pretty FIXME #15189
extern crate time;
use std::sync::{RWLock, Arc, Future};

2
src/test/bench/shootout-meteor.rs

@ -38,6 +38,8 @@
// ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
// OF THE POSSIBILITY OF SUCH DAMAGE.
// ignore-pretty FIXME #15189
#![feature(phase)]
#[phase(plugin)] extern crate green;

2
src/test/bench/shootout-spectralnorm.rs

@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// ignore-pretty FIXME #15189
#![feature(phase)]
#![allow(non_snake_case_functions)]
#[phase(plugin)] extern crate green;

21
src/test/compile-fail/issue-15167.rs Normal file

@ -0,0 +1,21 @@
// Copyright 2014 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 f should not be able to inject a reference to 'n'.
#![feature(macro_rules)]
macro_rules! f(() => (n))
fn main() -> (){
for n in range(0, 1) {
println!("{}", f!()); //~ ERROR unresolved name `n`
}
}

1
src/test/run-pass/backtrace.rs

@ -8,6 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// ignore-pretty FIXME #15189
// ignore-win32 FIXME #13259
extern crate native;

2
src/test/run-pass/deriving-cmp-generic-enum.rs

@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// ignore-pretty FIXME #15189
#[deriving(PartialEq, Eq, PartialOrd, Ord)]
enum E<T> {
E0,

2
src/test/run-pass/deriving-cmp-generic-struct-enum.rs

@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// ignore-pretty FIXME #15189
#![feature(struct_variant)]
#[deriving(PartialEq, Eq, PartialOrd, Ord)]

2
src/test/run-pass/deriving-cmp-generic-struct.rs

@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// ignore-pretty FIXME #15189
#[deriving(PartialEq, Eq, PartialOrd, Ord)]
struct S<T> {
x: T,

2
src/test/run-pass/deriving-cmp-generic-tuple-struct.rs

@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// ignore-pretty FIXME #15189
#[deriving(PartialEq, Eq, PartialOrd, Ord)]
struct TS<T>(T,T);

21
src/test/run-pass/issue-15189.rs Normal file

@ -0,0 +1,21 @@
// Copyright 2014 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.
// ignore-pretty
#![feature(macro_rules)]
macro_rules! third(($e:expr)=>({let x = 2; *$e.get(x)}))
fn main() {
let x = vec!(10u,11u,12u,13u);
let t = third!(x);
assert_eq!(t,12u);
}

2
src/test/run-pass/linear-for-loop.rs

@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// ignore-pretty FIXME #15189
extern crate debug;
pub fn main() {

2
src/test/run-pass/task-comm-3.rs

@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// ignore-pretty FIXME #15189
extern crate debug;
use std::task;

2
src/test/run-pass/unfold-cross-crate.rs

@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// ignore-pretty FIXME #15189
use std::iter::Unfold;
// Unfold had a bug with 'a that mean it didn't work

2
src/test/run-pass/utf8.rs

@ -8,6 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// ignore-pretty FIXME #15189
pub fn main() {
let yen: char = '¥'; // 0xa5
let c_cedilla: char = 'ç'; // 0xe7