Skip to content

Commit

Permalink
Auto merge of #42136 - petrochenkov:oldhard, r=nikomatsakis
Browse files Browse the repository at this point in the history
Turn sufficiently old compatibility lints into hard errors

It's been almost 7 months since #36894 was merged, so it's time to take the next step.

[breaking-change], needs crater run.

PRs/issues submitted to affected crates:
https://github.com/alexcrichton/ctest/pull/17
Sean1708/rusty-cheddar#55
m-r-r/helianto#3
azdle/virgil#1
rust-locale/rust-locale#24
mneumann/acyclic-network-rs#1
reem/rust-typemap#38

cc https://internals.rust-lang.org/t/moving-forward-on-forward-compatibility-lints/4204
cc #34537 #36887
Closes #36886
Closes #36888
Closes #36890
Closes #36891
Closes #36892
r? @nikomatsakis
  • Loading branch information
bors committed Jun 1, 2017
2 parents e0cc22b + 26d5c0e commit 67aa270
Show file tree
Hide file tree
Showing 29 changed files with 107 additions and 224 deletions.
58 changes: 0 additions & 58 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,68 +130,18 @@ declare_lint! {
"detect private items in public interfaces not caught by the old implementation"
}

declare_lint! {
pub INACCESSIBLE_EXTERN_CRATE,
Deny,
"use of inaccessible extern crate erroneously allowed"
}

declare_lint! {
pub INVALID_TYPE_PARAM_DEFAULT,
Deny,
"type parameter default erroneously allowed in invalid location"
}

declare_lint! {
pub ILLEGAL_FLOATING_POINT_CONSTANT_PATTERN,
Deny,
"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 RAW_POINTER_DERIVE,
Warn,
"uses of #[derive] with raw pointers are rarely correct"
}

declare_lint! {
pub HR_LIFETIME_IN_ASSOC_TYPE,
Deny,
"binding for associated type references higher-ranked lifetime \
that does not appear in the trait input types"
}

declare_lint! {
pub OVERLAPPING_INHERENT_IMPLS,
Deny,
"two overlapping inherent impls define an item with the same name were erroneously allowed"
}

declare_lint! {
pub RENAMED_AND_REMOVED_LINTS,
Warn,
"lints that have been renamed or removed"
}

declare_lint! {
pub SUPER_OR_SELF_IN_GLOBAL_PATH,
Deny,
"detects super or self keywords at the beginning of global path"
}

declare_lint! {
pub LIFETIME_UNDERSCORE,
Deny,
"lifetimes or labels named `'_` were erroneously allowed"
}

declare_lint! {
pub RESOLVE_TRAIT_ON_DEFAULTED_UNIT,
Warn,
Expand Down Expand Up @@ -280,17 +230,9 @@ impl LintPass for HardwiredLints {
TRIVIAL_CASTS,
TRIVIAL_NUMERIC_CASTS,
PRIVATE_IN_PUBLIC,
INACCESSIBLE_EXTERN_CRATE,
INVALID_TYPE_PARAM_DEFAULT,
ILLEGAL_FLOATING_POINT_CONSTANT_PATTERN,
ILLEGAL_STRUCT_OR_ENUM_CONSTANT_PATTERN,
CONST_ERR,
RAW_POINTER_DERIVE,
OVERLAPPING_INHERENT_IMPLS,
RENAMED_AND_REMOVED_LINTS,
SUPER_OR_SELF_IN_GLOBAL_PATH,
HR_LIFETIME_IN_ASSOC_TYPE,
LIFETIME_UNDERSCORE,
RESOLVE_TRAIT_ON_DEFAULTED_UNIT,
SAFE_EXTERN_STATICS,
PATTERNS_IN_FNS_WITHOUT_BODY,
Expand Down
21 changes: 6 additions & 15 deletions src/librustc_const_eval/pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

use eval;

use rustc::lint;
use rustc::middle::const_val::{ConstEvalErr, ConstVal};
use rustc::mir::{Field, BorrowKind, Mutability};
use rustc::ty::{self, TyCtxt, AdtDef, Ty, TypeVariants, Region};
Expand Down Expand Up @@ -644,27 +643,19 @@ impl<'a, 'gcx, 'tcx> PatternContext<'a, 'gcx, 'tcx> {
debug!("expr={:?} pat_ty={:?} pat_id={}", expr, pat_ty, pat_id);
match pat_ty.sty {
ty::TyFloat(_) => {
self.tcx.sess.add_lint(
lint::builtin::ILLEGAL_FLOATING_POINT_CONSTANT_PATTERN,
pat_id,
span,
format!("floating point constants cannot be used in patterns"));
self.tcx.sess.span_err(span, "floating point constants cannot be used in patterns");
}
ty::TyAdt(adt_def, _) if adt_def.is_union() => {
// Matching on union fields is unsafe, we can't hide it in constants
self.tcx.sess.span_err(span, "cannot use unions in constant patterns");
}
ty::TyAdt(adt_def, _) => {
if !self.tcx.has_attr(adt_def.did, "structural_match") {
self.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)]`",
self.tcx.item_path_str(adt_def.did),
self.tcx.item_path_str(adt_def.did)));
let msg = format!("to use a constant of type `{}` in a pattern, \
`{}` must be annotated with `#[derive(PartialEq, Eq)]`",
self.tcx.item_path_str(adt_def.did),
self.tcx.item_path_str(adt_def.did));
self.tcx.sess.span_err(span, &msg);
}
}
_ => { }
Expand Down
71 changes: 30 additions & 41 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
// - Create a lint defaulting to warn as normal, with ideally the same error
// message you would normally give
// - Add a suitable reference, typically an RFC or tracking issue. Go ahead
// and include the full URL.
// and include the full URL, sort items in ascending order of issue numbers.
// - Later, change lint to error
// - Eventually, remove lint
store.register_future_incompatible(sess,
Expand All @@ -189,48 +189,16 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
reference: "issue #34537 <https://github.com/rust-lang/rust/issues/34537>",
},
FutureIncompatibleInfo {
id: LintId::of(INACCESSIBLE_EXTERN_CRATE),
reference: "issue #36886 <https://github.com/rust-lang/rust/issues/36886>",
},
FutureIncompatibleInfo {
id: LintId::of(INVALID_TYPE_PARAM_DEFAULT),
reference: "issue #36887 <https://github.com/rust-lang/rust/issues/36887>",
},
FutureIncompatibleInfo {
id: LintId::of(SUPER_OR_SELF_IN_GLOBAL_PATH),
reference: "issue #36888 <https://github.com/rust-lang/rust/issues/36888>",
},
FutureIncompatibleInfo {
id: LintId::of(ILLEGAL_FLOATING_POINT_CONSTANT_PATTERN),
reference: "issue #36890 <https://github.com/rust-lang/rust/issues/36890>",
},
FutureIncompatibleInfo {
id: LintId::of(ILLEGAL_FLOATING_POINT_LITERAL_PATTERN),
reference: "issue #41620 <https://github.com/rust-lang/rust/issues/41620>",
},
FutureIncompatibleInfo {
id: LintId::of(ILLEGAL_STRUCT_OR_ENUM_CONSTANT_PATTERN),
reference: "issue #36891 <https://github.com/rust-lang/rust/issues/36891>",
},
FutureIncompatibleInfo {
id: LintId::of(HR_LIFETIME_IN_ASSOC_TYPE),
reference: "issue #33685 <https://github.com/rust-lang/rust/issues/33685>",
},
FutureIncompatibleInfo {
id: LintId::of(LIFETIME_UNDERSCORE),
reference: "issue #36892 <https://github.com/rust-lang/rust/issues/36892>",
},
FutureIncompatibleInfo {
id: LintId::of(RESOLVE_TRAIT_ON_DEFAULTED_UNIT),
reference: "issue #39216 <https://github.com/rust-lang/rust/issues/39216>",
id: LintId::of(PATTERNS_IN_FNS_WITHOUT_BODY),
reference: "issue #35203 <https://github.com/rust-lang/rust/issues/35203>",
},
FutureIncompatibleInfo {
id: LintId::of(SAFE_EXTERN_STATICS),
reference: "issue #36247 <https://github.com/rust-lang/rust/issues/35112>",
reference: "issue #36247 <https://github.com/rust-lang/rust/issues/36247>",
},
FutureIncompatibleInfo {
id: LintId::of(PATTERNS_IN_FNS_WITHOUT_BODY),
reference: "issue #35203 <https://github.com/rust-lang/rust/issues/35203>",
id: LintId::of(INVALID_TYPE_PARAM_DEFAULT),
reference: "issue #36887 <https://github.com/rust-lang/rust/issues/36887>",
},
FutureIncompatibleInfo {
id: LintId::of(EXTRA_REQUIREMENT_IN_IMPL),
Expand All @@ -248,18 +216,26 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
id: LintId::of(LEGACY_CONSTRUCTOR_VISIBILITY),
reference: "issue #39207 <https://github.com/rust-lang/rust/issues/39207>",
},
FutureIncompatibleInfo {
id: LintId::of(RESOLVE_TRAIT_ON_DEFAULTED_UNIT),
reference: "issue #39216 <https://github.com/rust-lang/rust/issues/39216>",
},
FutureIncompatibleInfo {
id: LintId::of(MISSING_FRAGMENT_SPECIFIER),
reference: "issue #40107 <https://github.com/rust-lang/rust/issues/40107>",
},
FutureIncompatibleInfo {
id: LintId::of(PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES),
reference: "issue #42238 <https://github.com/rust-lang/rust/issues/42238>",
id: LintId::of(ILLEGAL_FLOATING_POINT_LITERAL_PATTERN),
reference: "issue #41620 <https://github.com/rust-lang/rust/issues/41620>",
},
FutureIncompatibleInfo {
id: LintId::of(ANONYMOUS_PARAMETERS),
reference: "issue #41686 <https://github.com/rust-lang/rust/issues/41686>",
},
FutureIncompatibleInfo {
id: LintId::of(PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES),
reference: "issue #42238 <https://github.com/rust-lang/rust/issues/42238>",
}
]);

// Register renamed and removed lints
Expand All @@ -275,5 +251,18 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
store.register_removed("drop_with_repr_extern", "drop flags have been removed");
store.register_removed("transmute_from_fn_item_types",
"always cast functions before transmuting them");
store.register_removed("overlapping_inherent_impls", "converted into hard error, see #36889");
store.register_removed("hr_lifetime_in_assoc_type",
"converted into hard error, see https://github.com/rust-lang/rust/issues/33685");
store.register_removed("inaccessible_extern_crate",
"converted into hard error, see https://github.com/rust-lang/rust/issues/36886");
store.register_removed("super_or_self_in_global_path",
"converted into hard error, see https://github.com/rust-lang/rust/issues/36888");
store.register_removed("overlapping_inherent_impls",
"converted into hard error, see https://github.com/rust-lang/rust/issues/36889");
store.register_removed("illegal_floating_point_constant_pattern",
"converted into hard error, see https://github.com/rust-lang/rust/issues/36890");
store.register_removed("illegal_struct_or_enum_constant_pattern",
"converted into hard error, see https://github.com/rust-lang/rust/issues/36891");
store.register_removed("lifetime_underscore",
"converted into hard error, see https://github.com/rust-lang/rust/issues/36892");
}
25 changes: 7 additions & 18 deletions src/librustc_passes/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,10 @@ impl<'a> AstValidator<'a> {
&self.session.parse_sess.span_diagnostic
}

fn check_label(&self, label: Ident, span: Span, id: NodeId) {
if label.name == keywords::StaticLifetime.name() {
fn check_label(&self, label: Ident, span: Span) {
if label.name == keywords::StaticLifetime.name() || label.name == "'_" {
self.err_handler().span_err(span, &format!("invalid label name `{}`", label.name));
}
if label.name == "'_" {
self.session.add_lint(lint::builtin::LIFETIME_UNDERSCORE,
id,
span,
format!("invalid label name `{}`", label.name));
}
}

fn invalid_visibility(&self, vis: &Visibility, span: Span, note: Option<&str>) {
Expand Down Expand Up @@ -104,10 +98,7 @@ impl<'a> AstValidator<'a> {
impl<'a> Visitor<'a> for AstValidator<'a> {
fn visit_lifetime(&mut self, lt: &'a Lifetime) {
if lt.ident.name == "'_" {
self.session.add_lint(lint::builtin::LIFETIME_UNDERSCORE,
lt.id,
lt.span,
format!("invalid lifetime name `{}`", lt.ident));
self.err_handler().span_err(lt.span, &format!("invalid lifetime name `{}`", lt.ident));
}

visit::walk_lifetime(self, lt)
Expand All @@ -121,7 +112,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
ExprKind::ForLoop(.., Some(ident)) |
ExprKind::Break(Some(ident), _) |
ExprKind::Continue(Some(ident)) => {
self.check_label(ident.node, ident.span, expr.id);
self.check_label(ident.node, ident.span);
}
_ => {}
}
Expand Down Expand Up @@ -169,14 +160,12 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
visit::walk_ty(self, ty)
}

fn visit_path(&mut self, path: &'a Path, id: NodeId) {
fn visit_path(&mut self, path: &'a Path, _: NodeId) {
if path.segments.len() >= 2 && path.is_global() {
let ident = path.segments[1].identifier;
if token::Ident(ident).is_path_segment_keyword() {
self.session.add_lint(lint::builtin::SUPER_OR_SELF_IN_GLOBAL_PATH,
id,
path.span,
format!("global paths cannot start with `{}`", ident));
self.err_handler()
.span_err(path.span, &format!("global paths cannot start with `{}`", ident));
}
}

Expand Down
17 changes: 5 additions & 12 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,10 @@ impl<'a> NameBinding<'a> {
_ => false,
}
}

fn descr(&self) -> &'static str {
if self.is_extern_crate() { "extern crate" } else { self.def().kind_name() }
}
}

/// Interns the names of the primitive types.
Expand Down Expand Up @@ -3424,18 +3428,7 @@ impl<'a> Resolver<'a> {

for &PrivacyError(span, name, binding) in &self.privacy_errors {
if !reported_spans.insert(span) { continue }
if binding.is_extern_crate() {
// Warn when using an inaccessible extern crate.
let node_id = match binding.kind {
NameBindingKind::Import { directive, .. } => directive.id,
_ => unreachable!(),
};
let msg = format!("extern crate `{}` is private", name);
self.session.add_lint(lint::builtin::INACCESSIBLE_EXTERN_CRATE, node_id, span, msg);
} else {
let def = binding.def();
self.session.span_err(span, &format!("{} `{}` is private", def.kind_name(), name));
}
self.session.span_err(span, &format!("{} `{}` is private", binding.descr(), name));
}
}

Expand Down
15 changes: 7 additions & 8 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
let (ns, binding) = reexport_error.unwrap();
if ns == TypeNS && binding.is_extern_crate() {
let msg = format!("extern crate `{}` is private, and cannot be reexported \
(error E0364), consider declaring with `pub`",
(error E0365), consider declaring with `pub`",
ident);
self.session.add_lint(PRIVATE_IN_PUBLIC, directive.id, directive.span, msg);
} else if ns == TypeNS {
Expand Down Expand Up @@ -792,8 +792,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
self.record_def(directive.id, PathResolution::new(module.def().unwrap()));
}

// Miscellaneous post-processing, including recording reexports, reporting conflicts,
// reporting the PRIVATE_IN_PUBLIC lint, and reporting unresolved imports.
// Miscellaneous post-processing, including recording reexports,
// reporting conflicts, and reporting unresolved imports.
fn finalize_resolutions_in(&mut self, module: Module<'b>) {
// Since import resolution is finished, globs will not define any more names.
*module.globs.borrow_mut() = Vec::new();
Expand Down Expand Up @@ -838,13 +838,12 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
}

match binding.kind {
NameBindingKind::Import { binding: orig_binding, directive, .. } => {
NameBindingKind::Import { binding: orig_binding, .. } => {
if ns == TypeNS && orig_binding.is_variant() &&
!orig_binding.vis.is_at_least(binding.vis, &*self) {
let msg = format!("variant `{}` is private, and cannot be reexported \
(error E0364), consider declaring its enum as `pub`",
ident);
self.session.add_lint(PRIVATE_IN_PUBLIC, directive.id, binding.span, msg);
let msg = format!("variant `{}` is private, and cannot be reexported, \
consider declaring its enum as `pub`", ident);
self.session.span_err(binding.span, &msg);
}
}
NameBindingKind::Ambiguity { b1, b2, .. }
Expand Down
8 changes: 0 additions & 8 deletions src/libsyntax/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,20 +231,12 @@ fn mk_reexport_mod(cx: &mut TestCtxt,
-> (P<ast::Item>, Ident) {
let super_ = Ident::from_str("super");

// Generate imports with `#[allow(private_in_public)]` to work around issue #36768.
let allow_private_in_public = cx.ext_cx.attribute(DUMMY_SP, cx.ext_cx.meta_list(
DUMMY_SP,
Symbol::intern("allow"),
vec![cx.ext_cx.meta_list_item_word(DUMMY_SP, Symbol::intern("private_in_public"))],
));
let items = tests.into_iter().map(|r| {
cx.ext_cx.item_use_simple(DUMMY_SP, ast::Visibility::Public,
cx.ext_cx.path(DUMMY_SP, vec![super_, r]))
.map_attrs(|_| vec![allow_private_in_public.clone()])
}).chain(tested_submods.into_iter().map(|(r, sym)| {
let path = cx.ext_cx.path(DUMMY_SP, vec![super_, r, sym]);
cx.ext_cx.item_use_simple_(DUMMY_SP, ast::Visibility::Public, r, path)
.map_attrs(|_| vec![allow_private_in_public.clone()])
})).collect();

let reexport_mod = ast::Mod {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

#![allow(dead_code)]
#![feature(rustc_attrs)]
#![allow(hr_lifetime_in_assoc_type)]

trait Foo<'a> {
type Item;
Expand Down
Loading

0 comments on commit 67aa270

Please sign in to comment.