Skip to content

Commit fac5a07

Browse files
committed
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.
2 parents 6faff24 + 9d01db1 commit fac5a07

File tree

8 files changed

+153
-32
lines changed

8 files changed

+153
-32
lines changed

src/librustc/diagnostics.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -145,5 +145,6 @@ register_diagnostics!(
145145
E0166,
146146
E0167,
147147
E0168,
148-
E0169
148+
E0169,
149+
E0170
149150
)

src/librustc/middle/check_match.rs

+58-26
Original file line numberDiff line numberDiff line change
@@ -153,19 +153,14 @@ fn check_expr(cx: &mut MatchCheckCtxt, ex: &ast::Expr) {
153153
visit::walk_expr(cx, ex);
154154
match ex.node {
155155
ast::ExprMatch(ref scrut, ref arms, source) => {
156-
// First, check legality of move bindings.
157156
for arm in arms.iter() {
157+
// First, check legality of move bindings.
158158
check_legality_of_move_bindings(cx,
159159
arm.guard.is_some(),
160160
arm.pats.as_slice());
161-
for pat in arm.pats.iter() {
162-
check_legality_of_bindings_in_at_patterns(cx, &**pat);
163-
}
164-
}
165161

166-
// Second, if there is a guard on each arm, make sure it isn't
167-
// assigning or borrowing anything mutably.
168-
for arm in arms.iter() {
162+
// Second, if there is a guard on each arm, make sure it isn't
163+
// assigning or borrowing anything mutably.
169164
match arm.guard {
170165
Some(ref guard) => check_for_mutation_in_guard(cx, &**guard),
171166
None => {}
@@ -179,13 +174,23 @@ fn check_expr(cx: &mut MatchCheckCtxt, ex: &ast::Expr) {
179174
}).collect(), arm.guard.as_ref().map(|e| &**e))
180175
}).collect::<Vec<(Vec<P<Pat>>, Option<&ast::Expr>)>>();
181176

177+
// Bail out early if inlining failed.
182178
if static_inliner.failed {
183179
return;
184180
}
185181

186-
// Third, check if there are any references to NaN that we should warn about.
187-
for &(ref pats, _) in inlined_arms.iter() {
188-
check_for_static_nan(cx, pats.as_slice());
182+
for pat in inlined_arms
183+
.iter()
184+
.flat_map(|&(ref pats, _)| pats.iter()) {
185+
// Third, check legality of move bindings.
186+
check_legality_of_bindings_in_at_patterns(cx, &**pat);
187+
188+
// Fourth, check if there are any references to NaN that we should warn about.
189+
check_for_static_nan(cx, &**pat);
190+
191+
// Fifth, check if for any of the patterns that match an enumerated type
192+
// are bindings with the same name as one of the variants of said type.
193+
check_for_bindings_named_the_same_as_variants(cx, &**pat);
189194
}
190195

191196
// Fourth, check for unreachable arms.
@@ -239,21 +244,49 @@ fn is_expr_const_nan(tcx: &ty::ctxt, expr: &ast::Expr) -> bool {
239244
}
240245
}
241246

242-
// Check that we do not match against a static NaN (#6804)
243-
fn check_for_static_nan(cx: &MatchCheckCtxt, pats: &[P<Pat>]) {
244-
for pat in pats.iter() {
245-
walk_pat(&**pat, |p| {
246-
match p.node {
247-
ast::PatLit(ref expr) if is_expr_const_nan(cx.tcx, &**expr) => {
248-
span_warn!(cx.tcx.sess, p.span, E0003,
249-
"unmatchable NaN in pattern, \
250-
use the is_nan method in a guard instead");
247+
fn check_for_bindings_named_the_same_as_variants(cx: &MatchCheckCtxt, pat: &Pat) {
248+
walk_pat(pat, |p| {
249+
match p.node {
250+
ast::PatIdent(ast::BindByValue(ast::MutImmutable), ident, None) => {
251+
let pat_ty = ty::pat_ty(cx.tcx, p);
252+
if let ty::ty_enum(def_id, _) = pat_ty.sty {
253+
let def = cx.tcx.def_map.borrow().get(&p.id).cloned();
254+
if let Some(DefLocal(_)) = def {
255+
if ty::enum_variants(cx.tcx, def_id).iter().any(|variant|
256+
token::get_name(variant.name) == token::get_name(ident.node.name)
257+
&& variant.args.len() == 0
258+
) {
259+
span_warn!(cx.tcx.sess, p.span, E0170,
260+
"pattern binding `{}` is named the same as one \
261+
of the variants of the type `{}`",
262+
token::get_ident(ident.node).get(), ty_to_string(cx.tcx, pat_ty));
263+
span_help!(cx.tcx.sess, p.span,
264+
"if you meant to match on a variant, \
265+
consider making the path in the pattern qualified: `{}::{}`",
266+
ty_to_string(cx.tcx, pat_ty), token::get_ident(ident.node).get());
267+
}
268+
}
251269
}
252-
_ => ()
253270
}
254-
true
255-
});
256-
}
271+
_ => ()
272+
}
273+
true
274+
});
275+
}
276+
277+
// Check that we do not match against a static NaN (#6804)
278+
fn check_for_static_nan(cx: &MatchCheckCtxt, pat: &Pat) {
279+
walk_pat(pat, |p| {
280+
match p.node {
281+
ast::PatLit(ref expr) if is_expr_const_nan(cx.tcx, &**expr) => {
282+
span_warn!(cx.tcx.sess, p.span, E0003,
283+
"unmatchable NaN in pattern, \
284+
use the is_nan method in a guard instead");
285+
}
286+
_ => ()
287+
}
288+
true
289+
});
257290
}
258291

259292
// Check for unreachable patterns
@@ -414,8 +447,7 @@ fn construct_witness(cx: &MatchCheckCtxt, ctor: &Constructor,
414447
&Variant(vid) =>
415448
(vid, ty::enum_variant_with_id(cx.tcx, cid, vid).arg_names.is_some()),
416449
_ =>
417-
(cid, ty::lookup_struct_fields(cx.tcx, cid).iter()
418-
.any(|field| field.name != token::special_idents::unnamed_field.name))
450+
(cid, !ty::is_tuple_struct(cx.tcx, cid))
419451
};
420452
if is_structure {
421453
let fields = ty::lookup_struct_fields(cx.tcx, vid);

src/librustc/session/config.rs

+34
Original file line numberDiff line numberDiff line change
@@ -948,4 +948,38 @@ mod test {
948948
assert!(test_items.next().is_some());
949949
assert!(test_items.next().is_none());
950950
}
951+
952+
#[test]
953+
fn test_can_print_warnings() {
954+
{
955+
let matches = getopts(&[
956+
"-Awarnings".to_string()
957+
], optgroups().as_slice()).unwrap();
958+
let registry = diagnostics::registry::Registry::new(&[]);
959+
let sessopts = build_session_options(&matches);
960+
let sess = build_session(sessopts, None, registry);
961+
assert!(!sess.can_print_warnings);
962+
}
963+
964+
{
965+
let matches = getopts(&[
966+
"-Awarnings".to_string(),
967+
"-Dwarnings".to_string()
968+
], optgroups().as_slice()).unwrap();
969+
let registry = diagnostics::registry::Registry::new(&[]);
970+
let sessopts = build_session_options(&matches);
971+
let sess = build_session(sessopts, None, registry);
972+
assert!(sess.can_print_warnings);
973+
}
974+
975+
{
976+
let matches = getopts(&[
977+
"-Adead_code".to_string()
978+
], optgroups().as_slice()).unwrap();
979+
let registry = diagnostics::registry::Registry::new(&[]);
980+
let sessopts = build_session_options(&matches);
981+
let sess = build_session(sessopts, None, registry);
982+
assert!(sess.can_print_warnings);
983+
}
984+
}
951985
}

src/librustc/session/mod.rs

+19-3
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ pub struct Session {
5454
/// The maximum recursion limit for potentially infinitely recursive
5555
/// operations such as auto-dereference and monomorphization.
5656
pub recursion_limit: Cell<uint>,
57+
58+
pub can_print_warnings: bool
5759
}
5860

5961
impl Session {
@@ -82,13 +84,19 @@ impl Session {
8284
self.diagnostic().handler().abort_if_errors()
8385
}
8486
pub fn span_warn(&self, sp: Span, msg: &str) {
85-
self.diagnostic().span_warn(sp, msg)
87+
if self.can_print_warnings {
88+
self.diagnostic().span_warn(sp, msg)
89+
}
8690
}
8791
pub fn span_warn_with_code(&self, sp: Span, msg: &str, code: &str) {
88-
self.diagnostic().span_warn_with_code(sp, msg, code)
92+
if self.can_print_warnings {
93+
self.diagnostic().span_warn_with_code(sp, msg, code)
94+
}
8995
}
9096
pub fn warn(&self, msg: &str) {
91-
self.diagnostic().handler().warn(msg)
97+
if self.can_print_warnings {
98+
self.diagnostic().handler().warn(msg)
99+
}
92100
}
93101
pub fn opt_span_warn(&self, opt_sp: Option<Span>, msg: &str) {
94102
match opt_sp {
@@ -247,6 +255,13 @@ pub fn build_session_(sopts: config::Options,
247255
}
248256
);
249257

258+
let can_print_warnings = sopts.lint_opts
259+
.iter()
260+
.filter(|&&(ref key, _)| key.as_slice() == "warnings")
261+
.map(|&(_, ref level)| *level != lint::Allow)
262+
.last()
263+
.unwrap_or(true);
264+
250265
let sess = Session {
251266
target: target_cfg,
252267
opts: sopts,
@@ -265,6 +280,7 @@ pub fn build_session_(sopts: config::Options,
265280
crate_metadata: RefCell::new(Vec::new()),
266281
features: RefCell::new(feature_gate::Features::new()),
267282
recursion_limit: Cell::new(64),
283+
can_print_warnings: can_print_warnings
268284
};
269285

270286
sess.lint_store.borrow_mut().register_builtin(Some(&sess));

src/test/compile-fail/issue-12116.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ fn tail(source_list: &IntList) -> IntList {
1717
match source_list {
1818
&IntList::Cons(val, box ref next_list) => tail(next_list),
1919
&IntList::Cons(val, box Nil) => IntList::Cons(val, box Nil),
20-
//~^ ERROR: unreachable pattern
20+
//~^ ERROR unreachable pattern
21+
//~^^ WARN pattern binding `Nil` is named the same as one of the variants of the type `IntList`
2122
_ => panic!()
2223
}
2324
}

src/test/compile-fail/issue-14221.rs

+2
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ pub mod b {
1717
pub fn key(e: ::E) -> &'static str {
1818
match e {
1919
A => "A",
20+
//~^ WARN pattern binding `A` is named the same as one of the variants of the type `E`
2021
B => "B", //~ ERROR: unreachable pattern
22+
//~^ WARN pattern binding `B` is named the same as one of the variants of the type `E`
2123
}
2224
}
2325
}

src/test/compile-fail/lint-uppercase-variables.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ fn main() {
3333
match f.read(&mut buff) {
3434
Ok(cnt) => println!("read this many bytes: {}", cnt),
3535
Err(IoError{ kind: EndOfFile, .. }) => println!("Got end of file: {}", EndOfFile.to_string()),
36-
//~^ ERROR variable `EndOfFile` should have a snake case name such as `end_of_file`
36+
//~^ ERROR variable `EndOfFile` should have a snake case name such as `end_of_file`
37+
//~^^ WARN `EndOfFile` is named the same as one of the variants of the type `std::io::IoErrorKind`
3738
}
3839

3940
test(1);

src/test/run-pass/issue-19100.rs

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
enum Foo {
12+
Bar,
13+
Baz
14+
}
15+
16+
impl Foo {
17+
fn foo(&self) {
18+
match self {
19+
&
20+
Bar if true
21+
//~^ WARN pattern binding `Bar` is named the same as one of the variants of the type `Foo`
22+
//~^^ HELP to match on a variant, consider making the path in the pattern qualified: `Foo::Bar`
23+
=> println!("bar"),
24+
&
25+
Baz if false
26+
//~^ WARN pattern binding `Baz` is named the same as one of the variants of the type `Foo`
27+
//~^^ HELP to match on a variant, consider making the path in the pattern qualified: `Foo::Baz`
28+
=> println!("baz"),
29+
_ => ()
30+
}
31+
}
32+
}
33+
34+
fn main() {}

0 commit comments

Comments
 (0)