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

[beta] backports #81774

Merged
merged 10 commits into from
Feb 6, 2021
Merged
15 changes: 15 additions & 0 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use rustc_session::SessionLintStore;
use rustc_span::lev_distance::find_best_match_for_name;
use rustc_span::{symbol::Symbol, MultiSpan, Span, DUMMY_SP};
use rustc_target::abi::LayoutOf;
use tracing::debug;

use std::cell::Cell;
use std::slice;
Expand Down Expand Up @@ -335,6 +336,20 @@ impl LintStore {
}
}

/// True if this symbol represents a lint group name.
pub fn is_lint_group(&self, lint_name: Symbol) -> bool {
debug!(
"is_lint_group(lint_name={:?}, lint_groups={:?})",
lint_name,
self.lint_groups.keys().collect::<Vec<_>>()
);
let lint_name_str = &*lint_name.as_str();
self.lint_groups.contains_key(&lint_name_str) || {
let warnings_name_str = crate::WARNINGS.name_lower();
lint_name_str == &*warnings_name_str
}
}

/// 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
Expand Down
101 changes: 73 additions & 28 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc_ast::attr;
use rustc_ast::unwrap_or;
use rustc_ast_pretty::pprust;
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::{struct_span_err, Applicability};
use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::def_id::{CrateNum, LOCAL_CRATE};
use rustc_hir::{intravisit, HirId};
Expand All @@ -17,11 +17,15 @@ use rustc_middle::lint::{
};
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::TyCtxt;
use rustc_session::lint::{builtin, Level, Lint, LintId};
use rustc_session::lint::{
builtin::{self, FORBIDDEN_LINT_GROUPS},
Level, Lint, LintId,
};
use rustc_session::parse::feature_err;
use rustc_session::Session;
use rustc_span::symbol::{sym, Symbol};
use rustc_span::{source_map::MultiSpan, Span, DUMMY_SP};
use tracing::debug;

use std::cmp;

Expand Down Expand Up @@ -51,6 +55,7 @@ pub struct LintLevelsBuilder<'s> {
id_to_set: FxHashMap<HirId, u32>,
cur: u32,
warn_about_weird_lints: bool,
store: &'s LintStore,
}

pub struct BuilderPush {
Expand All @@ -59,13 +64,14 @@ pub struct BuilderPush {
}

impl<'s> LintLevelsBuilder<'s> {
pub fn new(sess: &'s Session, warn_about_weird_lints: bool, store: &LintStore) -> Self {
pub fn new(sess: &'s Session, warn_about_weird_lints: bool, store: &'s LintStore) -> Self {
let mut builder = LintLevelsBuilder {
sess,
sets: LintLevelSets::new(),
cur: 0,
id_to_set: Default::default(),
warn_about_weird_lints,
store,
};
builder.process_command_line(sess, store);
assert_eq!(builder.sets.list.len(), 1);
Expand Down Expand Up @@ -120,36 +126,75 @@ impl<'s> LintLevelsBuilder<'s> {
if let (Level::Forbid, old_src) =
self.sets.get_lint_level(id.lint, self.cur, Some(&specs), &self.sess)
{
let mut diag_builder = struct_span_err!(
self.sess,
src.span(),
E0453,
"{}({}) incompatible with previous forbid",
level.as_str(),
src.name(),
// Backwards compatibility check:
//
// We used to not consider `forbid(lint_group)`
// as preventing `allow(lint)` for some lint `lint` in
// `lint_group`. For now, issue a future-compatibility
// warning for this case.
let id_name = id.lint.name_lower();
let fcw_warning = match old_src {
LintLevelSource::Default => false,
LintLevelSource::Node(symbol, _, _) => self.store.is_lint_group(symbol),
LintLevelSource::CommandLine(symbol, _) => self.store.is_lint_group(symbol),
};
debug!(
"fcw_warning={:?}, specs.get(&id) = {:?}, old_src={:?}, id_name={:?}",
fcw_warning, specs, old_src, id_name
);
diag_builder.span_label(src.span(), "overruled by previous forbid");
match old_src {
LintLevelSource::Default => {
diag_builder.note(&format!(
"`forbid` lint level is the default for {}",
id.to_string()
));
}
LintLevelSource::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());

let decorate_diag_builder = |mut diag_builder: DiagnosticBuilder<'_>| {
diag_builder.span_label(src.span(), "overruled by previous forbid");
match old_src {
LintLevelSource::Default => {
diag_builder.note(&format!(
"`forbid` lint level is the default for {}",
id.to_string()
));
}
LintLevelSource::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());
}
}
LintLevelSource::CommandLine(_, _) => {
diag_builder.note("`forbid` lint level was set on command line");
}
}
LintLevelSource::CommandLine(_, _) => {
diag_builder.note("`forbid` lint level was set on command line");
}
diag_builder.emit();
};
if !fcw_warning {
let diag_builder = struct_span_err!(
self.sess,
src.span(),
E0453,
"{}({}) incompatible with previous forbid",
level.as_str(),
src.name(),
);
decorate_diag_builder(diag_builder);
} else {
self.struct_lint(
FORBIDDEN_LINT_GROUPS,
Some(src.span().into()),
|diag_builder| {
let diag_builder = diag_builder.build(&format!(
"{}({}) incompatible with previous forbid",
level.as_str(),
src.name(),
));
decorate_diag_builder(diag_builder);
},
);
}
diag_builder.emit();

// Retain the forbid lint level
return;
// Retain the forbid lint level, unless we are
// issuing a FCW. In the FCW case, we want to
// respect the new setting.
if !fcw_warning {
return;
}
}
}
specs.insert(id, (level, src));
Expand Down
11 changes: 6 additions & 5 deletions compiler/rustc_lint/src/panic_fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use rustc_parse_format::{ParseMode, Parser, Piece};
use rustc_span::{sym, InnerSpan};

declare_lint! {
/// The `panic_fmt` lint detects `panic!("..")` with `{` or `}` in the string literal.
/// The `non_fmt_panic` lint detects `panic!("..")` with `{` or `}` in the string literal
/// when it is not used as a format string.
///
/// ### Example
///
Expand All @@ -23,13 +24,13 @@ declare_lint! {
/// with a single argument does not use `format_args!()`.
/// A future edition of Rust will interpret this string as format string,
/// which would break this.
PANIC_FMT,
NON_FMT_PANIC,
Warn,
"detect braces in single-argument panic!() invocations",
report_in_external_macro
}

declare_lint_pass!(PanicFmt => [PANIC_FMT]);
declare_lint_pass!(PanicFmt => [NON_FMT_PANIC]);

impl<'tcx> LateLintPass<'tcx> for PanicFmt {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
Expand Down Expand Up @@ -92,7 +93,7 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc
[] => vec![fmt_span],
v => v.iter().map(|span| fmt_span.from_inner(*span)).collect(),
};
cx.struct_span_lint(PANIC_FMT, arg_spans, |lint| {
cx.struct_span_lint(NON_FMT_PANIC, arg_spans, |lint| {
let mut l = lint.build(match n_arguments {
1 => "panic message contains an unused formatting placeholder",
_ => "panic message contains unused formatting placeholders",
Expand Down Expand Up @@ -129,7 +130,7 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc
Some(v) if v.len() == 1 => "panic message contains a brace",
_ => "panic message contains braces",
};
cx.struct_span_lint(PANIC_FMT, brace_spans.unwrap_or(vec![expn.call_site]), |lint| {
cx.struct_span_lint(NON_FMT_PANIC, brace_spans.unwrap_or(vec![expn.call_site]), |lint| {
let mut l = lint.build(msg);
l.note("this message is not used as a format string, but will be in a future Rust edition");
if expn.call_site.contains(arg.span) {
Expand Down
37 changes: 37 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,42 @@ use crate::{declare_lint, declare_lint_pass, declare_tool_lint};
use rustc_span::edition::Edition;
use rustc_span::symbol::sym;

declare_lint! {
/// The `forbidden_lint_groups` lint detects violations of
/// `forbid` applied to a lint group. Due to a bug in the compiler,
/// these used to be overlooked entirely. They now generate a warning.
///
/// ### Example
///
/// ```rust
/// #![forbid(warnings)]
/// #![deny(bad_style)]
///
/// fn main() {}
/// ```
///
/// {{produces}}
///
/// ### Recommended fix
///
/// If your crate is using `#![forbid(warnings)]`,
/// we recommend that you change to `#![deny(warnings)]`.
///
/// ### Explanation
///
/// Due to a compiler bug, applying `forbid` to lint groups
/// previously had no effect. The bug is now fixed but instead of
/// enforcing `forbid` we issue this future-compatibility warning
/// to avoid breaking existing crates.
pub FORBIDDEN_LINT_GROUPS,
Warn,
"applying forbid to lint-groups",
@future_incompatible = FutureIncompatibleInfo {
reference: "issue #81670 <https://github.com/rust-lang/rust/issues/81670>",
edition: None,
};
}

declare_lint! {
/// The `ill_formed_attribute_input` lint detects ill-formed attribute
/// inputs that were previously accepted and used in practice.
Expand Down Expand Up @@ -2839,6 +2875,7 @@ declare_lint_pass! {
/// Does nothing as a lint pass, but registers some `Lint`s
/// that are used by other parts of the compiler.
HardwiredLints => [
FORBIDDEN_LINT_GROUPS,
ILLEGAL_FLOATING_POINT_LITERAL_PATTERN,
ARITHMETIC_OVERFLOW,
UNCONDITIONAL_PANIC,
Expand Down
14 changes: 11 additions & 3 deletions compiler/rustc_middle/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,17 @@ use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_errors::{DiagnosticBuilder, DiagnosticId};
use rustc_hir::HirId;
use rustc_session::lint::{builtin, Level, Lint, LintId};
use rustc_session::lint::{
builtin::{self, FORBIDDEN_LINT_GROUPS},
Level, Lint, LintId,
};
use rustc_session::{DiagnosticMessageId, Session};
use rustc_span::hygiene::MacroKind;
use rustc_span::source_map::{DesugaringKind, ExpnKind, MultiSpan};
use rustc_span::{symbol, Span, Symbol, DUMMY_SP};

/// How a lint level was set.
#[derive(Clone, Copy, PartialEq, Eq, HashStable)]
#[derive(Clone, Copy, PartialEq, Eq, HashStable, Debug)]
pub enum LintLevelSource {
/// Lint is at the default level as declared
/// in rustc or a plugin.
Expand Down Expand Up @@ -87,7 +90,12 @@ impl LintLevelSets {
// 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 {
//
// We exempt `FORBIDDEN_LINT_GROUPS` from this because it specifically
// triggers in cases (like #80988) where you have `forbid(warnings)`,
// and so if we turned that into an error, it'd defeat the purpose of the
// future compatibility warning.
if level == Level::Warn && LintId::of(lint) != LintId::of(FORBIDDEN_LINT_GROUPS) {
let (warnings_level, warnings_src) =
self.get_lint_id_level(LintId::of(builtin::WARNINGS), idx, aux);
if let Some(configured_warning_level) = warnings_level {
Expand Down
10 changes: 3 additions & 7 deletions compiler/rustc_mir_build/src/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::build::ForGuard::OutsideGuard;
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
use crate::thir::*;
use rustc_hir as hir;
use rustc_middle::middle::region;
use rustc_middle::mir::*;
use rustc_session::lint::builtin::UNSAFE_OP_IN_UNSAFE_FN;
use rustc_session::lint::Level;
Expand All @@ -13,7 +12,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
crate fn ast_block(
&mut self,
destination: Place<'tcx>,
scope: Option<region::Scope>,
block: BasicBlock,
ast_block: &'tcx hir::Block<'tcx>,
source_info: SourceInfo,
Expand All @@ -30,10 +28,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.in_opt_scope(opt_destruction_scope.map(|de| (de, source_info)), move |this| {
this.in_scope((region_scope, source_info), LintLevel::Inherited, move |this| {
if targeted_by_break {
this.in_breakable_scope(None, destination, scope, span, |this| {
this.in_breakable_scope(None, destination, span, |this| {
Some(this.ast_block_stmts(
destination,
scope,
block,
span,
stmts,
Expand All @@ -42,7 +39,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
))
})
} else {
this.ast_block_stmts(destination, scope, block, span, stmts, expr, safety_mode)
this.ast_block_stmts(destination, block, span, stmts, expr, safety_mode)
}
})
})
Expand All @@ -51,7 +48,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fn ast_block_stmts(
&mut self,
destination: Place<'tcx>,
scope: Option<region::Scope>,
mut block: BasicBlock,
span: Span,
stmts: Vec<StmtRef<'tcx>>,
Expand Down Expand Up @@ -186,7 +182,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
};
this.block_context.push(BlockFrame::TailExpr { tail_result_is_ignored, span });

unpack!(block = this.into(destination, scope, block, expr));
unpack!(block = this.into(destination, block, expr));
let popped = this.block_context.pop();

assert!(popped.map_or(false, |bf| bf.is_tail_expr()));
Expand Down
Loading