From a977df35d160c1fe7040c76a9276b64e7a7aedec Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Sun, 3 May 2020 23:11:34 +0200 Subject: [PATCH 01/16] Implement RFC 2585 --- src/librustc_feature/active.rs | 3 + src/librustc_middle/mir/mod.rs | 2 +- src/librustc_middle/mir/query.rs | 7 +++ src/librustc_mir/transform/check_unsafety.rs | 63 +++++++++++++++++--- src/librustc_mir_build/build/block.rs | 2 + src/librustc_session/lint/builtin.rs | 7 +++ src/librustc_span/symbol.rs | 1 + 7 files changed, 75 insertions(+), 10 deletions(-) diff --git a/src/librustc_feature/active.rs b/src/librustc_feature/active.rs index 90b2380d86450..fd35cb6c3f785 100644 --- a/src/librustc_feature/active.rs +++ b/src/librustc_feature/active.rs @@ -571,6 +571,9 @@ declare_features! ( /// Allows the use of `#[ffi_const]` on foreign functions. (active, ffi_const, "1.45.0", Some(58328), None), + /// No longer treat an unsafe function as an unsafe block. + (active, unsafe_block_in_unsafe_fn, "1.45.0", Some(71668), None), + // ------------------------------------------------------------------------- // feature-group-end: actual feature gates // ------------------------------------------------------------------------- diff --git a/src/librustc_middle/mir/mod.rs b/src/librustc_middle/mir/mod.rs index c279213e5bd0e..243f81459e760 100644 --- a/src/librustc_middle/mir/mod.rs +++ b/src/librustc_middle/mir/mod.rs @@ -408,7 +408,7 @@ impl<'tcx> Body<'tcx> { } } -#[derive(Copy, Clone, Debug, RustcEncodable, RustcDecodable, HashStable)] +#[derive(Copy, Clone, PartialEq, Eq, Debug, RustcEncodable, RustcDecodable, HashStable)] pub enum Safety { Safe, /// Unsafe because of a PushUnsafeBlock diff --git a/src/librustc_middle/mir/query.rs b/src/librustc_middle/mir/query.rs index 63b8d8c8da782..f19270e5561f0 100644 --- a/src/librustc_middle/mir/query.rs +++ b/src/librustc_middle/mir/query.rs @@ -15,10 +15,17 @@ use super::{Field, SourceInfo}; #[derive(Copy, Clone, PartialEq, RustcEncodable, RustcDecodable, HashStable)] pub enum UnsafetyViolationKind { + /// Only permitted in regular `fn`s, prohibitted in `const fn`s. General, /// Permitted both in `const fn`s and regular `fn`s. GeneralAndConstFn, + /// Borrow of packed field. + /// Has to be handled as a lint for backwards compatibility. BorrowPacked(hir::HirId), + /// 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), } #[derive(Copy, Clone, PartialEq, RustcEncodable, RustcDecodable, HashStable)] diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs index a335fa2de411b..bcadbd091076a 100644 --- a/src/librustc_mir/transform/check_unsafety.rs +++ b/src/librustc_mir/transform/check_unsafety.rs @@ -9,7 +9,7 @@ use rustc_middle::mir::*; 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, UNUSED_UNSAFE}; +use rustc_session::lint::builtin::{SAFE_PACKED_BORROWS, UNSAFE_OP_IN_UNSAFE_FN, UNUSED_UNSAFE}; use rustc_span::symbol::{sym, Symbol}; use std::ops::Bound; @@ -351,14 +351,35 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { violation.kind = UnsafetyViolationKind::General; } } + UnsafetyViolationKind::UnsafeFn(_) => { + bug!("`UnsafetyViolationKind::UnsafeFn` in an `Safe` context") + } + } + if !self.violations.contains(&violation) { + self.violations.push(violation) } + } + false + } + // With the RFC 2585, no longer allow `unsafe` operations in `unsafe fn`s + 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); if !self.violations.contains(&violation) { self.violations.push(violation) } } false } - // `unsafe` function bodies allow unsafe without additional unsafe blocks + // `unsafe` function bodies allow unsafe without additional unsafe blocks (before RFC 2585) Safety::BuiltinUnsafe | Safety::FnUnsafe => true, Safety::ExplicitUnsafe(hir_id) => { // mark unsafe block as used if there are any unsafe operations inside @@ -383,6 +404,9 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { self.violations.push(violation) } } + UnsafetyViolationKind::UnsafeFn(_) => bug!( + "`UnsafetyViolationKind::UnsafeFn` in an `ExplicitUnsafe` context" + ), } } } @@ -575,9 +599,12 @@ fn is_enclosed( kind: hir::ItemKind::Fn(ref sig, _, _), .. })) = tcx.hir().find(parent_id) { - match sig.header.unsafety { - hir::Unsafety::Unsafe => Some(("fn".to_string(), parent_id)), - hir::Unsafety::Normal => None, + if sig.header.unsafety == hir::Unsafety::Unsafe + && !tcx.features().unsafe_block_in_unsafe_fn + { + Some(("fn".to_string(), parent_id)) + } else { + None } } else { is_enclosed(tcx, used_unsafe, parent_id) @@ -630,16 +657,20 @@ 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() { // Report an error. match kind { UnsafetyViolationKind::GeneralAndConstFn | UnsafetyViolationKind::General => { + // once struct_span_err!( tcx.sess, source_info.span, E0133, - "{} is unsafe and requires unsafe function or block", - description + "{} is unsafe and requires unsafe function{}", + description, + or_block_msg, ) .span_label(source_info.span, &*description.as_str()) .note(&details.as_str()) @@ -655,8 +686,8 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: DefId) { source_info.span, |lint| { lint.build(&format!( - "{} is unsafe and requires unsafe function or block (error E0133)", - description + "{} is unsafe and requires unsafe function{} (error E0133)", + description, or_block_msg, )) .note(&details.as_str()) .emit() @@ -664,6 +695,20 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: DefId) { ) } } + UnsafetyViolationKind::UnsafeFn(lint_hir_id) => tcx.struct_span_lint_hir( + UNSAFE_OP_IN_UNSAFE_FN, + lint_hir_id, + source_info.span, + |lint| { + lint.build(&format!( + "{} is unsafe and requires unsafe block (error E0133)", + description + )) + .span_label(source_info.span, &*description.as_str()) + .note(&details.as_str()) + .emit(); + }, + ), } } diff --git a/src/librustc_mir_build/build/block.rs b/src/librustc_mir_build/build/block.rs index fa783ddcf409a..b052c839152de 100644 --- a/src/librustc_mir_build/build/block.rs +++ b/src/librustc_mir_build/build/block.rs @@ -217,6 +217,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { assert_eq!(self.push_unsafe_count, 0); 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 => {} _ => return, } self.unpushed_unsafe = Safety::ExplicitUnsafe(hir_id); diff --git a/src/librustc_session/lint/builtin.rs b/src/librustc_session/lint/builtin.rs index e55ddc26a9441..7112ac35b082b 100644 --- a/src/librustc_session/lint/builtin.rs +++ b/src/librustc_session/lint/builtin.rs @@ -526,6 +526,12 @@ declare_lint! { "using only a subset of a register for inline asm inputs", } +declare_lint! { + pub UNSAFE_OP_IN_UNSAFE_FN, + Allow, + "unsafe operations in unsafe functions without an explicit unsafe block are deprecated", +} + declare_lint_pass! { /// Does nothing as a lint pass, but registers some `Lint`s /// that are used by other parts of the compiler. @@ -597,6 +603,7 @@ declare_lint_pass! { SOFT_UNSTABLE, INLINE_NO_SANITIZE, ASM_SUB_REGISTER, + UNSAFE_OP_IN_UNSAFE_FN, ] } diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index 6a6098710e828..87952d409c896 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -806,6 +806,7 @@ symbols! { unmarked_api, unreachable_code, unrestricted_attribute_tokens, + unsafe_block_in_unsafe_fn, unsafe_no_drop_flag, unsized_locals, unsized_tuple_coercion, From 594c499db995abaacc10a74c0afbb9aeed48117d Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Sun, 3 May 2020 23:11:58 +0200 Subject: [PATCH 02/16] Add tests --- .../feature-gate-unsafe_block_in_unsafe_fn.rs | 11 +++++++ ...ture-gate-unsafe_block_in_unsafe_fn.stderr | 16 ++++++++++ .../unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs | 26 +++++++++++++++++ .../rfc-2585-unsafe_op_in_unsafe_fn.stderr | 29 +++++++++++++++++++ 4 files changed, 82 insertions(+) create mode 100644 src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.rs create mode 100644 src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.stderr create mode 100644 src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs create mode 100644 src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.stderr 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 new file mode 100644 index 0000000000000..80005fd4ef632 --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.rs @@ -0,0 +1,11 @@ +#![deny(unused_unsafe)] + +unsafe fn unsf() {} + +unsafe fn foo() { + unsafe { //~ ERROR unnecessary `unsafe` block + unsf() + } +} + +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 new file mode 100644 index 0000000000000..64874e590fb89 --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.stderr @@ -0,0 +1,16 @@ +error: unnecessary `unsafe` block + --> $DIR/feature-gate-unsafe_block_in_unsafe_fn.rs:6:5 + | +LL | unsafe fn foo() { + | --------------- because it's nested under this `unsafe` fn +LL | unsafe { + | ^^^^^^ unnecessary `unsafe` block + | +note: the lint level is defined here + --> $DIR/feature-gate-unsafe_block_in_unsafe_fn.rs:1:9 + | +LL | #![deny(unused_unsafe)] + | ^^^^^^^^^^^^^ + +error: aborting due to previous error + 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 new file mode 100644 index 0000000000000..0ffb2827bfde2 --- /dev/null +++ b/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs @@ -0,0 +1,26 @@ +#![feature(unsafe_block_in_unsafe_fn)] +#![warn(unsafe_op_in_unsafe_fn)] +#![deny(unused_unsafe)] + +unsafe fn unsf() {} + +unsafe fn foo() { + unsf(); + //~^ WARNING call to unsafe function is unsafe and requires unsafe block +} + +unsafe fn bar() { + unsafe { unsf() } // no error +} + +unsafe fn baz() { + unsafe { unsafe { unsf() } } + //~^ ERROR unnecessary `unsafe` block +} + +#[allow(unsafe_op_in_unsafe_fn)] +unsafe fn qux() { + unsf(); // no error +} + +fn main() {} 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 new file mode 100644 index 0000000000000..be6af2a11c40a --- /dev/null +++ b/src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.stderr @@ -0,0 +1,29 @@ +warning: call to unsafe function is unsafe and requires unsafe block (error E0133) + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:8:5 + | +LL | unsf(); + | ^^^^^^ call to unsafe function + | +note: the lint level is defined here + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:2:9 + | +LL | #![warn(unsafe_op_in_unsafe_fn)] + | ^^^^^^^^^^^^^^^^^^^^^^ + = note: consult the function's documentation for information on how to avoid undefined behavior + +error: unnecessary `unsafe` block + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:17:14 + | +LL | unsafe { unsafe { unsf() } } + | ------ ^^^^^^ unnecessary `unsafe` block + | | + | because it's nested under this `unsafe` block + | +note: the lint level is defined here + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:3:9 + | +LL | #![deny(unused_unsafe)] + | ^^^^^^^^^^^^^ + +error: aborting due to previous error; 1 warning emitted + From bb6791502846b62e752c99396953e4db7739f28c Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Wed, 13 May 2020 23:43:21 +0200 Subject: [PATCH 03/16] Apply suggestions from code review --- src/librustc_lint/levels.rs | 19 +++++- src/librustc_middle/mir/query.rs | 5 +- src/librustc_mir/transform/check_unsafety.rs | 67 ++++++++++++------- src/librustc_mir_build/build/block.rs | 12 +++- .../feature-gate-unsafe_block_in_unsafe_fn.rs | 11 +-- ...ture-gate-unsafe_block_in_unsafe_fn.stderr | 36 +++++++--- .../unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs | 8 ++- .../rfc-2585-unsafe_op_in_unsafe_fn.stderr | 17 ++++- 8 files changed, 125 insertions(+), 50 deletions(-) diff --git a/src/librustc_lint/levels.rs b/src/librustc_lint/levels.rs index 274e57ae64cac..7069472015402 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 f19270e5561f0..c63d7215867c2 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 bcadbd091076a..0434f3447bced 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 b052c839152de..4e4f0dc74cb7c 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 80005fd4ef632..bf33ac67fcae9 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 64874e590fb89..c5cad4a98d9ca 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 + = 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[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)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -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: 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 0ffb2827bfde2..7feb68b98577f 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 be6af2a11c40a..e812210498c4c 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`. From 3ce9d5c26982ff0516946dda7fa8d29580fa25b3 Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Thu, 14 May 2020 20:37:23 +0200 Subject: [PATCH 04/16] Add more cases to the test --- .../unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) 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 7feb68b98577f..618c911581597 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 @@ -1,16 +1,38 @@ #![feature(unsafe_block_in_unsafe_fn)] #![warn(unsafe_op_in_unsafe_fn)] #![deny(unused_unsafe)] +#![deny(safe_packed_borrows)] unsafe fn unsf() {} +const PTR: *const () = std::ptr::null(); +static mut VOID: () = (); + +#[repr(packed)] +pub struct Packed { + data: &'static u32, +} + +const PACKED: Packed = Packed { data: &0 }; unsafe fn foo() { unsf(); //~^ WARNING call to unsafe function is unsafe and requires unsafe block + *PTR; + //~^ WARNING dereference of raw pointer is unsafe and requires unsafe block + VOID = (); + //~^ WARNING use of mutable static is unsafe and requires unsafe block + &PACKED.data; // the level for the `safe_packed_borrows` lint is ignored + //~^ WARNING borrow of packed field is unsafe and requires unsafe block } unsafe fn bar() { - unsafe { unsf() } // no error + // no error + unsafe { + unsf(); + *PTR; + VOID = (); + &PACKED.data; + } } unsafe fn baz() { @@ -20,7 +42,11 @@ unsafe fn baz() { #[allow(unsafe_op_in_unsafe_fn)] unsafe fn qux() { - unsf(); // no error + // lint allowed -> no error + unsf(); + *PTR; + VOID = (); + &PACKED.data; unsafe { unsf() } //~^ ERROR unnecessary `unsafe` block From b3e012becc6539de9570eee6ce694f07879935d2 Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Tue, 19 May 2020 00:18:50 +0200 Subject: [PATCH 05/16] Fix inverted `if` condition --- src/librustc_mir/transform/check_unsafety.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs index 0434f3447bced..3cff214f441c0 100644 --- a/src/librustc_mir/transform/check_unsafety.rs +++ b/src/librustc_mir/transform/check_unsafety.rs @@ -676,7 +676,7 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: DefId) { { // Report an error. let unsafe_fn_msg = - if unsafe_op_in_unsafe_fn_allowed(tcx, lint_root) { "" } else { " function or" }; + if unsafe_op_in_unsafe_fn_allowed(tcx, lint_root) { " function or" } else { "" }; match kind { UnsafetyViolationKind::GeneralAndConstFn | UnsafetyViolationKind::General => { From a41f76321a05ce7941d843dc18ea11f7c8a11c6f Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Tue, 19 May 2020 00:19:31 +0200 Subject: [PATCH 06/16] Use the lowest of `unsafe_op_in_unsafe_fn` and `safe_borrow_packed` for packed borrows in unsafe fns --- src/librustc_middle/mir/query.rs | 4 +++ src/librustc_mir/transform/check_unsafety.rs | 33 +++++++++++++++++--- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/librustc_middle/mir/query.rs b/src/librustc_middle/mir/query.rs index c63d7215867c2..99bfb74c243b4 100644 --- a/src/librustc_middle/mir/query.rs +++ b/src/librustc_middle/mir/query.rs @@ -26,6 +26,10 @@ pub enum UnsafetyViolationKind { /// Has to be handled as a lint for backwards compatibility. /// Should stay gated under `#![feature(unsafe_block_in_unsafe_fn)]`. UnsafeFn, + /// Borrow of packed field 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)]`. + UnsafeFnBorrowPacked, } #[derive(Copy, Clone, PartialEq, RustcEncodable, RustcDecodable, HashStable)] diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs index 3cff214f441c0..177efa2fc0d6c 100644 --- a/src/librustc_mir/transform/check_unsafety.rs +++ b/src/librustc_mir/transform/check_unsafety.rs @@ -370,7 +370,8 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { violation.kind = UnsafetyViolationKind::General; } } - UnsafetyViolationKind::UnsafeFn => { + UnsafetyViolationKind::UnsafeFn + | UnsafetyViolationKind::UnsafeFnBorrowPacked => { bug!("`UnsafetyViolationKind::UnsafeFn` in an `Safe` context") } } @@ -385,8 +386,11 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { for violation in violations { let mut violation = *violation; - // FIXME(LeSeulArtichaut): what to do with `UnsafetyViolationKind::BorrowPacked`? - violation.kind = UnsafetyViolationKind::UnsafeFn; + if violation.kind == UnsafetyViolationKind::BorrowPacked { + violation.kind = UnsafetyViolationKind::UnsafeFnBorrowPacked; + } else { + violation.kind = UnsafetyViolationKind::UnsafeFn; + } if !self.violations.contains(&violation) { self.violations.push(violation) } @@ -418,7 +422,8 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> { self.violations.push(violation) } } - UnsafetyViolationKind::UnsafeFn => bug!( + UnsafetyViolationKind::UnsafeFn + | UnsafetyViolationKind::UnsafeFnBorrowPacked => bug!( "`UnsafetyViolationKind::UnsafeFn` in an `ExplicitUnsafe` context" ), } @@ -719,13 +724,31 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: DefId) { |lint| { lint.build(&format!( "{} is unsafe and requires unsafe block (error E0133)", - description + description, )) .span_label(source_info.span, &*description.as_str()) .note(&details.as_str()) .emit(); }, ), + UnsafetyViolationKind::UnsafeFnBorrowPacked => { + let lint = if tcx.lint_level_at_node(SAFE_PACKED_BORROWS, lint_root).0 + <= tcx.lint_level_at_node(UNSAFE_OP_IN_UNSAFE_FN, lint_root).0 + { + SAFE_PACKED_BORROWS + } else { + UNSAFE_OP_IN_UNSAFE_FN + }; + tcx.struct_span_lint_hir(&lint, lint_root, source_info.span, |lint| { + lint.build(&format!( + "{} is unsafe and requires unsafe block (error E0133)", + description, + )) + .span_label(source_info.span, &*description.as_str()) + .note(&details.as_str()) + .emit(); + }) + } } } From a3bae5ce73a65671ee35037ac988766973628295 Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Tue, 19 May 2020 12:16:40 +0200 Subject: [PATCH 07/16] Fix wrong conflict resolution --- src/librustc_mir/transform/check_unsafety.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs index 177efa2fc0d6c..7be640ae24310 100644 --- a/src/librustc_mir/transform/check_unsafety.rs +++ b/src/librustc_mir/transform/check_unsafety.rs @@ -204,18 +204,12 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> { if context.is_borrow() { if util::is_disaligned(self.tcx, self.body, self.param_env, *place) { - let source_info = self.source_info; - let lint_root = self.body.source_scopes[source_info.scope] - .local_data - .as_ref() - .assert_crate_local() - .lint_root; 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(lint_root), + UnsafetyViolationKind::BorrowPacked, ); } } From 925d5ac45f4a44dd61f1ebd4b7bd3cc89fcb2b5f Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Thu, 21 May 2020 23:53:12 +0200 Subject: [PATCH 08/16] Fix and bless tests --- .../unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs | 71 +++++++++-- .../rfc-2585-unsafe_op_in_unsafe_fn.stderr | 118 ++++++++++++++++-- 2 files changed, 168 insertions(+), 21 deletions(-) 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 618c911581597..6dfb97c230d66 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 @@ -1,5 +1,5 @@ #![feature(unsafe_block_in_unsafe_fn)] -#![warn(unsafe_op_in_unsafe_fn)] +#![deny(unsafe_op_in_unsafe_fn)] #![deny(unused_unsafe)] #![deny(safe_packed_borrows)] @@ -14,18 +14,34 @@ pub struct Packed { const PACKED: Packed = Packed { data: &0 }; -unsafe fn foo() { +unsafe fn deny_level() { unsf(); - //~^ WARNING call to unsafe function is unsafe and requires unsafe block + //~^ ERROR call to unsafe function is unsafe and requires unsafe block *PTR; - //~^ WARNING dereference of raw pointer is unsafe and requires unsafe block + //~^ ERROR dereference of raw pointer is unsafe and requires unsafe block VOID = (); - //~^ WARNING use of mutable static is unsafe and requires unsafe block - &PACKED.data; // the level for the `safe_packed_borrows` lint is ignored - //~^ WARNING borrow of packed field is unsafe and requires unsafe block + //~^ ERROR use of mutable static is unsafe and requires unsafe block + &PACKED.data; + //~^ ERROR borrow of packed field is unsafe and requires unsafe block + //~| WARNING this was previously accepted by the compiler but is being phased out } -unsafe fn bar() { +// Check that `unsafe_op_in_unsafe_fn` works starting from the `warn` level. +#[warn(unsafe_op_in_unsafe_fn)] +#[deny(warnings)] +unsafe fn warning_level() { + unsf(); + //~^ ERROR call to unsafe function is unsafe and requires unsafe block + *PTR; + //~^ ERROR dereference of raw pointer is unsafe and requires unsafe block + VOID = (); + //~^ ERROR use of mutable static is unsafe and requires unsafe block + &PACKED.data; + //~^ ERROR borrow of packed field is unsafe and requires unsafe block + //~| WARNING this was previously accepted by the compiler but is being phased out +} + +unsafe fn explicit_block() { // no error unsafe { unsf(); @@ -35,13 +51,25 @@ unsafe fn bar() { } } -unsafe fn baz() { +unsafe fn two_explicit_blocks() { unsafe { unsafe { unsf() } } //~^ ERROR unnecessary `unsafe` block } +#[warn(safe_packed_borrows)] +unsafe fn warn_packed_borrows() { + &PACKED.data; + //~^ WARNING borrow of packed field is unsafe and requires unsafe block + //~| WARNING this was previously accepted by the compiler but is being phased out +} + +#[allow(safe_packed_borrows)] +unsafe fn allow_packed_borrows() { + &PACKED.data; // `safe_packed_borrows` is allowed, no error +} + #[allow(unsafe_op_in_unsafe_fn)] -unsafe fn qux() { +unsafe fn allow_level() { // lint allowed -> no error unsf(); *PTR; @@ -52,7 +80,26 @@ unsafe fn qux() { //~^ ERROR unnecessary `unsafe` block } +unsafe fn nested_allow_level() { + #[allow(unsafe_op_in_unsafe_fn)] + { + // lint allowed -> no error + unsf(); + *PTR; + VOID = (); + &PACKED.data; + + unsafe { unsf() } + //~^ ERROR unnecessary `unsafe` block + } +} + fn main() { - unsf() - //~^ ERROR call to unsafe function is unsafe and requires unsafe function or block + unsf(); + //~^ ERROR call to unsafe function is unsafe and requires unsafe block + #[allow(unsafe_op_in_unsafe_fn)] + { + 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 e812210498c4c..efc53f0b28b29 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 @@ -1,5 +1,5 @@ -warning: call to unsafe function is unsafe and requires unsafe block (error E0133) - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:8:5 +error: call to unsafe function is unsafe and requires unsafe block (error E0133) + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:18:5 | LL | unsf(); | ^^^^^^ call to unsafe function @@ -7,12 +7,83 @@ LL | unsf(); note: the lint level is defined here --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:2:9 | -LL | #![warn(unsafe_op_in_unsafe_fn)] +LL | #![deny(unsafe_op_in_unsafe_fn)] | ^^^^^^^^^^^^^^^^^^^^^^ = note: consult the function's documentation for information on how to avoid undefined behavior +error: dereference of raw pointer is unsafe and requires unsafe block (error E0133) + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:20:5 + | +LL | *PTR; + | ^^^^ dereference of raw pointer + | + = note: raw pointers may be NULL, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior + +error: use of mutable static is unsafe and requires unsafe block (error E0133) + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:22:5 + | +LL | VOID = (); + | ^^^^^^^^^ use of mutable static + | + = note: mutable statics can be mutated by multiple threads: aliasing violations or data races will cause undefined behavior + +error: borrow of packed field is unsafe and requires unsafe block (error E0133) + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:24:5 + | +LL | &PACKED.data; + | ^^^^^^^^^^^^ borrow of packed field + | +note: the lint level is defined here + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:4:9 + | +LL | #![deny(safe_packed_borrows)] + | ^^^^^^^^^^^^^^^^^^^ + = 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 #46043 + = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior + +error: call to unsafe function is unsafe and requires unsafe block (error E0133) + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:33:5 + | +LL | unsf(); + | ^^^^^^ call to unsafe function + | +note: the lint level is defined here + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:31:8 + | +LL | #[deny(warnings)] + | ^^^^^^^^ + = note: `#[deny(unsafe_op_in_unsafe_fn)]` implied by `#[deny(warnings)]` + = note: consult the function's documentation for information on how to avoid undefined behavior + +error: dereference of raw pointer is unsafe and requires unsafe block (error E0133) + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:35:5 + | +LL | *PTR; + | ^^^^ dereference of raw pointer + | + = note: raw pointers may be NULL, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior + +error: use of mutable static is unsafe and requires unsafe block (error E0133) + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:37:5 + | +LL | VOID = (); + | ^^^^^^^^^ use of mutable static + | + = note: mutable statics can be mutated by multiple threads: aliasing violations or data races will cause undefined behavior + +error: borrow of packed field is unsafe and requires unsafe block (error E0133) + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:39:5 + | +LL | &PACKED.data; + | ^^^^^^^^^^^^ borrow of packed field + | + = 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 #46043 + = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior + error: unnecessary `unsafe` block - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:17:14 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:55:14 | LL | unsafe { unsafe { unsf() } } | ------ ^^^^^^ unnecessary `unsafe` block @@ -25,20 +96,49 @@ note: the lint level is defined here LL | #![deny(unused_unsafe)] | ^^^^^^^^^^^^^ +warning: borrow of packed field is unsafe and requires unsafe block (error E0133) + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:61:5 + | +LL | &PACKED.data; + | ^^^^^^^^^^^^ borrow of packed field + | +note: the lint level is defined here + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:59:8 + | +LL | #[warn(safe_packed_borrows)] + | ^^^^^^^^^^^^^^^^^^^ + = 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 #46043 + = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior + error: unnecessary `unsafe` block - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:25:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:79: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 +error: unnecessary `unsafe` block + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:92:9 | -LL | unsf() +LL | unsafe { unsf() } + | ^^^^^^ unnecessary `unsafe` block + +error[E0133]: call to unsafe function is unsafe and requires unsafe block + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:98: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 +error[E0133]: call to unsafe function is unsafe and requires unsafe function or block + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:102:9 + | +LL | unsf(); + | ^^^^^^ call to unsafe function + | + = note: consult the function's documentation for information on how to avoid undefined behavior + +error: aborting due to 13 previous errors; 1 warning emitted For more information about this error, try `rustc --explain E0133`. From 9671b44609d2adbf041fcc5f8b63183786417ba7 Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Fri, 22 May 2020 20:21:26 +0200 Subject: [PATCH 09/16] Add tests for packed borrows in unsafe fns --- ...c-2585-safe_packed_borrows-in-unsafe-fn.rs | 67 +++++++++++++++++++ ...85-safe_packed_borrows-in-unsafe-fn.stderr | 60 +++++++++++++++++ .../unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs | 29 -------- .../rfc-2585-unsafe_op_in_unsafe_fn.stderr | 66 ++++-------------- 4 files changed, 140 insertions(+), 82 deletions(-) create mode 100644 src/test/ui/unsafe/rfc-2585-safe_packed_borrows-in-unsafe-fn.rs create mode 100644 src/test/ui/unsafe/rfc-2585-safe_packed_borrows-in-unsafe-fn.stderr diff --git a/src/test/ui/unsafe/rfc-2585-safe_packed_borrows-in-unsafe-fn.rs b/src/test/ui/unsafe/rfc-2585-safe_packed_borrows-in-unsafe-fn.rs new file mode 100644 index 0000000000000..540612a7dce05 --- /dev/null +++ b/src/test/ui/unsafe/rfc-2585-safe_packed_borrows-in-unsafe-fn.rs @@ -0,0 +1,67 @@ +#![feature(unsafe_block_in_unsafe_fn)] + +#[repr(packed)] +pub struct Packed { + data: &'static u32, +} + +const PACKED: Packed = Packed { data: &0 }; + +#[allow(safe_packed_borrows)] +#[allow(unsafe_op_in_unsafe_fn)] +unsafe fn allow_allow() { + &PACKED.data; // allowed +} + +#[allow(safe_packed_borrows)] +#[warn(unsafe_op_in_unsafe_fn)] +unsafe fn allow_warn() { + &PACKED.data; // allowed +} + +#[allow(safe_packed_borrows)] +#[deny(unsafe_op_in_unsafe_fn)] +unsafe fn allow_deny() { + &PACKED.data; // allowed +} + +#[warn(safe_packed_borrows)] +#[allow(unsafe_op_in_unsafe_fn)] +unsafe fn warn_allow() { + &PACKED.data; // allowed +} + +#[warn(safe_packed_borrows)] +#[warn(unsafe_op_in_unsafe_fn)] +unsafe fn warn_warn() { + &PACKED.data; //~ WARN + //~| WARNING this was previously accepted by the compiler but is being phased out +} + +#[warn(safe_packed_borrows)] +#[deny(unsafe_op_in_unsafe_fn)] +unsafe fn warn_deny() { + &PACKED.data; //~ WARN + //~| WARNING this was previously accepted by the compiler but is being phased out +} + +#[deny(safe_packed_borrows)] +#[allow(unsafe_op_in_unsafe_fn)] +unsafe fn deny_allow() { + &PACKED.data; // allowed +} + +#[deny(safe_packed_borrows)] +#[warn(unsafe_op_in_unsafe_fn)] +unsafe fn deny_warn() { + &PACKED.data; //~ WARN +} + +#[deny(safe_packed_borrows)] +#[deny(unsafe_op_in_unsafe_fn)] +unsafe fn deny_deny() { + &PACKED.data; //~ ERROR + //~| WARNING this was previously accepted by the compiler but is being phased out +} + +fn main() {} diff --git a/src/test/ui/unsafe/rfc-2585-safe_packed_borrows-in-unsafe-fn.stderr b/src/test/ui/unsafe/rfc-2585-safe_packed_borrows-in-unsafe-fn.stderr new file mode 100644 index 0000000000000..fda15159643b6 --- /dev/null +++ b/src/test/ui/unsafe/rfc-2585-safe_packed_borrows-in-unsafe-fn.stderr @@ -0,0 +1,60 @@ +warning: borrow of packed field is unsafe and requires unsafe block (error E0133) + --> $DIR/rfc-2585-safe_packed_borrows-in-unsafe-fn.rs:37:5 + | +LL | &PACKED.data; + | ^^^^^^^^^^^^ borrow of packed field + | +note: the lint level is defined here + --> $DIR/rfc-2585-safe_packed_borrows-in-unsafe-fn.rs:34:8 + | +LL | #[warn(safe_packed_borrows)] + | ^^^^^^^^^^^^^^^^^^^ + = 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 #46043 + = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior + +warning: borrow of packed field is unsafe and requires unsafe block (error E0133) + --> $DIR/rfc-2585-safe_packed_borrows-in-unsafe-fn.rs:44:5 + | +LL | &PACKED.data; + | ^^^^^^^^^^^^ borrow of packed field + | +note: the lint level is defined here + --> $DIR/rfc-2585-safe_packed_borrows-in-unsafe-fn.rs:41:8 + | +LL | #[warn(safe_packed_borrows)] + | ^^^^^^^^^^^^^^^^^^^ + = 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 #46043 + = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior + +warning: borrow of packed field is unsafe and requires unsafe block (error E0133) + --> $DIR/rfc-2585-safe_packed_borrows-in-unsafe-fn.rs:57:5 + | +LL | &PACKED.data; + | ^^^^^^^^^^^^ borrow of packed field + | +note: the lint level is defined here + --> $DIR/rfc-2585-safe_packed_borrows-in-unsafe-fn.rs:55:8 + | +LL | #[warn(unsafe_op_in_unsafe_fn)] + | ^^^^^^^^^^^^^^^^^^^^^^ + = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior + +error: borrow of packed field is unsafe and requires unsafe block (error E0133) + --> $DIR/rfc-2585-safe_packed_borrows-in-unsafe-fn.rs:63:5 + | +LL | &PACKED.data; + | ^^^^^^^^^^^^ borrow of packed field + | +note: the lint level is defined here + --> $DIR/rfc-2585-safe_packed_borrows-in-unsafe-fn.rs:60:8 + | +LL | #[deny(safe_packed_borrows)] + | ^^^^^^^^^^^^^^^^^^^ + = 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 #46043 + = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior + +error: aborting due to previous error; 3 warnings emitted + 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 6dfb97c230d66..1e57b03ced48b 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 @@ -1,19 +1,11 @@ #![feature(unsafe_block_in_unsafe_fn)] #![deny(unsafe_op_in_unsafe_fn)] #![deny(unused_unsafe)] -#![deny(safe_packed_borrows)] unsafe fn unsf() {} const PTR: *const () = std::ptr::null(); static mut VOID: () = (); -#[repr(packed)] -pub struct Packed { - data: &'static u32, -} - -const PACKED: Packed = Packed { data: &0 }; - unsafe fn deny_level() { unsf(); //~^ ERROR call to unsafe function is unsafe and requires unsafe block @@ -21,9 +13,6 @@ unsafe fn deny_level() { //~^ ERROR dereference of raw pointer is unsafe and requires unsafe block VOID = (); //~^ ERROR use of mutable static is unsafe and requires unsafe block - &PACKED.data; - //~^ ERROR borrow of packed field is unsafe and requires unsafe block - //~| WARNING this was previously accepted by the compiler but is being phased out } // Check that `unsafe_op_in_unsafe_fn` works starting from the `warn` level. @@ -36,9 +25,6 @@ unsafe fn warning_level() { //~^ ERROR dereference of raw pointer is unsafe and requires unsafe block VOID = (); //~^ ERROR use of mutable static is unsafe and requires unsafe block - &PACKED.data; - //~^ ERROR borrow of packed field is unsafe and requires unsafe block - //~| WARNING this was previously accepted by the compiler but is being phased out } unsafe fn explicit_block() { @@ -47,7 +33,6 @@ unsafe fn explicit_block() { unsf(); *PTR; VOID = (); - &PACKED.data; } } @@ -56,25 +41,12 @@ unsafe fn two_explicit_blocks() { //~^ ERROR unnecessary `unsafe` block } -#[warn(safe_packed_borrows)] -unsafe fn warn_packed_borrows() { - &PACKED.data; - //~^ WARNING borrow of packed field is unsafe and requires unsafe block - //~| WARNING this was previously accepted by the compiler but is being phased out -} - -#[allow(safe_packed_borrows)] -unsafe fn allow_packed_borrows() { - &PACKED.data; // `safe_packed_borrows` is allowed, no error -} - #[allow(unsafe_op_in_unsafe_fn)] unsafe fn allow_level() { // lint allowed -> no error unsf(); *PTR; VOID = (); - &PACKED.data; unsafe { unsf() } //~^ ERROR unnecessary `unsafe` block @@ -87,7 +59,6 @@ unsafe fn nested_allow_level() { unsf(); *PTR; VOID = (); - &PACKED.data; unsafe { unsf() } //~^ ERROR unnecessary `unsafe` 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 efc53f0b28b29..cc595df12cc44 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 @@ -1,5 +1,5 @@ error: call to unsafe function is unsafe and requires unsafe block (error E0133) - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:18:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:10:5 | LL | unsf(); | ^^^^^^ call to unsafe function @@ -12,7 +12,7 @@ LL | #![deny(unsafe_op_in_unsafe_fn)] = note: consult the function's documentation for information on how to avoid undefined behavior error: dereference of raw pointer is unsafe and requires unsafe block (error E0133) - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:20:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:12:5 | LL | *PTR; | ^^^^ dereference of raw pointer @@ -20,36 +20,21 @@ LL | *PTR; = note: raw pointers may be NULL, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior error: use of mutable static is unsafe and requires unsafe block (error E0133) - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:22:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:14:5 | LL | VOID = (); | ^^^^^^^^^ use of mutable static | = note: mutable statics can be mutated by multiple threads: aliasing violations or data races will cause undefined behavior -error: borrow of packed field is unsafe and requires unsafe block (error E0133) - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:24:5 - | -LL | &PACKED.data; - | ^^^^^^^^^^^^ borrow of packed field - | -note: the lint level is defined here - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:4:9 - | -LL | #![deny(safe_packed_borrows)] - | ^^^^^^^^^^^^^^^^^^^ - = 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 #46043 - = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior - error: call to unsafe function is unsafe and requires unsafe block (error E0133) - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:33:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:22:5 | LL | unsf(); | ^^^^^^ call to unsafe function | note: the lint level is defined here - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:31:8 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:20:8 | LL | #[deny(warnings)] | ^^^^^^^^ @@ -57,7 +42,7 @@ LL | #[deny(warnings)] = note: consult the function's documentation for information on how to avoid undefined behavior error: dereference of raw pointer is unsafe and requires unsafe block (error E0133) - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:35:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:24:5 | LL | *PTR; | ^^^^ dereference of raw pointer @@ -65,25 +50,15 @@ LL | *PTR; = note: raw pointers may be NULL, dangling or unaligned; they can violate aliasing rules and cause data races: all of these are undefined behavior error: use of mutable static is unsafe and requires unsafe block (error E0133) - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:37:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:26:5 | LL | VOID = (); | ^^^^^^^^^ use of mutable static | = note: mutable statics can be mutated by multiple threads: aliasing violations or data races will cause undefined behavior -error: borrow of packed field is unsafe and requires unsafe block (error E0133) - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:39:5 - | -LL | &PACKED.data; - | ^^^^^^^^^^^^ borrow of packed field - | - = 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 #46043 - = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior - error: unnecessary `unsafe` block - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:55:14 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:40:14 | LL | unsafe { unsafe { unsf() } } | ------ ^^^^^^ unnecessary `unsafe` block @@ -96,35 +71,20 @@ note: the lint level is defined here LL | #![deny(unused_unsafe)] | ^^^^^^^^^^^^^ -warning: borrow of packed field is unsafe and requires unsafe block (error E0133) - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:61:5 - | -LL | &PACKED.data; - | ^^^^^^^^^^^^ borrow of packed field - | -note: the lint level is defined here - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:59:8 - | -LL | #[warn(safe_packed_borrows)] - | ^^^^^^^^^^^^^^^^^^^ - = 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 #46043 - = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior - error: unnecessary `unsafe` block - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:79:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:51:5 | LL | unsafe { unsf() } | ^^^^^^ unnecessary `unsafe` block error: unnecessary `unsafe` block - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:92:9 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:63:9 | LL | unsafe { unsf() } | ^^^^^^ unnecessary `unsafe` block error[E0133]: call to unsafe function is unsafe and requires unsafe block - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:98:5 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:69:5 | LL | unsf(); | ^^^^^^ call to unsafe function @@ -132,13 +92,13 @@ LL | unsf(); = note: consult the function's documentation for information on how to avoid undefined behavior error[E0133]: call to unsafe function is unsafe and requires unsafe function or block - --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:102:9 + --> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:73:9 | LL | unsf(); | ^^^^^^ call to unsafe function | = note: consult the function's documentation for information on how to avoid undefined behavior -error: aborting due to 13 previous errors; 1 warning emitted +error: aborting due to 11 previous errors For more information about this error, try `rustc --explain E0133`. From 3599ada976568d0b5326e890d4b0c2f22dcdc985 Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Sat, 23 May 2020 22:15:14 +0200 Subject: [PATCH 10/16] Mark deduplicated errors as expected in gate test --- .../ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.rs | 2 ++ 1 file changed, 2 insertions(+) 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 bf33ac67fcae9..61e512a12a18d 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,4 +1,6 @@ #![deny(unsafe_op_in_unsafe_fn)] //~^ ERROR the `unsafe_op_in_unsafe_fn` lint is unstable +//~| ERROR the `unsafe_op_in_unsafe_fn` lint is unstable +//~| ERROR the `unsafe_op_in_unsafe_fn` lint is unstable fn main() {} From 4a538d31e056c7563d7e29a6bc7c89e9e46d8ee8 Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Sat, 23 May 2020 22:22:01 +0200 Subject: [PATCH 11/16] Do not hardcode lint name --- src/librustc_lint/levels.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc_lint/levels.rs b/src/librustc_lint/levels.rs index 7069472015402..2e3fc162b530f 100644 --- a/src/librustc_lint/levels.rs +++ b/src/librustc_lint/levels.rs @@ -9,6 +9,7 @@ use rustc_errors::{struct_span_err, Applicability}; use rustc_hir as hir; use rustc_hir::def_id::{CrateNum, LOCAL_CRATE}; use rustc_hir::{intravisit, HirId}; +use rustc_lint::builtin::UNSAFE_OP_IN_UNSAFE_FN; use rustc_middle::hir::map::Map; use rustc_middle::lint::LintDiagnosticBuilder; use rustc_middle::lint::{struct_lint_level, LintLevelMap, LintLevelSets, LintSet, LintSource}; @@ -387,7 +388,7 @@ impl<'s> LintLevelsBuilder<'s> { } fn check_gated_lint(&self, name: Symbol, span: Span) { - if name.as_str() == "unsafe_op_in_unsafe_fn" + if name.as_str() == UNSAFE_OP_IN_UNSAFE_FN.name && !self.sess.features_untracked().unsafe_block_in_unsafe_fn { feature_err( From e3d27ec1c82e223277b16f30e2355056144ca0da Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Sat, 23 May 2020 22:32:40 +0200 Subject: [PATCH 12/16] Add explanation about taking the minimum of the two lints --- src/librustc_mir/transform/check_unsafety.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/librustc_mir/transform/check_unsafety.rs b/src/librustc_mir/transform/check_unsafety.rs index 7be640ae24310..1f01bc0e19513 100644 --- a/src/librustc_mir/transform/check_unsafety.rs +++ b/src/librustc_mir/transform/check_unsafety.rs @@ -726,6 +726,17 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: DefId) { }, ), UnsafetyViolationKind::UnsafeFnBorrowPacked => { + // When `unsafe_op_in_unsafe_fn` is disallowed, the behavior of safe and unsafe functions + // should be the same in terms of warnings and errors. Therefore, with `#[warn(safe_packed_borrows)]`, + // a safe packed borrow should emit a warning *but not an error* in an unsafe function, + // just like in a safe function, even if `unsafe_op_in_unsafe_fn` is `deny`. + // + // Also, `#[warn(unsafe_op_in_unsafe_fn)]` can't cause any new errors. Therefore, with + // `#[deny(safe_packed_borrows)]` and `#[warn(unsafe_op_in_unsafe_fn)]`, a packed borrow + // should only issue a warning for the sake of backwards compatibility. + // + // The solution those 2 expectations is to always take the minimum of both lints. + // This prevent any new errors (unless both lints are explicitely set to `deny`). let lint = if tcx.lint_level_at_node(SAFE_PACKED_BORROWS, lint_root).0 <= tcx.lint_level_at_node(UNSAFE_OP_IN_UNSAFE_FN, lint_root).0 { From 1b0885062266a21e67fdeb12320c92ac7c00742e Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Sat, 23 May 2020 22:39:07 +0200 Subject: [PATCH 13/16] Fix import --- src/librustc_lint/levels.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/librustc_lint/levels.rs b/src/librustc_lint/levels.rs index 2e3fc162b530f..8dd683ae7995d 100644 --- a/src/librustc_lint/levels.rs +++ b/src/librustc_lint/levels.rs @@ -9,7 +9,6 @@ use rustc_errors::{struct_span_err, Applicability}; use rustc_hir as hir; use rustc_hir::def_id::{CrateNum, LOCAL_CRATE}; use rustc_hir::{intravisit, HirId}; -use rustc_lint::builtin::UNSAFE_OP_IN_UNSAFE_FN; use rustc_middle::hir::map::Map; use rustc_middle::lint::LintDiagnosticBuilder; use rustc_middle::lint::{struct_lint_level, LintLevelMap, LintLevelSets, LintSet, LintSource}; @@ -388,7 +387,7 @@ impl<'s> LintLevelsBuilder<'s> { } fn check_gated_lint(&self, name: Symbol, span: Span) { - if name.as_str() == UNSAFE_OP_IN_UNSAFE_FN.name + if name.as_str() == builtin::UNSAFE_OP_IN_UNSAFE_FN.name && !self.sess.features_untracked().unsafe_block_in_unsafe_fn { feature_err( From 63066c0c06de27bc84f7fbbe683b59d4cb0441db Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Sat, 23 May 2020 23:11:55 +0200 Subject: [PATCH 14/16] Use `LintId`s to check for gated lints --- src/librustc_lint/levels.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc_lint/levels.rs b/src/librustc_lint/levels.rs index 8dd683ae7995d..3d2ddf12a0a1f 100644 --- a/src/librustc_lint/levels.rs +++ b/src/librustc_lint/levels.rs @@ -14,7 +14,7 @@ use rustc_middle::lint::LintDiagnosticBuilder; use rustc_middle::lint::{struct_lint_level, LintLevelMap, LintLevelSets, LintSet, LintSource}; use rustc_middle::ty::query::Providers; use rustc_middle::ty::TyCtxt; -use rustc_session::lint::{builtin, Level, Lint}; +use rustc_session::lint::{builtin, Level, Lint, LintId}; use rustc_session::parse::feature_err; use rustc_session::Session; use rustc_span::symbol::{sym, Symbol}; @@ -80,13 +80,13 @@ 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 }; for id in ids { + self.check_gated_lint(id, DUMMY_SP); let src = LintSource::CommandLine(lint_flag_val); specs.insert(id, (level, src)); } @@ -213,9 +213,9 @@ 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 { + self.check_gated_lint(*id, attr.span); specs.insert(*id, (level, src)); } } @@ -386,8 +386,8 @@ impl<'s> LintLevelsBuilder<'s> { BuilderPush { prev, changed: prev != self.cur } } - fn check_gated_lint(&self, name: Symbol, span: Span) { - if name.as_str() == builtin::UNSAFE_OP_IN_UNSAFE_FN.name + fn check_gated_lint(&self, id: LintId, span: Span) { + if id == LintId::of(builtin::UNSAFE_OP_IN_UNSAFE_FN) && !self.sess.features_untracked().unsafe_block_in_unsafe_fn { feature_err( From db684beb4e40aae70a38e22e4aa91251349bf49b Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Wed, 27 May 2020 19:32:00 +0200 Subject: [PATCH 15/16] Whitelist `unsafe_op_in_unsafe_fn` in rustdoc --- src/librustdoc/core.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index bf59b3f25734d..331ce1453e139 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -226,6 +226,11 @@ where { let warnings_lint_name = lint::builtin::WARNINGS.name; + // Whitelist feature-gated lints to avoid feature errors when trying to + // allow all lints. + // FIXME(LeSeulArtichaut): handle feature-gated lints properly. + let unsafe_op_in_unsafe_fn_name = rustc_lint::builtin::UNSAFE_OP_IN_UNSAFE_FN.name; + whitelisted_lints.push(warnings_lint_name.to_owned()); whitelisted_lints.extend(lint_opts.iter().map(|(lint, _)| lint).cloned()); @@ -236,7 +241,13 @@ where }; let lint_opts = lints() - .filter_map(|lint| if lint.name == warnings_lint_name { None } else { filter_call(lint) }) + .filter_map(|lint| { + if lint.name == warnings_lint_name || lint.name == unsafe_op_in_unsafe_fn_name { + None + } else { + filter_call(lint) + } + }) .chain(lint_opts.into_iter()) .collect::>(); From 0e3b31c641217542f02b40527b0d299d9534b920 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 28 May 2020 13:23:33 -0400 Subject: [PATCH 16/16] Update src/librustdoc/core.rs --- src/librustdoc/core.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 331ce1453e139..99ca7084c30dd 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -228,7 +228,7 @@ where // Whitelist feature-gated lints to avoid feature errors when trying to // allow all lints. - // FIXME(LeSeulArtichaut): handle feature-gated lints properly. + // FIXME(#72694): handle feature-gated lints properly. let unsafe_op_in_unsafe_fn_name = rustc_lint::builtin::UNSAFE_OP_IN_UNSAFE_FN.name; whitelisted_lints.push(warnings_lint_name.to_owned());