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

Fix ICE when a future-incompat-report has its command-line level capped #78663

Merged
merged 1 commit into from
Nov 3, 2020
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: 5 additions & 4 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ impl<'s> LintLevelsBuilder<'s> {

for &(ref lint_name, level) in &sess.opts.lint_opts {
store.check_lint_name_cmdline(sess, &lint_name, level);
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
Expand All @@ -88,7 +89,7 @@ impl<'s> LintLevelsBuilder<'s> {
};
for id in ids {
self.check_gated_lint(id, DUMMY_SP);
let src = LintSource::CommandLine(lint_flag_val);
let src = LintSource::CommandLine(lint_flag_val, orig_level);
specs.insert(id, (level, src));
}
}
Expand Down Expand Up @@ -123,7 +124,7 @@ impl<'s> LintLevelsBuilder<'s> {
diag_builder.note(&rationale.as_str());
}
}
LintSource::CommandLine(_) => {
LintSource::CommandLine(_, _) => {
diag_builder.note("`forbid` lint level was set on command line");
}
}
Expand Down Expand Up @@ -422,7 +423,7 @@ impl<'s> LintLevelsBuilder<'s> {
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::CommandLine(name, _) => name.to_string(),
};
let (lint_attr_name, lint_attr_span) = match *src {
LintSource::Node(name, span, _) => (name, span),
Expand All @@ -446,7 +447,7 @@ impl<'s> LintLevelsBuilder<'s> {
diag_builder.note(&rationale.as_str());
}
}
LintSource::CommandLine(_) => {
LintSource::CommandLine(_, _) => {
diag_builder.note("`forbid` lint level was set on command line");
}
}
Expand Down
14 changes: 8 additions & 6 deletions compiler/rustc_middle/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,25 @@ pub enum LintSource {
Node(Symbol, Span, Option<Symbol> /* RFC 2383 reason */),

/// Lint level was set by a command-line flag.
CommandLine(Symbol),
/// The provided `Level` is the level specified on the command line -
/// the actual level may be lower due to `--cap-lints`
CommandLine(Symbol, Level),
}

impl LintSource {
pub fn name(&self) -> Symbol {
match *self {
LintSource::Default => symbol::kw::Default,
LintSource::Node(name, _, _) => name,
LintSource::CommandLine(name) => name,
LintSource::CommandLine(name, _) => name,
}
}

pub fn span(&self) -> Span {
match *self {
LintSource::Default => DUMMY_SP,
LintSource::Node(_, span, _) => span,
LintSource::CommandLine(_) => DUMMY_SP,
LintSource::CommandLine(_, _) => DUMMY_SP,
}
}
}
Expand Down Expand Up @@ -279,12 +281,12 @@ pub fn struct_lint_level<'s, 'd>(
&format!("`#[{}({})]` on by default", level.as_str(), name),
);
}
LintSource::CommandLine(lint_flag_val) => {
let flag = match level {
LintSource::CommandLine(lint_flag_val, orig_level) => {
let flag = match orig_level {
Level::Warn => "-W",
Level::Deny => "-D",
Level::Forbid => "-F",
Level::Allow => panic!(),
Level::Allow => "-A",
Copy link
Member

@tmandry tmandry Nov 3, 2020

Choose a reason for hiding this comment

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

This would still be unexpected, no?

(feel free to ignore if I'm wrong or this is otherwise intentional)

Copy link
Member Author

Choose a reason for hiding this comment

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

You can pass -A warnings on the command line. Previously, we would never show a specific help message, since we would bail out when the lint is allowed. Now, we may emit a future-incompat report, where we will want to specify the command-line flag.

};
let hyphen_case_lint_name = name.replace("_", "-");
if lint_flag_val.as_str() == name {
Expand Down
10 changes: 10 additions & 0 deletions src/test/ui/lint/issue-78660-cap-lints-future-compat.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// compile-flags: -D warnings --cap-lints allow
// check-pass

// Regression test for issue #78660
// Tests that we don't ICE when a future-incompat-report lint has
// has a command-line source, but is capped to allow

fn main() {
["hi"].into_iter();
}
11 changes: 11 additions & 0 deletions src/test/ui/lint/issue-78660-cap-lints-future-compat.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Future incompatibility report: Future breakage date: None, diagnostic:
warning: this method call currently resolves to `<&[T; N] as IntoIterator>::into_iter` (due to autoref coercions), but that might change in the future when `IntoIterator` impls for arrays are added.
--> $DIR/issue-78660-cap-lints-future-compat.rs:9:12
|
LL | ["hi"].into_iter();
| ^^^^^^^^^ help: use `.iter()` instead of `.into_iter()` to avoid ambiguity: `iter`
|
= note: `-D array-into-iter` implied by `-D warnings`
Copy link
Member Author

Choose a reason for hiding this comment

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

We should suppress this message when a lint only shows up in the future incompat report. However, this only affects nightly, so I'll address it in a follow up PR.

= 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 #66145 <https://github.com/rust-lang/rust/issues/66145>