Skip to content

Commit

Permalink
Auto merge of #85788 - rylev:force-warns, r=nikomatsakis
Browse files Browse the repository at this point in the history
Support for force-warns

Implements #85512.

This PR adds a new command line option `force-warns` which will force the provided lints to warn even if they are allowed by some other mechanism such as `#![allow(warnings)]`.

Some remaining issues:
* #85512 mentions that `force-warns` should also be capable of taking lint groups instead of individual lints. This is not implemented.
* If a lint has a higher warning level than `warn`, this will cause that lint to warn instead. We probably want to allow the lint to error if it is set to a higher lint and is not allowed somewhere else.
* One test is currently ignored because it's not working - when a deny-by-default lint is allowed, it does not currently warn under `force-warns`. I'm not sure why, but I wanted to get this in before the weekend.

r? `@nikomatsakis`
  • Loading branch information
bors committed Jun 4, 2021
2 parents 289ada5 + 896898e commit 595088d
Show file tree
Hide file tree
Showing 28 changed files with 321 additions and 23 deletions.
37 changes: 24 additions & 13 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,14 @@ impl LintStore {
}
}

/// Checks the validity of lint names derived from the command line
pub fn check_lint_name_cmdline(&self, sess: &Session, lint_name: &str, level: Level) {
/// Checks the validity of lint names derived from the command line. Returns
/// true if the lint is valid, false otherwise.
pub fn check_lint_name_cmdline(
&self,
sess: &Session,
lint_name: &str,
level: Option<Level>,
) -> bool {
let db = match self.check_lint_name(lint_name, None) {
CheckLintNameResult::Ok(_) => None,
CheckLintNameResult::Warning(ref msg, _) => Some(sess.struct_warn(msg)),
Expand All @@ -361,18 +367,23 @@ impl LintStore {
};

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);
if let Some(level) = level {
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();
false
} else {
true
}
}

Expand Down
16 changes: 15 additions & 1 deletion compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl<'s> LintLevelsBuilder<'s> {
self.sets.lint_cap = sess.opts.lint_cap.unwrap_or(Level::Forbid);

for &(ref lint_name, level) in &sess.opts.lint_opts {
store.check_lint_name_cmdline(sess, &lint_name, level);
store.check_lint_name_cmdline(sess, &lint_name, Some(level));
let orig_level = level;

// If the cap is less than this specified level, e.g., if we've got
Expand All @@ -109,6 +109,16 @@ impl<'s> LintLevelsBuilder<'s> {
}
}

for lint_name in &sess.opts.force_warns {
let valid = store.check_lint_name_cmdline(sess, lint_name, None);
if valid {
let lints = store
.find_lints(lint_name)
.unwrap_or_else(|_| bug!("A valid lint failed to produce a lint ids"));
self.sets.force_warns.extend(&lints);
}
}

self.sets.list.push(LintSet::CommandLine { specs });
}

Expand Down Expand Up @@ -142,6 +152,9 @@ impl<'s> LintLevelsBuilder<'s> {
LintLevelSource::Default => false,
LintLevelSource::Node(symbol, _, _) => self.store.is_lint_group(symbol),
LintLevelSource::CommandLine(symbol, _) => self.store.is_lint_group(symbol),
LintLevelSource::ForceWarn(_symbol) => {
bug!("forced warn lint returned a forbid lint level")
}
};
debug!(
"fcw_warning={:?}, specs.get(&id) = {:?}, old_src={:?}, id_name={:?}",
Expand All @@ -166,6 +179,7 @@ impl<'s> LintLevelsBuilder<'s> {
LintLevelSource::CommandLine(_, _) => {
diag_builder.note("`forbid` lint level was set on command line");
}
_ => bug!("forced warn lint returned a forbid lint level"),
}
diag_builder.emit();
};
Expand Down
30 changes: 26 additions & 4 deletions compiler/rustc_middle/src/lint.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::cmp;

use crate::ich::StableHashingContext;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_errors::{DiagnosticBuilder, DiagnosticId};
use rustc_hir::HirId;
Expand All @@ -28,6 +28,9 @@ pub enum LintLevelSource {
/// The provided `Level` is the level specified on the command line.
/// (The actual level may be lower due to `--cap-lints`.)
CommandLine(Symbol, Level),

/// Lint is being forced to warn no matter what.
ForceWarn(Symbol),
}

impl LintLevelSource {
Expand All @@ -36,6 +39,7 @@ impl LintLevelSource {
LintLevelSource::Default => symbol::kw::Default,
LintLevelSource::Node(name, _, _) => name,
LintLevelSource::CommandLine(name, _) => name,
LintLevelSource::ForceWarn(name) => name,
}
}

Expand All @@ -44,6 +48,7 @@ impl LintLevelSource {
LintLevelSource::Default => DUMMY_SP,
LintLevelSource::Node(_, span, _) => span,
LintLevelSource::CommandLine(_, _) => DUMMY_SP,
LintLevelSource::ForceWarn(_) => DUMMY_SP,
}
}
}
Expand All @@ -55,6 +60,7 @@ pub type LevelAndSource = (Level, LintLevelSource);
pub struct LintLevelSets {
pub list: Vec<LintSet>,
pub lint_cap: Level,
pub force_warns: FxHashSet<LintId>,
}

#[derive(Debug)]
Expand All @@ -73,7 +79,11 @@ pub enum LintSet {

impl LintLevelSets {
pub fn new() -> Self {
LintLevelSets { list: Vec::new(), lint_cap: Level::Forbid }
LintLevelSets {
list: Vec::new(),
lint_cap: Level::Forbid,
force_warns: FxHashSet::default(),
}
}

pub fn get_lint_level(
Expand All @@ -83,6 +93,11 @@ impl LintLevelSets {
aux: Option<&FxHashMap<LintId, LevelAndSource>>,
sess: &Session,
) -> LevelAndSource {
// Check whether we should always warn
if self.force_warns.contains(&LintId::of(lint)) {
return (Level::Warn, LintLevelSource::ForceWarn(Symbol::intern(lint.name)));
}

let (level, mut src) = self.get_lint_id_level(LintId::of(lint), idx, aux);

// If `level` is none then we actually assume the default level for this
Expand Down Expand Up @@ -176,11 +191,11 @@ impl LintLevelMap {
impl<'a> HashStable<StableHashingContext<'a>> for LintLevelMap {
#[inline]
fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
let LintLevelMap { ref sets, ref id_to_set } = *self;
let LintLevelMap { ref sets, ref id_to_set, .. } = *self;

id_to_set.hash_stable(hcx, hasher);

let LintLevelSets { ref list, lint_cap } = *sets;
let LintLevelSets { ref list, lint_cap, .. } = *sets;

lint_cap.hash_stable(hcx, hasher);

Expand Down Expand Up @@ -346,6 +361,13 @@ pub fn struct_lint_level<'s, 'd>(
);
}
}
LintLevelSource::ForceWarn(_) => {
sess.diag_note_once(
&mut err,
DiagnosticMessageId::from(lint),
"warning forced by `force-warns` commandline option",
);
}
}

err.code(DiagnosticId::Lint { name, has_future_breakage });
Expand Down
30 changes: 26 additions & 4 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,7 @@ impl Default for Options {
optimize: OptLevel::No,
debuginfo: DebugInfo::None,
lint_opts: Vec::new(),
force_warns: Vec::new(),
lint_cap: None,
describe_lints: false,
output_types: OutputTypes(BTreeMap::new()),
Expand Down Expand Up @@ -1092,6 +1093,13 @@ pub fn rustc_short_optgroups() -> Vec<RustcOptGroup> {
level",
"LEVEL",
),
opt::multi_s(
"",
"force-warns",
"Specifiy lints that should warn even if \
they are allowed somewhere else",
"LINT",
),
opt::multi_s("C", "codegen", "Set a codegen option", "OPT[=VALUE]"),
opt::flag_s("V", "version", "Print version info and exit"),
opt::flag_s("v", "verbose", "Use verbose output"),
Expand Down Expand Up @@ -1156,7 +1164,8 @@ pub fn rustc_optgroups() -> Vec<RustcOptGroup> {
pub fn get_cmd_lint_options(
matches: &getopts::Matches,
error_format: ErrorOutputType,
) -> (Vec<(String, lint::Level)>, bool, Option<lint::Level>) {
debugging_opts: &DebuggingOptions,
) -> (Vec<(String, lint::Level)>, bool, Option<lint::Level>, Vec<String>) {
let mut lint_opts_with_position = vec![];
let mut describe_lints = false;

Expand Down Expand Up @@ -1189,7 +1198,18 @@ pub fn get_cmd_lint_options(
lint::Level::from_str(&cap)
.unwrap_or_else(|| early_error(error_format, &format!("unknown lint level: `{}`", cap)))
});
(lint_opts, describe_lints, lint_cap)

if !debugging_opts.unstable_options && matches.opt_present("force-warns") {
early_error(
error_format,
"the `-Z unstable-options` flag must also be passed to enable \
the flag `--force-warns=lints`",
);
}

let force_warns = matches.opt_strs("force-warns");

(lint_opts, describe_lints, lint_cap, force_warns)
}

/// Parses the `--color` flag.
Expand Down Expand Up @@ -1926,9 +1946,10 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
let crate_types = parse_crate_types_from_list(unparsed_crate_types)
.unwrap_or_else(|e| early_error(error_format, &e[..]));

let (lint_opts, describe_lints, lint_cap) = get_cmd_lint_options(matches, error_format);

let mut debugging_opts = DebuggingOptions::build(matches, error_format);
let (lint_opts, describe_lints, lint_cap, force_warns) =
get_cmd_lint_options(matches, error_format, &debugging_opts);

check_debug_option_stability(&debugging_opts, error_format, json_rendered);

if !debugging_opts.unstable_options && json_unused_externs {
Expand Down Expand Up @@ -2100,6 +2121,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
optimize: opt_level,
debuginfo,
lint_opts,
force_warns,
lint_cap,
describe_lints,
output_types,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ top_level_options!(
debuginfo: DebugInfo [TRACKED],
lint_opts: Vec<(String, lint::Level)> [TRACKED],
lint_cap: Option<lint::Level> [TRACKED],
force_warns: Vec<String> [TRACKED],
describe_lints: bool [UNTRACKED],
output_types: OutputTypes [TRACKED],
search_paths: Vec<SearchPath> [UNTRACKED],
Expand Down
21 changes: 21 additions & 0 deletions src/doc/unstable-book/src/compiler-flags/force-warns.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# `force-warns`

The tracking issue for this feature is: [#85512](https://github.com/rust-lang/rust/issues/85512).

------------------------

This feature allows you to cause any lint to produce a warning even if the lint has a different level by default or another level is set somewhere else. For instance, the `force-warns` option can be used to make a lint (e.g., `dead_code`) produce a warning even if that lint is allowed in code with `#![allow(dead_code)]`.

## Example

```rust,ignore (partial-example)
#![allow(dead_code)]
fn dead_function() {}
// This would normally not produce a warning even though the
// function is not used, because dead code is being allowed
fn main() {}
```

We can force a warning to be produced by providing `--force-warns dead_code` to rustc.
3 changes: 2 additions & 1 deletion src/librustdoc/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,8 @@ impl Options {
let generate_redirect_map = matches.opt_present("generate-redirect-map");
let show_type_layout = matches.opt_present("show-type-layout");

let (lint_opts, describe_lints, lint_cap) = get_cmd_lint_options(matches, error_format);
let (lint_opts, describe_lints, lint_cap, _) =
get_cmd_lint_options(matches, error_format, &debugging_opts);

Ok(Options {
input,
Expand Down
8 changes: 8 additions & 0 deletions src/librustdoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,14 @@ fn opts() -> Vec<RustcOptGroup> {
"LEVEL",
)
}),
unstable("force-warns", |o| {
o.optopt(
"",
"force-warns",
"Lints that will warn even if allowed somewhere else",
"LINTS",
)
}),
unstable("index-page", |o| {
o.optopt("", "index-page", "Markdown file to be used as index page", "PATH")
}),
Expand Down
1 change: 1 addition & 0 deletions src/test/run-make/unstable-flag-required/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@

all:
$(RUSTDOC) --output-format=json x.html 2>&1 | diff - output-format-json.stderr
$(RUSTC) --force-warns dead_code x.rs 2>&1 | diff - force-warns.stderr
2 changes: 2 additions & 0 deletions src/test/run-make/unstable-flag-required/force-warns.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
error: the `-Z unstable-options` flag must also be passed to enable the flag `--force-warns=lints`

11 changes: 11 additions & 0 deletions src/test/ui/lint/force-warn/force-allowed-by-default-lint.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// compile-flags: --force-warns elided_lifetimes_in_paths
// check-pass

struct Foo<'a> {
x: &'a u32,
}

fn foo(x: &Foo) {}
//~^ WARN hidden lifetime parameters in types are deprecated

fn main() {}
10 changes: 10 additions & 0 deletions src/test/ui/lint/force-warn/force-allowed-by-default-lint.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
warning: hidden lifetime parameters in types are deprecated
--> $DIR/force-allowed-by-default-lint.rs:8:12
|
LL | fn foo(x: &Foo) {}
| ^^^- help: indicate the anonymous lifetime: `<'_>`
|
= note: warning forced by `force-warns` commandline option

warning: 1 warning emitted

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// compile-flags: --force-warns const_err
// check-pass

#![allow(const_err)]
const C: i32 = 1 / 0;
//~^ WARN any use of this value will cause an error
//~| WARN this was previously accepted by the compiler

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
warning: any use of this value will cause an error
--> $DIR/force-allowed-deny-by-default-lint.rs:5:16
|
LL | const C: i32 = 1 / 0;
| ---------------^^^^^-
| |
| attempt to divide `1_i32` by zero
|
= note: warning forced by `force-warns` commandline option
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #71800 <https://github.com/rust-lang/rust/issues/71800>

warning: 1 warning emitted

9 changes: 9 additions & 0 deletions src/test/ui/lint/force-warn/force-allowed-warning.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// compile-flags: --force-warns dead_code
// check-pass

#![allow(dead_code)]

fn dead_function() {}
//~^ WARN function is never used

fn main() {}
10 changes: 10 additions & 0 deletions src/test/ui/lint/force-warn/force-allowed-warning.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
warning: function is never used: `dead_function`
--> $DIR/force-allowed-warning.rs:6:4
|
LL | fn dead_function() {}
| ^^^^^^^^^^^^^
|
= note: warning forced by `force-warns` commandline option

warning: 1 warning emitted

Loading

0 comments on commit 595088d

Please sign in to comment.