Rollup merge of rust-lang#32199 - nikomatsakis:limiting-constants-in-patterns-2, r=pnkfelix

Restrict constants in patterns

This implements [RFC 1445](https://github.com/rust-lang/rfcs/blob/master/text/1445-restrict-constants-in-patterns.md). The primary change is to limit the types of constants used in patterns to those that *derive* `Eq` (note that implementing `Eq` is not sufficient). This has two main effects:

1. Floating point constants are linted, and will eventually be disallowed. This is because floating point constants do not implement `Eq` but only `PartialEq`. This check replaces the existing special case code that aimed to detect the use of `NaN`.
2. Structs and enums must derive `Eq` to be usable within a match.

This is a [breaking-change]: 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.

cc rust-lang#31434

r? @pnkfelix

Author: Manishearth

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(PartialEq, 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,

src/librustc/middle/check_match.rs

@@ -478,15 +478,24 @@ 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) {
-                            const_expr_to_pat(self.tcx, const_expr, pat.span).map(|new_pat| {
-
-                                if let Some(ref mut renaming_map) = self.renaming_map {
-                                    // Record any renamings we do here
-                                    record_renamings(const_expr, &pat, renaming_map);
+                            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
+                                        record_renamings(const_expr, &pat, map);
+                                    }
+                                    new_pat
                                 }
-
-                                new_pat
-                            })
+                                Err(def_id) => {
+                                    self.failed = true;
+                                    self.tcx.sess.span_err(
+                                        pat.span,
+                                        &format!("constants of the type `{}` \
+                                                  cannot be used in patterns",
+                                                 self.tcx.item_path_str(def_id)));
+                                    pat
+                                }
+                            }
                         } else {
                             self.failed = true;
                             span_err!(self.tcx.sess, pat.span, E0158,

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,10 +324,41 @@ impl ConstVal {
     }
 }
 
-pub fn const_expr_to_pat(tcx: &TyCtxt, expr: &Expr, span: Span) -> P<hir::Pat> {
+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(PartialEq, 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(exprs.iter().map(|expr| const_expr_to_pat(tcx, &expr, span)).collect()),
+            PatKind::Tup(try!(exprs.iter()
+                                  .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();
@@ -336,31 +368,41 @@ pub fn const_expr_to_pat(tcx: &TyCtxt, expr: &Expr, span: Span) -> P<hir::Pat> {
             let path = match def.full_def() {
                 Def::Struct(def_id) => def_to_path(tcx, def_id),
                 Def::Variant(_, variant_did) => def_to_path(tcx, variant_did),
-                Def::Fn(..) => return P(hir::Pat {
+                Def::Fn(..) => return Ok(P(hir::Pat {
                     id: expr.id,
                     node: PatKind::Lit(P(expr.clone())),
                     span: span,
-                }),
+                })),
                 _ => unreachable!()
             };
-            let pats = args.iter().map(|expr| const_expr_to_pat(tcx, &expr, span)).collect();
+            let pats = try!(args.iter()
+                                .map(|expr| const_expr_to_pat(tcx, &**expr,
+                                                              pat_id, span))
+                                .collect());
             PatKind::TupleStruct(path, Some(pats))
         }
 
         hir::ExprStruct(ref path, ref fields, None) => {
-            let field_pats = fields.iter().map(|field| codemap::Spanned {
-                span: codemap::DUMMY_SP,
-                node: hir::FieldPat {
-                    name: field.name.node,
-                    pat: const_expr_to_pat(tcx, &field.expr, span),
-                    is_shorthand: false,
-                },
-            }).collect();
+            let field_pats =
+                try!(fields.iter()
+                           .map(|field| Ok(codemap::Spanned {
+                               span: codemap::DUMMY_SP,
+                               node: hir::FieldPat {
+                                   name: field.name.node,
+                                   pat: try!(const_expr_to_pat(tcx, &field.expr,
+                                                               pat_id, span)),
+                                   is_shorthand: false,
+                               },
+                           }))
+                           .collect());
             PatKind::Struct(path.clone(), field_pats, false)
         }
 
         hir::ExprVec(ref exprs) => {
-            let pats = exprs.iter().map(|expr| const_expr_to_pat(tcx, &expr, span)).collect();
+            let pats = try!(exprs.iter()
+                                 .map(|expr| const_expr_to_pat(tcx, &expr,
+                                                               pat_id, span))
+                                 .collect());
             PatKind::Vec(pats, None, hir::HirVec::new())
         }
 
@@ -373,15 +415,15 @@ pub fn const_expr_to_pat(tcx: &TyCtxt, expr: &Expr, span: Span) -> P<hir::Pat> {
                 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!(),
             }
         }
 
         _ => PatKind::Lit(P(expr.clone()))
     };
-    P(hir::Pat { id: expr.id, node: pat, span: span })
+    Ok(P(hir::Pat { id: expr.id, node: pat, span: span }))
 }
 
 pub fn eval_const_expr(tcx: &TyCtxt, e: &Expr) -> ConstVal {

src/librustc/session/mod.rs

@@ -246,7 +246,13 @@ impl Session {
         let lint_id = lint::LintId::of(lint);
         let mut lints = self.lints.borrow_mut();
         match lints.get_mut(&id) {
-            Some(arr) => { arr.push((lint_id, sp, msg)); return; }
+            Some(arr) => {
+                let tuple = (lint_id, sp, msg);
+                if !arr.contains(&tuple) {
+                    arr.push(tuple);
+                }
+                return;
+            }
             None => {}
         }
         lints.insert(id, vec!((lint_id, sp, msg)));

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 <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

src/librustc_mir/hair/cx/pattern.rs

@@ -90,9 +90,16 @@ impl<'patcx, 'cx, 'tcx> PatCx<'patcx, 'cx, 'tcx> {
                         let substs = Some(self.cx.tcx.node_id_item_substs(pat.id).substs);
                         match const_eval::lookup_const_by_id(self.cx.tcx, def_id, substs) {
                             Some((const_expr, _const_ty)) => {
-                                let pat = const_eval::const_expr_to_pat(self.cx.tcx, const_expr,
-                                                                        pat.span);
-                                return self.to_pattern(&pat);
+                                match const_eval::const_expr_to_pat(self.cx.tcx,
+                                                                    const_expr,
+                                                                    pat.id,
+                                                                    pat.span) {
+                                    Ok(pat) =>
+                                        return self.to_pattern(&pat),
+                                    Err(_) =>
+                                        self.cx.tcx.sess.span_bug(
+                                            pat.span, "illegal constant"),
+                                }
                             }
                             None => {
                                 self.cx.tcx.sess.span_bug(

src/libstd/num/f32.rs

@@ -1152,9 +1152,10 @@ impl f32 {
     #[stable(feature = "rust1", since = "1.0.0")]
     #[inline]
     pub fn asinh(self) -> f32 {
-        match self {
-            NEG_INFINITY => NEG_INFINITY,
-            x => (x + ((x * x) + 1.0).sqrt()).ln(),
+        if self == NEG_INFINITY {
+            NEG_INFINITY
+        } else {
+            (self + ((self * self) + 1.0).sqrt()).ln()
         }
     }
 

src/libstd/num/f64.rs

@@ -1023,9 +1023,10 @@ impl f64 {
     #[stable(feature = "rust1", since = "1.0.0")]
     #[inline]
     pub fn asinh(self) -> f64 {
-        match self {
-            NEG_INFINITY => NEG_INFINITY,
-            x => (x + ((x * x) + 1.0).sqrt()).ln(),
+        if self == NEG_INFINITY {
+            NEG_INFINITY
+        } else {
+            (self + ((self * self) + 1.0).sqrt()).ln()
         }
     }
 

src/libsyntax/codemap.rs

@@ -1304,6 +1304,31 @@ impl CodeMap {
         return a;
     }
 
+    /// Check if the backtrace `subtrace` contains `suptrace` as a prefix.
+    pub fn more_specific_trace(&self,
+                              mut subtrace: ExpnId,
+                              suptrace: ExpnId)
+                              -> bool {
+        loop {
+            if subtrace == suptrace {
+                return true;
+            }
+
+            let stop = self.with_expn_info(subtrace, |opt_expn_info| {
+                if let Some(expn_info) = opt_expn_info {
+                    subtrace = expn_info.call_site.expn_id;
+                    false
+                } else {
+                    true
+                }
+            });
+
+            if stop {
+                return false;
+            }
+        }
+    }
+
     pub fn record_expansion(&self, expn_info: ExpnInfo) -> ExpnId {
         let mut expansions = self.expansions.borrow_mut();
         expansions.push(expn_info);

src/libsyntax/ext/expand.rs

@@ -33,7 +33,7 @@ use visit::Visitor;
 use std_inject;
 
 use std::collections::HashSet;
-
+use std::env;
 
 pub fn expand_expr(e: P<ast::Expr>, fld: &mut MacroExpander) -> P<ast::Expr> {
     let expr_span = e.span;
@@ -1275,11 +1275,41 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
 }
 
 fn new_span(cx: &ExtCtxt, sp: Span) -> Span {
-    /* this discards information in the case of macro-defining macros */
-    Span {
-        lo: sp.lo,
-        hi: sp.hi,
-        expn_id: cx.backtrace(),
+    debug!("new_span(sp={:?})", sp);
+
+    if cx.codemap().more_specific_trace(sp.expn_id, cx.backtrace()) {
+        // If the span we are looking at has a backtrace that has more
+        // detail than our current backtrace, then we keep that
+        // backtrace.  Honestly, I have no idea if this makes sense,
+        // because I have no idea why we are stripping the backtrace
+        // below. But the reason I made this change is because, in
+        // deriving, we were generating attributes with a specific
+        // backtrace, which was essential for `#[structural_match]` to
+        // be properly supported, but these backtraces were being
+        // stripped and replaced with a null backtrace. Sort of
+        // unclear why this is the case. --nmatsakis
+        debug!("new_span: keeping trace from {:?} because it is more specific",
+               sp.expn_id);
+        sp
+    } else {
+        // This discards information in the case of macro-defining macros.
+        //
+        // The comment above was originally added in
+        // b7ec2488ff2f29681fe28691d20fd2c260a9e454 in Feb 2012. I
+        // *THINK* the reason we are doing this is because we want to
+        // replace the backtrace of the macro contents with the
+        // backtrace that contains the macro use. But it's pretty
+        // unclear to me. --nmatsakis
+        let sp1 = Span {
+            lo: sp.lo,
+            hi: sp.hi,
+            expn_id: cx.backtrace(),
+        };
+        debug!("new_span({:?}) = {:?}", sp, sp1);
+        if sp.expn_id.into_u32() == 0 && env::var_os("NDM").is_some() {
+            panic!("NDM");
+        }
+        sp1
     }
 }