diff --git a/src/librustc_lint/levels.rs b/src/librustc_lint/levels.rs index 274e57ae64c..70694720154 100644 --- a/src/librustc_lint/levels.rs +++ b/src/librustc_lint/levels.rs @@ -17,8 +17,8 @@ use rustc_middle::ty::TyCtxt; use rustc_session::lint::{builtin, Level, Lint}; use rustc_session::parse::feature_err; use rustc_session::Session; -use rustc_span::source_map::MultiSpan; use rustc_span::symbol::{sym, Symbol}; +use rustc_span::{source_map::MultiSpan, Span, DUMMY_SP}; use std::cmp; @@ -80,6 +80,8 @@ impl<'s> LintLevelsBuilder<'s> { let level = cmp::min(level, self.sets.lint_cap); let lint_flag_val = Symbol::intern(lint_name); + self.check_gated_lint(lint_flag_val, DUMMY_SP); + let ids = match store.find_lints(&lint_name) { Ok(ids) => ids, Err(_) => continue, // errors handled in check_lint_name_cmdline above @@ -211,6 +213,7 @@ impl<'s> LintLevelsBuilder<'s> { 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) => { + self.check_gated_lint(name, attr.span); let src = LintSource::Node(name, li.span(), reason); for id in ids { specs.insert(*id, (level, src)); @@ -383,6 +386,20 @@ impl<'s> LintLevelsBuilder<'s> { BuilderPush { prev, changed: prev != self.cur } } + fn check_gated_lint(&self, name: Symbol, span: Span) { + if name.as_str() == "unsafe_op_in_unsafe_fn" + && !self.sess.features_untracked().unsafe_block_in_unsafe_fn + { + feature_err( + &self.sess.parse_sess, + sym::unsafe_block_in_unsafe_fn, + span, + "the `unsafe_op_in_unsafe_fn` lint is unstable", + ) + .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; diff --git a/src/librustc_middle/mir/query.rs b/src/librustc_middle/mir/query.rs index f19270e5561..c63d7215867 100644 --- a/src/librustc_middle/mir/query.rs +++ b/src/librustc_middle/mir/query.rs @@ -21,16 +21,17 @@ pub enum UnsafetyViolationKind { GeneralAndConstFn, /// Borrow of packed field. /// Has to be handled as a lint for backwards compatibility. - BorrowPacked(hir::HirId), + BorrowPacked, /// Unsafe operation in an `unsafe fn` but outside an `unsafe` block. /// Has to be handled as a lint for backwards compatibility. /// Should stay gated under `#![feature(unsafe_block_in_unsafe_fn)]`. - UnsafeFn(hir::HirId), + UnsafeFn, } #[derive(Copy, Clone, PartialEq, RustcEncodable, RustcDecodable, HashStable)] pub struct UnsafetyViolation { pub source_info: SourceInfo, + pub lint_root: hir::HirId, pub description: Symbol, pub details: Symbol, pub kind: UnsafetyViolationKind, diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs index bcadbd09107..0434f3447bc 100644 --- a/src/librustc_mir/transform/check_unsafety.rs +++ b/src/librustc_mir/transform/check_unsafety.rs @@ -2,6 +2,7 @@ use rustc_data_structures::fx::FxHashSet; use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def_id::{DefId, LocalDefId}; +use rustc_hir::hir_id::HirId; use rustc_hir::intravisit; use rustc_hir::Node; use rustc_middle::mir::visit::{MutatingUseContext, PlaceContext, Visitor}; @@ -10,6 +11,7 @@ use rustc_middle::ty::cast::CastTy; use rustc_middle::ty::query::Providers; use rustc_middle::ty::{self, TyCtxt}; use rustc_session::lint::builtin::{SAFE_PACKED_BORROWS, UNSAFE_OP_IN_UNSAFE_FN, UNUSED_UNSAFE}; +use rustc_session::lint::Level; use rustc_span::symbol::{sym, Symbol}; use std::ops::Bound; @@ -220,7 +222,18 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { for (i, elem) in place.projection.iter().enumerate() { let proj_base = &place.projection[..i]; - let old_source_info = self.source_info; + if context.is_borrow() { + if util::is_disaligned(self.tcx, self.body, self.param_env, *place) { + self.require_unsafe( + "borrow of packed field", + "fields of packed structs might be misaligned: dereferencing a \ + misaligned pointer or even just creating a misaligned reference \ + is undefined behavior", + UnsafetyViolationKind::BorrowPacked, + ); + } + } + let source_info = self.source_info; if let [] = proj_base { let decl = &self.body.local_decls[place.local]; if decl.internal { @@ -301,7 +314,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { } _ => {} } - self.source_info = old_source_info; + self.source_info = source_info; } } } @@ -314,9 +327,15 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { kind: UnsafetyViolationKind, ) { let source_info = self.source_info; + let lint_root = self.body.source_scopes[self.source_info.scope] + .local_data + .as_ref() + .assert_crate_local() + .lint_root; self.register_violations( &[UnsafetyViolation { source_info, + lint_root, description: Symbol::intern(description), details: Symbol::intern(details), kind, @@ -343,7 +362,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { match violation.kind { UnsafetyViolationKind::GeneralAndConstFn | UnsafetyViolationKind::General => {} - UnsafetyViolationKind::BorrowPacked(_) => { + UnsafetyViolationKind::BorrowPacked => { if self.min_const_fn { // const fns don't need to be backwards compatible and can // emit these violations as a hard error instead of a backwards @@ -351,7 +370,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { violation.kind = UnsafetyViolationKind::General; } } - UnsafetyViolationKind::UnsafeFn(_) => { + UnsafetyViolationKind::UnsafeFn => { bug!("`UnsafetyViolationKind::UnsafeFn` in an `Safe` context") } } @@ -365,14 +384,9 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { Safety::FnUnsafe if self.tcx.features().unsafe_block_in_unsafe_fn => { for violation in violations { let mut violation = *violation; - let lint_root = self.body.source_scopes[self.source_info.scope] - .local_data - .as_ref() - .assert_crate_local() - .lint_root; // FIXME(LeSeulArtichaut): what to do with `UnsafetyViolationKind::BorrowPacked`? - violation.kind = UnsafetyViolationKind::UnsafeFn(lint_root); + violation.kind = UnsafetyViolationKind::UnsafeFn; if !self.violations.contains(&violation) { self.violations.push(violation) } @@ -394,7 +408,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { UnsafetyViolationKind::GeneralAndConstFn => {} // these things are forbidden in const fns UnsafetyViolationKind::General - | UnsafetyViolationKind::BorrowPacked(_) => { + | UnsafetyViolationKind::BorrowPacked => { let mut violation = *violation; // const fns don't need to be backwards compatible and can // emit these violations as a hard error instead of a backwards @@ -404,7 +418,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { self.violations.push(violation) } } - UnsafetyViolationKind::UnsafeFn(_) => bug!( + UnsafetyViolationKind::UnsafeFn => bug!( "`UnsafetyViolationKind::UnsafeFn` in an `ExplicitUnsafe` context" ), } @@ -657,10 +671,13 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: DefId) { let UnsafetyCheckResult { violations, unsafe_blocks } = tcx.unsafety_check_result(def_id.expect_local()); - let or_block_msg = if tcx.features().unsafe_block_in_unsafe_fn { "" } else { " or block" }; - - for &UnsafetyViolation { source_info, description, details, kind } in violations.iter() { + for &UnsafetyViolation { source_info, lint_root, description, details, kind } in + violations.iter() + { // Report an error. + let unsafe_fn_msg = + if unsafe_op_in_unsafe_fn_allowed(tcx, lint_root) { "" } else { " function or" }; + match kind { UnsafetyViolationKind::GeneralAndConstFn | UnsafetyViolationKind::General => { // once @@ -668,26 +685,26 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: DefId) { tcx.sess, source_info.span, E0133, - "{} is unsafe and requires unsafe function{}", + "{} is unsafe and requires unsafe{} block", description, - or_block_msg, + unsafe_fn_msg, ) .span_label(source_info.span, &*description.as_str()) .note(&details.as_str()) .emit(); } - UnsafetyViolationKind::BorrowPacked(lint_hir_id) => { + UnsafetyViolationKind::BorrowPacked => { if let Some(impl_def_id) = builtin_derive_def_id(tcx, def_id) { tcx.ensure().unsafe_derive_on_repr_packed(impl_def_id); } else { tcx.struct_span_lint_hir( SAFE_PACKED_BORROWS, - lint_hir_id, + lint_root, source_info.span, |lint| { lint.build(&format!( - "{} is unsafe and requires unsafe function{} (error E0133)", - description, or_block_msg, + "{} is unsafe and requires unsafe{} block (error E0133)", + description, unsafe_fn_msg, )) .note(&details.as_str()) .emit() @@ -695,9 +712,9 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: DefId) { ) } } - UnsafetyViolationKind::UnsafeFn(lint_hir_id) => tcx.struct_span_lint_hir( + UnsafetyViolationKind::UnsafeFn => tcx.struct_span_lint_hir( UNSAFE_OP_IN_UNSAFE_FN, - lint_hir_id, + lint_root, source_info.span, |lint| { lint.build(&format!( @@ -728,3 +745,7 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: DefId) { report_unused_unsafe(tcx, &unsafe_used, block_id); } } + +fn unsafe_op_in_unsafe_fn_allowed(tcx: TyCtxt<'_>, id: HirId) -> bool { + tcx.lint_level_at_node(UNSAFE_OP_IN_UNSAFE_FN, id).0 == Level::Allow +} diff --git a/src/librustc_mir_build/build/block.rs b/src/librustc_mir_build/build/block.rs index b052c839152..4e4f0dc74cb 100644 --- a/src/librustc_mir_build/build/block.rs +++ b/src/librustc_mir_build/build/block.rs @@ -4,6 +4,8 @@ use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder}; use crate::hair::*; use rustc_hir as hir; use rustc_middle::mir::*; +use rustc_session::lint::builtin::UNSAFE_OP_IN_UNSAFE_FN; +use rustc_session::lint::Level; use rustc_span::Span; impl<'a, 'tcx> Builder<'a, 'tcx> { @@ -218,7 +220,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { match self.unpushed_unsafe { Safety::Safe => {} // no longer treat `unsafe fn`s as `unsafe` contexts (see RFC #2585) - Safety::FnUnsafe if self.hir.tcx().features().unsafe_block_in_unsafe_fn => {} + Safety::FnUnsafe + if self.hir.tcx().lint_level_at_node(UNSAFE_OP_IN_UNSAFE_FN, hir_id).0 + != Level::Allow => {} _ => return, } self.unpushed_unsafe = Safety::ExplicitUnsafe(hir_id); @@ -233,7 +237,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { .push_unsafe_count .checked_sub(1) .unwrap_or_else(|| span_bug!(span, "unsafe count underflow")); - if self.push_unsafe_count == 0 { Some(self.unpushed_unsafe) } else { None } + if self.push_unsafe_count == 0 { + Some(self.unpushed_unsafe) + } else { + None + } } }; diff --git a/src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.rs b/src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.rs index 80005fd4ef6..bf33ac67fca 100644 --- a/src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.rs +++ b/src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.rs @@ -1,11 +1,4 @@ -#![deny(unused_unsafe)] - -unsafe fn unsf() {} - -unsafe fn foo() { - unsafe { //~ ERROR unnecessary `unsafe` block - unsf() - } -} +#![deny(unsafe_op_in_unsafe_fn)] +//~^ ERROR the `unsafe_op_in_unsafe_fn` lint is unstable fn main() {} diff --git a/src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.stderr b/src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.stderr index 64874e590fb..c5cad4a98d9 100644 --- a/src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.stderr +++ b/src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.stderr @@ -1,16 +1,30 @@ -error: unnecessary `unsafe` block - --> $DIR/feature-gate-unsafe_block_in_unsafe_fn.rs:6:5 +error[E0658]: the `unsafe_op_in_unsafe_fn` lint is unstable + --> $DIR/feature-gate-unsafe_block_in_unsafe_fn.rs:1:1 | -LL | unsafe fn foo() { - | --------------- because it's nested under this `unsafe` fn -LL | unsafe { - | ^^^^^^ unnecessary `unsafe` block +LL | #![deny(unsafe_op_in_unsafe_fn)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -note: the lint level is defined here - --> $DIR/feature-gate-unsafe_block_in_unsafe_fn.rs:1:9 - | -LL | #![deny(unused_unsafe)] - | ^^^^^^^^^^^^^ + = note: see issue #71668 for more information + = help: add `#![feature(unsafe_block_in_unsafe_fn)]` to the crate attributes to enable -error: aborting due to previous error +error[E0658]: the `unsafe_op_in_unsafe_fn` lint is unstable + --> $DIR/feature-gate-unsafe_block_in_unsafe_fn.rs:1:1 + | +LL | #![deny(unsafe_op_in_unsafe_fn)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #71668 for more information + = help: add `#![feature(unsafe_block_in_unsafe_fn)]` to the crate attributes to enable +error[E0658]: the `unsafe_op_in_unsafe_fn` lint is unstable + --> $DIR/feature-gate-unsafe_block_in_unsafe_fn.rs:1:1 + | +LL | #![deny(unsafe_op_in_unsafe_fn)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #71668 for more information + = help: add `#![feature(unsafe_block_in_unsafe_fn)]` to the crate attributes to enable + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs b/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs index 0ffb2827bfd..7feb68b9857 100644 --- a/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs +++ b/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs @@ -21,6 +21,12 @@ unsafe fn baz() { #[allow(unsafe_op_in_unsafe_fn)] unsafe fn qux() { unsf(); // no error + + unsafe { unsf() } + //~^ ERROR unnecessary `unsafe` block } -fn main() {} +fn main() { + unsf() + //~^ ERROR call to unsafe function is unsafe and requires unsafe function or block +} diff --git a/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.stderr b/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.stderr index be6af2a11c4..e812210498c 100644 --- a/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.stderr +++ b/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.stderr @@ -25,5 +25,20 @@ note: the lint level is defined here LL | #![deny(unused_unsafe)] | ^^^^^^^^^^^^^ -error: aborting due to previous error; 1 warning emitted +error: unnecessary `unsafe` block + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:25:5 + | +LL | unsafe { unsf() } + | ^^^^^^ unnecessary `unsafe` block +error[E0133]: call to unsafe function is unsafe and requires unsafe function or block + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:30:5 + | +LL | unsf() + | ^^^^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior + +error: aborting due to 3 previous errors; 1 warning emitted + +For more information about this error, try `rustc --explain E0133`.