Move type parameter shadowing errors to resolve

For some reason type checking did this. Further it didn't consider
hygiene.

Author: matthewjasper

src/librustc_resolve/diagnostics.rs

@@ -169,12 +169,14 @@ impl<'a> Resolver<'a> {
                 err
             }
             ResolutionError::NameAlreadyUsedInParameterList(name, first_use_span) => {
-                let mut err = struct_span_err!(self.session,
-                                                span,
-                                                E0403,
-                                                "the name `{}` is already used for a generic \
-                                                parameter in this list of generic parameters",
-                                                name);
+                let mut err = struct_span_err!(
+                    self.session,
+                    span,
+                    E0403,
+                    "the name `{}` is already used for a generic \
+                     parameter in this item's generic parameters",
+                    name,
+                );
                 err.span_label(span, "already used");
                 err.span_label(first_use_span, format!("first use of `{}`", name));
                 err

src/librustc_resolve/error_codes.rs

@@ -526,15 +526,25 @@ Some type parameters have the same name.
 Erroneous code example:
 
 ```compile_fail,E0403
-fn foo<T, T>(s: T, u: T) {} // error: the name `T` is already used for a type
-                            //        parameter in this type parameter list
+fn f<T, T>(s: T, u: T) {} // error: the name `T` is already used for a generic
+                          //        parameter in this item's generic parameters
 ```
 
 Please verify that none of the type parameters are misspelled, and rename any
 clashing parameters. Example:
 
 ```
-fn foo<T, Y>(s: T, u: Y) {} // ok!
+fn f<T, Y>(s: T, u: Y) {} // ok!
+```
+
+Type parameters in an associated item also cannot shadow parameters from the
+containing item:
+
+```compile_fail,E0403
+trait Foo<T> {
+    fn do_something(&self) -> T;
+    fn do_something_else<T: Clone>(&self, bar: T);
+}
 ```
 "##,
 

src/librustc_resolve/late.rs

@@ -111,6 +111,24 @@ crate enum RibKind<'a> {
     TyParamAsConstParamTy,
 }
 
+impl RibKind<'_> {
+    // Whether this rib kind contains generic parameters, as opposed to local
+    // variables.
+    crate fn contains_params(&self) -> bool {
+        match self {
+            NormalRibKind
+            | FnItemRibKind
+            | ConstantItemRibKind
+            | ModuleRibKind(_)
+            | MacroDefinition(_) => false,
+            AssocItemRibKind
+            | ItemRibKind
+            | ForwardTyParamBanRibKind
+            | TyParamAsConstParamTy => true,
+        }
+    }
+}
+
 /// A single local scope.
 ///
 /// A rib represents a scope names can live in. Note that these appear in many places, not just
@@ -798,6 +816,19 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
                 let mut function_type_rib = Rib::new(rib_kind);
                 let mut function_value_rib = Rib::new(rib_kind);
                 let mut seen_bindings = FxHashMap::default();
+                // We also can't shadow bindings from the parent item
+                if let AssocItemRibKind = rib_kind {
+                    let mut add_bindings_for_ns = |ns| {
+                        let parent_rib = self.ribs[ns].iter()
+                            .rfind(|rib| if let ItemRibKind = rib.kind { true } else { false })
+                            .expect("associated item outside of an item");
+                        seen_bindings.extend(
+                            parent_rib.bindings.iter().map(|(ident, _)| (*ident, ident.span)),
+                        );
+                    };
+                    add_bindings_for_ns(ValueNS);
+                    add_bindings_for_ns(TypeNS);
+                }
                 for param in &generics.params {
                     match param.kind {
                         GenericParamKind::Lifetime { .. } => {}

src/librustc_resolve/lib.rs

@@ -1499,7 +1499,7 @@ impl<'a> Resolver<'a> {
             debug!("walk rib\n{:?}", ribs[i].bindings);
             // Use the rib kind to determine whether we are resolving parameters
             // (modern hygiene) or local variables (legacy hygiene).
-            let rib_ident = if let AssocItemRibKind | ItemRibKind = ribs[i].kind {
+            let rib_ident = if ribs[i].kind.contains_params() {
                 modern_ident
             } else {
                 ident

src/librustc_typeck/check/wfcheck.rs

@@ -203,7 +203,6 @@ fn check_associated_item(
                 fcx.register_wf_obligation(ty, span, code.clone());
             }
             ty::AssocKind::Method => {
-                reject_shadowing_parameters(fcx.tcx, item.def_id);
                 let sig = fcx.tcx.fn_sig(item.def_id);
                 let sig = fcx.normalize_associated_types_in(span, &sig);
                 check_fn_or_method(tcx, fcx, span, sig,
@@ -998,34 +997,6 @@ fn report_bivariance(tcx: TyCtxt<'_>, span: Span, param_name: ast::Name) {
     err.emit();
 }
 
-fn reject_shadowing_parameters(tcx: TyCtxt<'_>, def_id: DefId) {
-    let generics = tcx.generics_of(def_id);
-    let parent = tcx.generics_of(generics.parent.unwrap());
-    let impl_params: FxHashMap<_, _> = parent.params.iter().flat_map(|param| match param.kind {
-        GenericParamDefKind::Lifetime => None,
-        GenericParamDefKind::Type { .. } | GenericParamDefKind::Const => {
-            Some((param.name, param.def_id))
-        }
-    }).collect();
-
-    for method_param in &generics.params {
-        // Shadowing is checked in `resolve_lifetime`.
-        if let GenericParamDefKind::Lifetime = method_param.kind {
-            continue
-        }
-        if impl_params.contains_key(&method_param.name) {
-            // Tighten up the span to focus on only the shadowing type.
-            let type_span = tcx.def_span(method_param.def_id);
-
-            // The expectation here is that the original trait declaration is
-            // local so it should be okay to just unwrap everything.
-            let trait_def_id = impl_params[&method_param.name];
-            let trait_decl_span = tcx.def_span(trait_def_id);
-            error_194(tcx, type_span, trait_decl_span, &method_param.name.as_str()[..]);
-        }
-    }
-}
-
 /// Feature gates RFC 2056 -- trivial bounds, checking for global bounds that
 /// aren't true.
 fn check_false_global_bounds(fcx: &FnCtxt<'_, '_>, span: Span, id: hir::HirId) {
@@ -1152,12 +1123,3 @@ fn error_392(
     err.span_label(span, "unused parameter");
     err
 }
-
-fn error_194(tcx: TyCtxt<'_>, span: Span, trait_decl_span: Span, name: &str) {
-    struct_span_err!(tcx.sess, span, E0194,
-                     "type parameter `{}` shadows another type parameter of the same name",
-                     name)
-        .span_label(span, "shadows another type parameter")
-        .span_label(trait_decl_span, format!("first `{}` declared here", name))
-        .emit();
-}

src/librustc_typeck/error_codes.rs

@@ -1718,22 +1718,6 @@ Since we know for certain that `Wrapper<u32>` implements `Clone`, there's no
 reason to also specify it in a `where` clause.
 "##,
 
-E0194: r##"
-A type parameter was declared which shadows an existing one. An example of this
-error:
-
-```compile_fail,E0194
-trait Foo<T> {
-    fn do_something(&self) -> T;
-    fn do_something_else<T: Clone>(&self, bar: T);
-}
-```
-
-In this example, the trait `Foo` and the trait method `do_something_else` both
-define a type parameter `T`. This is not allowed: if the method wishes to
-define a type parameter, it must use a different name for it.
-"##,
-
 E0195: r##"
 Your method's lifetime parameters do not match the trait declaration.
 Erroneous code example:
@@ -4837,6 +4821,7 @@ register_diagnostics! {
 //  E0188, // can not cast an immutable reference to a mutable pointer
 //  E0189, // deprecated: can only cast a boxed pointer to a boxed object
 //  E0190, // deprecated: can only cast a &-pointer to an &-object
+//  E0194, // merged into E0403
 //  E0196, // cannot determine a type for this closure
     E0203, // type parameter has more than one relaxed default bound,
            // and only one is supported

src/test/ui/duplicate/duplicate-type-parameter.stderr

@@ -1,52 +1,52 @@
-error[E0403]: the name `T` is already used for a generic parameter in this list of generic parameters
+error[E0403]: the name `T` is already used for a generic parameter in this item's generic parameters
   --> $DIR/duplicate-type-parameter.rs:1:12
    |
 LL | type Foo<T,T> = Option<T>;
    |          - ^ already used
    |          |
    |          first use of `T`
 
-error[E0403]: the name `T` is already used for a generic parameter in this list of generic parameters
+error[E0403]: the name `T` is already used for a generic parameter in this item's generic parameters
   --> $DIR/duplicate-type-parameter.rs:4:14
    |
 LL | struct Bar<T,T>(T);
    |            - ^ already used
    |            |
    |            first use of `T`
 
-error[E0403]: the name `T` is already used for a generic parameter in this list of generic parameters
+error[E0403]: the name `T` is already used for a generic parameter in this item's generic parameters
   --> $DIR/duplicate-type-parameter.rs:7:14
    |
 LL | struct Baz<T,T> {
    |            - ^ already used
    |            |
    |            first use of `T`
 
-error[E0403]: the name `T` is already used for a generic parameter in this list of generic parameters
+error[E0403]: the name `T` is already used for a generic parameter in this item's generic parameters
   --> $DIR/duplicate-type-parameter.rs:12:12
    |
 LL | enum Boo<T,T> {
    |          - ^ already used
    |          |
    |          first use of `T`
 
-error[E0403]: the name `T` is already used for a generic parameter in this list of generic parameters
+error[E0403]: the name `T` is already used for a generic parameter in this item's generic parameters
   --> $DIR/duplicate-type-parameter.rs:18:11
    |
 LL | fn quux<T,T>(x: T) {}
    |         - ^ already used
    |         |
    |         first use of `T`
 
-error[E0403]: the name `T` is already used for a generic parameter in this list of generic parameters
+error[E0403]: the name `T` is already used for a generic parameter in this item's generic parameters
   --> $DIR/duplicate-type-parameter.rs:21:13
    |
 LL | trait Qux<T,T> {}
    |           - ^ already used
    |           |
    |           first use of `T`
 
-error[E0403]: the name `T` is already used for a generic parameter in this list of generic parameters
+error[E0403]: the name `T` is already used for a generic parameter in this item's generic parameters
   --> $DIR/duplicate-type-parameter.rs:24:8
    |
 LL | impl<T,T> Qux<T,T> for Option<T> {}

src/test/ui/error-codes/E0194.rs

@@ -1,7 +1,7 @@
 trait Foo<T> {
     fn do_something(&self) -> T;
     fn do_something_else<T: Clone>(&self, bar: T);
-    //~^ ERROR E0194
+    //~^ ERROR E0403
 }
 
 fn main() {

src/test/ui/error-codes/E0194.stderr

@@ -1,12 +1,12 @@
-error[E0194]: type parameter `T` shadows another type parameter of the same name
+error[E0403]: the name `T` is already used for a generic parameter in this item's generic parameters
   --> $DIR/E0194.rs:3:26
    |
 LL | trait Foo<T> {
-   |           - first `T` declared here
+   |           - first use of `T`
 LL |     fn do_something(&self) -> T;
 LL |     fn do_something_else<T: Clone>(&self, bar: T);
-   |                          ^ shadows another type parameter
+   |                          ^ already used
 
 error: aborting due to previous error
 
-For more information about this error, try `rustc --explain E0194`.
+For more information about this error, try `rustc --explain E0403`.

src/test/ui/error-codes/E0403.stderr

@@ -1,4 +1,4 @@
-error[E0403]: the name `T` is already used for a generic parameter in this list of generic parameters
+error[E0403]: the name `T` is already used for a generic parameter in this item's generic parameters
   --> $DIR/E0403.rs:1:11
    |
 LL | fn foo<T, T>(s: T, u: T) {}