Skip to content

Improve lint for type alias bounds #48909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,15 @@ pub enum TyParamBound {
RegionTyParamBound(Lifetime),
}

impl TyParamBound {
pub fn span(&self) -> Span {
match self {
&TraitTyParamBound(ref t, ..) => t.span,
&RegionTyParamBound(ref l) => l.span,
}
}
}

/// A modifier on a bound, currently this is only used for `?Sized`, where the
/// modifier is `Maybe`. Negative bounds should also be handled here.
#[derive(Copy, Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
Expand Down Expand Up @@ -570,6 +579,16 @@ pub enum WherePredicate {
EqPredicate(WhereEqPredicate),
}

impl WherePredicate {
pub fn span(&self) -> Span {
match self {
&WherePredicate::BoundPredicate(ref p) => p.span,
&WherePredicate::RegionPredicate(ref p) => p.span,
&WherePredicate::EqPredicate(ref p) => p.span,
}
}
}

/// A type bound, eg `for<'c> Foo: Send+Clone+'c`
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
pub struct WhereBoundPredicate {
Expand Down
104 changes: 86 additions & 18 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ use syntax::attr;
use syntax::feature_gate::{AttributeGate, AttributeType, Stability, deprecated_attributes};
use syntax_pos::{BytePos, Span, SyntaxContext};
use syntax::symbol::keywords;
use syntax::errors::DiagnosticBuilder;

use rustc::hir::{self, PatKind};
use rustc::hir::intravisit::FnKind;
Expand Down Expand Up @@ -1316,48 +1317,115 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnreachablePub {
}
}

/// Lint for trait and lifetime bounds that are (accidentally) accepted by the parser, but
/// ignored later.
/// Lint for trait and lifetime bounds in type aliases being mostly ignored:
/// They are relevant when using associated types, but otherwise neither checked
/// at definition site nor enforced at use site.

pub struct IgnoredGenericBounds;
pub struct TypeAliasBounds;

declare_lint! {
IGNORED_GENERIC_BOUNDS,
TYPE_ALIAS_BOUNDS,
Warn,
"these generic bounds are ignored"
"bounds in type aliases are not enforced"
}

impl LintPass for IgnoredGenericBounds {
impl LintPass for TypeAliasBounds {
fn get_lints(&self) -> LintArray {
lint_array!(IGNORED_GENERIC_BOUNDS)
lint_array!(TYPE_ALIAS_BOUNDS)
}
}

impl EarlyLintPass for IgnoredGenericBounds {
fn check_item(&mut self, cx: &EarlyContext, item: &ast::Item) {
let type_alias_generics = match item.node {
ast::ItemKind::Ty(_, ref generics) => generics,
impl TypeAliasBounds {
fn is_type_variable_assoc(qpath: &hir::QPath) -> bool {
match *qpath {
hir::QPath::TypeRelative(ref ty, _) => {
// If this is a type variable, we found a `T::Assoc`.
match ty.node {
hir::TyPath(hir::QPath::Resolved(None, ref path)) => {
match path.def {
Def::TyParam(_) => true,
_ => false
}
}
_ => false
}
}
hir::QPath::Resolved(..) => false,
}
}

fn suggest_changing_assoc_types(ty: &hir::Ty, err: &mut DiagnosticBuilder) {
// Access to associates types should use `<T as Bound>::Assoc`, which does not need a
// bound. Let's see if this type does that.

// We use a HIR visitor to walk the type.
use rustc::hir::intravisit::{self, Visitor};
use syntax::ast::NodeId;
struct WalkAssocTypes<'a, 'db> where 'db: 'a {
err: &'a mut DiagnosticBuilder<'db>
}
impl<'a, 'db, 'v> Visitor<'v> for WalkAssocTypes<'a, 'db> {
fn nested_visit_map<'this>(&'this mut self) -> intravisit::NestedVisitorMap<'this, 'v>
{
intravisit::NestedVisitorMap::None
}

fn visit_qpath(&mut self, qpath: &'v hir::QPath, id: NodeId, span: Span) {
if TypeAliasBounds::is_type_variable_assoc(qpath) {
self.err.span_help(span,
"use fully disambiguated paths (i.e., `<T as Trait>::Assoc`) to refer to \
associated types in type aliases");
}
intravisit::walk_qpath(self, qpath, id, span)
}
}

// Let's go for a walk!
let mut visitor = WalkAssocTypes { err };
visitor.visit_ty(ty);
}
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TypeAliasBounds {
fn check_item(&mut self, cx: &LateContext, item: &hir::Item) {
let (ty, type_alias_generics) = match item.node {
hir::ItemTy(ref ty, ref generics) => (&*ty, generics),
_ => return,
};
let mut suggested_changing_assoc_types = false;
// There must not be a where clause
if !type_alias_generics.where_clause.predicates.is_empty() {
let spans : Vec<_> = type_alias_generics.where_clause.predicates.iter()
.map(|pred| pred.span()).collect();
cx.span_lint(IGNORED_GENERIC_BOUNDS, spans,
"where clauses are ignored in type aliases");
let mut err = cx.struct_span_lint(TYPE_ALIAS_BOUNDS, spans,
"where clauses are not enforced in type aliases");
err.help("the clause will not be checked when the type alias is used, \
and should be removed");
if !suggested_changing_assoc_types {
TypeAliasBounds::suggest_changing_assoc_types(ty, &mut err);
suggested_changing_assoc_types = true;
}
err.emit();
}
// The parameters must not have bounds
for param in type_alias_generics.params.iter() {
let spans : Vec<_> = match param {
&ast::GenericParam::Lifetime(ref l) => l.bounds.iter().map(|b| b.span).collect(),
&ast::GenericParam::Type(ref ty) => ty.bounds.iter().map(|b| b.span()).collect(),
&hir::GenericParam::Lifetime(ref l) => l.bounds.iter().map(|b| b.span).collect(),
&hir::GenericParam::Type(ref ty) => ty.bounds.iter().map(|b| b.span()).collect(),
};
if !spans.is_empty() {
cx.span_lint(
IGNORED_GENERIC_BOUNDS,
let mut err = cx.struct_span_lint(
TYPE_ALIAS_BOUNDS,
spans,
"bounds on generic parameters are ignored in type aliases",
"bounds on generic parameters are not enforced in type aliases",
);
err.help("the bound will not be checked when the type alias is used, \
and should be removed");
if !suggested_changing_assoc_types {
TypeAliasBounds::suggest_changing_assoc_types(ty, &mut err);
suggested_changing_assoc_types = true;
}
err.emit();
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
UnusedImportBraces,
AnonymousParameters,
UnusedDocComment,
IgnoredGenericBounds,
);

add_early_builtin_with_new!(sess,
Expand Down Expand Up @@ -139,6 +138,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
MutableTransmutes,
UnionsWithDropFields,
UnreachablePub,
TypeAliasBounds,
);

add_builtin_with_new!(sess,
Expand Down
1 change: 0 additions & 1 deletion src/test/compile-fail/issue-17994.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,4 @@

trait Tr {}
type Huh<T> where T: Tr = isize; //~ ERROR type parameter `T` is unused
//~| WARNING where clauses are ignored in type aliases
fn main() {}
2 changes: 0 additions & 2 deletions src/test/compile-fail/private-in-public-warn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ mod traits {
pub trait PubTr {}

pub type Alias<T: PrivTr> = T; //~ ERROR private trait `traits::PrivTr` in public interface
//~^ WARNING bounds on generic parameters are ignored
//~| WARNING hard error
pub trait Tr1: PrivTr {} //~ ERROR private trait `traits::PrivTr` in public interface
//~^ WARNING hard error
Expand All @@ -85,7 +84,6 @@ mod traits_where {
pub type Alias<T> where T: PrivTr = T;
//~^ ERROR private trait `traits_where::PrivTr` in public interface
//~| WARNING hard error
//~| WARNING where clauses are ignored in type aliases
pub trait Tr2<T> where T: PrivTr {}
//~^ ERROR private trait `traits_where::PrivTr` in public interface
//~| WARNING hard error
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,31 +10,7 @@

#![allow(dead_code, non_camel_case_types)]

use std::rc::Rc;

type SVec<T: Send+Send> = Vec<T>;
//~^ WARN bounds on generic parameters are ignored in type aliases
type VVec<'b, 'a: 'b+'b> = Vec<&'a i32>;
//~^ WARN bounds on generic parameters are ignored in type aliases
type WVec<'b, T: 'b+'b> = Vec<T>;
//~^ WARN bounds on generic parameters are ignored in type aliases
type W2Vec<'b, T> where T: 'b, T: 'b = Vec<T>;
//~^ WARN where clauses are ignored in type aliases

fn foo<'a>(y: &'a i32) {
// If the bounds above would matter, the code below would be rejected.
let mut x : SVec<_> = Vec::new();
x.push(Rc::new(42));

let mut x : VVec<'static, 'a> = Vec::new();
x.push(y);

let mut x : WVec<'static, & 'a i32> = Vec::new();
x.push(y);

let mut x : W2Vec<'static, & 'a i32> = Vec::new();
x.push(y);
}
// Test that bounds on higher-kinded lifetime binders are rejected.

fn bar1<'a, 'b>(
x: &'a i32,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,94 +1,68 @@
error: lifetime bounds cannot be used in this context
--> $DIR/param-bounds-ignored.rs:42:22
--> $DIR/higher-lifetime-bounds.rs:18:22
|
LL | f: for<'xa, 'xb: 'xa+'xa> fn(&'xa i32, &'xb i32) -> &'xa i32)
| ^^^ ^^^

error: lifetime bounds cannot be used in this context
--> $DIR/param-bounds-ignored.rs:50:34
--> $DIR/higher-lifetime-bounds.rs:26:34
|
LL | fn bar2<'a, 'b, F: for<'xa, 'xb: 'xa> Fn(&'xa i32, &'xb i32) -> &'xa i32>(
| ^^^

error: lifetime bounds cannot be used in this context
--> $DIR/param-bounds-ignored.rs:65:28
--> $DIR/higher-lifetime-bounds.rs:41:28
|
LL | where F: for<'xa, 'xb: 'xa> Fn(&'xa i32, &'xb i32) -> &'xa i32
| ^^^

error: lifetime bounds cannot be used in this context
--> $DIR/param-bounds-ignored.rs:77:25
--> $DIR/higher-lifetime-bounds.rs:53:25
|
LL | where for<'xa, 'xb: 'xa> F: Fn(&'xa i32, &'xb i32) -> &'xa i32
| ^^^

error: lifetime bounds cannot be used in this context
--> $DIR/param-bounds-ignored.rs:85:28
--> $DIR/higher-lifetime-bounds.rs:61:28
|
LL | struct S1<F: for<'xa, 'xb: 'xa> Fn(&'xa i32, &'xb i32) -> &'xa i32>(F);
| ^^^

error: lifetime bounds cannot be used in this context
--> $DIR/param-bounds-ignored.rs:87:40
--> $DIR/higher-lifetime-bounds.rs:63:40
|
LL | struct S2<F>(F) where F: for<'xa, 'xb: 'xa> Fn(&'xa i32, &'xb i32) -> &'xa i32;
| ^^^

error: lifetime bounds cannot be used in this context
--> $DIR/param-bounds-ignored.rs:89:37
--> $DIR/higher-lifetime-bounds.rs:65:37
|
LL | struct S3<F>(F) where for<'xa, 'xb: 'xa> F: Fn(&'xa i32, &'xb i32) -> &'xa i32;
| ^^^

error: lifetime bounds cannot be used in this context
--> $DIR/param-bounds-ignored.rs:92:29
--> $DIR/higher-lifetime-bounds.rs:68:29
|
LL | struct S_fnty(for<'xa, 'xb: 'xa> fn(&'xa i32, &'xb i32) -> &'xa i32);
| ^^^

error: lifetime bounds cannot be used in this context
--> $DIR/param-bounds-ignored.rs:95:29
--> $DIR/higher-lifetime-bounds.rs:71:29
|
LL | type T1 = Box<for<'xa, 'xb: 'xa> Fn(&'xa i32, &'xb i32) -> &'xa i32>;
| ^^^

error: lifetime bounds cannot be used in this context
--> $DIR/param-bounds-ignored.rs:99:34
--> $DIR/higher-lifetime-bounds.rs:75:34
|
LL | let _ : Option<for<'xa, 'xb: 'xa> fn(&'xa i32, &'xb i32) -> &'xa i32> = None;
| ^^^

error: lifetime bounds cannot be used in this context
--> $DIR/param-bounds-ignored.rs:101:38
--> $DIR/higher-lifetime-bounds.rs:77:38
|
LL | let _ : Option<Box<for<'xa, 'xb: 'xa> Fn(&'xa i32, &'xb i32) -> &'xa i32>> = None;
| ^^^

warning: bounds on generic parameters are ignored in type aliases
--> $DIR/param-bounds-ignored.rs:15:14
|
LL | type SVec<T: Send+Send> = Vec<T>;
| ^^^^ ^^^^
|
= note: #[warn(ignored_generic_bounds)] on by default

warning: bounds on generic parameters are ignored in type aliases
--> $DIR/param-bounds-ignored.rs:17:19
|
LL | type VVec<'b, 'a: 'b+'b> = Vec<&'a i32>;
| ^^ ^^

warning: bounds on generic parameters are ignored in type aliases
--> $DIR/param-bounds-ignored.rs:19:18
|
LL | type WVec<'b, T: 'b+'b> = Vec<T>;
| ^^ ^^

warning: where clauses are ignored in type aliases
--> $DIR/param-bounds-ignored.rs:21:25
|
LL | type W2Vec<'b, T> where T: 'b, T: 'b = Vec<T>;
| ^^^^^ ^^^^^

error: aborting due to 11 previous errors

Loading