Skip to content

Commit 7c0c7ef

Browse files
alexcrichtonalexcrichton
committed
Rollup merge of rust-lang#48909 - RalfJung:type_alias_bounds, r=petrochenkov
Improve lint for type alias bounds First of all, I learned just today that I was wrong assuming that the bounds in type aliases are entirely ignored: It turns out they are used to resolve associated types in type aliases. So: ```rust type T1<U: Bound> = U::Assoc; // compiles type T2<U> = U::Assoc; // fails type T3<U> = <U as Bound>::Assoc; // "correct" way to write this, maybe? ``` I am sorry for creating this mess. This PR changes the wording of the lint accordingly. Moreover, since just removing the bound is no longer always a possible fix, I tried to detect cases like `T1` above and show a helpful message to the user: ``` warning: bounds on generic parameters are not enforced in type aliases --> $DIR/type-alias-bounds.rs:57:12 | LL | type T1<U: Bound> = U::Assoc; //~ WARN not enforced in type aliases | ^^^^^ | = help: the bound will not be checked when the type alias is used, and should be removed help: use absolute paths (i.e., <T as Trait>::Assoc) to refer to associated types in type aliases --> $DIR/type-alias-bounds.rs:57:21 | LL | type T1<U: Bound> = U::Assoc; //~ WARN not enforced in type aliases | ^^^^^^^^ ``` I am not sure if I got this entirely right. Ideally, we could provide a suggestion involving the correct trait and type name -- however, while I have access to the HIR in the lint, I do not know how to get access to the resolved name information, like which trait `Assoc` belongs to above. The lint does not even run if that resolution fails, so I assume that information is available *somewhere*... This is a follow-up for (parts of) rust-lang#48326. Also see rust-lang#21903. This changes the name of a lint, but that lint was just merged to master yesterday and has never even been on beta.
2 parents f836ae4 + c05d234 commit 7c0c7ef

9 files changed

+270
-84
lines changed

‎src/librustc/hir/mod.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,15 @@ pub enum TyParamBound {
395395
RegionTyParamBound(Lifetime),
396396
}
397397

398+
impl TyParamBound {
399+
pub fn span(&self) -> Span {
400+
match self {
401+
&TraitTyParamBound(ref t, ..) => t.span,
402+
&RegionTyParamBound(ref l) => l.span,
403+
}
404+
}
405+
}
406+
398407
/// A modifier on a bound, currently this is only used for `?Sized`, where the
399408
/// modifier is `Maybe`. Negative bounds should also be handled here.
400409
#[derive(Copy, Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
@@ -571,6 +580,16 @@ pub enum WherePredicate {
571580
EqPredicate(WhereEqPredicate),
572581
}
573582

583+
impl WherePredicate {
584+
pub fn span(&self) -> Span {
585+
match self {
586+
&WherePredicate::BoundPredicate(ref p) => p.span,
587+
&WherePredicate::RegionPredicate(ref p) => p.span,
588+
&WherePredicate::EqPredicate(ref p) => p.span,
589+
}
590+
}
591+
}
592+
574593
/// A type bound, eg `for<'c> Foo: Send+Clone+'c`
575594
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
576595
pub struct WhereBoundPredicate {

‎src/librustc_lint/builtin.rs

Lines changed: 86 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ use syntax::attr;
4646
use syntax::feature_gate::{AttributeGate, AttributeType, Stability, deprecated_attributes};
4747
use syntax_pos::{BytePos, Span, SyntaxContext};
4848
use syntax::symbol::keywords;
49+
use syntax::errors::DiagnosticBuilder;
4950

5051
use rustc::hir::{self, PatKind};
5152
use rustc::hir::intravisit::FnKind;
@@ -1316,48 +1317,115 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnreachablePub {
13161317
}
13171318
}
13181319

1319-
/// Lint for trait and lifetime bounds that are (accidentally) accepted by the parser, but
1320-
/// ignored later.
1320+
/// Lint for trait and lifetime bounds in type aliases being mostly ignored:
1321+
/// They are relevant when using associated types, but otherwise neither checked
1322+
/// at definition site nor enforced at use site.
13211323
1322-
pub struct IgnoredGenericBounds;
1324+
pub struct TypeAliasBounds;
13231325

13241326
declare_lint! {
1325-
IGNORED_GENERIC_BOUNDS,
1327+
TYPE_ALIAS_BOUNDS,
13261328
Warn,
1327-
"these generic bounds are ignored"
1329+
"bounds in type aliases are not enforced"
13281330
}
13291331

1330-
impl LintPass for IgnoredGenericBounds {
1332+
impl LintPass for TypeAliasBounds {
13311333
fn get_lints(&self) -> LintArray {
1332-
lint_array!(IGNORED_GENERIC_BOUNDS)
1334+
lint_array!(TYPE_ALIAS_BOUNDS)
13331335
}
13341336
}
13351337

1336-
impl EarlyLintPass for IgnoredGenericBounds {
1337-
fn check_item(&mut self, cx: &EarlyContext, item: &ast::Item) {
1338-
let type_alias_generics = match item.node {
1339-
ast::ItemKind::Ty(_, ref generics) => generics,
1338+
impl TypeAliasBounds {
1339+
fn is_type_variable_assoc(qpath: &hir::QPath) -> bool {
1340+
match *qpath {
1341+
hir::QPath::TypeRelative(ref ty, _) => {
1342+
// If this is a type variable, we found a `T::Assoc`.
1343+
match ty.node {
1344+
hir::TyPath(hir::QPath::Resolved(None, ref path)) => {
1345+
match path.def {
1346+
Def::TyParam(_) => true,
1347+
_ => false
1348+
}
1349+
}
1350+
_ => false
1351+
}
1352+
}
1353+
hir::QPath::Resolved(..) => false,
1354+
}
1355+
}
1356+
1357+
fn suggest_changing_assoc_types(ty: &hir::Ty, err: &mut DiagnosticBuilder) {
1358+
// Access to associates types should use `<T as Bound>::Assoc`, which does not need a
1359+
// bound. Let's see if this type does that.
1360+
1361+
// We use a HIR visitor to walk the type.
1362+
use rustc::hir::intravisit::{self, Visitor};
1363+
use syntax::ast::NodeId;
1364+
struct WalkAssocTypes<'a, 'db> where 'db: 'a {
1365+
err: &'a mut DiagnosticBuilder<'db>
1366+
}
1367+
impl<'a, 'db, 'v> Visitor<'v> for WalkAssocTypes<'a, 'db> {
1368+
fn nested_visit_map<'this>(&'this mut self) -> intravisit::NestedVisitorMap<'this, 'v>
1369+
{
1370+
intravisit::NestedVisitorMap::None
1371+
}
1372+
1373+
fn visit_qpath(&mut self, qpath: &'v hir::QPath, id: NodeId, span: Span) {
1374+
if TypeAliasBounds::is_type_variable_assoc(qpath) {
1375+
self.err.span_help(span,
1376+
"use fully disambiguated paths (i.e., `<T as Trait>::Assoc`) to refer to \
1377+
associated types in type aliases");
1378+
}
1379+
intravisit::walk_qpath(self, qpath, id, span)
1380+
}
1381+
}
1382+
1383+
// Let's go for a walk!
1384+
let mut visitor = WalkAssocTypes { err };
1385+
visitor.visit_ty(ty);
1386+
}
1387+
}
1388+
1389+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TypeAliasBounds {
1390+
fn check_item(&mut self, cx: &LateContext, item: &hir::Item) {
1391+
let (ty, type_alias_generics) = match item.node {
1392+
hir::ItemTy(ref ty, ref generics) => (&*ty, generics),
13401393
_ => return,
13411394
};
1395+
let mut suggested_changing_assoc_types = false;
13421396
// There must not be a where clause
13431397
if !type_alias_generics.where_clause.predicates.is_empty() {
13441398
let spans : Vec<_> = type_alias_generics.where_clause.predicates.iter()
13451399
.map(|pred| pred.span()).collect();
1346-
cx.span_lint(IGNORED_GENERIC_BOUNDS, spans,
1347-
"where clauses are ignored in type aliases");
1400+
let mut err = cx.struct_span_lint(TYPE_ALIAS_BOUNDS, spans,
1401+
"where clauses are not enforced in type aliases");
1402+
err.help("the clause will not be checked when the type alias is used, \
1403+
and should be removed");
1404+
if !suggested_changing_assoc_types {
1405+
TypeAliasBounds::suggest_changing_assoc_types(ty, &mut err);
1406+
suggested_changing_assoc_types = true;
1407+
}
1408+
err.emit();
13481409
}
13491410
// The parameters must not have bounds
13501411
for param in type_alias_generics.params.iter() {
13511412
let spans : Vec<_> = match param {
1352-
&ast::GenericParam::Lifetime(ref l) => l.bounds.iter().map(|b| b.span).collect(),
1353-
&ast::GenericParam::Type(ref ty) => ty.bounds.iter().map(|b| b.span()).collect(),
1413+
&hir::GenericParam::Lifetime(ref l) => l.bounds.iter().map(|b| b.span).collect(),
1414+
&hir::GenericParam::Type(ref ty) => ty.bounds.iter().map(|b| b.span()).collect(),
13541415
};
13551416
if !spans.is_empty() {
1356-
cx.span_lint(
1357-
IGNORED_GENERIC_BOUNDS,
1417+
let mut err = cx.struct_span_lint(
1418+
TYPE_ALIAS_BOUNDS,
13581419
spans,
1359-
"bounds on generic parameters are ignored in type aliases",
1420+
"bounds on generic parameters are not enforced in type aliases",
13601421
);
1422+
err.help("the bound will not be checked when the type alias is used, \
1423+
and should be removed");
1424+
if !suggested_changing_assoc_types {
1425+
TypeAliasBounds::suggest_changing_assoc_types(ty, &mut err);
1426+
suggested_changing_assoc_types = true;
1427+
}
1428+
err.emit();
13611429
}
13621430
}
13631431
}

‎src/librustc_lint/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
109109
UnusedImportBraces,
110110
AnonymousParameters,
111111
UnusedDocComment,
112-
IgnoredGenericBounds,
113112
);
114113

115114
add_early_builtin_with_new!(sess,
@@ -139,6 +138,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
139138
MutableTransmutes,
140139
UnionsWithDropFields,
141140
UnreachablePub,
141+
TypeAliasBounds,
142142
);
143143

144144
add_builtin_with_new!(sess,

‎src/test/compile-fail/issue-17994.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,4 @@
1010

1111
trait Tr {}
1212
type Huh<T> where T: Tr = isize; //~ ERROR type parameter `T` is unused
13-
//~| WARNING where clauses are ignored in type aliases
1413
fn main() {}

‎src/test/compile-fail/private-in-public-warn.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ mod traits {
5858
pub trait PubTr {}
5959

6060
pub type Alias<T: PrivTr> = T; //~ ERROR private trait `traits::PrivTr` in public interface
61-
//~^ WARNING bounds on generic parameters are ignored
6261
//~| WARNING hard error
6362
pub trait Tr1: PrivTr {} //~ ERROR private trait `traits::PrivTr` in public interface
6463
//~^ WARNING hard error
@@ -85,7 +84,6 @@ mod traits_where {
8584
pub type Alias<T> where T: PrivTr = T;
8685
//~^ ERROR private trait `traits_where::PrivTr` in public interface
8786
//~| WARNING hard error
88-
//~| WARNING where clauses are ignored in type aliases
8987
pub trait Tr2<T> where T: PrivTr {}
9088
//~^ ERROR private trait `traits_where::PrivTr` in public interface
9189
//~| WARNING hard error

‎src/test/ui/param-bounds-ignored.rs renamed to ‎src/test/ui/higher-lifetime-bounds.rs

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -10,31 +10,7 @@
1010

1111
#![allow(dead_code, non_camel_case_types)]
1212

13-
use std::rc::Rc;
14-
15-
type SVec<T: Send+Send> = Vec<T>;
16-
//~^ WARN bounds on generic parameters are ignored in type aliases
17-
type VVec<'b, 'a: 'b+'b> = Vec<&'a i32>;
18-
//~^ WARN bounds on generic parameters are ignored in type aliases
19-
type WVec<'b, T: 'b+'b> = Vec<T>;
20-
//~^ WARN bounds on generic parameters are ignored in type aliases
21-
type W2Vec<'b, T> where T: 'b, T: 'b = Vec<T>;
22-
//~^ WARN where clauses are ignored in type aliases
23-
24-
fn foo<'a>(y: &'a i32) {
25-
// If the bounds above would matter, the code below would be rejected.
26-
let mut x : SVec<_> = Vec::new();
27-
x.push(Rc::new(42));
28-
29-
let mut x : VVec<'static, 'a> = Vec::new();
30-
x.push(y);
31-
32-
let mut x : WVec<'static, & 'a i32> = Vec::new();
33-
x.push(y);
34-
35-
let mut x : W2Vec<'static, & 'a i32> = Vec::new();
36-
x.push(y);
37-
}
13+
// Test that bounds on higher-kinded lifetime binders are rejected.
3814

3915
fn bar1<'a, 'b>(
4016
x: &'a i32,

‎src/test/ui/param-bounds-ignored.stderr renamed to ‎src/test/ui/higher-lifetime-bounds.stderr

Lines changed: 11 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,94 +1,68 @@
11
error: lifetime bounds cannot be used in this context
2-
--> $DIR/param-bounds-ignored.rs:42:22
2+
--> $DIR/higher-lifetime-bounds.rs:18:22
33
|
44
LL | f: for<'xa, 'xb: 'xa+'xa> fn(&'xa i32, &'xb i32) -> &'xa i32)
55
| ^^^ ^^^
66

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

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

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

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

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

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

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

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

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

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

67-
warning: bounds on generic parameters are ignored in type aliases
68-
--> $DIR/param-bounds-ignored.rs:15:14
69-
|
70-
LL | type SVec<T: Send+Send> = Vec<T>;
71-
| ^^^^ ^^^^
72-
|
73-
= note: #[warn(ignored_generic_bounds)] on by default
74-
75-
warning: bounds on generic parameters are ignored in type aliases
76-
--> $DIR/param-bounds-ignored.rs:17:19
77-
|
78-
LL | type VVec<'b, 'a: 'b+'b> = Vec<&'a i32>;
79-
| ^^ ^^
80-
81-
warning: bounds on generic parameters are ignored in type aliases
82-
--> $DIR/param-bounds-ignored.rs:19:18
83-
|
84-
LL | type WVec<'b, T: 'b+'b> = Vec<T>;
85-
| ^^ ^^
86-
87-
warning: where clauses are ignored in type aliases
88-
--> $DIR/param-bounds-ignored.rs:21:25
89-
|
90-
LL | type W2Vec<'b, T> where T: 'b, T: 'b = Vec<T>;
91-
| ^^^^^ ^^^^^
92-
9367
error: aborting due to 11 previous errors
9468

0 commit comments

Comments
 (0)