Skip to content
Permalink
Browse files

librustc: Stop desugaring `for` expressions and translate them directly.

This makes edge cases in which the `Iterator` trait was not in scope
and/or `Option` or its variants were not in scope work properly.

This breaks code that looks like:

    struct MyStruct { ... }

    impl MyStruct {
        fn next(&mut self) -> Option<int> { ... }
    }

    for x in MyStruct { ... } { ... }

Change ad-hoc `next` methods like the above to implementations of the
`Iterator` trait. For example:

    impl Iterator<int> for MyStruct {
        fn next(&mut self) -> Option<int> { ... }
    }

Closes #15392.

[breaking-change]
  • Loading branch information...
pcwalton committed Jul 22, 2014
1 parent 7f2e63e commit caa564bea3d5f5a24d0797c4769184c1ea0abaff
Showing with 614 additions and 163 deletions.
  1. +4 −1 src/libcore/char.rs
  2. +6 −2 src/libcore/fmt/float.rs
  3. +6 −2 src/libcore/fmt/num.rs
  4. +1 −0 src/libcore/iter.rs
  5. +6 −1 src/libcore/option.rs
  6. +4 −1 src/libcore/ptr.rs
  7. +5 −2 src/libcore/str.rs
  8. +4 −4 src/libcoretest/iter.rs
  9. +6 −0 src/librustc/middle/borrowck/check_loans.rs
  10. +6 −1 src/librustc/middle/borrowck/mod.rs
  11. +39 −2 src/librustc/middle/cfg/construct.rs
  12. +4 −0 src/librustc/middle/check_loop.rs
  13. +18 −0 src/librustc/middle/check_match.rs
  14. +4 −4 src/librustc/middle/dead.rs
  15. +12 −2 src/librustc/middle/expr_use_visitor.rs
  16. +2 −0 src/librustc/middle/lang_items.rs
  17. +52 −14 src/librustc/middle/liveness.rs
  18. +3 −4 src/librustc/middle/mem_categorization.rs
  19. +2 −2 src/librustc/middle/reachable.rs
  20. +11 −1 src/librustc/middle/region.rs
  21. +36 −1 src/librustc/middle/resolve.rs
  22. +25 −0 src/librustc/middle/trans/_match.rs
  23. +130 −0 src/librustc/middle/trans/controlflow.rs
  24. +18 −3 src/librustc/middle/trans/debuginfo.rs
  25. +7 −0 src/librustc/middle/trans/expr.rs
  26. +6 −4 src/librustc/middle/ty.rs
  27. +5 −1 src/librustc/middle/typeck/check/_match.rs
  28. +89 −2 src/librustc/middle/typeck/check/mod.rs
  29. +16 −0 src/librustc/middle/typeck/check/regionck.rs
  30. +2 −1 src/librustc/middle/typeck/check/vtable.rs
  31. +2 −2 src/librustc/middle/typeck/coherence.rs
  32. +1 −1 src/librustc/util/common.rs
  33. +2 −1 src/librustc_back/svh.rs
  34. +4 −1 src/libstd/io/tempfile.rs
  35. +7 −97 src/libsyntax/ext/expand.rs
  36. +1 −0 src/libunicode/decompose.rs
  37. +4 −3 src/test/bench/shootout-meteor.rs
  38. +31 −0 src/test/compile-fail/for-loop-bogosity.rs
  39. +6 −1 src/test/compile-fail/issue-15167.rs
  40. +2 −2 src/test/compile-fail/vec-mut-iter-borrow.rs
  41. +1 −0 src/test/debuginfo/lexical-scope-in-for-loop.rs
  42. +24 −0 src/test/run-pass/for-loop-goofiness.rs
@@ -17,7 +17,10 @@

use mem::transmute;
use option::{None, Option, Some};
use iter::{Iterator, range_step};
use iter::range_step;

#[cfg(stage0)]
use iter::Iterator; // NOTE(stage0): Remove after snapshot.

// UTF-8 ranges and tags for encoding characters
static TAG_CONT: u8 = 0b1000_0000u8;
@@ -13,15 +13,19 @@
use char;
use collections::Collection;
use fmt;
use iter::{Iterator, range, DoubleEndedIterator};
use iter::{range, DoubleEndedIterator};
use num::{Float, FPNaN, FPInfinite, ToPrimitive, Primitive};
use num::{Zero, One, cast};
use option::{None, Some};
use result::Ok;
use slice::{ImmutableVector, MutableVector};
use slice;
use str::StrSlice;

#[cfg(stage0)]
use iter::Iterator; // NOTE(stage0): Remove after snapshot.
#[cfg(stage0)]
use option::{Some, None}; // NOTE(stage0): Remove after snapshot.

/// A flag that specifies whether to use exponential (scientific) notation.
pub enum ExponentFormat {
/// Do not use exponential notation.
@@ -16,11 +16,15 @@

use collections::Collection;
use fmt;
use iter::{Iterator, DoubleEndedIterator};
use iter::DoubleEndedIterator;
use num::{Int, cast, zero};
use option::{Some, None};
use slice::{ImmutableVector, MutableVector};

#[cfg(stage0)]
use iter::Iterator; // NOTE(stage0): Remove after snapshot.
#[cfg(stage0)]
use option::{Some, None}; // NOTE(stage0): Remove after snapshot.

/// A type that represents a specific radix
trait GenericRadix {
/// The number of digits.
@@ -95,6 +95,7 @@ pub trait Extendable<A>: FromIterator<A> {
/// is returned. A concrete Iterator implementation may choose to behave however
/// it wishes, either by returning `None` infinitely, or by doing something
/// else.
#[lang="iterator"]
pub trait Iterator<A> {
/// Advance the iterator and return the next value. Return `None` when the end is reached.
fn next(&mut self) -> Option<A>;
@@ -147,7 +147,12 @@ use iter::{Iterator, DoubleEndedIterator, FromIterator, ExactSize};
use mem;
use slice;

/// The `Option`
// Note that this is not a lang item per se, but it has a hidden dependency on
// `Iterator`, which is one. The compiler assumes that the `next` method of
// `Iterator` is an enumeration with one type parameter and two variants,
// which basically means it must be `Option`.

/// The `Option` type.
#[deriving(Clone, PartialEq, PartialOrd, Eq, Ord, Show)]
pub enum Option<T> {
/// No value
@@ -90,11 +90,14 @@
use mem;
use clone::Clone;
use intrinsics;
use iter::{range, Iterator};
use iter::range;
use option::{Some, None, Option};

use cmp::{PartialEq, Eq, PartialOrd, Equiv, Ordering, Less, Equal, Greater};

#[cfg(stage0)]
use iter::Iterator; // NOTE(stage0): Remove after snapshot.

pub use intrinsics::copy_memory;
pub use intrinsics::copy_nonoverlapping_memory;
pub use intrinsics::set_memory;
@@ -28,7 +28,7 @@ use iter::{Map, Iterator};
use iter::{DoubleEndedIterator, ExactSize};
use iter::range;
use num::{CheckedMul, Saturating};
use option::{None, Option, Some};
use option::{Option, None, Some};
use raw::Repr;
use slice::ImmutableVector;
use slice;
@@ -1027,9 +1027,12 @@ pub mod traits {
use cmp::{Ord, Ordering, Less, Equal, Greater, PartialEq, PartialOrd, Equiv, Eq};
use collections::Collection;
use iter::Iterator;
use option::{Option, Some, None};
use option::{Option, Some};
use str::{Str, StrSlice, eq_slice};

#[cfg(stage0)]
use option::None; // NOTE(stage0): Remove after snapshot.

impl<'a> Ord for &'a str {
#[inline]
fn cmp(&self, other: & &'a str) -> Ordering {
@@ -137,8 +137,8 @@ fn test_iterator_take_while() {
let ys = [0u, 1, 2, 3, 5, 13];
let mut it = xs.iter().take_while(|&x| *x < 15u);
let mut i = 0;
for &x in it {
assert_eq!(x, ys[i]);
for x in it {
assert_eq!(*x, ys[i]);
i += 1;
}
assert_eq!(i, ys.len());
@@ -150,8 +150,8 @@ fn test_iterator_skip_while() {
let ys = [15, 16, 17, 19];
let mut it = xs.iter().skip_while(|&x| *x < 15u);
let mut i = 0;
for &x in it {
assert_eq!(x, ys[i]);
for x in it {
assert_eq!(*x, ys[i]);
i += 1;
}
assert_eq!(i, ys.len());
@@ -440,6 +440,7 @@ impl<'a> CheckLoanCtxt<'a> {
euv::AddrOf(..) |
euv::AutoRef(..) |
euv::ClosureInvocation(..) |
euv::ForLoop(..) |
euv::RefBinding(..) => {
format!("previous borrow of `{}` occurs here",
self.bccx.loan_path_to_string(&*old_loan.loan_path))
@@ -668,6 +669,11 @@ impl<'a> CheckLoanCtxt<'a> {
return;
}

// Initializations are OK.
if mode == euv::Init {
return
}

// For immutable local variables, assignments are legal
// if they cannot already have been assigned
if self.is_local_variable_or_arg(assignee_cmt.clone()) {
@@ -651,7 +651,8 @@ impl<'a> BorrowckCtxt<'a> {
euv::OverloadedOperator |
euv::AddrOf |
euv::RefBinding |
euv::AutoRef => {
euv::AutoRef |
euv::ForLoop => {
format!("cannot borrow {} as mutable", descr)
}
euv::ClosureInvocation => {
@@ -712,6 +713,10 @@ impl<'a> BorrowckCtxt<'a> {
BorrowViolation(euv::ClosureInvocation) => {
"closure invocation"
}

BorrowViolation(euv::ForLoop) => {
"`for` loop"
}
};

match cause {
@@ -240,7 +240,7 @@ impl<'a> CFGBuilder<'a> {
// v 3
// [expr]
//
// Note that `break` and `loop` statements
// Note that `break` and `continue` statements
// may cause additional edges.

// Is the condition considered part of the loop?
@@ -258,7 +258,44 @@ impl<'a> CFGBuilder<'a> {
expr_exit
}

ast::ExprForLoop(..) => fail!("non-desugared expr_for_loop"),
ast::ExprForLoop(ref pat, ref head, ref body, _) => {
//
// [pred]
// |
// v 1
// [head]
// |
// v 2
// [loopback] <--+ 7
// | |
// v 3 |
// +------[cond] |
// | | |
// | v 5 |
// | [pat] |
// | | |
// | v 6 |
// v 4 [body] -----+
// [expr]
//
// Note that `break` and `continue` statements
// may cause additional edges.

let head = self.expr(head.clone(), pred); // 1
let loopback = self.add_dummy_node([head]); // 2
let cond = self.add_dummy_node([loopback]); // 3
let expr_exit = self.add_node(expr.id, [cond]); // 4
self.loop_scopes.push(LoopScope {
loop_id: expr.id,
continue_index: loopback,
break_index: expr_exit,
});
let pat = self.pat(&**pat, cond); // 5
let body = self.block(&**body, pat); // 6
self.add_contained_edge(body, loopback); // 7
self.loop_scopes.pop();
expr_exit
}

ast::ExprLoop(ref body, _) => {
//
@@ -42,6 +42,10 @@ impl<'a> Visitor<Context> for CheckLoopVisitor<'a> {
ast::ExprLoop(ref b, _) => {
self.visit_block(&**b, Loop);
}
ast::ExprForLoop(_, ref e, ref b, _) => {
self.visit_expr(&**e, cx);
self.visit_block(&**b, Loop);
}
ast::ExprFnBlock(_, ref b) |
ast::ExprProc(_, ref b) |
ast::ExprUnboxedFn(_, ref b) => {
@@ -170,6 +170,24 @@ fn check_expr(cx: &mut MatchCheckCtxt, ex: &Expr) {
.collect();
check_exhaustive(cx, ex.span, &matrix);
},
ExprForLoop(ref pat, _, _, _) => {
let mut static_inliner = StaticInliner {
tcx: cx.tcx
};
match is_refutable(cx, static_inliner.fold_pat(*pat)) {
Some(uncovered_pat) => {
cx.tcx.sess.span_err(
pat.span,
format!("refutable pattern in `for` loop binding: \
`{}` not covered",
pat_to_string(&*uncovered_pat)).as_slice());
},
None => {}
}

// Check legality of move bindings.
check_legality_of_move_bindings(cx, false, [ *pat ]);
}
_ => ()
}
}
@@ -354,11 +354,11 @@ fn create_and_seed_worklist(tcx: &ty::ctxt,
// depending on whether a crate is built as bin or lib, and we want
// the warning to be consistent, we also seed the worklist with
// exported symbols.
for &id in exported_items.iter() {
worklist.push(id);
for id in exported_items.iter() {
worklist.push(*id);
}
for &id in reachable_symbols.iter() {
worklist.push(id);
for id in reachable_symbols.iter() {
worklist.push(*id);
}

// Seed entry point
@@ -79,7 +79,8 @@ pub enum LoanCause {
AutoRef,
RefBinding,
OverloadedOperator,
ClosureInvocation
ClosureInvocation,
ForLoop,
}

#[deriving(PartialEq,Show)]
@@ -395,7 +396,16 @@ impl<'d,'t,TYPER:mc::Typer> ExprUseVisitor<'d,'t,TYPER> {
self.walk_block(&**blk);
}

ast::ExprForLoop(..) => fail!("non-desugared expr_for_loop"),
ast::ExprForLoop(ref pat, ref head, ref blk, _) => {
// The pattern lives as long as the block.
debug!("walk_expr for loop case: blk id={}", blk.id);
self.walk_expr(&**head);

let head_cmt = return_if_err!(self.mc.cat_expr(&**head));
self.walk_pat(head_cmt, pat.clone());

self.walk_block(&**blk);
}

ast::ExprUnary(_, ref lhs) => {
if !self.walk_overloaded_operator(expr, &**lhs, []) {
@@ -299,5 +299,7 @@ lets_do_this! {
NoShareItem, "no_share_bound", no_share_bound;
ManagedItem, "managed_bound", managed_bound;

IteratorItem, "iterator", iterator;

StackExhaustedLangItem, "stack_exhausted", stack_exhausted;
}
Oops, something went wrong.

5 comments on commit caa564b

@bors

This comment has been minimized.

Copy link
Contributor

bors replied Jul 25, 2014

saw approval from pnkfelix
at pcwalton@caa564b

@bors

This comment has been minimized.

Copy link
Contributor

bors replied Jul 25, 2014

merging pcwalton/rust/dedesugar-for = caa564b into auto

@bors

This comment has been minimized.

Copy link
Contributor

bors replied Jul 25, 2014

pcwalton/rust/dedesugar-for = caa564b merged ok, testing candidate = b9035c2

@bors

This comment has been minimized.

Copy link
Contributor

bors replied Jul 25, 2014

fast-forwarding master to auto = b9035c2

Please sign in to comment.
You can’t perform that action at this time.