auto merge of #19115 : jakub-/rust/issue-19100, r=alexcrichton
...of the type being matched. This change will result in a better diagnostic for code like the following: ```rust enum Enum { Foo, Bar } fn f(x: Enum) { match x { Foo => (), Bar => () } } ``` which would currently simply fail with an unreachable pattern error on the 2nd arm. The user is advised to either use a qualified path in the patterns or import the variants explicitly into the scope.
This commit is contained in:
commit
fac5a07679
8 changed files with 153 additions and 32 deletions
|
@ -145,5 +145,6 @@ register_diagnostics!(
|
||||||
E0166,
|
E0166,
|
||||||
E0167,
|
E0167,
|
||||||
E0168,
|
E0168,
|
||||||
E0169
|
E0169,
|
||||||
|
E0170
|
||||||
)
|
)
|
||||||
|
|
|
@ -153,19 +153,14 @@ fn check_expr(cx: &mut MatchCheckCtxt, ex: &ast::Expr) {
|
||||||
visit::walk_expr(cx, ex);
|
visit::walk_expr(cx, ex);
|
||||||
match ex.node {
|
match ex.node {
|
||||||
ast::ExprMatch(ref scrut, ref arms, source) => {
|
ast::ExprMatch(ref scrut, ref arms, source) => {
|
||||||
// First, check legality of move bindings.
|
|
||||||
for arm in arms.iter() {
|
for arm in arms.iter() {
|
||||||
|
// First, check legality of move bindings.
|
||||||
check_legality_of_move_bindings(cx,
|
check_legality_of_move_bindings(cx,
|
||||||
arm.guard.is_some(),
|
arm.guard.is_some(),
|
||||||
arm.pats.as_slice());
|
arm.pats.as_slice());
|
||||||
for pat in arm.pats.iter() {
|
|
||||||
check_legality_of_bindings_in_at_patterns(cx, &**pat);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Second, if there is a guard on each arm, make sure it isn't
|
// Second, if there is a guard on each arm, make sure it isn't
|
||||||
// assigning or borrowing anything mutably.
|
// assigning or borrowing anything mutably.
|
||||||
for arm in arms.iter() {
|
|
||||||
match arm.guard {
|
match arm.guard {
|
||||||
Some(ref guard) => check_for_mutation_in_guard(cx, &**guard),
|
Some(ref guard) => check_for_mutation_in_guard(cx, &**guard),
|
||||||
None => {}
|
None => {}
|
||||||
|
@ -179,13 +174,23 @@ fn check_expr(cx: &mut MatchCheckCtxt, ex: &ast::Expr) {
|
||||||
}).collect(), arm.guard.as_ref().map(|e| &**e))
|
}).collect(), arm.guard.as_ref().map(|e| &**e))
|
||||||
}).collect::<Vec<(Vec<P<Pat>>, Option<&ast::Expr>)>>();
|
}).collect::<Vec<(Vec<P<Pat>>, Option<&ast::Expr>)>>();
|
||||||
|
|
||||||
|
// Bail out early if inlining failed.
|
||||||
if static_inliner.failed {
|
if static_inliner.failed {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Third, check if there are any references to NaN that we should warn about.
|
for pat in inlined_arms
|
||||||
for &(ref pats, _) in inlined_arms.iter() {
|
.iter()
|
||||||
check_for_static_nan(cx, pats.as_slice());
|
.flat_map(|&(ref pats, _)| pats.iter()) {
|
||||||
|
// Third, check legality of move bindings.
|
||||||
|
check_legality_of_bindings_in_at_patterns(cx, &**pat);
|
||||||
|
|
||||||
|
// Fourth, check if there are any references to NaN that we should warn about.
|
||||||
|
check_for_static_nan(cx, &**pat);
|
||||||
|
|
||||||
|
// Fifth, check if for any of the patterns that match an enumerated type
|
||||||
|
// are bindings with the same name as one of the variants of said type.
|
||||||
|
check_for_bindings_named_the_same_as_variants(cx, &**pat);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Fourth, check for unreachable arms.
|
// Fourth, check for unreachable arms.
|
||||||
|
@ -239,10 +244,39 @@ fn is_expr_const_nan(tcx: &ty::ctxt, expr: &ast::Expr) -> bool {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn check_for_bindings_named_the_same_as_variants(cx: &MatchCheckCtxt, pat: &Pat) {
|
||||||
|
walk_pat(pat, |p| {
|
||||||
|
match p.node {
|
||||||
|
ast::PatIdent(ast::BindByValue(ast::MutImmutable), ident, None) => {
|
||||||
|
let pat_ty = ty::pat_ty(cx.tcx, p);
|
||||||
|
if let ty::ty_enum(def_id, _) = pat_ty.sty {
|
||||||
|
let def = cx.tcx.def_map.borrow().get(&p.id).cloned();
|
||||||
|
if let Some(DefLocal(_)) = def {
|
||||||
|
if ty::enum_variants(cx.tcx, def_id).iter().any(|variant|
|
||||||
|
token::get_name(variant.name) == token::get_name(ident.node.name)
|
||||||
|
&& variant.args.len() == 0
|
||||||
|
) {
|
||||||
|
span_warn!(cx.tcx.sess, p.span, E0170,
|
||||||
|
"pattern binding `{}` is named the same as one \
|
||||||
|
of the variants of the type `{}`",
|
||||||
|
token::get_ident(ident.node).get(), ty_to_string(cx.tcx, pat_ty));
|
||||||
|
span_help!(cx.tcx.sess, p.span,
|
||||||
|
"if you meant to match on a variant, \
|
||||||
|
consider making the path in the pattern qualified: `{}::{}`",
|
||||||
|
ty_to_string(cx.tcx, pat_ty), token::get_ident(ident.node).get());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
_ => ()
|
||||||
|
}
|
||||||
|
true
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
// Check that we do not match against a static NaN (#6804)
|
// Check that we do not match against a static NaN (#6804)
|
||||||
fn check_for_static_nan(cx: &MatchCheckCtxt, pats: &[P<Pat>]) {
|
fn check_for_static_nan(cx: &MatchCheckCtxt, pat: &Pat) {
|
||||||
for pat in pats.iter() {
|
walk_pat(pat, |p| {
|
||||||
walk_pat(&**pat, |p| {
|
|
||||||
match p.node {
|
match p.node {
|
||||||
ast::PatLit(ref expr) if is_expr_const_nan(cx.tcx, &**expr) => {
|
ast::PatLit(ref expr) if is_expr_const_nan(cx.tcx, &**expr) => {
|
||||||
span_warn!(cx.tcx.sess, p.span, E0003,
|
span_warn!(cx.tcx.sess, p.span, E0003,
|
||||||
|
@ -253,7 +287,6 @@ fn check_for_static_nan(cx: &MatchCheckCtxt, pats: &[P<Pat>]) {
|
||||||
}
|
}
|
||||||
true
|
true
|
||||||
});
|
});
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check for unreachable patterns
|
// Check for unreachable patterns
|
||||||
|
@ -414,8 +447,7 @@ fn construct_witness(cx: &MatchCheckCtxt, ctor: &Constructor,
|
||||||
&Variant(vid) =>
|
&Variant(vid) =>
|
||||||
(vid, ty::enum_variant_with_id(cx.tcx, cid, vid).arg_names.is_some()),
|
(vid, ty::enum_variant_with_id(cx.tcx, cid, vid).arg_names.is_some()),
|
||||||
_ =>
|
_ =>
|
||||||
(cid, ty::lookup_struct_fields(cx.tcx, cid).iter()
|
(cid, !ty::is_tuple_struct(cx.tcx, cid))
|
||||||
.any(|field| field.name != token::special_idents::unnamed_field.name))
|
|
||||||
};
|
};
|
||||||
if is_structure {
|
if is_structure {
|
||||||
let fields = ty::lookup_struct_fields(cx.tcx, vid);
|
let fields = ty::lookup_struct_fields(cx.tcx, vid);
|
||||||
|
|
|
@ -948,4 +948,38 @@ mod test {
|
||||||
assert!(test_items.next().is_some());
|
assert!(test_items.next().is_some());
|
||||||
assert!(test_items.next().is_none());
|
assert!(test_items.next().is_none());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_can_print_warnings() {
|
||||||
|
{
|
||||||
|
let matches = getopts(&[
|
||||||
|
"-Awarnings".to_string()
|
||||||
|
], optgroups().as_slice()).unwrap();
|
||||||
|
let registry = diagnostics::registry::Registry::new(&[]);
|
||||||
|
let sessopts = build_session_options(&matches);
|
||||||
|
let sess = build_session(sessopts, None, registry);
|
||||||
|
assert!(!sess.can_print_warnings);
|
||||||
|
}
|
||||||
|
|
||||||
|
{
|
||||||
|
let matches = getopts(&[
|
||||||
|
"-Awarnings".to_string(),
|
||||||
|
"-Dwarnings".to_string()
|
||||||
|
], optgroups().as_slice()).unwrap();
|
||||||
|
let registry = diagnostics::registry::Registry::new(&[]);
|
||||||
|
let sessopts = build_session_options(&matches);
|
||||||
|
let sess = build_session(sessopts, None, registry);
|
||||||
|
assert!(sess.can_print_warnings);
|
||||||
|
}
|
||||||
|
|
||||||
|
{
|
||||||
|
let matches = getopts(&[
|
||||||
|
"-Adead_code".to_string()
|
||||||
|
], optgroups().as_slice()).unwrap();
|
||||||
|
let registry = diagnostics::registry::Registry::new(&[]);
|
||||||
|
let sessopts = build_session_options(&matches);
|
||||||
|
let sess = build_session(sessopts, None, registry);
|
||||||
|
assert!(sess.can_print_warnings);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -54,6 +54,8 @@ pub struct Session {
|
||||||
/// The maximum recursion limit for potentially infinitely recursive
|
/// The maximum recursion limit for potentially infinitely recursive
|
||||||
/// operations such as auto-dereference and monomorphization.
|
/// operations such as auto-dereference and monomorphization.
|
||||||
pub recursion_limit: Cell<uint>,
|
pub recursion_limit: Cell<uint>,
|
||||||
|
|
||||||
|
pub can_print_warnings: bool
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Session {
|
impl Session {
|
||||||
|
@ -82,14 +84,20 @@ impl Session {
|
||||||
self.diagnostic().handler().abort_if_errors()
|
self.diagnostic().handler().abort_if_errors()
|
||||||
}
|
}
|
||||||
pub fn span_warn(&self, sp: Span, msg: &str) {
|
pub fn span_warn(&self, sp: Span, msg: &str) {
|
||||||
|
if self.can_print_warnings {
|
||||||
self.diagnostic().span_warn(sp, msg)
|
self.diagnostic().span_warn(sp, msg)
|
||||||
}
|
}
|
||||||
|
}
|
||||||
pub fn span_warn_with_code(&self, sp: Span, msg: &str, code: &str) {
|
pub fn span_warn_with_code(&self, sp: Span, msg: &str, code: &str) {
|
||||||
|
if self.can_print_warnings {
|
||||||
self.diagnostic().span_warn_with_code(sp, msg, code)
|
self.diagnostic().span_warn_with_code(sp, msg, code)
|
||||||
}
|
}
|
||||||
|
}
|
||||||
pub fn warn(&self, msg: &str) {
|
pub fn warn(&self, msg: &str) {
|
||||||
|
if self.can_print_warnings {
|
||||||
self.diagnostic().handler().warn(msg)
|
self.diagnostic().handler().warn(msg)
|
||||||
}
|
}
|
||||||
|
}
|
||||||
pub fn opt_span_warn(&self, opt_sp: Option<Span>, msg: &str) {
|
pub fn opt_span_warn(&self, opt_sp: Option<Span>, msg: &str) {
|
||||||
match opt_sp {
|
match opt_sp {
|
||||||
Some(sp) => self.span_warn(sp, msg),
|
Some(sp) => self.span_warn(sp, msg),
|
||||||
|
@ -247,6 +255,13 @@ pub fn build_session_(sopts: config::Options,
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
|
|
||||||
|
let can_print_warnings = sopts.lint_opts
|
||||||
|
.iter()
|
||||||
|
.filter(|&&(ref key, _)| key.as_slice() == "warnings")
|
||||||
|
.map(|&(_, ref level)| *level != lint::Allow)
|
||||||
|
.last()
|
||||||
|
.unwrap_or(true);
|
||||||
|
|
||||||
let sess = Session {
|
let sess = Session {
|
||||||
target: target_cfg,
|
target: target_cfg,
|
||||||
opts: sopts,
|
opts: sopts,
|
||||||
|
@ -265,6 +280,7 @@ pub fn build_session_(sopts: config::Options,
|
||||||
crate_metadata: RefCell::new(Vec::new()),
|
crate_metadata: RefCell::new(Vec::new()),
|
||||||
features: RefCell::new(feature_gate::Features::new()),
|
features: RefCell::new(feature_gate::Features::new()),
|
||||||
recursion_limit: Cell::new(64),
|
recursion_limit: Cell::new(64),
|
||||||
|
can_print_warnings: can_print_warnings
|
||||||
};
|
};
|
||||||
|
|
||||||
sess.lint_store.borrow_mut().register_builtin(Some(&sess));
|
sess.lint_store.borrow_mut().register_builtin(Some(&sess));
|
||||||
|
|
|
@ -17,7 +17,8 @@ fn tail(source_list: &IntList) -> IntList {
|
||||||
match source_list {
|
match source_list {
|
||||||
&IntList::Cons(val, box ref next_list) => tail(next_list),
|
&IntList::Cons(val, box ref next_list) => tail(next_list),
|
||||||
&IntList::Cons(val, box Nil) => IntList::Cons(val, box Nil),
|
&IntList::Cons(val, box Nil) => IntList::Cons(val, box Nil),
|
||||||
//~^ ERROR: unreachable pattern
|
//~^ ERROR unreachable pattern
|
||||||
|
//~^^ WARN pattern binding `Nil` is named the same as one of the variants of the type `IntList`
|
||||||
_ => panic!()
|
_ => panic!()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -17,7 +17,9 @@ pub mod b {
|
||||||
pub fn key(e: ::E) -> &'static str {
|
pub fn key(e: ::E) -> &'static str {
|
||||||
match e {
|
match e {
|
||||||
A => "A",
|
A => "A",
|
||||||
|
//~^ WARN pattern binding `A` is named the same as one of the variants of the type `E`
|
||||||
B => "B", //~ ERROR: unreachable pattern
|
B => "B", //~ ERROR: unreachable pattern
|
||||||
|
//~^ WARN pattern binding `B` is named the same as one of the variants of the type `E`
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -33,7 +33,8 @@ fn main() {
|
||||||
match f.read(&mut buff) {
|
match f.read(&mut buff) {
|
||||||
Ok(cnt) => println!("read this many bytes: {}", cnt),
|
Ok(cnt) => println!("read this many bytes: {}", cnt),
|
||||||
Err(IoError{ kind: EndOfFile, .. }) => println!("Got end of file: {}", EndOfFile.to_string()),
|
Err(IoError{ kind: EndOfFile, .. }) => println!("Got end of file: {}", EndOfFile.to_string()),
|
||||||
//~^ ERROR variable `EndOfFile` should have a snake case name such as `end_of_file`
|
//~^ ERROR variable `EndOfFile` should have a snake case name such as `end_of_file`
|
||||||
|
//~^^ WARN `EndOfFile` is named the same as one of the variants of the type `std::io::IoErrorKind`
|
||||||
}
|
}
|
||||||
|
|
||||||
test(1);
|
test(1);
|
||||||
|
|
34
src/test/run-pass/issue-19100.rs
Normal file
34
src/test/run-pass/issue-19100.rs
Normal file
|
@ -0,0 +1,34 @@
|
||||||
|
// Copyright 2014 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 <LICENSE-APACHE or
|
||||||
|
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
|
||||||
|
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
|
||||||
|
// option. This file may not be copied, modified, or distributed
|
||||||
|
// except according to those terms.
|
||||||
|
|
||||||
|
enum Foo {
|
||||||
|
Bar,
|
||||||
|
Baz
|
||||||
|
}
|
||||||
|
|
||||||
|
impl Foo {
|
||||||
|
fn foo(&self) {
|
||||||
|
match self {
|
||||||
|
&
|
||||||
|
Bar if true
|
||||||
|
//~^ WARN pattern binding `Bar` is named the same as one of the variants of the type `Foo`
|
||||||
|
//~^^ HELP to match on a variant, consider making the path in the pattern qualified: `Foo::Bar`
|
||||||
|
=> println!("bar"),
|
||||||
|
&
|
||||||
|
Baz if false
|
||||||
|
//~^ WARN pattern binding `Baz` is named the same as one of the variants of the type `Foo`
|
||||||
|
//~^^ HELP to match on a variant, consider making the path in the pattern qualified: `Foo::Baz`
|
||||||
|
=> println!("baz"),
|
||||||
|
_ => ()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn main() {}
|
Loading…
Reference in a new issue