Skip to content

Commit

Permalink
issue a future-compat lint for constants of invalid type
Browse files Browse the repository at this point in the history
This is a [breaking-change]: according to RFC rust-lang#1445, constants used as
patterns must be of a type that *derives* `Eq`. If you encounter a
problem, you are most likely using a constant in an expression where the
type of the constant is some struct that does not currently implement
`Eq`. Something like the following:

```rust
struct SomeType { ... }
const SOME_CONST: SomeType = ...;

match foo {
    SOME_CONST => ...
}
```

The easiest and most future compatible fix is to annotate the type in
question with `#[derive(Eq)]` (note that merely *implementing* `Eq` is
not enough, it must be *derived*):

```rust
struct SomeType { ... }
const SOME_CONST: SomeType = ...;

match foo {
    SOME_CONST => ...
}
```

Another good option is to rewrite the match arm to use an `if`
condition (this is also particularly good for floating point types,
which implement `PartialEq` but not `Eq`):

```rust
match foo {
    c if c == SOME_CONST => ...
}
```

Finally, a third alternative is to tag the type with
`#[structural_match]`; but this is not recommended, as the attribute is
never expected to be stabilized. Please see RFC rust-lang#1445 for more details.
  • Loading branch information
nikomatsakis committed Mar 25, 2016
1 parent 05baf64 commit f69eb8e
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 9 deletions.
15 changes: 15 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,19 @@ declare_lint! {
"type parameter default erroneously allowed in invalid location"
}

declare_lint! {
pub ILLEGAL_FLOATING_POINT_CONSTANT_PATTERN,
Warn,
"floating-point constants cannot be used in patterns"
}

declare_lint! {
pub ILLEGAL_STRUCT_OR_ENUM_CONSTANT_PATTERN,
Deny,
"constants of struct or enum type can only be used in a pattern if \
the struct or enum has `#[derive(Eq)]`"
}

declare_lint! {
pub MATCH_OF_UNIT_VARIANT_VIA_PAREN_DOTDOT,
Deny,
Expand Down Expand Up @@ -193,6 +206,8 @@ impl LintPass for HardwiredLints {
PRIVATE_IN_PUBLIC,
INACCESSIBLE_EXTERN_CRATE,
INVALID_TYPE_PARAM_DEFAULT,
ILLEGAL_FLOATING_POINT_CONSTANT_PATTERN,
ILLEGAL_STRUCT_OR_ENUM_CONSTANT_PATTERN,
MATCH_OF_UNIT_VARIANT_VIA_PAREN_DOTDOT,
CONST_ERR,
RAW_POINTER_DERIVE,
Expand Down
3 changes: 1 addition & 2 deletions src/librustc/middle/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ impl<'a, 'tcx> Folder for StaticInliner<'a, 'tcx> {
Some(Def::Const(did)) => {
let substs = Some(self.tcx.node_id_item_substs(pat.id).substs);
if let Some((const_expr, _)) = lookup_const_by_id(self.tcx, did, substs) {
match const_expr_to_pat(self.tcx, const_expr, pat.span) {
match const_expr_to_pat(self.tcx, const_expr, pat.id, pat.span) {
Ok(new_pat) => {
if let Some(ref mut map) = self.renaming_map {
// Record any renamings we do here
Expand All @@ -487,7 +487,6 @@ impl<'a, 'tcx> Folder for StaticInliner<'a, 'tcx> {
new_pat
}
Err(def_id) => {
// TODO back-compat
self.failed = true;
self.tcx.sess.span_err(
pat.span,
Expand Down
46 changes: 39 additions & 7 deletions src/librustc/middle/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use self::EvalHint::*;

use front::map as ast_map;
use front::map::blocks::FnLikeNode;
use lint;
use middle::cstore::{self, CrateStore, InlinedItem};
use middle::{infer, subst, traits};
use middle::def::Def;
Expand Down Expand Up @@ -323,13 +324,41 @@ impl ConstVal {
}
}

pub fn const_expr_to_pat(tcx: &ty::TyCtxt, expr: &Expr, span: Span)
pub fn const_expr_to_pat(tcx: &ty::TyCtxt, expr: &Expr, pat_id: ast::NodeId, span: Span)
-> Result<P<hir::Pat>, DefId> {
let pat_ty = tcx.expr_ty(expr);
debug!("expr={:?} pat_ty={:?} pat_id={}", expr, pat_ty, pat_id);
match pat_ty.sty {
ty::TyFloat(_) => {
tcx.sess.add_lint(
lint::builtin::ILLEGAL_FLOATING_POINT_CONSTANT_PATTERN,
pat_id,
span,
format!("floating point constants cannot be used in patterns"));
}
ty::TyEnum(adt_def, _) |
ty::TyStruct(adt_def, _) => {
if !tcx.has_attr(adt_def.did, "structural_match") {
tcx.sess.add_lint(
lint::builtin::ILLEGAL_STRUCT_OR_ENUM_CONSTANT_PATTERN,
pat_id,
span,
format!("to use a constant of type `{}` \
in a pattern, \
`{}` must be annotated with `#[derive(Eq)]`",
tcx.item_path_str(adt_def.did),
tcx.item_path_str(adt_def.did)));
}
}
_ => { }
}

let pat = match expr.node {
hir::ExprTup(ref exprs) =>
PatKind::Tup(try!(exprs.iter()
.map(|expr| const_expr_to_pat(tcx, &expr, span))
.collect())),
.map(|expr| const_expr_to_pat(tcx, &expr,
pat_id, span))
.collect())),

hir::ExprCall(ref callee, ref args) => {
let def = *tcx.def_map.borrow().get(&callee.id).unwrap();
Expand All @@ -347,7 +376,8 @@ pub fn const_expr_to_pat(tcx: &ty::TyCtxt, expr: &Expr, span: Span)
_ => unreachable!()
};
let pats = try!(args.iter()
.map(|expr| const_expr_to_pat(tcx, &**expr, span))
.map(|expr| const_expr_to_pat(tcx, &**expr,
pat_id, span))
.collect());
PatKind::TupleStruct(path, Some(pats))
}
Expand All @@ -359,7 +389,8 @@ pub fn const_expr_to_pat(tcx: &ty::TyCtxt, expr: &Expr, span: Span)
span: codemap::DUMMY_SP,
node: hir::FieldPat {
name: field.name.node,
pat: try!(const_expr_to_pat(tcx, &field.expr, span)),
pat: try!(const_expr_to_pat(tcx, &field.expr,
pat_id, span)),
is_shorthand: false,
},
}))
Expand All @@ -369,7 +400,8 @@ pub fn const_expr_to_pat(tcx: &ty::TyCtxt, expr: &Expr, span: Span)

hir::ExprVec(ref exprs) => {
let pats = try!(exprs.iter()
.map(|expr| const_expr_to_pat(tcx, &expr, span))
.map(|expr| const_expr_to_pat(tcx, &expr,
pat_id, span))
.collect());
PatKind::Vec(pats, None, hir::HirVec::new())
}
Expand All @@ -383,7 +415,7 @@ pub fn const_expr_to_pat(tcx: &ty::TyCtxt, expr: &Expr, span: Span)
Some(Def::AssociatedConst(def_id)) => {
let substs = Some(tcx.node_id_item_substs(expr.id).substs);
let (expr, _ty) = lookup_const_by_id(tcx, def_id, substs).unwrap();
return const_expr_to_pat(tcx, expr, span);
return const_expr_to_pat(tcx, expr, pat_id, span);
},
_ => unreachable!(),
}
Expand Down
8 changes: 8 additions & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,14 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
id: LintId::of(OVERLAPPING_INHERENT_IMPLS),
reference: "issue #22889 <https://github.com/rust-lang/rust/issues/22889>",
},
FutureIncompatibleInfo {
id: LintId::of(ILLEGAL_FLOATING_POINT_CONSTANT_PATTERN),
reference: "RFC 1445 <https://github.com/rust-lang/rfcs/pull/1445>",
},
FutureIncompatibleInfo {
id: LintId::of(ILLEGAL_STRUCT_OR_ENUM_CONSTANT_PATTERN),
reference: "RFC 1445 <https://github.com/rust-lang/rfcs/pull/1445>",
},
]);

// We have one lint pass defined specially
Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/hair/cx/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ impl<'patcx, 'cx, 'tcx> PatCx<'patcx, 'cx, 'tcx> {
Some((const_expr, _const_ty)) => {
match const_eval::const_expr_to_pat(self.cx.tcx,
const_expr,
pat.id,
pat.span) {
Ok(pat) =>
return self.to_pattern(&pat),
Expand Down

0 comments on commit f69eb8e

Please sign in to comment.