Auto merge of #38920 - petrochenkov:selfimpl, r=eddyb

Partially implement RFC 1647 (`Self` in impl headers)

The name resolution part is easy, but the typeck part contains an unexpected problem.

It turns out that `Self` type *depends* on bounds and `where` clauses, so we need to convert them first to determine what the `Self` type is! If bounds/`where` clauses can refer to `Self` then we have a cyclic dependency.
This is required to support impls like this:
```
// Found in libcollections
impl<I: IntoIterator> SpecExtend<I> for LinkedList<I::Item> { .... }
                                                      ^^^^^ associated type `Item` is found using information from bounds

```
I'm not yet sure how to resolve this issue.
One possible solution (that feels hacky) is to make two passes over generics - first collect predicates ignoring everything involving `Self`, then determine `Self`, then collect predicates again without ignoring anything. (Some kind of lazy on-demand checking or something looks like a proper solution.)

This patch in its current state doesn't solve the problem with `Self` in bounds, so the only observable things it does is improving error messages and supporting `impl Trait<Self> for Type {}`.

There's also a question about feature gating. It's non-trivial to *detect* "newly resolved" `Self`s to feature gate them, but it's simple to *enable* the new resolution behavior when the feature gate is already specified. Alternatively this can be considered a bug fix and merged without a feature gate.

cc https://github.com/rust-lang/rust/issues/38864
r? @nikomatsakis
cc @eddyb
Whitespace ignoring diff https://github.com/rust-lang/rust/pull/38920/files?w=1

5 files changed, 122 insertions, 67 deletions

diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index 8a206664a7d..b5e2715ab4f 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -1548,7 +1548,10 @@ impl<'a> Resolver<'a> {
             }
 
             ItemKind::DefaultImpl(_, ref trait_ref) => {
-                self.with_optional_trait_ref(Some(trait_ref), |_, _| {});
+                self.with_optional_trait_ref(Some(trait_ref), |this, _| {
+                    // Resolve type arguments in trait path
+                    visit::walk_trait_ref(this, trait_ref);
+                });
             }
             ItemKind::Impl(.., ref generics, ref opt_trait_ref, ref self_type, ref impl_items) =>
                 self.resolve_implementation(generics,
@@ -1712,7 +1715,6 @@ impl<'a> Resolver<'a> {
                 new_val = Some((def.def_id(), trait_ref.clone()));
                 new_id = Some(def.def_id());
             }
-            visit::walk_trait_ref(self, trait_ref);
         }
         let original_trait_ref = replace(&mut self.current_trait_ref, new_val);
         let result = f(self, new_id);
@@ -1740,60 +1742,66 @@ impl<'a> Resolver<'a> {
                               impl_items: &[ImplItem]) {
         // If applicable, create a rib for the type parameters.
         self.with_type_parameter_rib(HasTypeParameters(generics, ItemRibKind), |this| {
-            // Resolve the type parameters.
-            this.visit_generics(generics);
-
-            // Resolve the trait reference, if necessary.
-            this.with_optional_trait_ref(opt_trait_reference.as_ref(), |this, trait_id| {
-                // Resolve the self type.
-                this.visit_ty(self_type);
-
-                let item_def_id = this.definitions.local_def_id(item_id);
-                this.with_self_rib(Def::SelfTy(trait_id, Some(item_def_id)), |this| {
-                    this.with_current_self_type(self_type, |this| {
-                        for impl_item in impl_items {
-                            this.check_proc_macro_attrs(&impl_item.attrs);
-                            this.resolve_visibility(&impl_item.vis);
-                            match impl_item.node {
-                                ImplItemKind::Const(..) => {
-                                    // If this is a trait impl, ensure the const
-                                    // exists in trait
-                                    this.check_trait_item(impl_item.ident.name,
-                                                          ValueNS,
-                                                          impl_item.span,
-                                        |n, s| ResolutionError::ConstNotMemberOfTrait(n, s));
-                                    visit::walk_impl_item(this, impl_item);
-                                }
-                                ImplItemKind::Method(ref sig, _) => {
-                                    // If this is a trait impl, ensure the method
-                                    // exists in trait
-                                    this.check_trait_item(impl_item.ident.name,
-                                                          ValueNS,
-                                                          impl_item.span,
-                                        |n, s| ResolutionError::MethodNotMemberOfTrait(n, s));
-
-                                    // We also need a new scope for the method-
-                                    // specific type parameters.
-                                    let type_parameters =
-                                        HasTypeParameters(&sig.generics,
-                                                          MethodRibKind(!sig.decl.has_self()));
-                                    this.with_type_parameter_rib(type_parameters, |this| {
+            // Dummy self type for better errors if `Self` is used in the trait path.
+            this.with_self_rib(Def::SelfTy(None, None), |this| {
+                // Resolve the trait reference, if necessary.
+                this.with_optional_trait_ref(opt_trait_reference.as_ref(), |this, trait_id| {
+                    let item_def_id = this.definitions.local_def_id(item_id);
+                    this.with_self_rib(Def::SelfTy(trait_id, Some(item_def_id)), |this| {
+                        if let Some(trait_ref) = opt_trait_reference.as_ref() {
+                            // Resolve type arguments in trait path
+                            visit::walk_trait_ref(this, trait_ref);
+                        }
+                        // Resolve the self type.
+                        this.visit_ty(self_type);
+                        // Resolve the type parameters.
+                        this.visit_generics(generics);
+                        this.with_current_self_type(self_type, |this| {
+                            for impl_item in impl_items {
+                                this.check_proc_macro_attrs(&impl_item.attrs);
+                                this.resolve_visibility(&impl_item.vis);
+                                match impl_item.node {
+                                    ImplItemKind::Const(..) => {
+                                        // If this is a trait impl, ensure the const
+                                        // exists in trait
+                                        this.check_trait_item(impl_item.ident.name,
+                                                            ValueNS,
+                                                            impl_item.span,
+                                            |n, s| ResolutionError::ConstNotMemberOfTrait(n, s));
                                         visit::walk_impl_item(this, impl_item);
-                                    });
-                                }
-                                ImplItemKind::Type(ref ty) => {
-                                    // If this is a trait impl, ensure the type
-                                    // exists in trait
-                                    this.check_trait_item(impl_item.ident.name,
-                                                          TypeNS,
-                                                          impl_item.span,
-                                        |n, s| ResolutionError::TypeNotMemberOfTrait(n, s));
-
-                                    this.visit_ty(ty);
+                                    }
+                                    ImplItemKind::Method(ref sig, _) => {
+                                        // If this is a trait impl, ensure the method
+                                        // exists in trait
+                                        this.check_trait_item(impl_item.ident.name,
+                                                            ValueNS,
+                                                            impl_item.span,
+                                            |n, s| ResolutionError::MethodNotMemberOfTrait(n, s));
+
+                                        // We also need a new scope for the method-
+                                        // specific type parameters.
+                                        let type_parameters =
+                                            HasTypeParameters(&sig.generics,
+                                                            MethodRibKind(!sig.decl.has_self()));
+                                        this.with_type_parameter_rib(type_parameters, |this| {
+                                            visit::walk_impl_item(this, impl_item);
+                                        });
+                                    }
+                                    ImplItemKind::Type(ref ty) => {
+                                        // If this is a trait impl, ensure the type
+                                        // exists in trait
+                                        this.check_trait_item(impl_item.ident.name,
+                                                            TypeNS,
+                                                            impl_item.span,
+                                            |n, s| ResolutionError::TypeNotMemberOfTrait(n, s));
+
+                                        this.visit_ty(ty);
+                                    }
+                                    ImplItemKind::Macro(_) =>
+                                        panic!("unexpanded macro in resolve!"),
                                 }
-                                ImplItemKind::Macro(_) => panic!("unexpanded macro in resolve!"),
                             }
-                        }
+                        });
                     });
                 });
             });
diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs
index e4529b58f03..838dbef8ecf 100644
--- a/src/librustc_typeck/astconv.rs
+++ b/src/librustc_typeck/astconv.rs
@@ -1401,11 +1401,23 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
 
                 assert_eq!(opt_self_ty, None);
                 tcx.prohibit_type_params(&path.segments);
-                let ty = tcx.item_type(def_id);
-                if let Some(free_substs) = self.get_free_substs() {
-                    ty.subst(tcx, free_substs)
+
+                // FIXME: Self type is not always computed when we are here because type parameter
+                // bounds may affect Self type and have to be converted before it.
+                let ty = if def_id.is_local() {
+                    tcx.item_types.borrow().get(&def_id).cloned()
                 } else {
-                    ty
+                    Some(tcx.item_type(def_id))
+                };
+                if let Some(ty) = ty {
+                    if let Some(free_substs) = self.get_free_substs() {
+                        ty.subst(tcx, free_substs)
+                    } else {
+                        ty
+                    }
+                } else {
+                    tcx.sess.span_err(span, "`Self` type is used before it's determined");
+                    tcx.types.err
                 }
             }
             Def::SelfTy(Some(_), None) => {
diff --git a/src/test/compile-fail/resolve-self-in-impl-2.rs b/src/test/compile-fail/resolve-self-in-impl-2.rs
new file mode 100644
index 00000000000..adc208a0202
--- /dev/null
+++ b/src/test/compile-fail/resolve-self-in-impl-2.rs
@@ -0,0 +1,17 @@
+// Copyright 2016 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.
+
+struct S<T = u8>(T);
+trait Tr<T = u8> {}
+
+impl Self for S {} //~ ERROR expected trait, found self type `Self`
+impl Self::N for S {} //~ ERROR cannot find trait `N` in `Self`
+
+fn main() {}
diff --git a/src/test/compile-fail/resolve-self-in-impl.rs b/src/test/compile-fail/resolve-self-in-impl.rs
new file mode 100644
index 00000000000..82037b6c9ed
--- /dev/null
+++ b/src/test/compile-fail/resolve-self-in-impl.rs
@@ -0,0 +1,26 @@
+// Copyright 2016 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.
+
+struct S<T = u8>(T);
+trait Tr<T = u8> {}
+
+impl Tr<Self> for S {} // OK
+
+// FIXME: `Self` cannot be used in bounds because it depends on bounds itself.
+impl<T: Tr<Self>> Tr<T> for S {} //~ ERROR `Self` type is used before it's determined
+impl<T = Self> Tr<T> for S {} //~ ERROR `Self` type is used before it's determined
+impl Tr for S where Self: Copy {} //~ ERROR `Self` type is used before it's determined
+impl Tr for S where S<Self>: Copy {} //~ ERROR `Self` type is used before it's determined
+impl Tr for Self {} //~ ERROR `Self` type is used before it's determined
+impl Tr for S<Self> {} //~ ERROR `Self` type is used before it's determined
+impl Self {} //~ ERROR `Self` type is used before it's determined
+impl S<Self> {} //~ ERROR `Self` type is used before it's determined
+
+fn main() {}
diff --git a/src/test/ui/resolve/issue-23305.stderr b/src/test/ui/resolve/issue-23305.stderr
index 70de352d73e..881f04300ed 100644
--- a/src/test/ui/resolve/issue-23305.stderr
+++ b/src/test/ui/resolve/issue-23305.stderr
@@ -1,16 +1,8 @@
-error[E0411]: cannot find type `Self` in this scope
+error: `Self` type is used before it's determined
   --> $DIR/issue-23305.rs:15:12
    |
 15 | impl ToNbt<Self> {}
-   |            ^^^^ `Self` is only available in traits and impls
-
-error[E0038]: the trait `ToNbt` cannot be made into an object
-  --> $DIR/issue-23305.rs:15:6
-   |
-15 | impl ToNbt<Self> {}
-   |      ^^^^^^^^^^^ the trait `ToNbt` cannot be made into an object
-   |
-   = note: method `new` has no receiver
+   |            ^^^^
 
 error: aborting due to previous error