Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict constants in patterns #32199

Merged
merged 10 commits into from
Mar 26, 2016
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(PartialEq, 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
25 changes: 17 additions & 8 deletions src/librustc/middle/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
74 changes: 58 additions & 16 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,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();
Expand All @@ -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())
}

Expand All @@ -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 {
Expand Down
8 changes: 7 additions & 1 deletion src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
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
13 changes: 10 additions & 3 deletions src/librustc_mir/hair/cx/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
7 changes: 4 additions & 3 deletions src/libstd/num/f32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I personally would have left this as a match and just changed its first clause to x if x == NEG_INFINITY => x)

}
}

Expand Down
7 changes: 4 additions & 3 deletions src/libstd/num/f64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(as above, I personally would have left this as a match and just changed its first clause to x if x == NEG_INFINITY => x)

}
}

Expand Down
25 changes: 25 additions & 0 deletions src/libsyntax/codemap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1304,6 +1304,31 @@ impl CodeMap {
return a;
}

/// Check if the backtrace `subtrace` contains `suptrace` as a prefix.
pub fn more_specific_trace(&self,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: the name seems to imply that this method ought to produce a more specific backtrace. Perhaps “is_supertrace” or something would be more suitable?

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);
Expand Down
42 changes: 36 additions & 6 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, oops -- that was a debugging remnant, obviously :) should have left a // TODO, like I usually do

}
sp1
}
}

Expand Down
9 changes: 8 additions & 1 deletion src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),

Expand Down Expand Up @@ -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),

Expand Down Expand Up @@ -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);
}
}
Expand Down
1 change: 1 addition & 0 deletions src/libsyntax_ext/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ crate-type = ["dylib"]

[dependencies]
fmt_macros = { path = "../libfmt_macros" }
log = { path = "../liblog" }
syntax = { path = "../libsyntax" }
Loading