Skip to content

Commit

Permalink
librustc: Stop desugaring for expressions and translate them directly.
Browse files Browse the repository at this point in the history
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 25, 2014
1 parent 7f2e63e commit caa564b
Show file tree
Hide file tree
Showing 42 changed files with 614 additions and 163 deletions.
5 changes: 4 additions & 1 deletion src/libcore/char.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 6 additions & 2 deletions src/libcore/fmt/float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 6 additions & 2 deletions src/libcore/fmt/num.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions src/libcore/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down
7 changes: 6 additions & 1 deletion src/libcore/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion src/libcore/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 5 additions & 2 deletions src/libcore/str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions src/libcoretest/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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());
Expand Down
6 changes: 6 additions & 0 deletions src/librustc/middle/borrowck/check_loans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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()) {
Expand Down
7 changes: 6 additions & 1 deletion src/librustc/middle/borrowck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down Expand Up @@ -712,6 +713,10 @@ impl<'a> BorrowckCtxt<'a> {
BorrowViolation(euv::ClosureInvocation) => {
"closure invocation"
}

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

match cause {
Expand Down
41 changes: 39 additions & 2 deletions src/librustc/middle/cfg/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand All @@ -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, _) => {
//
Expand Down
4 changes: 4 additions & 0 deletions src/librustc/middle/check_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
18 changes: 18 additions & 0 deletions src/librustc/middle/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]);
}
_ => ()
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/librustc/middle/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 12 additions & 2 deletions src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ pub enum LoanCause {
AutoRef,
RefBinding,
OverloadedOperator,
ClosureInvocation
ClosureInvocation,
ForLoop,
}

#[deriving(PartialEq,Show)]
Expand Down Expand Up @@ -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, []) {
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/middle/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Loading

5 comments on commit caa564b

@bors
Copy link
Contributor

@bors bors commented on caa564b Jul 25, 2014

Choose a reason for hiding this comment

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

saw approval from pnkfelix
at pcwalton@caa564b

@bors
Copy link
Contributor

@bors bors commented on caa564b Jul 25, 2014

Choose a reason for hiding this comment

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

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

@bors
Copy link
Contributor

@bors bors commented on caa564b Jul 25, 2014

Choose a reason for hiding this comment

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

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

@bors
Copy link
Contributor

@bors bors commented on caa564b Jul 25, 2014

Choose a reason for hiding this comment

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

fast-forwarding master to auto = b9035c2

Please sign in to comment.