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.

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

Make --force-warns a normal lint level option #86970

Merged
merged 2 commits into from
Jul 19, 2021
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
24 changes: 5 additions & 19 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ use rustc_span::symbol::{sym, Symbol};
use rustc_span::{source_map::MultiSpan, Span, DUMMY_SP};
use tracing::debug;

use std::cmp;

fn lint_levels(tcx: TyCtxt<'_>, (): ()) -> LintLevelMap {
let store = unerased_lint_store(tcx);
let crate_attrs = tcx.hir().attrs(CRATE_HIR_ID);
Expand Down Expand Up @@ -91,36 +89,24 @@ impl<'s> LintLevelsBuilder<'s> {
for &(ref lint_name, level) in &sess.opts.lint_opts {
store.check_lint_name_cmdline(sess, &lint_name, level, self.crate_attrs);
let orig_level = level;

// If the cap is less than this specified level, e.g., if we've got
// `--cap-lints allow` but we've also got `-D foo` then we ignore
// this specification as the lint cap will set it to allow anyway.
let level = cmp::min(level, self.sets.lint_cap);

let lint_flag_val = Symbol::intern(lint_name);

let ids = match store.find_lints(&lint_name) {
Ok(ids) => ids,
Err(_) => continue, // errors handled in check_lint_name_cmdline above
};
for id in ids {
// ForceWarn and Forbid cannot be overriden
if let Some((Level::ForceWarn | Level::Forbid, _)) = specs.get(&id) {
Copy link
Member

Choose a reason for hiding this comment

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

Is this behavior documented anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sort of. However, the fact that forbid currently overrides the others isn't specified, and force-warns isn't documented at all because it's unstable. Given that the behavior is intrinsic to the definition of these lint levels, I don't think it's too surprising. It might be worth documenting it in the long run though?

continue;
}

self.check_gated_lint(id, DUMMY_SP);
let src = LintLevelSource::CommandLine(lint_flag_val, orig_level);
specs.insert(id, (level, src));
}
}

for lint_name in &sess.opts.force_warns {
store.check_lint_name_cmdline(sess, lint_name, Level::ForceWarn, self.crate_attrs);
let lints = store
.find_lints(lint_name)
.unwrap_or_else(|_| bug!("A valid lint failed to produce a lint ids"));
for id in lints {
let src = LintLevelSource::CommandLine(Symbol::intern(lint_name), Level::ForceWarn);
specs.insert(id, (Level::ForceWarn, src));
}
}

self.cur = self.sets.list.push(LintSet { specs, parent: COMMAND_LINE });
}

Expand Down
39 changes: 13 additions & 26 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,6 @@ 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 @@ -1172,20 +1171,20 @@ pub fn get_cmd_lint_options(
matches: &getopts::Matches,
error_format: ErrorOutputType,
debugging_opts: &DebuggingOptions,
) -> (Vec<(String, lint::Level)>, bool, Option<lint::Level>, Vec<String>) {
) -> (Vec<(String, lint::Level)>, bool, Option<lint::Level>) {
let mut lint_opts_with_position = vec![];
let mut describe_lints = false;

for level in [lint::Allow, lint::Warn, lint::Deny, lint::Forbid] {
for (passed_arg_pos, lint_name) in matches.opt_strs_pos(level.as_str()) {
let arg_pos = if let lint::Forbid = level {
// HACK: forbid is always specified last, so it can't be overridden.
// FIXME: remove this once <https://github.com/rust-lang/rust/issues/70819> is
// fixed and `forbid` works as expected.
usize::MAX
} else {
passed_arg_pos
};
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`",
);
}

for level in [lint::Allow, lint::Warn, lint::ForceWarn, lint::Deny, lint::Forbid] {
for (arg_pos, lint_name) in matches.opt_strs_pos(level.as_str()) {
if lint_name == "help" {
describe_lints = true;
} else {
Expand All @@ -1206,18 +1205,7 @@ pub fn get_cmd_lint_options(
.unwrap_or_else(|| early_error(error_format, &format!("unknown lint level: `{}`", 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").into_iter().map(|name| name.replace('-', "_")).collect();

(lint_opts, describe_lints, lint_cap, force_warns)
(lint_opts, describe_lints, lint_cap)
}

/// Parses the `--color` flag.
Expand Down Expand Up @@ -1955,7 +1943,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
.unwrap_or_else(|e| early_error(error_format, &e[..]));

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

check_debug_option_stability(&debugging_opts, error_format, json_rendered);
Expand Down Expand Up @@ -2129,7 +2117,6 @@ 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: 0 additions & 1 deletion compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ top_level_options!(
debuginfo: DebugInfo [TRACKED],
lint_opts: Vec<(String, lint::Level)> [TRACKED_NO_CRATE_HASH],
lint_cap: Option<lint::Level> [TRACKED_NO_CRATE_HASH],
force_warns: Vec<String> [TRACKED_NO_CRATE_HASH],
describe_lints: bool [UNTRACKED],
output_types: OutputTypes [TRACKED],
search_paths: Vec<SearchPath> [UNTRACKED],
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ 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, _) =
let (lint_opts, describe_lints, lint_cap) =
get_cmd_lint_options(matches, error_format, &debugging_opts);

Ok(Options {
Expand Down
11 changes: 11 additions & 0 deletions src/test/ui/lint/cli-lint-override.forbid_warn.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: extern declarations without an explicit ABI are deprecated
--> $DIR/cli-lint-override.rs:12:1
|
LL | extern fn foo() {}
| ^^^^^^^^^^^^^^^ ABI should be specified here
|
= note: requested on the command line with `-F missing-abi`
= help: the default ABI is C

error: aborting due to previous error

11 changes: 11 additions & 0 deletions src/test/ui/lint/cli-lint-override.force_warn_deny.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
warning: extern declarations without an explicit ABI are deprecated
--> $DIR/cli-lint-override.rs:12:1
|
LL | extern fn foo() {}
| ^^^^^^^^^^^^^^^ ABI should be specified here
|
= note: requested on the command line with `--force-warns missing-abi`
= help: the default ABI is C

warning: 1 warning emitted

17 changes: 17 additions & 0 deletions src/test/ui/lint/cli-lint-override.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Tests that subsequent lints specified via the command line override
// each other, except for ForceWarn and Forbid, which cannot be overriden.
//
// revisions: warn_deny forbid_warn force_warn_deny
//
//[warn_deny] compile-flags: --warn missing_abi --deny missing_abi
//[forbid_warn] compile-flags: --warn missing_abi --forbid missing_abi
//[force_warn_deny] compile-flags: -Z unstable-options --force-warns missing_abi --allow missing_abi
//[force_warn_deny] check-pass


extern fn foo() {}
//[warn_deny]~^ ERROR extern declarations without an explicit ABI are deprecated
//[forbid_warn]~^^ ERROR extern declarations without an explicit ABI are deprecated
//[force_warn_deny]~^^^ WARN extern declarations without an explicit ABI are deprecated

fn main() {}
11 changes: 11 additions & 0 deletions src/test/ui/lint/cli-lint-override.warn_deny.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: extern declarations without an explicit ABI are deprecated
--> $DIR/cli-lint-override.rs:12:1
|
LL | extern fn foo() {}
| ^^^^^^^^^^^^^^^ ABI should be specified here
|
= note: requested on the command line with `-D missing-abi`
= help: the default ABI is C

error: aborting due to previous error

7 changes: 7 additions & 0 deletions src/test/ui/lint/cli-unknown-force-warn.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Checks that rustc correctly errors when passed an invalid lint with
// `--force-warns`. This is a regression test for issue #86958.
//
// compile-flags: -Z unstable-options --force-warns foo-qux
// error-pattern: unknown lint: `foo_qux`

fn main() {}
15 changes: 15 additions & 0 deletions src/test/ui/lint/cli-unknown-force-warn.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0602]: unknown lint: `foo_qux`
|
= note: requested on the command line with `--force-warns foo_qux`

error[E0602]: unknown lint: `foo_qux`
Copy link
Member

Choose a reason for hiding this comment

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

Any idea why this error gets emitted multiple times?

Copy link
Contributor Author

@inquisitivecrystal inquisitivecrystal Jul 15, 2021

Choose a reason for hiding this comment

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

This happens for some other cases as well. I think it's a product of the way the tests are run, though I'm not totally sure?

|
= note: requested on the command line with `--force-warns foo_qux`

error[E0602]: unknown lint: `foo_qux`
|
= note: requested on the command line with `--force-warns foo_qux`

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0602`.