Skip to content
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

Warn about ignored generic bounds in for #48326

Merged
merged 5 commits into from
Mar 9, 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
21 changes: 21 additions & 0 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,17 @@ impl<'a, 'tcx> hir_visit::Visitor<'tcx> for LateContext<'a, 'tcx> {
hir_visit::walk_generics(self, g);
}

fn visit_where_predicate(&mut self, p: &'tcx hir::WherePredicate) {
run_lints!(self, check_where_predicate, late_passes, p);
hir_visit::walk_where_predicate(self, p);
}

fn visit_poly_trait_ref(&mut self, t: &'tcx hir::PolyTraitRef,
m: hir::TraitBoundModifier) {
run_lints!(self, check_poly_trait_ref, late_passes, t, m);
hir_visit::walk_poly_trait_ref(self, t, m);
}

fn visit_trait_item(&mut self, trait_item: &'tcx hir::TraitItem) {
let generics = self.generics.take();
self.generics = Some(&trait_item.generics);
Expand Down Expand Up @@ -955,6 +966,16 @@ impl<'a> ast_visit::Visitor<'a> for EarlyContext<'a> {
ast_visit::walk_generics(self, g);
}

fn visit_where_predicate(&mut self, p: &'a ast::WherePredicate) {
run_lints!(self, check_where_predicate, early_passes, p);
ast_visit::walk_where_predicate(self, p);
}

fn visit_poly_trait_ref(&mut self, t: &'a ast::PolyTraitRef, m: &'a ast::TraitBoundModifier) {
run_lints!(self, check_poly_trait_ref, early_passes, t, m);
ast_visit::walk_poly_trait_ref(self, t, m);
}

fn visit_trait_item(&mut self, trait_item: &'a ast::TraitItem) {
self.with_lint_attrs(trait_item.id, &trait_item.attrs, |cx| {
run_lints!(cx, check_trait_item, early_passes, trait_item);
Expand Down
6 changes: 6 additions & 0 deletions src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ pub trait LateLintPass<'a, 'tcx>: LintPass {
fn check_ty(&mut self, _: &LateContext<'a, 'tcx>, _: &'tcx hir::Ty) { }
fn check_generic_param(&mut self, _: &LateContext<'a, 'tcx>, _: &'tcx hir::GenericParam) { }
fn check_generics(&mut self, _: &LateContext<'a, 'tcx>, _: &'tcx hir::Generics) { }
fn check_where_predicate(&mut self, _: &LateContext<'a, 'tcx>, _: &'tcx hir::WherePredicate) { }
fn check_poly_trait_ref(&mut self, _: &LateContext<'a, 'tcx>, _: &'tcx hir::PolyTraitRef,
_: hir::TraitBoundModifier) { }
fn check_fn(&mut self,
_: &LateContext<'a, 'tcx>,
_: FnKind<'tcx>,
Expand Down Expand Up @@ -230,6 +233,9 @@ pub trait EarlyLintPass: LintPass {
fn check_ty(&mut self, _: &EarlyContext, _: &ast::Ty) { }
fn check_generic_param(&mut self, _: &EarlyContext, _: &ast::GenericParam) { }
fn check_generics(&mut self, _: &EarlyContext, _: &ast::Generics) { }
fn check_where_predicate(&mut self, _: &EarlyContext, _: &ast::WherePredicate) { }
fn check_poly_trait_ref(&mut self, _: &EarlyContext, _: &ast::PolyTraitRef,
_: &ast::TraitBoundModifier) { }
fn check_fn(&mut self, _: &EarlyContext,
_: ast_visit::FnKind, _: &ast::FnDecl, _: Span, _: ast::NodeId) { }
fn check_fn_post(&mut self, _: &EarlyContext,
Expand Down
47 changes: 47 additions & 0 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1386,3 +1386,50 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnreachablePub {
self.perform_lint(cx, "item", impl_item.id, &impl_item.vis, impl_item.span, false);
}
}

/// Lint for trait and lifetime bounds that are (accidentally) accepted by the parser, but
/// ignored later.

pub struct IgnoredGenericBounds;

declare_lint! {
IGNORED_GENERIC_BOUNDS,
Warn,
"these generic bounds are ignored"
}

impl LintPass for IgnoredGenericBounds {
fn get_lints(&self) -> LintArray {
lint_array!(IGNORED_GENERIC_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,
_ => return,
};
// 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");
}
// 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(),
};
if !spans.is_empty() {
cx.span_lint(
IGNORED_GENERIC_BOUNDS,
spans,
"bounds on generic parameters are ignored in type aliases",
);
}
}
}
}
1 change: 1 addition & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
AnonymousParameters,
IllegalFloatLiteralPattern,
UnusedDocComment,
IgnoredGenericBounds,
);

add_early_builtin_with_new!(sess,
Expand Down
41 changes: 41 additions & 0 deletions src/librustc_passes/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,33 @@ impl<'a> AstValidator<'a> {
in patterns")
}
}

fn check_late_bound_lifetime_defs(&self, params: &Vec<GenericParam>) {
// Check: Only lifetime parameters
let non_lifetime_param_spans : Vec<_> = params.iter()
.filter_map(|param| match *param {
GenericParam::Lifetime(_) => None,
GenericParam::Type(ref t) => Some(t.span),
}).collect();
if !non_lifetime_param_spans.is_empty() {
self.err_handler().span_err(non_lifetime_param_spans,
"only lifetime parameters can be used in this context");
}

// Check: No bounds on lifetime parameters
for param in params.iter() {
match *param {
GenericParam::Lifetime(ref l) => {
if !l.bounds.is_empty() {
let spans : Vec<_> = l.bounds.iter().map(|b| b.span).collect();
self.err_handler().span_err(spans,
"lifetime bounds cannot be used in this context");
}
}
GenericParam::Type(_) => {}
}
}
}
}

impl<'a> Visitor<'a> for AstValidator<'a> {
Expand All @@ -157,6 +184,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
struct_span_err!(self.session, span, E0561,
"patterns aren't allowed in function pointer types").emit();
});
self.check_late_bound_lifetime_defs(&bfty.generic_params);
}
TyKind::TraitObject(ref bounds, ..) => {
let mut any_lifetime_bounds = false;
Expand Down Expand Up @@ -417,6 +445,19 @@ impl<'a> Visitor<'a> for AstValidator<'a> {

visit::walk_pat(self, pat)
}

fn visit_where_predicate(&mut self, p: &'a WherePredicate) {
if let &WherePredicate::BoundPredicate(ref bound_predicate) = p {
// A type binding, eg `for<'c> Foo: Send+Clone+'c`
self.check_late_bound_lifetime_defs(&bound_predicate.bound_generic_params);
}
visit::walk_where_predicate(self, p);
}

fn visit_poly_trait_ref(&mut self, t: &'a PolyTraitRef, m: &'a TraitBoundModifier) {
self.check_late_bound_lifetime_defs(&t.bound_generic_params);
visit::walk_poly_trait_ref(self, t, m);
}
}

// Bans nested `impl Trait`, e.g. `impl Into<impl Debug>`.
Expand Down
41 changes: 1 addition & 40 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,39 +355,6 @@ fn is_param<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
}
}

fn ensure_no_param_bounds(tcx: TyCtxt,
span: Span,
generics: &hir::Generics,
thing: &'static str) {
let mut warn = false;

for ty_param in generics.ty_params() {
if !ty_param.bounds.is_empty() {
warn = true;
}
}

for lft_param in generics.lifetimes() {
if !lft_param.bounds.is_empty() {
warn = true;
}
}

if !generics.where_clause.predicates.is_empty() {
warn = true;
}

if warn {
// According to accepted RFC #XXX, we should
// eventually accept these, but it will not be
// part of this PR. Still, convert to warning to
// make bootstrapping easier.
span_warn!(tcx.sess, span, E0122,
"generic bounds are ignored in {}",
thing);
}
}

fn convert_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item_id: ast::NodeId) {
let it = tcx.hir.expect_item(item_id);
debug!("convert: item {} with id {}", it.name, it.id);
Expand Down Expand Up @@ -448,13 +415,7 @@ fn convert_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item_id: ast::NodeId) {
convert_variant_ctor(tcx, struct_def.id());
}
},
hir::ItemTy(_, ref generics) => {
ensure_no_param_bounds(tcx, it.span, generics, "type aliases");
tcx.generics_of(def_id);
tcx.type_of(def_id);
tcx.predicates_of(def_id);
}
hir::ItemStatic(..) | hir::ItemConst(..) | hir::ItemFn(..) => {
hir::ItemTy(..) | hir::ItemStatic(..) | hir::ItemConst(..) | hir::ItemFn(..) => {
tcx.generics_of(def_id);
tcx.type_of(def_id);
tcx.predicates_of(def_id);
Expand Down
21 changes: 1 addition & 20 deletions src/librustc_typeck/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1524,26 +1524,6 @@ static BAR: _ = "test"; // error, explicitly write out the type instead
```
"##,

E0122: r##"
An attempt was made to add a generic constraint to a type alias. This constraint
is entirely ignored. For backwards compatibility, Rust still allows this with a
warning. Consider the example below:

```
trait Foo{}

type MyType<R: Foo> = (R, ());

fn main() {
let t: MyType<u32>;
}
```

We're able to declare a variable of type `MyType<u32>`, despite the fact that
`u32` does not implement `Foo`. As a result, one should avoid using generic
constraints in concert with type aliases.
"##,

E0124: r##"
You declared two fields of a struct with the same name. Erroneous code
example:
Expand Down Expand Up @@ -4758,6 +4738,7 @@ register_diagnostics! {
// E0086,
// E0103,
// E0104,
// E0122, // bounds in type aliases are ignored, turned into proper lint
// E0123,
// E0127,
// E0129,
Expand Down
19 changes: 19 additions & 0 deletions src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,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 @@ -404,6 +413,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.
///
/// E.g. `for<'c> Foo: Send+Clone+'c`
Expand Down
15 changes: 3 additions & 12 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4827,6 +4827,7 @@ impl<'a> Parser<'a> {
}
));
// FIXME: Decide what should be used here, `=` or `==`.
// FIXME: We are just dropping the binders in lifetime_defs on the floor here.
} else if self.eat(&token::Eq) || self.eat(&token::EqEq) {
let rhs_ty = self.parse_ty()?;
where_clause.predicates.push(ast::WherePredicate::EqPredicate(
Expand Down Expand Up @@ -5484,18 +5485,8 @@ impl<'a> Parser<'a> {
self.expect_lt()?;
let params = self.parse_generic_params()?;
self.expect_gt()?;

let first_non_lifetime_param_span = params.iter()
.filter_map(|param| match *param {
ast::GenericParam::Lifetime(_) => None,
ast::GenericParam::Type(ref t) => Some(t.span),
})
.next();

if let Some(span) = first_non_lifetime_param_span {
self.span_err(span, "only lifetime parameters can be used in this context");
}

// We rely on AST validation to rule out invalid cases: There must not be type
// parameters, and the lifetime parameters must not have bounds.
Ok(params)
} else {
Ok(Vec::new())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

type Foo<T: std::ops::Add> = T; //~ WARNING E0122
type A = for<'b, 'a: 'b> fn(); //~ ERROR lifetime bounds cannot be used in this context
type B = for<'b, 'a: 'b,> fn(); //~ ERROR lifetime bounds cannot be used in this context
type C = for<'b, 'a: 'b +> fn(); //~ ERROR lifetime bounds cannot be used in this context
type D = for<'a, T> fn(); //~ ERROR only lifetime parameters can be used in this context
type E = for<T> Fn(); //~ ERROR only lifetime parameters can be used in this context

type Bar<T> where T: std::ops::Add = T; //~ WARNING E0122
fn main() {}
3 changes: 1 addition & 2 deletions src/test/compile-fail/dst-bad-assign-3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@

#![feature(unsized_tuple_coercion)]

type Fat<T: ?Sized> = (isize, &'static str, T);
//~^ WARNING bounds are ignored
type Fat<T> = (isize, &'static str, T);

#[derive(PartialEq,Eq)]
struct Bar;
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/issue-17994.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@

trait Tr {}
type Huh<T> where T: Tr = isize; //~ ERROR type parameter `T` is unused
//~| WARNING E0122
//~| WARNING where clauses are ignored in type aliases
fn main() {}
4 changes: 2 additions & 2 deletions src/test/compile-fail/issue-23046.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@

pub enum Expr<'var, VAR> {
Let(Box<Expr<'var, VAR>>,
Box<for<'v: 'var> Fn(Expr<'v, VAR>) -> Expr<'v, VAR> + 'var>)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the use of bounds was intentional here (#23046).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to check whether they are totally ignored during lowering to ty representation or just not enforced properly (and still may cause ICEs or something).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I will bring the bound back. So far I haven't observed anything that would react to these bounds, and AFAIK @eddyb also said they are ignored.

Are you saying I should check the lowering code whether they are ignored? I'm not familiar with that code at all, not even sure where it lives, but I can certainly try.^^

Box<for<'v> Fn(Expr<'v, VAR>) -> Expr<'v, VAR> + 'var>)
}

pub fn add<'var, VAR>
(a: Expr<'var, VAR>, b: Expr<'var, VAR>) -> Expr<'var, VAR> {
loop {}
}

pub fn let_<'var, VAR, F: for<'v: 'var> Fn(Expr<'v, VAR>) -> Expr<'v, VAR>>
pub fn let_<'var, VAR, F: for<'v> Fn(Expr<'v, VAR>) -> Expr<'v, VAR>>
(a: Expr<'var, VAR>, b: F) -> Expr<'var, VAR> {
loop {}
}
Expand Down
4 changes: 2 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,7 @@ mod traits {
pub trait PubTr {}

pub type Alias<T: PrivTr> = T; //~ ERROR private trait `traits::PrivTr` in public interface
//~^ WARN bounds are ignored in type aliases
//~^ 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 +85,7 @@ mod traits_where {
pub type Alias<T> where T: PrivTr = T;
//~^ ERROR private trait `traits_where::PrivTr` in public interface
//~| WARNING hard error
//~| WARNING E0122
//~| 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
Loading