From 5bc286806023ef4d63bceec4ba703399ba9ee2f7 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 3 Feb 2016 16:38:48 -0500 Subject: [PATCH 01/10] make `const_expr_to_pat` fallible (but never have it actually fail) --- src/librustc/middle/check_match.rs | 26 +++++++++++++------ src/librustc/middle/const_eval.rs | 40 ++++++++++++++++++----------- src/librustc_mir/hair/cx/pattern.rs | 12 ++++++--- 3 files changed, 52 insertions(+), 26 deletions(-) diff --git a/src/librustc/middle/check_match.rs b/src/librustc/middle/check_match.rs index b5e1d5899967f..77b0995827808 100644 --- a/src/librustc/middle/check_match.rs +++ b/src/librustc/middle/check_match.rs @@ -478,15 +478,25 @@ 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.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) => { + // TODO back-compat + 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, diff --git a/src/librustc/middle/const_eval.rs b/src/librustc/middle/const_eval.rs index a8c2a73e72f51..af1e9d60be441 100644 --- a/src/librustc/middle/const_eval.rs +++ b/src/librustc/middle/const_eval.rs @@ -323,10 +323,13 @@ impl ConstVal { } } -pub fn const_expr_to_pat(tcx: &TyCtxt, expr: &Expr, span: Span) -> P { +pub fn const_expr_to_pat(tcx: &ty::TyCtxt, expr: &Expr, span: Span) + -> Result, DefId> { 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, span)) + .collect())), hir::ExprCall(ref callee, ref args) => { let def = *tcx.def_map.borrow().get(&callee.id).unwrap(); @@ -336,31 +339,38 @@ pub fn const_expr_to_pat(tcx: &TyCtxt, expr: &Expr, span: Span) -> P { 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, 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, 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, span)) + .collect()); PatKind::Vec(pats, None, hir::HirVec::new()) } @@ -381,7 +391,7 @@ pub fn const_expr_to_pat(tcx: &TyCtxt, expr: &Expr, span: Span) -> P { _ => 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 { diff --git a/src/librustc_mir/hair/cx/pattern.rs b/src/librustc_mir/hair/cx/pattern.rs index a987377837417..bfb8d1c401aaa 100644 --- a/src/librustc_mir/hair/cx/pattern.rs +++ b/src/librustc_mir/hair/cx/pattern.rs @@ -90,9 +90,15 @@ 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.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( From 99c2a6b335d953eca64e631f3e9946b1cc6643e1 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 11 Mar 2016 13:27:42 -0500 Subject: [PATCH 02/10] modify #[deriving(Eq)] to emit #[structural_match] to careful use of the span from deriving, we can permit it in stable code if it derives from deriving (not-even-a-pun intended) --- src/libsyntax/feature_gate.rs | 9 +++++- src/libsyntax_ext/deriving/mod.rs | 48 +++++++++++++++++++++++++++++-- src/libsyntax_ext/lib.rs | 1 + 3 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 299b7d8b9ba07..8e7f4876ee2ab 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, 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), @@ -676,7 +683,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); } } diff --git a/src/libsyntax_ext/deriving/mod.rs b/src/libsyntax_ext/deriving/mod.rs index 75de5c56ea139..dbd3cbd4fbaa3 100644 --- a/src/libsyntax_ext/deriving/mod.rs +++ b/src/libsyntax_ext/deriving/mod.rs @@ -78,7 +78,10 @@ fn expand_derive(cx: &mut ExtCtxt, mitem: &MetaItem, annotatable: Annotatable) -> Annotatable { - annotatable.map_item_or(|item| { + debug!("expand_derive: span = {:?}", span); + debug!("expand_derive: mitem = {:?}", mitem); + debug!("expand_derive: annotatable input = {:?}", annotatable); + let annot = annotatable.map_item_or(|item| { item.map(|mut item| { if mitem.value_str().is_some() { cx.span_err(mitem.span, "unexpected value in `derive`"); @@ -107,6 +110,45 @@ fn expand_derive(cx: &mut ExtCtxt, continue; } + // RFC #1445. `#[derive(Eq)]` adds a (trusted) + // `#[structural_match]` attribute. + if &tname[..] == "Eq" { + // This span is **very** sensitive and crucial to + // getting the stability behavior we want. What we + // are doing is marking `#[structural_match]` with + // the span of the `#[deriving(Eq)]` attribute + // (the entire attribute, not just the `Eq` part), + // but with the current backtrace. The current + // backtrace will contain a topmost entry that IS + // this `#[deriving(Eq)]` attribute and with the + // "allow-unstable" flag set to true. + // + // Note that we do NOT use the span of the `Eq` + // text itself. You might think this is + // equivalent, because the `Eq` appears within the + // `#[deriving(Eq)]` attribute, and hence we would + // inherit the "allows unstable" from the + // backtrace. But in fact this is not always the + // case. The actual source text that led to + // deriving can be `#[$attr]`, for example, where + // `$attr == deriving(Eq)`. In that case, the + // "#[structural_match]" would be considered to + // originate not from the deriving call but from + // text outside the deriving call, and hence would + // be forbidden from using unstable + // content. + // + // See tests src/run-pass/rfc1445 for + // examples. --nmatsakis + let span = Span { expn_id: cx.backtrace(), .. span }; + assert!(cx.parse_sess.codemap().span_allows_unstable(span)); + debug!("inserting structural_match with span {:?}", span); + let structural_match = intern_and_get_ident("structural_match"); + item.attrs.push(cx.attribute(span, + cx.meta_word(span, + structural_match))); + } + // #[derive(Foo, Bar)] expands to #[derive_Foo] #[derive_Bar] item.attrs.push(cx.attribute(titem.span, cx.meta_word(titem.span, intern_and_get_ident(&format!("derive_{}", tname))))); @@ -117,7 +159,9 @@ fn expand_derive(cx: &mut ExtCtxt, }, |a| { cx.span_err(span, "`derive` can only be applied to items"); a - }) + }); + debug!("expand_derive: annotatable output = {:?}", annot); + annot } macro_rules! derive_traits { diff --git a/src/libsyntax_ext/lib.rs b/src/libsyntax_ext/lib.rs index 97531d4279d4b..f214ecdc3368d 100644 --- a/src/libsyntax_ext/lib.rs +++ b/src/libsyntax_ext/lib.rs @@ -24,6 +24,7 @@ #![feature(str_char)] extern crate fmt_macros; +#[macro_use] extern crate log; #[macro_use] extern crate syntax; From 05baf645e47a0ed3893f2413696e56be180249ff Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 11 Mar 2016 13:29:06 -0500 Subject: [PATCH 03/10] do not overwrite spans as eagerly this was required to preserve the span from the #[structural_match] attribute -- but honestly I am not 100% sure if it makes sense. --- src/libsyntax/codemap.rs | 25 ++++++++++++++++++++++ src/libsyntax/ext/expand.rs | 42 +++++++++++++++++++++++++++++++------ 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index 804ca6705ecbb..f771ee95bd121 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 5bfdab791d638..8550617560df3 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, fld: &mut MacroExpander) -> P { 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 } } From f69eb8efbe5dbc373426bf0ff021b49f37db41cb Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 11 Mar 2016 13:30:32 -0500 Subject: [PATCH 04/10] 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 f2371c1819fc4..5d257fc7b2f22 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 77b0995827808..3414d509d953f 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 af1e9d60be441..dfeb5a5e3f1ab 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 4288f6258ae0d..9ed21117cebca 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 bfb8d1c401aaa..a582a4622a6d0 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), From 73b4f06b83fd7a7ab4bcc9bf2ac97844f3b27df5 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 11 Mar 2016 15:36:24 -0500 Subject: [PATCH 05/10] suppress duplicate lints --- src/librustc/session/mod.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index b198eda181208..e0982656c69bc 100644 --- a/src/librustc/session/mod.rs +++ b/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))); From 56ebf2b046fd4f0b9d05a90ff1e8a38b62be0325 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 11 Mar 2016 13:36:46 -0500 Subject: [PATCH 06/10] fallout in existing tests --- src/libstd/num/f32.rs | 7 ++++--- src/libstd/num/f64.rs | 7 ++++--- src/test/compile-fail/issue-6804.rs | 8 ++++++++ src/test/debuginfo/constant-in-match-pattern.rs | 3 +++ src/test/run-pass/associated-const-match-patterns.rs | 1 + src/test/run-pass/empty-struct-braces.rs | 3 +++ src/test/run-pass/issue-12860.rs | 2 -- src/test/run-pass/match-arm-statics.rs | 11 ++++++++++- 8 files changed, 33 insertions(+), 9 deletions(-) diff --git a/src/libstd/num/f32.rs b/src/libstd/num/f32.rs index e78d46b22e940..6fc26bb7eed71 100644 --- a/src/libstd/num/f32.rs +++ b/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() } } diff --git a/src/libstd/num/f64.rs b/src/libstd/num/f64.rs index cea5a9edd680b..93e5969a275c3 100644 --- a/src/libstd/num/f64.rs +++ b/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() } } diff --git a/src/test/compile-fail/issue-6804.rs b/src/test/compile-fail/issue-6804.rs index ffab194149e12..1cb5dbccf2105 100644 --- a/src/test/compile-fail/issue-6804.rs +++ b/src/test/compile-fail/issue-6804.rs @@ -24,9 +24,17 @@ fn main() { //~ ERROR compilation successful _ => {}, }; //~^^^ WARNING unmatchable NaN in pattern, use the is_nan method in a guard instead + //~| WARNING floating point constants cannot be used + //~| WARNING this was previously accepted + //~| WARNING floating point constants cannot be used + //~| WARNING this was previously accepted match [x, 1.0] { [NAN, _] => {}, _ => {}, }; //~^^^ WARNING unmatchable NaN in pattern, use the is_nan method in a guard instead + //~| WARNING floating point constants cannot be used + //~| WARNING this was previously accepted + //~| WARNING floating point constants cannot be used + //~| WARNING this was previously accepted } diff --git a/src/test/debuginfo/constant-in-match-pattern.rs b/src/test/debuginfo/constant-in-match-pattern.rs index fb40400a4429c..6974238ac72f2 100644 --- a/src/test/debuginfo/constant-in-match-pattern.rs +++ b/src/test/debuginfo/constant-in-match-pattern.rs @@ -21,15 +21,18 @@ const CONSTANT: u64 = 3; +#[derive(PartialEq, Eq)] struct Struct { a: isize, b: usize, } const STRUCT: Struct = Struct { a: 1, b: 2 }; +#[derive(PartialEq, Eq)] struct TupleStruct(u32); const TUPLE_STRUCT: TupleStruct = TupleStruct(4); +#[derive(PartialEq, Eq)] enum Enum { Variant1(char), Variant2 { a: u8 }, diff --git a/src/test/run-pass/associated-const-match-patterns.rs b/src/test/run-pass/associated-const-match-patterns.rs index 605ca6b65e2cf..01d1b27bfc99f 100644 --- a/src/test/run-pass/associated-const-match-patterns.rs +++ b/src/test/run-pass/associated-const-match-patterns.rs @@ -17,6 +17,7 @@ use empty_struct::XEmpty2 as XFoo; struct Foo; +#[derive(PartialEq, Eq)] enum Bar { Var1, Var2, diff --git a/src/test/run-pass/empty-struct-braces.rs b/src/test/run-pass/empty-struct-braces.rs index 85ae77f20f155..0060150fbece0 100644 --- a/src/test/run-pass/empty-struct-braces.rs +++ b/src/test/run-pass/empty-struct-braces.rs @@ -18,7 +18,10 @@ use empty_struct::*; struct Empty1 {} struct Empty2; + +#[derive(PartialEq, Eq)] struct Empty3 {} + const Empty3: Empty3 = Empty3 {}; enum E { diff --git a/src/test/run-pass/issue-12860.rs b/src/test/run-pass/issue-12860.rs index c854747bcf7bd..5c9ee74472b12 100644 --- a/src/test/run-pass/issue-12860.rs +++ b/src/test/run-pass/issue-12860.rs @@ -8,8 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// pretty-expanded FIXME #23616 - #![feature(collections)] extern crate collections; diff --git a/src/test/run-pass/match-arm-statics.rs b/src/test/run-pass/match-arm-statics.rs index 43ff69fe75e6c..9700ed247959b 100644 --- a/src/test/run-pass/match-arm-statics.rs +++ b/src/test/run-pass/match-arm-statics.rs @@ -9,18 +9,24 @@ // except according to those terms. +#[derive(PartialEq, Eq)] struct NewBool(bool); +#[derive(PartialEq, Eq)] enum Direction { North, East, South, West } + +#[derive(PartialEq, Eq)] struct Foo { bar: Option, baz: NewBool } + +#[derive(PartialEq, Eq)] enum EnumWithStructVariants { Variant1(bool), Variant2 { @@ -37,7 +43,7 @@ const VARIANT2_NORTH: EnumWithStructVariants = EnumWithStructVariants::Variant2 dir: Direction::North }; pub mod glfw { - #[derive(Copy, Clone)] + #[derive(Copy, Clone, PartialEq, Eq)] pub struct InputState(usize); pub const RELEASE : InputState = InputState(0); @@ -82,6 +88,7 @@ fn issue_14576() { _ => unreachable!() } + #[derive(PartialEq, Eq)] enum C { D = 3, E = 4 } const F : C = C::D; @@ -89,6 +96,7 @@ fn issue_14576() { } fn issue_13731() { + #[derive(PartialEq, Eq)] enum A { AA(()) } const B: A = A::AA(()); @@ -99,6 +107,7 @@ fn issue_13731() { fn issue_15393() { #![allow(dead_code)] + #[derive(PartialEq, Eq)] struct Flags { bits: usize } From 7f661ec417616ea7036f0cc4aefc7034592a3647 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 11 Mar 2016 13:36:55 -0500 Subject: [PATCH 07/10] new tests for RFC #1445 --- src/test/compile-fail/issue-6804.rs | 4 -- src/test/compile-fail/rfc1445/feature-gate.rs | 36 +++++++++++++++++ .../rfc1445/match-forbidden-without-eq.rs | 39 +++++++++++++++++++ .../rfc1445/eq-allows-match-on-ty-in-macro.rs | 32 +++++++++++++++ src/test/run-pass/rfc1445/eq-allows-match.rs | 26 +++++++++++++ 5 files changed, 133 insertions(+), 4 deletions(-) create mode 100644 src/test/compile-fail/rfc1445/feature-gate.rs create mode 100644 src/test/compile-fail/rfc1445/match-forbidden-without-eq.rs create mode 100644 src/test/run-pass/rfc1445/eq-allows-match-on-ty-in-macro.rs create mode 100644 src/test/run-pass/rfc1445/eq-allows-match.rs diff --git a/src/test/compile-fail/issue-6804.rs b/src/test/compile-fail/issue-6804.rs index 1cb5dbccf2105..f6b7e13c4f5e9 100644 --- a/src/test/compile-fail/issue-6804.rs +++ b/src/test/compile-fail/issue-6804.rs @@ -26,8 +26,6 @@ fn main() { //~ ERROR compilation successful //~^^^ WARNING unmatchable NaN in pattern, use the is_nan method in a guard instead //~| WARNING floating point constants cannot be used //~| WARNING this was previously accepted - //~| WARNING floating point constants cannot be used - //~| WARNING this was previously accepted match [x, 1.0] { [NAN, _] => {}, _ => {}, @@ -35,6 +33,4 @@ fn main() { //~ ERROR compilation successful //~^^^ WARNING unmatchable NaN in pattern, use the is_nan method in a guard instead //~| WARNING floating point constants cannot be used //~| WARNING this was previously accepted - //~| WARNING floating point constants cannot be used - //~| WARNING this was previously accepted } diff --git a/src/test/compile-fail/rfc1445/feature-gate.rs b/src/test/compile-fail/rfc1445/feature-gate.rs new file mode 100644 index 0000000000000..1f2d7819e26d8 --- /dev/null +++ b/src/test/compile-fail/rfc1445/feature-gate.rs @@ -0,0 +1,36 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that structural match is only permitted with a feature gate, +// and that if a feature gate is supplied, it permits the type to be +// used in a match. + +// revisions: with_gate no_gate + +#![allow(dead_code)] +#![deny(future_incompatible)] +#![feature(rustc_attrs)] +#![cfg_attr(with_gate, feature(structural_match))] + +#[structural_match] //[no_gate]~ ERROR semantics of constant patterns is not yet settled +struct Foo { + x: u32 +} + +const FOO: Foo = Foo { x: 0 }; + +#[rustc_error] +fn main() { //[with_gate]~ ERROR compilation successful + let y = Foo { x: 1 }; + match y { + FOO => { } + _ => { } + } +} diff --git a/src/test/compile-fail/rfc1445/match-forbidden-without-eq.rs b/src/test/compile-fail/rfc1445/match-forbidden-without-eq.rs new file mode 100644 index 0000000000000..b3e465688fdd4 --- /dev/null +++ b/src/test/compile-fail/rfc1445/match-forbidden-without-eq.rs @@ -0,0 +1,39 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![allow(dead_code)] +#![deny(future_incompatible)] + +use std::f32; + +#[derive(PartialEq)] +struct Foo { + x: u32 +} + +const FOO: Foo = Foo { x: 0 }; + +fn main() { + let y = Foo { x: 1 }; + match y { + FOO => { } + //~^ ERROR must be annotated with `#[derive(Eq)]` + //~| WARNING will become a hard error + _ => { } + } + + let x = 0.0; + match x { + f32::INFINITY => { } + //~^ ERROR floating point constants cannot be used in patterns + //~| WARNING will become a hard error + _ => { } + } +} diff --git a/src/test/run-pass/rfc1445/eq-allows-match-on-ty-in-macro.rs b/src/test/run-pass/rfc1445/eq-allows-match-on-ty-in-macro.rs new file mode 100644 index 0000000000000..241fe6c6ab1e9 --- /dev/null +++ b/src/test/run-pass/rfc1445/eq-allows-match-on-ty-in-macro.rs @@ -0,0 +1,32 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![allow(dead_code)] + +macro_rules! foo { + (#[$attr:meta] $x:ident) => { + #[$attr] + struct $x { + x: u32 + } + } +} + +foo! { #[derive(PartialEq, Eq)] Foo } + +const FOO: Foo = Foo { x: 0 }; + +fn main() { + let y = Foo { x: 1 }; + match y { + FOO => { } + _ => { } + } +} diff --git a/src/test/run-pass/rfc1445/eq-allows-match.rs b/src/test/run-pass/rfc1445/eq-allows-match.rs new file mode 100644 index 0000000000000..f02a45625c9f6 --- /dev/null +++ b/src/test/run-pass/rfc1445/eq-allows-match.rs @@ -0,0 +1,26 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![allow(dead_code)] + +#[derive(PartialEq, Eq)] +struct Foo { + x: u32 +} + +const FOO: Foo = Foo { x: 0 }; + +fn main() { + let y = Foo { x: 1 }; + match y { + FOO => { } + _ => { } + } +} From 93e44432e1055a529c4923709a6533114bb7820f Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 25 Mar 2016 10:02:56 -0400 Subject: [PATCH 08/10] check for both partialeq and eq --- src/librustc/lint/builtin.rs | 2 +- src/librustc/middle/const_eval.rs | 2 +- src/libsyntax_ext/deriving/mod.rs | 81 ++++++++++--------- .../match-requires-both-partialeq-and-eq.rs | 35 ++++++++ 4 files changed, 82 insertions(+), 38 deletions(-) create mode 100644 src/test/compile-fail/rfc1445/match-requires-both-partialeq-and-eq.rs diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 5d257fc7b2f22..879b894562092 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -146,7 +146,7 @@ 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)]`" + the struct or enum has `#[derive(PartialEq, Eq)]`" } declare_lint! { diff --git a/src/librustc/middle/const_eval.rs b/src/librustc/middle/const_eval.rs index dfeb5a5e3f1ab..c102822e8c126 100644 --- a/src/librustc/middle/const_eval.rs +++ b/src/librustc/middle/const_eval.rs @@ -345,7 +345,7 @@ pub fn const_expr_to_pat(tcx: &ty::TyCtxt, expr: &Expr, pat_id: ast::NodeId, spa span, format!("to use a constant of type `{}` \ in a pattern, \ - `{}` must be annotated with `#[derive(Eq)]`", + `{}` must be annotated with `#[derive(PartialEq, Eq)]`", tcx.item_path_str(adt_def.did), tcx.item_path_str(adt_def.did))); } diff --git a/src/libsyntax_ext/deriving/mod.rs b/src/libsyntax_ext/deriving/mod.rs index dbd3cbd4fbaa3..1774167e83000 100644 --- a/src/libsyntax_ext/deriving/mod.rs +++ b/src/libsyntax_ext/deriving/mod.rs @@ -92,6 +92,9 @@ fn expand_derive(cx: &mut ExtCtxt, cx.span_warn(mitem.span, "empty trait list in `derive`"); } + let mut found_partial_eq = false; + let mut found_eq = false; + for titem in traits.iter().rev() { let tname = match titem.node { MetaItemKind::Word(ref tname) => tname, @@ -110,43 +113,10 @@ fn expand_derive(cx: &mut ExtCtxt, continue; } - // RFC #1445. `#[derive(Eq)]` adds a (trusted) - // `#[structural_match]` attribute. if &tname[..] == "Eq" { - // This span is **very** sensitive and crucial to - // getting the stability behavior we want. What we - // are doing is marking `#[structural_match]` with - // the span of the `#[deriving(Eq)]` attribute - // (the entire attribute, not just the `Eq` part), - // but with the current backtrace. The current - // backtrace will contain a topmost entry that IS - // this `#[deriving(Eq)]` attribute and with the - // "allow-unstable" flag set to true. - // - // Note that we do NOT use the span of the `Eq` - // text itself. You might think this is - // equivalent, because the `Eq` appears within the - // `#[deriving(Eq)]` attribute, and hence we would - // inherit the "allows unstable" from the - // backtrace. But in fact this is not always the - // case. The actual source text that led to - // deriving can be `#[$attr]`, for example, where - // `$attr == deriving(Eq)`. In that case, the - // "#[structural_match]" would be considered to - // originate not from the deriving call but from - // text outside the deriving call, and hence would - // be forbidden from using unstable - // content. - // - // See tests src/run-pass/rfc1445 for - // examples. --nmatsakis - let span = Span { expn_id: cx.backtrace(), .. span }; - assert!(cx.parse_sess.codemap().span_allows_unstable(span)); - debug!("inserting structural_match with span {:?}", span); - let structural_match = intern_and_get_ident("structural_match"); - item.attrs.push(cx.attribute(span, - cx.meta_word(span, - structural_match))); + found_eq = true; + } else if &tname[..] == "PartialEq" { + found_partial_eq = true; } // #[derive(Foo, Bar)] expands to #[derive_Foo] #[derive_Bar] @@ -154,6 +124,45 @@ fn expand_derive(cx: &mut ExtCtxt, intern_and_get_ident(&format!("derive_{}", tname))))); } + // RFC #1445. `#[derive(PartialEq, Eq)]` adds a (trusted) + // `#[structural_match]` attribute. + if found_partial_eq && found_eq { + // This span is **very** sensitive and crucial to + // getting the stability behavior we want. What we are + // doing is marking `#[structural_match]` with the + // span of the `#[deriving(...)]` attribute (the + // entire attribute, not just the `PartialEq` or `Eq` + // part), but with the current backtrace. The current + // backtrace will contain a topmost entry that IS this + // `#[deriving(...)]` attribute and with the + // "allow-unstable" flag set to true. + // + // Note that we do NOT use the span of the `Eq` + // text itself. You might think this is + // equivalent, because the `Eq` appears within the + // `#[deriving(Eq)]` attribute, and hence we would + // inherit the "allows unstable" from the + // backtrace. But in fact this is not always the + // case. The actual source text that led to + // deriving can be `#[$attr]`, for example, where + // `$attr == deriving(Eq)`. In that case, the + // "#[structural_match]" would be considered to + // originate not from the deriving call but from + // text outside the deriving call, and hence would + // be forbidden from using unstable + // content. + // + // See tests src/run-pass/rfc1445 for + // examples. --nmatsakis + let span = Span { expn_id: cx.backtrace(), .. span }; + assert!(cx.parse_sess.codemap().span_allows_unstable(span)); + debug!("inserting structural_match with span {:?}", span); + let structural_match = intern_and_get_ident("structural_match"); + item.attrs.push(cx.attribute(span, + cx.meta_word(span, + structural_match))); + } + item }) }, |a| { diff --git a/src/test/compile-fail/rfc1445/match-requires-both-partialeq-and-eq.rs b/src/test/compile-fail/rfc1445/match-requires-both-partialeq-and-eq.rs new file mode 100644 index 0000000000000..029df08ebc37a --- /dev/null +++ b/src/test/compile-fail/rfc1445/match-requires-both-partialeq-and-eq.rs @@ -0,0 +1,35 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![allow(dead_code)] +#![deny(future_incompatible)] + +#[derive(Eq)] +struct Foo { + x: u32 +} + +impl PartialEq for Foo { + fn eq(&self, _: &Foo) -> bool { + false // ha ha sucker! + } +} + +const FOO: Foo = Foo { x: 0 }; + +fn main() { + let y = Foo { x: 1 }; + match y { + FOO => { } + //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` + //~| WARNING will become a hard error + _ => { } + } +} From 944dc4aa654633c0b027fa6fe30f5ed0690a73b9 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 25 Mar 2016 14:39:24 -0400 Subject: [PATCH 09/10] fix cargo.toml for new dependency --- src/libsyntax_ext/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libsyntax_ext/Cargo.toml b/src/libsyntax_ext/Cargo.toml index e137815cd32fd..671f3e4a7e330 100644 --- a/src/libsyntax_ext/Cargo.toml +++ b/src/libsyntax_ext/Cargo.toml @@ -10,4 +10,5 @@ crate-type = ["dylib"] [dependencies] fmt_macros = { path = "../libfmt_macros" } +log = { path = "../liblog" } syntax = { path = "../libsyntax" } From 2536ae55a4fe3bb9d96ddd53e790631e76a6a11b Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 25 Mar 2016 15:14:45 -0400 Subject: [PATCH 10/10] fix error message --- src/test/compile-fail/rfc1445/match-forbidden-without-eq.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/compile-fail/rfc1445/match-forbidden-without-eq.rs b/src/test/compile-fail/rfc1445/match-forbidden-without-eq.rs index b3e465688fdd4..c573e3e8e28b2 100644 --- a/src/test/compile-fail/rfc1445/match-forbidden-without-eq.rs +++ b/src/test/compile-fail/rfc1445/match-forbidden-without-eq.rs @@ -24,7 +24,7 @@ fn main() { let y = Foo { x: 1 }; match y { FOO => { } - //~^ ERROR must be annotated with `#[derive(Eq)]` + //~^ ERROR must be annotated with `#[derive(PartialEq, Eq)]` //~| WARNING will become a hard error _ => { } }