Skip to content

rustc: Fix bugs in renamed and removed lints and re-add raw_pointer_derive #30878

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 1 commit into from
Jan 16, 2016
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
9 changes: 8 additions & 1 deletion src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,12 @@ declare_lint! {
"unit struct or enum variant erroneously allowed to match via path::ident(..)"
}

declare_lint! {
pub RAW_POINTER_DERIVE,
Warn,
"uses of #[derive] with raw pointers are rarely correct"
}

/// Does nothing as a lint pass, but registers some `Lint`s
/// which are used by other parts of the compiler.
#[derive(Copy, Clone)]
Expand Down Expand Up @@ -166,7 +172,8 @@ impl LintPass for HardwiredLints {
PRIVATE_IN_PUBLIC,
INVALID_TYPE_PARAM_DEFAULT,
MATCH_OF_UNIT_VARIANT_VIA_PAREN_DOTDOT,
CONST_ERR
CONST_ERR,
RAW_POINTER_DERIVE
)
}
}
Expand Down
184 changes: 149 additions & 35 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,21 +219,10 @@ impl LintStore {
{
match self.by_name.get(lint_name) {
Some(&Id(lint_id)) => Ok(lint_id),
Some(&Renamed(ref new_name, lint_id)) => {
let warning = format!("lint {} has been renamed to {}",
lint_name, new_name);
match span {
Some(span) => sess.span_warn(span, &warning[..]),
None => sess.warn(&warning[..]),
};
Some(&Renamed(_, lint_id)) => {
Ok(lint_id)
},
Some(&Removed(ref reason)) => {
let warning = format!("lint {} has been removed: {}", lint_name, reason);
match span {
Some(span) => sess.span_warn(span, &warning[..]),
None => sess.warn(&warning[..])
}
Err(FindLintError::Removed)
},
None => Err(FindLintError::NotFound)
Expand All @@ -242,8 +231,12 @@ impl LintStore {

pub fn process_command_line(&mut self, sess: &Session) {
for &(ref lint_name, level) in &sess.opts.lint_opts {
check_lint_name_cmdline(sess, self,
&lint_name[..], level);

match self.find_lint(&lint_name[..], sess, None) {
Ok(lint_id) => self.set_level(lint_id, (level, CommandLine)),
Err(FindLintError::Removed) => { }
Err(_) => {
match self.lint_groups.iter().map(|(&x, pair)| (x, pair.0.clone()))
.collect::<FnvHashMap<&'static str,
Expand All @@ -255,8 +248,11 @@ impl LintStore {
self.set_level(*lint_id, (level, CommandLine)))
.collect::<Vec<()>>();
}
None => sess.err(&format!("unknown {} flag: {}",
level.as_str(), lint_name)),
None => {
// The lint or lint group doesn't exist.
// This is an error, but it was handled
// by check_lint_name_cmdline.
}
}
}
}
Expand Down Expand Up @@ -331,29 +327,39 @@ pub fn gather_attrs(attrs: &[ast::Attribute])
-> Vec<Result<(InternedString, Level, Span), Span>> {
let mut out = vec!();
for attr in attrs {
let level = match Level::from_str(&attr.name()) {
None => continue,
Some(lvl) => lvl,
};
let r = gather_attr(attr);
out.extend(r.into_iter());
}
out
}

attr::mark_used(attr);
pub fn gather_attr(attr: &ast::Attribute)
-> Vec<Result<(InternedString, Level, Span), Span>> {
let mut out = vec!();

let meta = &attr.node.value;
let metas = match meta.node {
ast::MetaList(_, ref metas) => metas,
_ => {
out.push(Err(meta.span));
continue;
}
};
let level = match Level::from_str(&attr.name()) {
None => return out,
Some(lvl) => lvl,
};

for meta in metas {
out.push(match meta.node {
ast::MetaWord(ref lint_name) => Ok((lint_name.clone(), level, meta.span)),
_ => Err(meta.span),
});
attr::mark_used(attr);

let meta = &attr.node.value;
let metas = match meta.node {
ast::MetaList(_, ref metas) => metas,
_ => {
out.push(Err(meta.span));
return out;
}
};

for meta in metas {
out.push(match meta.node {
ast::MetaWord(ref lint_name) => Ok((lint_name.clone(), level, meta.span)),
_ => Err(meta.span),
});
}

out
}

Expand Down Expand Up @@ -555,9 +561,9 @@ pub trait LintContext: Sized {
(*lint_id, level, span))
.collect(),
None => {
self.span_lint(builtin::UNKNOWN_LINTS, span,
&format!("unknown `{}` attribute: `{}`",
level.as_str(), lint_name));
// The lint or lint group doesn't exist.
// This is an error, but it was handled
// by check_lint_name_attribute.
continue;
}
}
Expand Down Expand Up @@ -869,6 +875,7 @@ impl<'a, 'tcx, 'v> hir_visit::Visitor<'v> for LateContext<'a, 'tcx> {
}

fn visit_attribute(&mut self, attr: &ast::Attribute) {
check_lint_name_attribute(self, attr);
run_lints!(self, check_attribute, late_passes, attr);
}
}
Expand Down Expand Up @@ -1082,6 +1089,113 @@ impl LateLintPass for GatherNodeLevels {
}
}

enum CheckLintNameResult<'a> {
Ok,
// Lint doesn't exist
NoLint,
// The lint is either renamed or removed and a warning was
// generated in the DiagnosticBuilder
Mentioned(DiagnosticBuilder<'a>)
}

/// Checks the name of a lint for its existence, and whether it was
/// renamed or removed. Generates a DiagnosticBuilder containing a
/// warning for renamed and removed lints. This is over both lint
/// names from attributes and those passed on the command line. Since
/// it emits non-fatal warnings and there are *two* lint passes that
/// inspect attributes, this is only run from the late pass to avoid
/// printing duplicate warnings.
fn check_lint_name<'a>(sess: &'a Session,
lint_cx: &LintStore,
lint_name: &str,
span: Option<Span>) -> CheckLintNameResult<'a> {
match lint_cx.by_name.get(lint_name) {
Some(&Renamed(ref new_name, _)) => {
let warning = format!("lint {} has been renamed to {}",
lint_name, new_name);
let db = match span {
Some(span) => sess.struct_span_warn(span, &warning[..]),
None => sess.struct_warn(&warning[..]),
};
CheckLintNameResult::Mentioned(db)
},
Some(&Removed(ref reason)) => {
let warning = format!("lint {} has been removed: {}", lint_name, reason);
let db = match span {
Some(span) => sess.struct_span_warn(span, &warning[..]),
None => sess.struct_warn(&warning[..])
};
CheckLintNameResult::Mentioned(db)
},
None => {
match lint_cx.lint_groups.get(lint_name) {
None => {
CheckLintNameResult::NoLint
}
Some(_) => {
/* lint group exists */
CheckLintNameResult::Ok
}
}
}
Some(_) => {
/* lint exists */
CheckLintNameResult::Ok
}
}
}

// Checks the validity of lint names derived from attributes
fn check_lint_name_attribute(cx: &LateContext, attr: &ast::Attribute) {
for result in gather_attr(attr) {
match result {
Err(_) => {
// Malformed lint attr. Reported by with_lint_attrs
continue;
}
Ok((lint_name, _, span)) => {
match check_lint_name(&cx.tcx.sess, &cx.lints, &lint_name[..], Some(span)) {
CheckLintNameResult::Ok => (),
CheckLintNameResult::Mentioned(mut db) => {
db.emit();
}
CheckLintNameResult::NoLint => {
cx.span_lint(builtin::UNKNOWN_LINTS, span,
&format!("unknown lint: `{}`",
lint_name));
}
}
}
}
}
}

// Checks the validity of lint names derived from the command line
fn check_lint_name_cmdline(sess: &Session, lint_cx: &LintStore,
lint_name: &str, level: Level) {
let db = match check_lint_name(sess, lint_cx, lint_name, None) {
CheckLintNameResult::Ok => None,
CheckLintNameResult::Mentioned(db) => Some(db),
CheckLintNameResult::NoLint => {
Some(sess.struct_err(&format!("unknown lint: `{}`", lint_name)))
}
};

if let Some(mut db) = db {
let msg = format!("requested on the command line with `{} {}`",
match level {
Level::Allow => "-A",
Level::Warn => "-W",
Level::Deny => "-D",
Level::Forbid => "-F",
},
lint_name);
db.note(&msg);
db.emit();
}
}


/// Perform lint checking on a crate.
///
/// Consumes the `lint_store` field of the `Session`.
Expand Down
7 changes: 5 additions & 2 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,12 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
// We have one lint pass defined specially
store.register_late_pass(sess, false, box lint::GatherNodeLevels);

// Insert temporary renamings for a one-time deprecation
// Register renamed and removed lints
store.register_renamed("unknown_features", "unused_features");

store.register_removed("unsigned_negation", "replaced by negate_unsigned feature gate");
store.register_removed("negate_unsigned", "cast a signed value instead");
store.register_removed("raw_pointer_derive", "using derive with raw pointers is ok");
// This was renamed to raw_pointer_derive, which was then removed,
// so it is also considered removed
store.register_removed("raw_pointer_deriving", "using derive with raw pointers is ok");
}
14 changes: 14 additions & 0 deletions src/test/compile-fail/lint-malformed.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright 2016 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.

#![deny = "foo"] //~ ERR malformed lint attribute
#![allow(bar = "baz")] //~ ERR malformed lint attribute

fn main() { }
20 changes: 20 additions & 0 deletions src/test/compile-fail/lint-removed-cmdline.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2016 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.

// The raw_pointer_derived lint warns about its removal
// cc #30346

// compile-flags:-D raw_pointer_derive

// error-pattern:lint raw_pointer_derive has been removed
// error-pattern:requested on the command line with `-D raw_pointer_derive`

#[deny(warnings)]
fn main() { let unused = (); }
16 changes: 16 additions & 0 deletions src/test/compile-fail/lint-removed.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2016 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.

// The raw_pointer_derived lint only warns about its own removal
// cc #30346

#[deny(raw_pointer_derive)] //~ WARN raw_pointer_derive has been removed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether the diagnostic for #[deny(raw_pointer_deriving)] would only report about it being renamed, or also that it has been removed? I suspect its former and I think we might want the behaviour resembling the later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are probably right, but I don't think it's worth the effort to change. What will happen is they will get the renamed warning, change the name, then get the removed warning. It is a minor hassle for a scenario that is never going to come up in practice with this particular lint, and may never happen again.

#[deny(warnings)]
fn main() { let unused = (); } //~ ERR unused
18 changes: 18 additions & 0 deletions src/test/compile-fail/lint-renamed-cmdline.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright 2016 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.

// compile-flags:-D unknown_features

// error-pattern:lint unknown_features has been renamed to unused_features
// error-pattern:requested on the command line with `-D unknown_features`
// error-pattern:unused

#[deny(unused)]
fn main() { let unused = (); }
13 changes: 13 additions & 0 deletions src/test/compile-fail/lint-renamed.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright 2016 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.

#[deny(unknown_features)] //~ WARN lint unknown_features has been renamed to unused_features
#[deny(unused)]
fn main() { let unused = (); } //~ ERR unused
16 changes: 16 additions & 0 deletions src/test/compile-fail/lint-unknown-lint-cmdline.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2016 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.

// compile-flags:-D bogus

// error-pattern:unknown lint
// error-pattern:requested on the command line with `-D bogus`

fn main() { }
13 changes: 13 additions & 0 deletions src/test/compile-fail/lint-unknown-lint.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright 2016 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.

#![allow(not_a_real_lint)] //~ WARN unknown lint
#![deny(unused)]
fn main() { let unused = (); } //~ ERR unused variable