Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Minimum lint levels for C-future-compatibility issues #59658

Closed
wants to merge 6 commits into from
Closed

Minimum lint levels for C-future-compatibility issues #59658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8b2bf1d
minimum-lint-levels: Introduce basics.
Centril Apr 1, 2019
5e541e2
minimum-lint-levels: Emit unused_attributes when e.g. allow(foo) has …
Centril Apr 3, 2019
a122019
minimum-lint-levels: Use declare_unsuppressable_lint! for ILLEGAL_FLO…
Centril Apr 7, 2019
8471a95
minimum-lint-levels: Add dedicated tests.
Centril Apr 7, 2019
b566b20
minimum-lint-levels: Adjust tests.
Centril Apr 1, 2019
6b13fad
minimum-lint-levels: --bless tests.
Centril Apr 7, 2019
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
44 changes: 25 additions & 19 deletions src/librustc/lint/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ declare_lint! {
"constant evaluation detected erroneous expression"
}

declare_lint! {
pub UNUSED_ATTRIBUTES,
Warn,
"detects attributes that were not used by the compiler"
}

declare_lint! {
pub UNUSED_IMPORTS,
Warn,
Expand Down Expand Up @@ -119,7 +125,7 @@ declare_lint! {
"detects trivial casts of numeric types which could be removed"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub PRIVATE_IN_PUBLIC,
Warn,
"detect private items in public interfaces not caught by the old implementation"
Expand All @@ -137,7 +143,7 @@ declare_lint! {
"detect public re-exports of private extern crates"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub INVALID_TYPE_PARAM_DEFAULT,
Deny,
"type parameter default erroneously allowed in invalid location"
Expand All @@ -149,62 +155,62 @@ declare_lint! {
"lints that have been renamed or removed"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub SAFE_EXTERN_STATICS,
Deny,
"safe access to extern statics was erroneously allowed"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub SAFE_PACKED_BORROWS,
Warn,
"safe borrows of fields of packed structs were was erroneously allowed"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub PATTERNS_IN_FNS_WITHOUT_BODY,
Warn,
"patterns in functions without body were erroneously allowed"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub LEGACY_DIRECTORY_OWNERSHIP,
Deny,
"non-inline, non-`#[path]` modules (e.g., `mod foo;`) were erroneously allowed in some files \
not named `mod.rs`"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub LEGACY_CONSTRUCTOR_VISIBILITY,
Deny,
"detects use of struct constructors that would be invisible with new visibility rules"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub MISSING_FRAGMENT_SPECIFIER,
Deny,
"detects missing fragment specifiers in unused `macro_rules!` patterns"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES,
Deny,
"detects parenthesized generic parameters in type and module names"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub LATE_BOUND_LIFETIME_ARGUMENTS,
Warn,
"detects generic lifetime arguments in path segments with late bound lifetime parameters"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub INCOHERENT_FUNDAMENTAL_IMPLS,
Deny,
"potentially-conflicting impls were erroneously allowed"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub ORDER_DEPENDENT_TRAIT_OBJECTS,
Deny,
"trait-object types were treated as different depending on marker-trait order"
Expand Down Expand Up @@ -247,7 +253,7 @@ declare_lint! {
"detects lifetime parameters that are never used"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub TYVAR_BEHIND_RAW_POINTER,
Warn,
"raw pointer to an inference variable"
Expand All @@ -272,7 +278,7 @@ declare_lint! {
instead of `crate`, `self`, or an extern crate name"
}

declare_lint! {
declare_unsuppressable_lint! {
pub ILLEGAL_FLOATING_POINT_LITERAL_PATTERN,
Warn,
"floating-point literals cannot be used in patterns"
Expand Down Expand Up @@ -320,7 +326,7 @@ declare_lint! {
"detects code samples in docs of private items not documented by rustdoc"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub WHERE_CLAUSES_OBJECT_SAFETY,
Warn,
"checks the object safety of where clauses"
Expand Down Expand Up @@ -352,7 +358,7 @@ declare_lint! {
"outlives requirements can be inferred"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub DUPLICATE_MATCHER_BINDING_NAME,
Deny,
"duplicate macro matcher binding name"
Expand All @@ -366,7 +372,7 @@ pub mod parser {
"detects the use of `?` as a macro separator"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub ILL_FORMED_ATTRIBUTE_INPUT,
Warn,
"ill-formed attribute inputs that were previously accepted and used in practice"
Expand All @@ -380,13 +386,13 @@ declare_lint! {
report_in_external_macro: true
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub AMBIGUOUS_ASSOCIATED_ITEMS,
Warn,
"ambiguous associated items"
}

declare_lint! {
declare_lint! { // FIXME(centril): consider using `declare_unsuppressable_lint`
pub NESTED_IMPL_TRAIT,
Warn,
"nested occurrence of `impl Trait` type"
Expand Down
71 changes: 60 additions & 11 deletions src/librustc/lint/levels.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl LintLevelSets {
Err(_) => continue, // errors handled in check_lint_name_cmdline above
};
for id in ids {
let src = LintSource::CommandLine(lint_flag_val);
let src = LintSource::CommandLine(lint_flag_val, None);
specs.insert(id, (level, src));
}
}
Expand All @@ -90,18 +90,28 @@ impl LintLevelSets {
// lint.
let mut level = level.unwrap_or_else(|| lint.default_level(sess));

// Ensure we don't go below the minimum level of the lint.
// Note that we allow `--cap-lints` to cap `WARNINGS`,
// but we will never allow `--cap-lints` to cap the lint itself.
let warn_level = cmp::max(level, lint.min_level);

// If we're about to issue a warning, check at the last minute for any
// directives against the warnings "lint". If, for example, there's an
// `allow(warnings)` in scope then we want to respect that instead.
if level == Level::Warn {
if warn_level == Level::Warn {
let (warnings_level, warnings_src) =
self.get_lint_id_level(LintId::of(lint::builtin::WARNINGS),
idx,
aux);
if let Some(configured_warning_level) = warnings_level {
if configured_warning_level != Level::Warn {
let orig_level = Some(level);
level = configured_warning_level;
src = warnings_src;
src = match warnings_src {
LintSource::CommandLine(s, _) => LintSource::CommandLine(s, orig_level),
LintSource::Node(n, _, s, r) => LintSource::Node(n, orig_level, s, r),
other => other,
};
}
}
}
Expand Down Expand Up @@ -290,7 +300,7 @@ impl<'a> LintLevelsBuilder<'a> {
let name = meta_item.path.segments.last().expect("empty lint name").ident.name;
match store.check_lint_name(&name.as_str(), tool_name) {
CheckLintNameResult::Ok(ids) => {
let src = LintSource::Node(name, li.span(), reason);
let src = LintSource::Node(name, None, li.span(), reason);
for id in ids {
specs.insert(*id, (level, src));
}
Expand All @@ -301,7 +311,7 @@ impl<'a> LintLevelsBuilder<'a> {
Ok(ids) => {
let complete_name = &format!("{}::{}", tool_name.unwrap(), name);
let src = LintSource::Node(
Symbol::intern(complete_name), li.span(), reason
Symbol::intern(complete_name), None, li.span(), reason
);
for id in ids {
specs.insert(*id, (level, src));
Expand Down Expand Up @@ -334,7 +344,7 @@ impl<'a> LintLevelsBuilder<'a> {
).emit();

let src = LintSource::Node(
Symbol::intern(&new_lint_name), li.span(), reason
Symbol::intern(&new_lint_name), None, li.span(), reason
);
for id in ids {
specs.insert(*id, (level, src));
Expand Down Expand Up @@ -403,6 +413,8 @@ impl<'a> LintLevelsBuilder<'a> {
}

for (id, &(level, ref src)) in specs.iter() {
self.lint_higher_minimum_attr_lint(id.lint, level, src, &specs);

if level == Level::Forbid {
continue
}
Expand All @@ -412,11 +424,11 @@ impl<'a> LintLevelsBuilder<'a> {
};
let forbidden_lint_name = match forbid_src {
LintSource::Default => id.to_string(),
LintSource::Node(name, _, _) => name.to_string(),
LintSource::CommandLine(name) => name.to_string(),
LintSource::Node(name, _, _, _) => name.to_string(),
LintSource::CommandLine(name, _) => name.to_string(),
};
let (lint_attr_name, lint_attr_span) = match *src {
LintSource::Node(name, span, _) => (name, span),
LintSource::Node(name, _, span, _) => (name, span),
_ => continue,
};
let mut diag_builder = struct_span_err!(self.sess,
Expand All @@ -429,14 +441,14 @@ impl<'a> LintLevelsBuilder<'a> {
diag_builder.span_label(lint_attr_span, "overruled by previous forbid");
match forbid_src {
LintSource::Default => {},
LintSource::Node(_, forbid_source_span, reason) => {
LintSource::Node(_, _, forbid_source_span, reason) => {
diag_builder.span_label(forbid_source_span,
"`forbid` level set here");
if let Some(rationale) = reason {
diag_builder.note(&rationale.as_str());
}
},
LintSource::CommandLine(_) => {
LintSource::CommandLine(_, _) => {
diag_builder.note("`forbid` lint level was set on command line");
}
}
Expand All @@ -460,6 +472,43 @@ impl<'a> LintLevelsBuilder<'a> {
}
}

/// If we have e.g. `#[allow($some_future_compat_lint)]` this will have
/// no effect as `min_level > Allow`. We want to tell the user about this.
fn lint_higher_minimum_attr_lint(
&self,
lint: &'static Lint,
level: Level,
src: &LintSource,
specs: &FxHashMap<LintId, (Level, LintSource)>,
) {
let min_level = lint.min_level;
if min_level <= level {
return;
}

if let LintSource::Node(name, _, span, _) = src {
// Get the `unused_attributes` lint specs:
let unused = builtin::UNUSED_ATTRIBUTES;
let (lvl, src) = self.sets.get_lint_level(unused, self.cur, Some(&specs), &self.sess);

// Construct base diagnostic for `unused_attributes`:
let level_str = level.as_str();
let msg = format!("#[{}({})] has no effect", level_str, name);
let multi_span = Some((*span).into());
let mut err = lint::struct_lint_level(self.sess, unused, lvl, src, multi_span, &msg);

// Add notes about minimum levels and what the user should do here:
err.note(&format!("the minimum lint level for `{}` is `{}`", name, min_level.as_str()))
.note(&format!("the lint level cannot be reduced to `{}`", level_str))
.help(&format!("remove the #[{}({})] directive", level_str, name));

// If it is a future compat lint, warn the user about it.
crate::lint::check_future_compatibility(self.sess, lint, &mut err, Some(name));

err.emit();
}
}

/// Called after `push` when the scope of a set of attributes are exited.
pub fn pop(&mut self, push: BuilderPush) {
self.cur = push.prev;
Expand Down
Loading