From 819d0180634e9b5ac725f79671f69837d93a37a5 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 5 Jan 2016 13:43:57 -0500 Subject: [PATCH 1/5] improve visibility of future-incompatibilities (mildly, at least) --- src/librustc/lint/builtin.rs | 3 +++ src/librustc/lint/context.rs | 22 +++++++++++++++++++--- src/librustc_trans/trans/base.rs | 1 + 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 26f663b1c9d79..05a22aedfff55 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -16,6 +16,9 @@ use lint::{LintPass, LateLintPass, LintArray}; +// name of the future-incompatible group +pub const FUTURE_INCOMPATIBLE: &'static str = "future_incompatible"; + declare_lint! { pub CONST_ERR, Warn, diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index 1ed873f0508d5..4a5a44f19f34b 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -363,8 +363,12 @@ pub fn gather_attrs(attrs: &[ast::Attribute]) /// in trans that run after the main lint pass is finished. Most /// lints elsewhere in the compiler should call /// `Session::add_lint()` instead. -pub fn raw_emit_lint(sess: &Session, lint: &'static Lint, - lvlsrc: LevelSource, span: Option, msg: &str) { +pub fn raw_emit_lint(sess: &Session, + lints: &LintStore, + lint: &'static Lint, + lvlsrc: LevelSource, + span: Option, + msg: &str) { let (mut level, source) = lvlsrc; if level == Allow { return } @@ -399,6 +403,18 @@ pub fn raw_emit_lint(sess: &Session, lint: &'static Lint, _ => sess.bug("impossible level in raw_emit_lint"), } + // Check for future incompatibility lints and issue a stronger warning. + let future_incompat_lints = &lints.lint_groups[builtin::FUTURE_INCOMPATIBLE]; + let this_id = LintId::of(lint); + if future_incompat_lints.0.iter().any(|&id| id == this_id) { + let msg = "this lint will become a HARD ERROR in a future release!"; + if let Some(sp) = span { + sess.span_note(sp, msg); + } else { + sess.note(msg); + } + } + if let Some(span) = def { sess.span_note(span, "lint level defined here"); } @@ -428,7 +444,7 @@ pub trait LintContext: Sized { Some(&pair) => pair, }; - raw_emit_lint(&self.sess(), lint, (level, src), span, msg); + raw_emit_lint(&self.sess(), self.lints(), lint, (level, src), span, msg); } /// Emit a lint at the appropriate level, for a particular span. diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index dde6e3935b25d..971eb6bc5743a 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -2204,6 +2204,7 @@ fn enum_variant_size_lint(ccx: &CrateContext, enum_def: &hir::EnumDef, sp: Span, // Use lint::raw_emit_lint rather than sess.add_lint because the lint-printing // pass for the latter already ran. lint::raw_emit_lint(&ccx.tcx().sess, + &ccx.tcx().sess.lint_store.borrow(), lint::builtin::VARIANT_SIZE_DIFFERENCES, *lvlsrc.unwrap(), Some(sp), From e01ae5e6c11cafafca2e1d28fcc631c1b6fe1914 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 7 Jan 2016 00:45:13 +0100 Subject: [PATCH 2/5] extend warning cycle to cover matching unit-structs via `S(..)` (this makes them handled like enum unit-variants.) --- src/librustc_typeck/check/_match.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 38c714fa8f292..baba66778d7f1 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -636,6 +636,12 @@ pub fn check_pat_enum<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>, let report_bad_struct_kind = |is_warning| { bad_struct_kind_err(tcx.sess, pat.span, path, is_warning); if is_warning { + // 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."); return } @@ -676,10 +682,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 { - span_note!(tcx.sess, pat.span, - "this warning will become a HARD ERROR in a future release. \ - See RFC 218 for details."); } } (variant.fields @@ -693,7 +695,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 From 0903f805d9d7fe49f569d0d6b71375a7806d0947 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 7 Jan 2016 01:03:36 +0100 Subject: [PATCH 3/5] updated test to reflect loosening of check (for issue #30379). --- src/test/compile-fail/empty-struct-unit-pat.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/test/compile-fail/empty-struct-unit-pat.rs b/src/test/compile-fail/empty-struct-unit-pat.rs index 6cb9a3f007f0c..7e13f539bb043 100644 --- a/src/test/compile-fail/empty-struct-unit-pat.rs +++ b/src/test/compile-fail/empty-struct-unit-pat.rs @@ -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; @@ -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; @@ -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 { From a1bcfc5dba74ec98b3e11bd691367c62b703a101 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 8 Jan 2016 02:07:28 +0100 Subject: [PATCH 4/5] Added proper lint for the unit variant/struct warning. --- src/librustc/lint/builtin.rs | 8 +++++++ src/librustc_lint/lib.rs | 3 +++ src/librustc_typeck/check/_match.rs | 33 ++++++++++++++++------------- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 05a22aedfff55..0f176cf9f006a 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -120,6 +120,13 @@ declare_lint! { Allow, "detects trivial casts of numeric types which could be removed" } + +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)] @@ -144,6 +151,7 @@ impl LintPass for HardwiredLints { FAT_PTR_TRANSMUTES, TRIVIAL_CASTS, TRIVIAL_NUMERIC_CASTS, + MATCH_OF_UNIT_VARIANT_VIA_PAREN_DOTDOT, CONST_ERR ) } diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 1d7431404f545..5dd7768cb5c6c 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -146,6 +146,9 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { UNUSED_MUT, UNREACHABLE_CODE, UNUSED_MUST_USE, UNUSED_UNSAFE, PATH_STATEMENTS, UNUSED_ATTRIBUTES); + add_lint_group!(sess, FUTURE_INCOMPATIBLE, + MATCH_OF_UNIT_VARIANT_VIA_PAREN_DOTDOT); + // We have one lint pass defined specially store.register_late_pass(sess, false, box lint::GatherNodeLevels); diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index baba66778d7f1..992c3026e4bb5 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -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; @@ -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; } } @@ -580,10 +581,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>, @@ -634,17 +646,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 { - // 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."); - 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 { From 5e2e81bf01fba8105316ad16b8926e62f0e3466c Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 8 Jan 2016 16:19:13 +0100 Subject: [PATCH 5/5] The lint warnings are not reported since we report the errors first and then exit. I think that behavior is fine, so I am removing the expected warnings from these tests. --- src/test/compile-fail/match-pattern-field-mismatch-2.rs | 1 - src/test/compile-fail/pattern-error-continue.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/src/test/compile-fail/match-pattern-field-mismatch-2.rs b/src/test/compile-fail/match-pattern-field-mismatch-2.rs index 17debdabb61f0..e63ddf6c7fd9b 100644 --- a/src/test/compile-fail/match-pattern-field-mismatch-2.rs +++ b/src/test/compile-fail/match-pattern-field-mismatch-2.rs @@ -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 } } } diff --git a/src/test/compile-fail/pattern-error-continue.rs b/src/test/compile-fail/pattern-error-continue.rs index 1721d1f0ae11c..aa7202574abfc 100644 --- a/src/test/compile-fail/pattern-error-continue.rs +++ b/src/test/compile-fail/pattern-error-continue.rs @@ -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' {