Skip to content

Commit

Permalink
unsafety checking: no longer care about is_min_const_fn
Browse files Browse the repository at this point in the history 
Rejecting the forbidden unsafe ops is done by const checking, not by unsafety checking
  • Loading branch information
@RalfJung
RalfJung committed Apr 25, 2021
1 parent 5da10c0 commit 9082078
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 53 deletions.
4 changes: 1 addition & 3 deletions compiler/rustc_middle/src/mir/query.rs
View file
Expand Up @@ -19,10 +19,8 @@ use super::{Field, SourceInfo};

#[derive(Copy, Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)]
pub enum UnsafetyViolationKind {
/// Only permitted in regular `fn`s, prohibited in `const fn`s.
/// Unsafe operation outside `unsafe`
General,
/// Permitted both in `const fn`s and regular `fn`s.
GeneralAndConstFn,
/// Unsafe operation in an `unsafe fn` but outside an `unsafe` block.
/// Has to be handled as a lint for backwards compatibility.
UnsafeFn,
Expand Down
67 changes: 17 additions & 50 deletions compiler/rustc_mir/src/transform/check_unsafety.rs
View file
Expand Up @@ -15,13 +15,10 @@ use rustc_session::lint::Level;

use std::ops::Bound;

use crate::const_eval::is_min_const_fn;

pub struct UnsafetyChecker<'a, 'tcx> {
body: &'a Body<'tcx>,
body_did: LocalDefId,
const_context: bool,
min_const_fn: bool,
violations: Vec<UnsafetyViolation>,
source_info: SourceInfo,
tcx: TyCtxt<'tcx>,
Expand All @@ -34,21 +31,15 @@ pub struct UnsafetyChecker<'a, 'tcx> {
impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
fn new(
const_context: bool,
min_const_fn: bool,
body: &'a Body<'tcx>,
body_did: LocalDefId,
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
) -> Self {
// sanity check
if min_const_fn {
assert!(const_context);
}
Self {
body,
body_did,
const_context,
min_const_fn,
violations: vec![],
source_info: SourceInfo::outermost(body.span),
tcx,
Expand Down Expand Up @@ -84,7 +75,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
let sig = func_ty.fn_sig(self.tcx);
if let hir::Unsafety::Unsafe = sig.unsafety() {
self.require_unsafe(
UnsafetyViolationKind::GeneralAndConstFn,
UnsafetyViolationKind::General,
UnsafetyViolationDetails::CallToUnsafeFunction,
)
}
Expand Down Expand Up @@ -134,7 +125,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
match self.tcx.layout_scalar_valid_range(def.did) {
(Bound::Unbounded, Bound::Unbounded) => {}
_ => self.require_unsafe(
UnsafetyViolationKind::GeneralAndConstFn,
UnsafetyViolationKind::General,
UnsafetyViolationDetails::InitializingTypeWith,
),
}
Expand Down Expand Up @@ -213,7 +204,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
let base_ty = base.ty(self.body, self.tcx).ty;
if base_ty.is_unsafe_ptr() {
self.require_unsafe(
UnsafetyViolationKind::GeneralAndConstFn,
UnsafetyViolationKind::General,
UnsafetyViolationDetails::DerefOfRawPointer,
)
}
Expand Down Expand Up @@ -258,15 +249,15 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
);
if !nodrop {
self.require_unsafe(
UnsafetyViolationKind::GeneralAndConstFn,
UnsafetyViolationKind::General,
UnsafetyViolationDetails::AssignToDroppingUnionField,
);
} else {
// write to non-drop union field, safe
}
} else {
self.require_unsafe(
UnsafetyViolationKind::GeneralAndConstFn,
UnsafetyViolationKind::General,
UnsafetyViolationDetails::AccessToUnionField,
)
}
Expand All @@ -277,6 +268,9 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {

impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
fn require_unsafe(&mut self, kind: UnsafetyViolationKind, details: UnsafetyViolationDetails) {
// Violations can turn out to be `UnsafeFn` during analysis, but they should not start out as such.
assert_ne!(kind, UnsafetyViolationKind::UnsafeFn);

let source_info = self.source_info;
let lint_root = self.body.source_scopes[self.source_info.scope]
.local_data
Expand Down Expand Up @@ -304,8 +298,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
Safety::Safe => {
for violation in violations {
match violation.kind {
UnsafetyViolationKind::GeneralAndConstFn
| UnsafetyViolationKind::General => {}
UnsafetyViolationKind::General => {}
UnsafetyViolationKind::UnsafeFn => {
bug!("`UnsafetyViolationKind::UnsafeFn` in an `Safe` context")
}
Expand Down Expand Up @@ -334,29 +327,6 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
if !violations.is_empty() {
self.used_unsafe.insert(hir_id);
}
// only some unsafety is allowed in const fn
if self.min_const_fn {
for violation in violations {
match violation.kind {
// these unsafe things are stable in const fn
UnsafetyViolationKind::GeneralAndConstFn => {}
// these things are forbidden in const fns
UnsafetyViolationKind::General => {
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
// compat lint
violation.kind = UnsafetyViolationKind::General;
if !self.violations.contains(&violation) {
self.violations.push(violation)
}
}
UnsafetyViolationKind::UnsafeFn => bug!(
"`UnsafetyViolationKind::UnsafeFn` in an `ExplicitUnsafe` context"
),
}
}
}
true
}
};
Expand Down Expand Up @@ -394,7 +364,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
} else {
continue;
};
self.require_unsafe(UnsafetyViolationKind::GeneralAndConstFn, details);
self.require_unsafe(UnsafetyViolationKind::General, details);
}
}
}
Expand All @@ -412,7 +382,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
// Is `callee_features` a subset of `calling_features`?
if !callee_features.iter().all(|feature| self_features.contains(feature)) {
self.require_unsafe(
UnsafetyViolationKind::GeneralAndConstFn,
UnsafetyViolationKind::General,
UnsafetyViolationDetails::CallToFunctionWith,
)
}
Expand Down Expand Up @@ -494,15 +464,12 @@ fn unsafety_check_result<'tcx>(
let param_env = tcx.param_env(def.did);

let id = tcx.hir().local_def_id_to_hir_id(def.did);
let (const_context, min_const_fn) = match tcx.hir().body_owner_kind(id) {
hir::BodyOwnerKind::Closure => (false, false),
hir::BodyOwnerKind::Fn => {
(tcx.is_const_fn_raw(def.did.to_def_id()), is_min_const_fn(tcx, def.did.to_def_id()))
}
hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(_) => (true, false),
let const_context = match tcx.hir().body_owner_kind(id) {
hir::BodyOwnerKind::Closure => false,
hir::BodyOwnerKind::Fn => tcx.is_const_fn_raw(def.did.to_def_id()),
hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(_) => true,
};
let mut checker =
UnsafetyChecker::new(const_context, min_const_fn, body, def.did, tcx, param_env);
let mut checker = UnsafetyChecker::new(const_context, body, def.did, tcx, param_env);
checker.visit_body(&body);

check_unused_unsafe(tcx, def.did, &checker.used_unsafe, &mut checker.inherited_blocks);
Expand Down Expand Up @@ -577,7 +544,7 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) {
if unsafe_op_in_unsafe_fn_allowed(tcx, lint_root) { " function or" } else { "" };

match kind {
UnsafetyViolationKind::GeneralAndConstFn | UnsafetyViolationKind::General => {
UnsafetyViolationKind::General => {
// once
struct_span_err!(
tcx.sess,
Expand Down

0 comments on commit 9082078

Please sign in to comment.