Skip to content

Downgrade unit struct match via S(..) warnings to errors #30753

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,12 @@ declare_lint! {
"type parameter default erroneously allowed in invalid location"
}

declare_lint! {
pub MATCH_OF_UNIT_VARIANT_VIA_PAREN_DOTDOT,
Warn,
"unit struct or enum variant erroneously allowed to match via path::ident(..)"
}

/// Does nothing as a lint pass, but registers some `Lint`s
/// which are used by other parts of the compiler.
#[derive(Copy, Clone)]
Expand All @@ -159,6 +165,7 @@ impl LintPass for HardwiredLints {
TRIVIAL_NUMERIC_CASTS,
PRIVATE_IN_PUBLIC,
INVALID_TYPE_PARAM_DEFAULT,
MATCH_OF_UNIT_VARIANT_VIA_PAREN_DOTDOT,
CONST_ERR
)
}
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
UNUSED_UNSAFE, PATH_STATEMENTS, UNUSED_ATTRIBUTES);

add_lint_group!(sess, FUTURE_INCOMPATIBLE,
PRIVATE_IN_PUBLIC, INVALID_TYPE_PARAM_DEFAULT);
PRIVATE_IN_PUBLIC, INVALID_TYPE_PARAM_DEFAULT,
MATCH_OF_UNIT_VARIANT_VIA_PAREN_DOTDOT);

// We have one lint pass defined specially
store.register_late_pass(sess, false, box lint::GatherNodeLevels);
Expand Down
38 changes: 22 additions & 16 deletions src/librustc_typeck/check/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use check::{check_expr, check_expr_has_type, check_expr_with_expectation};
use check::{check_expr_coercable_to_type, demand, FnCtxt, Expectation};
use check::{check_expr_with_lvalue_pref};
use check::{instantiate_path, resolve_ty_and_def_ufcs, structurally_resolved_type};
use lint;
use require_same_types;
use util::nodemap::FnvHashMap;
use session::Session;
Expand Down Expand Up @@ -138,7 +139,7 @@ pub fn check_pat<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>,
if pat_is_resolved_const(&tcx.def_map.borrow(), pat) => {
if let hir::PatEnum(ref path, ref subpats) = pat.node {
if !(subpats.is_some() && subpats.as_ref().unwrap().is_empty()) {
bad_struct_kind_err(tcx.sess, pat.span, path, false);
bad_struct_kind_err(tcx.sess, pat, path, false);
return;
}
}
Expand Down Expand Up @@ -590,10 +591,21 @@ pub fn check_pat_struct<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>, pat: &'tcx hir::Pat,
}

// This function exists due to the warning "diagnostic code E0164 already used"
fn bad_struct_kind_err(sess: &Session, span: Span, path: &hir::Path, is_warning: bool) {
fn bad_struct_kind_err(sess: &Session, pat: &hir::Pat, path: &hir::Path, lint: bool) {
let name = pprust::path_to_string(path);
span_err_or_warn!(is_warning, sess, span, E0164,
"`{}` does not name a tuple variant or a tuple struct", name);
let msg = format!("`{}` does not name a tuple variant or a tuple struct", name);
if lint {
let expanded_msg =
format!("{}; RFC 218 disallowed matching of unit variants or unit structs via {}(..)",
msg,
name);
sess.add_lint(lint::builtin::MATCH_OF_UNIT_VARIANT_VIA_PAREN_DOTDOT,
pat.id,
pat.span,
expanded_msg);
} else {
span_err!(sess, pat.span, E0164, "{}", msg);
}
}

pub fn check_pat_enum<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>,
Expand Down Expand Up @@ -657,11 +669,8 @@ pub fn check_pat_enum<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>,
opt_ty, def, pat.span, pat.id);

let report_bad_struct_kind = |is_warning| {
bad_struct_kind_err(tcx.sess, pat.span, path, is_warning);
if is_warning {
return;
}

bad_struct_kind_err(tcx.sess, pat, path, is_warning);
if is_warning { return; }
fcx.write_error(pat.id);
if let Some(subpats) = subpats {
for pat in subpats {
Expand Down Expand Up @@ -699,12 +708,6 @@ pub fn check_pat_enum<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>,
report_bad_struct_kind(is_special_case);
if !is_special_case {
return
} else {
// Boo! Too painful to attach this to the actual warning,
// it should go away at some point though.
tcx.sess.span_note_without_error(pat.span,
"this warning will become a HARD ERROR in a future release. \
See RFC 218 for details.");
}
}
(variant.fields
Expand All @@ -718,7 +721,10 @@ pub fn check_pat_enum<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>,
ty::TyStruct(struct_def, expected_substs) => {
let variant = struct_def.struct_variant();
if is_tuple_struct_pat && variant.kind() != ty::VariantKind::Tuple {
report_bad_struct_kind(false);
// Matching unit structs with tuple variant patterns (`UnitVariant(..)`)
// is allowed for backward compatibility.
let is_special_case = variant.kind() == ty::VariantKind::Unit;
report_bad_struct_kind(is_special_case);
return;
}
(variant.fields
Expand Down
8 changes: 6 additions & 2 deletions src/test/compile-fail/empty-struct-unit-pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

// Can't use unit struct as enum pattern

#![feature(rustc_attrs)]
// remove prior feature after warning cycle and promoting warnings to errors
#![feature(braced_empty_structs)]

struct Empty1;
Expand All @@ -18,7 +20,9 @@ enum E {
Empty2
}

fn main() {
// remove attribute after warning cycle and promoting warnings to errors
#[rustc_error]
fn main() { //~ ERROR: compilation successful
let e1 = Empty1;
let e2 = E::Empty2;

Expand All @@ -27,7 +31,7 @@ fn main() {
// Empty1() => () // ERROR `Empty1` does not name a tuple variant or a tuple struct
// }
match e1 {
Empty1(..) => () //~ ERROR `Empty1` does not name a tuple variant or a tuple struct
Empty1(..) => () //~ WARN `Empty1` does not name a tuple variant or a tuple struct
}
// Rejected by parser as yet
// match e2 {
Expand Down
1 change: 0 additions & 1 deletion src/test/compile-fail/match-pattern-field-mismatch-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ fn main() {
color::cmyk(_, _, _, _) => { }
color::no_color(_) => { }
//~^ ERROR this pattern has 1 field, but the corresponding variant has no fields
//~^^ WARN `color::no_color` does not name a tuple variant or a tuple struct
}
}
}
1 change: 0 additions & 1 deletion src/test/compile-fail/pattern-error-continue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ fn main() {
match A::B(1, 2) {
A::B(_, _, _) => (), //~ ERROR this pattern has 3 fields, but
A::D(_) => (), //~ ERROR this pattern has 1 field, but
//~^ WARN `A::D` does not name a tuple variant or a tuple struct
_ => ()
}
match 'c' {
Expand Down