Report errors for type parameters that are not constrained, either by

nikomatsakis · nikomatsakis · commit 91eedfe18b9b · 2015-02-18T10:24:55.000-05:00
variance or an associated type.
diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs
@@ -2955,6 +2955,13 @@ impl<'tcx> TyS<'tcx> {
         assert_eq!(r, Some(self));
         walker
     }
+
+    pub fn as_opt_param_ty(&self) -> Option<ty::ParamTy> {
+        match self.sty {
+            ty::ty_param(ref d) => Some(d.clone()),
+            _ => None,
+        }
+    }
 }
 
 pub fn walk_ty<'tcx, F>(ty_root: Ty<'tcx>, mut f: F)
diff --git a/src/librustc_typeck/check/wf.rs b/src/librustc_typeck/check/wf.rs
@@ -10,21 +10,22 @@
 
 use astconv::AstConv;
 use check::{FnCtxt, Inherited, blank_fn_ctxt, vtable, regionck};
+use constrained_type_params::identify_constrained_type_params;
 use CrateCtxt;
 use middle::region;
-use middle::subst;
+use middle::subst::{self, TypeSpace, FnSpace, ParamSpace, SelfSpace};
 use middle::traits;
 use middle::ty::{self, Ty};
 use middle::ty::liberate_late_bound_regions;
 use middle::ty_fold::{TypeFolder, TypeFoldable, super_fold_ty};
-use util::ppaux::Repr;
+use util::ppaux::{Repr, UserString};
 
 use std::collections::HashSet;
 use syntax::ast;
 use syntax::ast_util::{local_def};
 use syntax::attr;
 use syntax::codemap::Span;
-use syntax::parse::token;
+use syntax::parse::token::{self, special_idents};
 use syntax::visit;
 use syntax::visit::Visitor;
 
@@ -38,6 +39,10 @@ impl<'ccx, 'tcx> CheckTypeWellFormedVisitor<'ccx, 'tcx> {
         CheckTypeWellFormedVisitor { ccx: ccx, cache: HashSet::new() }
     }
 
+    fn tcx(&self) -> &ty::ctxt<'tcx> {
+        self.ccx.tcx
+    }
+
     /// Checks that the field types (in a struct def'n) or argument types (in an enum def'n) are
     /// well-formed, meaning that they do not require any constraints not declared in the struct
     /// definition itself. For example, this definition would be illegal:
@@ -96,23 +101,29 @@ impl<'ccx, 'tcx> CheckTypeWellFormedVisitor<'ccx, 'tcx> {
             ast::ItemConst(..) => {
                 self.check_item_type(item);
             }
-            ast::ItemStruct(ref struct_def, _) => {
+            ast::ItemStruct(ref struct_def, ref ast_generics) => {
                 self.check_type_defn(item, |fcx| {
                     vec![struct_variant(fcx, &**struct_def)]
                 });
+
+                self.check_variances_for_type_defn(item, ast_generics);
             }
-            ast::ItemEnum(ref enum_def, _) => {
+            ast::ItemEnum(ref enum_def, ref ast_generics) => {
                 self.check_type_defn(item, |fcx| {
                     enum_variants(fcx, enum_def)
                 });
+
+                self.check_variances_for_type_defn(item, ast_generics);
             }
-            ast::ItemTrait(..) => {
+            ast::ItemTrait(_, ref ast_generics, _, _) => {
                 let trait_predicates =
                     ty::lookup_predicates(ccx.tcx, local_def(item.id));
                 reject_non_type_param_bounds(
                     ccx.tcx,
                     item.span,
                     &trait_predicates);
+                self.check_variances(item, ast_generics, &trait_predicates,
+                                     self.tcx().lang_items.phantom_fn());
             }
             _ => {}
         }
@@ -280,6 +291,123 @@ impl<'ccx, 'tcx> CheckTypeWellFormedVisitor<'ccx, 'tcx> {
             }
         });
     }
+
+    fn check_variances_for_type_defn(&self,
+                                     item: &ast::Item,
+                                     ast_generics: &ast::Generics)
+    {
+        let item_def_id = local_def(item.id);
+        let predicates = ty::lookup_predicates(self.tcx(), item_def_id);
+        self.check_variances(item,
+                             ast_generics,
+                             &predicates,
+                             self.tcx().lang_items.phantom_data());
+    }
+
+    fn check_variances(&self,
+                       item: &ast::Item,
+                       ast_generics: &ast::Generics,
+                       ty_predicates: &ty::GenericPredicates<'tcx>,
+                       suggested_marker_id: Option<ast::DefId>)
+    {
+        let variance_lang_items = &[
+            self.tcx().lang_items.phantom_fn(),
+            self.tcx().lang_items.phantom_data(),
+        ];
+
+        let item_def_id = local_def(item.id);
+        let is_lang_item = variance_lang_items.iter().any(|n| *n == Some(item_def_id));
+        if is_lang_item {
+            return;
+        }
+
+        let variances = ty::item_variances(self.tcx(), item_def_id);
+
+        let mut constrained_parameters: HashSet<_> =
+            variances.types
+            .iter_enumerated()
+            .filter(|&(_, _, &variance)| variance != ty::Bivariant)
+            .map(|(space, index, _)| self.param_ty(ast_generics, space, index))
+            .collect();
+
+        identify_constrained_type_params(self.tcx(),
+                                         ty_predicates.predicates.as_slice(),
+                                         None,
+                                         &mut constrained_parameters);
+
+        for (space, index, _) in variances.types.iter_enumerated() {
+            let param_ty = self.param_ty(ast_generics, space, index);
+            if constrained_parameters.contains(&param_ty) {
+                continue;
+            }
+            let span = self.ty_param_span(ast_generics, item, space, index);
+            self.report_bivariance(span, param_ty.name, suggested_marker_id);
+        }
+
+        for (space, index, &variance) in variances.regions.iter_enumerated() {
+            if variance != ty::Bivariant {
+                continue;
+            }
+
+            assert_eq!(space, TypeSpace);
+            let span = ast_generics.lifetimes[index].lifetime.span;
+            let name = ast_generics.lifetimes[index].lifetime.name;
+            self.report_bivariance(span, name, suggested_marker_id);
+        }
+    }
+
+    fn param_ty(&self,
+                ast_generics: &ast::Generics,
+                space: ParamSpace,
+                index: usize)
+                -> ty::ParamTy
+    {
+        let name = match space {
+            TypeSpace => ast_generics.ty_params[index].ident.name,
+            SelfSpace => special_idents::type_self.name,
+            FnSpace => self.tcx().sess.bug("Fn space occupied?"),
+        };
+
+        ty::ParamTy { space: space, idx: index as u32, name: name }
+    }
+
+    fn ty_param_span(&self,
+                     ast_generics: &ast::Generics,
+                     item: &ast::Item,
+                     space: ParamSpace,
+                     index: usize)
+                     -> Span
+    {
+        match space {
+            TypeSpace => ast_generics.ty_params[index].span,
+            SelfSpace => item.span,
+            FnSpace => self.tcx().sess.span_bug(item.span, "Fn space occupied?"),
+        }
+    }
+
+    fn report_bivariance(&self,
+                         span: Span,
+                         param_name: ast::Name,
+                         suggested_marker_id: Option<ast::DefId>)
+    {
+        self.tcx().sess.span_err(
+            span,
+            &format!("parameter `{}` is never used",
+                     param_name.user_string(self.tcx()))[]);
+
+        match suggested_marker_id {
+            Some(def_id) => {
+                self.tcx().sess.span_help(
+                    span,
+                    format!("consider removing `{}` or using a marker such as `{}`",
+                            param_name.user_string(self.tcx()),
+                            ty::item_path_str(self.tcx(), def_id)).as_slice());
+            }
+            None => {
+                // no lang items, no help!
+            }
+        }
+    }
 }
 
 // Reject any predicates that do not involve a type parameter.
@@ -347,9 +475,9 @@ impl<'ccx, 'tcx, 'v> Visitor<'v> for CheckTypeWellFormedVisitor<'ccx, 'tcx> {
         match fk {
             visit::FkFnBlock | visit::FkItemFn(..) => {}
             visit::FkMethod(..) => {
-                match ty::impl_or_trait_item(self.ccx.tcx, local_def(id)) {
+                match ty::impl_or_trait_item(self.tcx(), local_def(id)) {
                     ty::ImplOrTraitItem::MethodTraitItem(ty_method) => {
-                        reject_shadowing_type_parameters(self.ccx.tcx, span, &ty_method.generics)
+                        reject_shadowing_type_parameters(self.tcx(), span, &ty_method.generics)
                     }
                     _ => {}
                 }
@@ -363,14 +491,14 @@ impl<'ccx, 'tcx, 'v> Visitor<'v> for CheckTypeWellFormedVisitor<'ccx, 'tcx> {
             &ast::TraitItem::ProvidedMethod(_) |
             &ast::TraitItem::TypeTraitItem(_) => {},
             &ast::TraitItem::RequiredMethod(ref method) => {
-                match ty::impl_or_trait_item(self.ccx.tcx, local_def(method.id)) {
+                match ty::impl_or_trait_item(self.tcx(), local_def(method.id)) {
                     ty::ImplOrTraitItem::MethodTraitItem(ty_method) => {
                         reject_non_type_param_bounds(
-                            self.ccx.tcx,
+                            self.tcx(),
                             method.span,
                             &ty_method.predicates);
                         reject_shadowing_type_parameters(
-                            self.ccx.tcx,
+                            self.tcx(),
                             method.span,
                             &ty_method.generics);
                     }
diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs
@@ -87,6 +87,7 @@ There are some shortcomings in this design:
 
 use astconv::{self, AstConv, ty_of_arg, ast_ty_to_ty, ast_region_to_region};
 use middle::def;
+use constrained_type_params::identify_constrained_type_params;
 use middle::lang_items::SizedTraitLangItem;
 use middle::region;
 use middle::resolve_lifetime;
@@ -1960,51 +1961,15 @@ fn enforce_impl_ty_params_are_constrained<'tcx>(tcx: &ty::ctxt<'tcx>,
     let mut input_parameters: HashSet<_> =
         impl_trait_ref.iter()
                       .flat_map(|t| t.input_types().iter()) // Types in trait ref, if any
-                      .chain(Some(impl_scheme.ty).iter())  // Self type, always
+                      .chain(Some(impl_scheme.ty).iter())   // Self type, always
                       .flat_map(|t| t.walk())
-                      .filter_map(to_opt_param_ty)
+                      .filter_map(|t| t.as_opt_param_ty())
                       .collect();
 
-    loop {
-        let num_inputs = input_parameters.len();
-
-        let projection_predicates =
-            impl_predicates.predicates
-                           .iter()
-                           .filter_map(|predicate| {
-                               match *predicate {
-                                   // Ignore higher-ranked binders. For the purposes
-                                   // of this check, they don't matter because they
-                                   // only affect named regions, and we're just
-                                   // concerned about type parameters here.
-                                   ty::Predicate::Projection(ref data) => Some(data.0.clone()),
-                                   _ => None,
-                               }
-                           });
-
-        for projection in projection_predicates {
-            // Special case: watch out for some kind of sneaky attempt
-            // to project out an associated type defined by this very trait.
-            if Some(projection.projection_ty.trait_ref.clone()) == impl_trait_ref {
-                continue;
-            }
-
-            let relies_only_on_inputs =
-                projection.projection_ty.trait_ref.input_types().iter()
-                .flat_map(|t| t.walk())
-                .filter_map(to_opt_param_ty)
-                .all(|t| input_parameters.contains(&t));
-
-            if relies_only_on_inputs {
-                input_parameters.extend(
-                    projection.ty.walk().filter_map(to_opt_param_ty));
-            }
-        }
-
-        if input_parameters.len() == num_inputs {
-            break;
-        }
-    }
+    identify_constrained_type_params(tcx,
+                                     impl_predicates.predicates.as_slice(),
+                                     impl_trait_ref,
+                                     &mut input_parameters);
 
     for (index, ty_param) in ast_generics.ty_params.iter().enumerate() {
         let param_ty = ty::ParamTy { space: TypeSpace,
@@ -2025,11 +1990,4 @@ fn enforce_impl_ty_params_are_constrained<'tcx>(tcx: &ty::ctxt<'tcx>,
             }
         }
     }
-
-    fn to_opt_param_ty<'tcx>(ty: Ty<'tcx>) -> Option<ty::ParamTy> {
-        match ty.sty {
-            ty::ty_param(ref d) => Some(d.clone()),
-            _ => None,
-        }
-    }
 }
diff --git a/src/librustc_typeck/constrained_type_params.rs b/src/librustc_typeck/constrained_type_params.rs
@@ -23,16 +23,16 @@ pub fn identify_constrained_type_params<'tcx>(_tcx: &ty::ctxt<'tcx>,
 
         let projection_predicates =
             predicates.iter()
-            .filter_map(|predicate| {
-                match *predicate {
-                    // Ignore higher-ranked binders. For the purposes
-                    // of this check, they don't matter because they
-                    // only affect named regions, and we're just
-                    // concerned about type parameters here.
-                    ty::Predicate::Projection(ref data) => Some(data.0.clone()),
-                    _ => None,
-                }
-            });
+                      .filter_map(|predicate| {
+                          match *predicate {
+                              // Ignore higher-ranked binders. For the purposes
+                              // of this check, they don't matter because they
+                              // only affect named regions, and we're just
+                              // concerned about type parameters here.
+                              ty::Predicate::Projection(ref data) => Some(data.0.clone()),
+                              _ => None,
+                          }
+                      });
 
         for projection in projection_predicates {
             // Special case: watch out for some kind of sneaky attempt
diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs
@@ -123,6 +123,7 @@ mod check;
 mod rscope;
 mod astconv;
 mod collect;
+mod constrained_type_params;
 mod coherence;
 mod variance;
 
diff --git a/src/test/compile-fail/issue-17904-2.rs b/src/test/compile-fail/issue-17904-2.rs
@@ -0,0 +1,16 @@
+// 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.
+
+// Test that we can parse a unit struct with a where clause, even if
+// it leads to a error later on since `T` is unused.
+
+struct Foo<T> where T: Copy; //~ ERROR parameter `T` is never used
+
+fn main() {}

Original file line number	Diff line number	Diff line change
`@@ -2955,6 +2955,13 @@ impl<'tcx> TyS<'tcx> {`
`2955`	`2955`	`assert_eq!(r, Some(self));`
`2956`	`2956`	`walker`
`2957`	`2957`	`}`
	`2958`	`+`
	`2959`	`+ pub fn as_opt_param_ty(&self) -> Option<ty::ParamTy> {`
	`2960`	`+ match self.sty {`
	`2961`	`+ ty::ty_param(ref d) => Some(d.clone()),`
	`2962`	`+ _ => None,`
	`2963`	`+ }`
	`2964`	`+ }`
`2958`	`2965`	`}`
`2959`	`2966`
`2960`	`2967`	`pub fn walk_ty<'tcx, F>(ty_root: Ty<'tcx>, mut f: F)`