nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2025-01-27 07:03:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

Apply suggestions from code review

Browse Source
This commit is contained in:
LeSeulArtichaut 2020-05-13 23:43:21 +02:00
parent 594c499db9
commit bb67915028
8 changed files with 126 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/librustc_lint/levels.rs
View File

@ -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;

5
src/librustc_middle/mir/query.rs
View File

@ -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,

67
src/librustc_mir/transform/check_unsafety.rs
View File

@ -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
}

12
src/librustc_mir_build/build/block.rs
View File

@ -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
}
}
};

11
src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.rs
View File

@ -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() {}

38
src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.stderr
View File

@ -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 <https://github.com/rust-lang/rust/issues/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 <https://github.com/rust-lang/rust/issues/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 <https://github.com/rust-lang/rust/issues/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`.

8
src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs
View File

@ -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
}

17
src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.stderr
View File

@ -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`.