summary refs log tree commit diff
path: root/src/libsyntax
diff options
context:
space:
mode:
authorManish Goregaokar <manishsmail@gmail.com>2016-03-26 09:07:21 +0530
committerManish Goregaokar <manishsmail@gmail.com>2016-03-26 09:07:21 +0530
commit128b2ad8298f33984c64b98cfb354bb932b740ef (patch)
tree0a04ffa5af03d56995d8df5f1bc753bc6e5dbc04 /src/libsyntax
parentb8b17a54cf1f6d216d83c5679cf9d7951656e36c (diff)
parent2536ae55a4fe3bb9d96ddd53e790631e76a6a11b (diff)
downloadrust-128b2ad8298f33984c64b98cfb354bb932b740ef.tar.gz
rust-128b2ad8298f33984c64b98cfb354bb932b740ef.zip
Rollup merge of #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 #1445 for more details.

cc https://github.com/rust-lang/rust/issues/31434

r? @pnkfelix
Diffstat (limited to 'src/libsyntax')
-rw-r--r--src/libsyntax/codemap.rs25
-rw-r--r--src/libsyntax/ext/expand.rs42
-rw-r--r--src/libsyntax/feature_gate.rs9
3 files changed, 69 insertions, 7 deletions
diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs
index 804ca6705ec..f771ee95bd1 100644
--- a/src/libsyntax/codemap.rs
+++ b/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);
diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs
index 5bfdab791d6..8550617560d 100644
--- a/src/libsyntax/ext/expand.rs
+++ b/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
     }
 }
 
diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs
index 80e1ae111a2..cd88aaa4597 100644
--- a/src/libsyntax/feature_gate.rs
+++ b/src/libsyntax/feature_gate.rs
@@ -109,6 +109,8 @@ const KNOWN_FEATURES: &'static [(&'static str, &'static str, Option<u32>, Status
     // to bootstrap fix for #5723.
     ("issue_5723_bootstrap", "1.0.0", None, Accepted),
 
+    ("structural_match", "1.8.0", Some(31434), Active),
+
     // A way to temporarily opt out of opt in copy. This will *never* be accepted.
     ("opt_out_copy", "1.0.0", None, Removed),
 
@@ -304,6 +306,11 @@ pub const KNOWN_ATTRIBUTES: &'static [(&'static str, AttributeType, AttributeGat
     ("link_args", Normal, Ungated),
     ("macro_escape", Normal, Ungated),
 
+    // RFC #1445.
+    ("structural_match", Whitelisted, Gated("structural_match",
+                                            "the semantics of constant patterns is \
+                                             not yet settled")),
+
     // Not used any more, but we can't feature gate it
     ("no_stack_check", Normal, Ungated),
 
@@ -682,7 +689,7 @@ impl<'a> Context<'a> {
     fn gate_feature(&self, feature: &str, span: Span, explain: &str) {
         let has_feature = self.has_feature(feature);
         debug!("gate_feature(feature = {:?}, span = {:?}); has? {}", feature, span, has_feature);
-        if !has_feature {
+        if !has_feature && !self.cm.span_allows_unstable(span) {
             emit_feature_err(self.span_handler, feature, span, GateIssue::Language, explain);
         }
     }