From f69eb8efbe5dbc373426bf0ff021b49f37db41cb Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 11 Mar 2016 13:30:32 -0500 Subject: [PATCH] issue a future-compat lint for constants of invalid type This is a [breaking-change]: according to RFC #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 #1445 for more details. --- src/librustc/lint/builtin.rs | 15 ++++++++++ src/librustc/middle/check_match.rs | 3 +- src/librustc/middle/const_eval.rs | 46 ++++++++++++++++++++++++----- src/librustc_lint/lib.rs | 8 +++++ src/librustc_mir/hair/cx/pattern.rs | 1 + 5 files changed, 64 insertions(+), 9 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index f2371c1819f..5d257fc7b2f 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -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, @@ -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, diff --git a/src/librustc/middle/check_match.rs b/src/librustc/middle/check_match.rs index 77b09958278..3414d509d95 100644 --- a/src/librustc/middle/check_match.rs +++ b/src/librustc/middle/check_match.rs @@ -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 @@ -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, diff --git a/src/librustc/middle/const_eval.rs b/src/librustc/middle/const_eval.rs index af1e9d60be4..dfeb5a5e3f1 100644 --- a/src/librustc/middle/const_eval.rs +++ b/src/librustc/middle/const_eval.rs @@ -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; @@ -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, 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(); @@ -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)) } @@ -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, }, })) @@ -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()) } @@ -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!(), } diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 4288f6258ae..9ed21117ceb 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -179,6 +179,14 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { id: LintId::of(OVERLAPPING_INHERENT_IMPLS), reference: "issue #22889 ", }, + FutureIncompatibleInfo { + id: LintId::of(ILLEGAL_FLOATING_POINT_CONSTANT_PATTERN), + reference: "RFC 1445 ", + }, + FutureIncompatibleInfo { + id: LintId::of(ILLEGAL_STRUCT_OR_ENUM_CONSTANT_PATTERN), + reference: "RFC 1445 ", + }, ]); // We have one lint pass defined specially diff --git a/src/librustc_mir/hair/cx/pattern.rs b/src/librustc_mir/hair/cx/pattern.rs index bfb8d1c401a..a582a4622a6 100644 --- a/src/librustc_mir/hair/cx/pattern.rs +++ b/src/librustc_mir/hair/cx/pattern.rs @@ -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),