Auto merge of #30878 - brson:raw-pointer-derive, r=brson

This adds back the raw_pointer_derive lint as a 'removed' lint, so that its removal does not cause errors (#30346) but warnings.

In the process I discovered regressions in the code for renamed and removed lints, which didn't appear to have any tests. The addition of a second lint pass (ast vs. hir) meant that attributes were being inspected twice, renamed and removed warnings printed twice. I restructured the code so these tests are only done once and added tests. Unfortunately it makes the patch more complicated for the needed beta backport.

r? @nikomatsakis
This commit is contained in:
bors 2016-01-15 23:26:38 +00:00
commit cee9463d24
10 changed files with 276 additions and 42 deletions

View File

@ -136,6 +136,12 @@
"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)]
@ -163,7 +169,8 @@ fn get_lints(&self) -> LintArray {
PRIVATE_IN_PUBLIC,
INVALID_TYPE_PARAM_DEFAULT,
MATCH_OF_UNIT_VARIANT_VIA_PAREN_DOTDOT,
CONST_ERR
CONST_ERR,
RAW_POINTER_DERIVE
)
}
}

View File

@ -247,21 +247,10 @@ fn find_lint(&self, lint_name: &str, sess: &Session, span: Option<Span>)
{
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)
@ -270,8 +259,12 @@ fn find_lint(&self, lint_name: &str, sess: &Session, span: Option<Span>)
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,
@ -283,8 +276,11 @@ pub fn process_command_line(&mut self, sess: &Session) {
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.
}
}
}
}
@ -359,32 +355,42 @@ 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,
};
attr::mark_used(attr);
let meta = &attr.node.value;
let metas = match meta.node {
ast::MetaList(_, ref metas) => metas,
_ => {
out.push(Err(meta.span));
continue;
}
};
for meta in metas {
out.push(match meta.node {
ast::MetaWord(ref lint_name) => Ok((lint_name.clone(), level, meta.span)),
_ => Err(meta.span),
});
}
let r = gather_attr(attr);
out.extend(r.into_iter());
}
out
}
pub fn gather_attr(attr: &ast::Attribute)
-> Vec<Result<(InternedString, Level, Span), Span>> {
let mut out = vec!();
let level = match Level::from_str(&attr.name()) {
None => return out,
Some(lvl) => lvl,
};
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
}
/// Emit a lint as a warning or an error (or not at all)
/// according to `level`.
///
@ -587,9 +593,9 @@ fn with_lint_attrs<F>(&mut self,
(*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;
}
}
@ -901,6 +907,7 @@ fn visit_path_list_item(&mut self, prefix: &hir::Path, item: &hir::PathListItem)
}
fn visit_attribute(&mut self, attr: &ast::Attribute) {
check_lint_name_attribute(self, attr);
run_lints!(self, check_attribute, late_passes, attr);
}
}
@ -1114,6 +1121,113 @@ fn check_item(&mut self, cx: &LateContext, it: &hir::Item) {
}
}
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`.

View File

@ -172,9 +172,12 @@ macro_rules! add_lint_group {
// 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");
}

View File

@ -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() { }

View File

@ -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 = (); }

View File

@ -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
#[deny(warnings)]
fn main() { let unused = (); } //~ ERR unused

View File

@ -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 = (); }

View File

@ -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

View File

@ -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() { }

View File

@ -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