Auto merge of #24422 - pnkfelix:typeck-highlevel-before-bodies, r=nikomatsakis

typeck: Do high-level structural/signature checks before function body checks.

This avoids various ICEs, e.g. premature calls to cat_expr that yield the dreaded "cat_expr Errd" ICE.

However, it also means that some early error feedback is now not provided.  This may be for the best, because the error feedback were were providing in some of those cases were false positives -- it was spurious feedback and a distraction from the real problem.

So it is not 100% clear whether we actually want to put this change in or not.  I think its a net win, but others might disagree.

(Kudos to @arielb1 for suggesting this modification.)

Author: bors

src/librustc_typeck/check/mod.rs

@@ -441,10 +441,11 @@ fn static_inherited_fields<'a, 'tcx>(ccx: &'a CrateCtxt<'a, 'tcx>)
 }
 
 struct CheckItemTypesVisitor<'a, 'tcx: 'a> { ccx: &'a CrateCtxt<'a, 'tcx> }
+struct CheckItemBodiesVisitor<'a, 'tcx: 'a> { ccx: &'a CrateCtxt<'a, 'tcx> }
 
 impl<'a, 'tcx> Visitor<'tcx> for CheckItemTypesVisitor<'a, 'tcx> {
     fn visit_item(&mut self, i: &'tcx ast::Item) {
-        check_item(self.ccx, i);
+        check_item_type(self.ccx, i);
         visit::walk_item(self, i);
     }
 
@@ -460,6 +461,13 @@ impl<'a, 'tcx> Visitor<'tcx> for CheckItemTypesVisitor<'a, 'tcx> {
     }
 }
 
+impl<'a, 'tcx> Visitor<'tcx> for CheckItemBodiesVisitor<'a, 'tcx> {
+    fn visit_item(&mut self, i: &'tcx ast::Item) {
+        check_item_body(self.ccx, i);
+        visit::walk_item(self, i);
+    }
+}
+
 pub fn check_item_types(ccx: &CrateCtxt) {
     let krate = ccx.tcx.map.krate();
     let mut visit = wf::CheckTypeWellFormedVisitor::new(ccx);
@@ -474,6 +482,11 @@ pub fn check_item_types(ccx: &CrateCtxt) {
 
     ccx.tcx.sess.abort_if_errors();
 
+    let mut visit = CheckItemBodiesVisitor { ccx: ccx };
+    visit::walk_crate(&mut visit, krate);
+
+    ccx.tcx.sess.abort_if_errors();
+
     for drop_method_did in ccx.tcx.destructors.borrow().iter() {
         if drop_method_did.krate == ast::LOCAL_CRATE {
             let drop_impl_did = ccx.tcx.map.get_parent_did(drop_method_did.node);
@@ -713,13 +726,13 @@ pub fn check_struct(ccx: &CrateCtxt, id: ast::NodeId, span: Span) {
     }
 }
 
-pub fn check_item<'a,'tcx>(ccx: &CrateCtxt<'a,'tcx>, it: &'tcx ast::Item) {
-    debug!("check_item(it.id={}, it.ident={})",
+pub fn check_item_type<'a,'tcx>(ccx: &CrateCtxt<'a,'tcx>, it: &'tcx ast::Item) {
+    debug!("check_item_type(it.id={}, it.ident={})",
            it.id,
            ty::item_path_str(ccx.tcx, local_def(it.id)));
     let _indenter = indenter();
-
     match it.node {
+      // Consts can play a role in type-checking, so they are included here.
       ast::ItemStatic(_, _, ref e) |
       ast::ItemConst(_, ref e) => check_const(ccx, it.span, &**e, it.id),
       ast::ItemEnum(ref enum_definition, _) => {
@@ -728,16 +741,9 @@ pub fn check_item<'a,'tcx>(ccx: &CrateCtxt<'a,'tcx>, it: &'tcx ast::Item) {
                             &enum_definition.variants,
                             it.id);
       }
-      ast::ItemFn(ref decl, _, _, _, ref body) => {
-        let fn_pty = ty::lookup_item_type(ccx.tcx, ast_util::local_def(it.id));
-        let param_env = ParameterEnvironment::for_item(ccx.tcx, it.id);
-        check_bare_fn(ccx, &**decl, &**body, it.id, it.span, fn_pty.ty, param_env);
-      }
+      ast::ItemFn(_, _, _, _, _) => {} // entirely within check_item_body
       ast::ItemImpl(_, _, _, _, _, ref impl_items) => {
-        debug!("ItemImpl {} with id {}", token::get_ident(it.ident), it.id);
-
-        let impl_pty = ty::lookup_item_type(ccx.tcx, ast_util::local_def(it.id));
-
+          debug!("ItemImpl {} with id {}", token::get_ident(it.ident), it.id);
           match ty::impl_trait_ref(ccx.tcx, local_def(it.id)) {
               Some(impl_trait_ref) => {
                 check_impl_items_against_trait(ccx,
@@ -747,39 +753,9 @@ pub fn check_item<'a,'tcx>(ccx: &CrateCtxt<'a,'tcx>, it: &'tcx ast::Item) {
               }
               None => { }
           }
-
-        for impl_item in impl_items {
-            match impl_item.node {
-                ast::MethodImplItem(ref sig, ref body) => {
-                    check_method_body(ccx, &impl_pty.generics, sig, body,
-                                      impl_item.id, impl_item.span);
-                }
-                ast::TypeImplItem(_) |
-                ast::MacImplItem(_) => {
-                    // Nothing to do here.
-                }
-            }
-        }
-
       }
-      ast::ItemTrait(_, ref generics, _, ref trait_items) => {
+      ast::ItemTrait(_, ref generics, _, _) => {
         check_trait_on_unimplemented(ccx, generics, it);
-        let trait_def = ty::lookup_trait_def(ccx.tcx, local_def(it.id));
-        for trait_item in trait_items {
-            match trait_item.node {
-                ast::MethodTraitItem(_, None) => {
-                    // Nothing to do, since required methods don't have
-                    // bodies to check.
-                }
-                ast::MethodTraitItem(ref sig, Some(ref body)) => {
-                    check_method_body(ccx, &trait_def.generics, sig, body,
-                                      trait_item.id, trait_item.span);
-                }
-                ast::TypeTraitItem(..) => {
-                    // Nothing to do.
-                }
-            }
-        }
       }
       ast::ItemStruct(..) => {
         check_struct(ccx, it.id, it.span);
@@ -814,6 +790,57 @@ pub fn check_item<'a,'tcx>(ccx: &CrateCtxt<'a,'tcx>, it: &'tcx ast::Item) {
     }
 }
 
+pub fn check_item_body<'a,'tcx>(ccx: &CrateCtxt<'a,'tcx>, it: &'tcx ast::Item) {
+    debug!("check_item_body(it.id={}, it.ident={})",
+           it.id,
+           ty::item_path_str(ccx.tcx, local_def(it.id)));
+    let _indenter = indenter();
+    match it.node {
+      ast::ItemFn(ref decl, _, _, _, ref body) => {
+        let fn_pty = ty::lookup_item_type(ccx.tcx, ast_util::local_def(it.id));
+        let param_env = ParameterEnvironment::for_item(ccx.tcx, it.id);
+        check_bare_fn(ccx, &**decl, &**body, it.id, it.span, fn_pty.ty, param_env);
+      }
+      ast::ItemImpl(_, _, _, _, _, ref impl_items) => {
+        debug!("ItemImpl {} with id {}", token::get_ident(it.ident), it.id);
+
+        let impl_pty = ty::lookup_item_type(ccx.tcx, ast_util::local_def(it.id));
+
+        for impl_item in impl_items {
+            match impl_item.node {
+                ast::MethodImplItem(ref sig, ref body) => {
+                    check_method_body(ccx, &impl_pty.generics, sig, body,
+                                      impl_item.id, impl_item.span);
+                }
+                ast::TypeImplItem(_) |
+                ast::MacImplItem(_) => {
+                    // Nothing to do here.
+                }
+            }
+        }
+      }
+      ast::ItemTrait(_, _, _, ref trait_items) => {
+        let trait_def = ty::lookup_trait_def(ccx.tcx, local_def(it.id));
+        for trait_item in trait_items {
+            match trait_item.node {
+                ast::MethodTraitItem(_, None) => {
+                    // Nothing to do, since required methods don't have
+                    // bodies to check.
+                }
+                ast::MethodTraitItem(ref sig, Some(ref body)) => {
+                    check_method_body(ccx, &trait_def.generics, sig, body,
+                                      trait_item.id, trait_item.span);
+                }
+                ast::TypeTraitItem(..) => {
+                    // Nothing to do.
+                }
+            }
+        }
+      }
+      _ => {/* nothing to do */ }
+    }
+}
+
 fn check_trait_on_unimplemented<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
                                generics: &ast::Generics,
                                item: &ast::Item) {

src/test/compile-fail/associated-types-no-suitable-supertrait-2.rs

@@ -0,0 +1,31 @@
+// 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.
+
+// Check that we get an error when you use `<Self as Get>::Value` in
+// the trait definition but `Self` does not, in fact, implement `Get`.
+//
+// See also associated-types-no-suitable-supertrait.rs, which checks
+// that we see the same error when making this mistake on an impl
+// rather than the default method impl.
+//
+// See also run-pass/associated-types-projection-to-unrelated-trait.rs,
+// which checks that the trait interface itself is not considered an
+// error as long as all impls satisfy the constraint.
+
+trait Get : ::std::marker::MarkerTrait {
+    type Value;
+}
+
+trait Other {
+    fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) {}
+    //~^ ERROR the trait `Get` is not implemented for the type `Self`
+}
+
+fn main() { }

src/test/compile-fail/associated-types-no-suitable-supertrait.rs

@@ -10,20 +10,30 @@
 
 // Check that we get an error when you use `<Self as Get>::Value` in
 // the trait definition but `Self` does not, in fact, implement `Get`.
+//
+// See also associated-types-no-suitable-supertrait-2.rs, which checks
+// that we see the same error if we get around to checking the default
+// method body.
+//
+// See also run-pass/associated-types-projection-to-unrelated-trait.rs,
+// which checks that the trait interface itself is not considered an
+// error as long as all impls satisfy the constraint.
 
 trait Get : ::std::marker::MarkerTrait {
     type Value;
 }
 
 trait Other {
     fn uhoh<U:Get>(&self, foo: U, bar: <Self as Get>::Value) {}
-    //~^ ERROR the trait `Get` is not implemented for the type `Self`
+    // (note that we no longer catch the error here, since the
+    //  error below aborts compilation.
+    //  See also associated-types-no-suitable-supertrait-2.rs
+    //  which checks that this error would be caught eventually.)
 }
 
 impl<T:Get> Other for T {
     fn uhoh<U:Get>(&self, foo: U, bar: <(T, U) as Get>::Value) {}
     //~^ ERROR the trait `Get` is not implemented for the type `(T, U)`
-    //~| ERROR the trait `Get` is not implemented for the type `(T, U)`
 }
 
 fn main() { }

src/test/compile-fail/enum-to-float-cast-2.rs

@@ -0,0 +1,28 @@
+// Copyright 2012 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.
+
+// Tests that enum-to-float casts are disallowed.
+
+enum E {
+    L0 = -1,
+    H0 = 1
+}
+
+enum F {
+    L1 = 1,
+    H1 = 0xFFFFFFFFFFFFFFFF
+}
+
+pub fn main() {
+    let a = E::L0 as f32;  //~ ERROR illegal cast
+    let c = F::H1 as f32;  //~ ERROR illegal cast
+    assert_eq!(a, -1.0f32);
+    assert_eq!(c, -1.0f32);
+}

src/test/compile-fail/enum-to-float-cast.rs

@@ -24,12 +24,8 @@ static C0: f32 = E::L0 as f32; //~ ERROR illegal cast
 static C1: f32 = F::H1 as f32; //~ ERROR illegal cast
 
 pub fn main() {
-    let a = E::L0 as f32;  //~ ERROR illegal cast
     let b = C0;
-    let c = F::H1 as f32;  //~ ERROR illegal cast
     let d = C1;
-    assert_eq!(a, -1.0f32);
     assert_eq!(b, -1.0f32);
-    assert_eq!(c, -1.0f32);
     assert_eq!(d, -1.0f32);
 }

src/test/compile-fail/issue-16048.rs

@@ -29,7 +29,7 @@ impl<'a> Test<'a> for Foo<'a> {
 impl<'a> NoLifetime for Foo<'a> {
     fn get<'p, T : Test<'a>>(&self) -> T {
 //~^ ERROR lifetime parameters or bounds on method `get` do not match the trait declaration
-        return *self as T; //~ ERROR non-scalar cast: `Foo<'a>` as `T`
+        return *self as T;
     }
 }
 

src/test/compile-fail/issue-19244-1.rs

@@ -12,7 +12,5 @@ const TUP: (usize,) = (42,);
 
 fn main() {
     let a: [isize; TUP.1];
-    //~^ ERROR array length constant evaluation error: tuple index out of bounds
-    //~| ERROR attempted out-of-bounds tuple index
-    //~| ERROR attempted out-of-bounds tuple index
+    //~^ ERROR attempted out-of-bounds tuple index
 }

src/test/compile-fail/issue-19244-2.rs

@@ -13,7 +13,5 @@ const STRUCT: MyStruct = MyStruct { field: 42 };
 
 fn main() {
     let a: [isize; STRUCT.nonexistent_field];
-    //~^ ERROR array length constant evaluation error: nonexistent struct field
-    //~| ERROR attempted access of field `nonexistent_field`
-    //~| ERROR attempted access of field `nonexistent_field`
+    //~^ ERROR attempted access of field `nonexistent_field`
 }

src/test/compile-fail/issue-2063.rs

@@ -28,7 +28,10 @@ impl to_str_2 for t {
 }
 
 fn new_t(x: t) {
-    x.my_to_string(); //~ ERROR does not implement
+    x.my_to_string();
+    // (there used to be an error emitted right here as well. It was
+    // spurious, at best; if `t` did exist as a type, it clearly would
+    // have an impl of the `to_str_2` trait.)
 }
 
 fn main() {

src/test/compile-fail/issue-23729.rs

@@ -0,0 +1,43 @@
+// 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.
+
+// Regression test for #23729
+
+fn main() {
+    let fib = {
+        struct Recurrence {
+            mem: [u64; 2],
+            pos: usize,
+        }
+
+        impl Iterator for Recurrence {
+            //~^ ERROR not all trait items implemented, missing: `Item` [E0046]
+            #[inline]
+            fn next(&mut self) -> Option<u64> {
+                if self.pos < 2 {
+                    let next_val = self.mem[self.pos];
+                    self.pos += 1;
+                    Some(next_val)
+                } else {
+                    let next_val = (self.mem[0] + self.mem[1]);
+                    self.mem[0] = self.mem[1];
+                    self.mem[1] = next_val;
+                    Some(next_val)
+                }
+            }
+        }
+
+        Recurrence { mem: [0, 1], pos: 0 }
+    };
+
+    for e in fib.take(10) {
+        println!("{}", e)
+    }
+}