Skip to content

Commit

Permalink
Rollup merge of #105147 - nbdd0121:inline_const_unsafe, r=oli-obk
Browse files Browse the repository at this point in the history
Allow unsafe through inline const

Handle similar to closures.

Address #104087 (comment)

Note that this PR does not fix the issue for `unsafe { [0; function_requiring_unsafe()] }`. This is fundamentally unfixable for MIR unsafeck IMO.

This PR also does not fix unsafety checking for inline const in pattern position. It actually breaks it, allowing unsafe functions to be used in inline const in pattern position without unsafe blocks. Inline const in pattern position is not visible in MIR so ignored by MIR unsafety checking (currently it is also not checked by borrow checker, which is the reason why it's considered an incomplete feature).

`@rustbot` label: +T-lang +F-inline_const
  • Loading branch information
matthiaskrgr committed Dec 13, 2022
2 parents aa5b179 + d6dc912 commit 7357cfb
Show file tree
Hide file tree
Showing 12 changed files with 177 additions and 19 deletions.
7 changes: 6 additions & 1 deletion compiler/rustc_mir_build/src/build/custom/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use rustc_ast::Attribute;
use rustc_data_structures::fx::FxHashMap;
use rustc_hir::def_id::DefId;
use rustc_hir::HirId;
use rustc_index::vec::IndexVec;
use rustc_middle::{
mir::*,
Expand All @@ -33,6 +34,7 @@ mod parse;
pub(super) fn build_custom_mir<'tcx>(
tcx: TyCtxt<'tcx>,
did: DefId,
hir_id: HirId,
thir: &Thir<'tcx>,
expr: ExprId,
params: &IndexVec<ParamId, Param<'tcx>>,
Expand Down Expand Up @@ -67,7 +69,10 @@ pub(super) fn build_custom_mir<'tcx>(
parent_scope: None,
inlined: None,
inlined_parent_scope: None,
local_data: ClearCrossCrate::Clear,
local_data: ClearCrossCrate::Set(SourceScopeLocalData {
lint_root: hir_id,
safety: Safety::Safe,
}),
});
body.injection_phase = Some(parse_attribute(attr));

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,7 @@ fn construct_fn<'tcx>(
return custom::build_custom_mir(
tcx,
fn_def.did.to_def_id(),
fn_id,
thir,
expr,
arguments,
Expand Down
34 changes: 19 additions & 15 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,18 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
fn unsafe_op_in_unsafe_fn_allowed(&self) -> bool {
self.tcx.lint_level_at_node(UNSAFE_OP_IN_UNSAFE_FN, self.hir_context).0 == Level::Allow
}

/// Handle closures/generators/inline-consts, which is unsafecked with their parent body.
fn visit_inner_body(&mut self, def: ty::WithOptConstParam<LocalDefId>) {
if let Ok((inner_thir, expr)) = self.tcx.thir_body(def) {
let inner_thir = &inner_thir.borrow();
let hir_context = self.tcx.hir().local_def_id_to_hir_id(def.did);
let mut inner_visitor = UnsafetyVisitor { thir: inner_thir, hir_context, ..*self };
inner_visitor.visit_expr(&inner_thir[expr]);
// Unsafe blocks can be used in the inner body, make sure to take it into account
self.safety_context = inner_visitor.safety_context;
}
}
}

// Searches for accesses to layout constrained fields.
Expand Down Expand Up @@ -408,16 +420,11 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
} else {
ty::WithOptConstParam::unknown(closure_id)
};
let (closure_thir, expr) = self.tcx.thir_body(closure_def).unwrap_or_else(|_| {
(self.tcx.alloc_steal_thir(Thir::new()), ExprId::from_u32(0))
});
let closure_thir = &closure_thir.borrow();
let hir_context = self.tcx.hir().local_def_id_to_hir_id(closure_id);
let mut closure_visitor =
UnsafetyVisitor { thir: closure_thir, hir_context, ..*self };
closure_visitor.visit_expr(&closure_thir[expr]);
// Unsafe blocks can be used in closures, make sure to take it into account
self.safety_context = closure_visitor.safety_context;
self.visit_inner_body(closure_def);
}
ExprKind::ConstBlock { did, substs: _ } => {
let def_id = did.expect_local();
self.visit_inner_body(ty::WithOptConstParam::unknown(def_id));
}
ExprKind::Field { lhs, .. } => {
let lhs = &self.thir[lhs];
Expand Down Expand Up @@ -612,11 +619,8 @@ pub fn check_unsafety<'tcx>(tcx: TyCtxt<'tcx>, def: ty::WithOptConstParam<LocalD
return;
}

// Closures are handled by their owner, if it has a body
if tcx.is_closure(def.did.to_def_id()) {
let hir = tcx.hir();
let owner = hir.enclosing_body_owner(hir.local_def_id_to_hir_id(def.did));
tcx.ensure().thir_check_unsafety(owner);
// Closures and inline consts are handled by their owner, if it has a body
if tcx.is_typeck_child(def.did.to_def_id()) {
return;
}

Expand Down
35 changes: 32 additions & 3 deletions compiler/rustc_mir_transform/src/check_unsafety.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::struct_span_err;
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::hir_id::HirId;
use rustc_hir::intravisit;
Expand Down Expand Up @@ -134,6 +135,28 @@ impl<'tcx> Visitor<'tcx> for UnsafetyChecker<'_, 'tcx> {
self.super_rvalue(rvalue, location);
}

fn visit_operand(&mut self, op: &Operand<'tcx>, location: Location) {
if let Operand::Constant(constant) = op {
let maybe_uneval = match constant.literal {
ConstantKind::Val(..) | ConstantKind::Ty(_) => None,
ConstantKind::Unevaluated(uv, _) => Some(uv),
};

if let Some(uv) = maybe_uneval {
if uv.promoted.is_none() {
let def_id = uv.def.def_id_for_type_of();
if self.tcx.def_kind(def_id) == DefKind::InlineConst {
let local_def_id = def_id.expect_local();
let UnsafetyCheckResult { violations, used_unsafe_blocks, .. } =
self.tcx.unsafety_check_result(local_def_id);
self.register_violations(violations, used_unsafe_blocks.iter().copied());
}
}
}
}
self.super_operand(op, location);
}

fn visit_place(&mut self, place: &Place<'tcx>, context: PlaceContext, _location: Location) {
// On types with `scalar_valid_range`, prevent
// * `&mut x.field`
Expand Down Expand Up @@ -410,6 +433,12 @@ impl<'tcx> intravisit::Visitor<'tcx> for UnusedUnsafeVisitor<'_, 'tcx> {
intravisit::walk_block(self, block);
}

fn visit_anon_const(&mut self, c: &'tcx hir::AnonConst) {
if matches!(self.tcx.def_kind(c.def_id), DefKind::InlineConst) {
self.visit_body(self.tcx.hir().body(c.body))
}
}

fn visit_fn(
&mut self,
fk: intravisit::FnKind<'tcx>,
Expand Down Expand Up @@ -484,7 +513,7 @@ fn unsafety_check_result<'tcx>(
let mut checker = UnsafetyChecker::new(body, def.did, tcx, param_env);
checker.visit_body(&body);

let unused_unsafes = (!tcx.is_closure(def.did.to_def_id()))
let unused_unsafes = (!tcx.is_typeck_child(def.did.to_def_id()))
.then(|| check_unused_unsafe(tcx, def.did, &checker.used_unsafe_blocks));

tcx.arena.alloc(UnsafetyCheckResult {
Expand Down Expand Up @@ -516,8 +545,8 @@ fn report_unused_unsafe(tcx: TyCtxt<'_>, kind: UnusedUnsafe, id: HirId) {
pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: LocalDefId) {
debug!("check_unsafety({:?})", def_id);

// closures are handled by their parent fn.
if tcx.is_closure(def_id.to_def_id()) {
// closures and inline consts are handled by their parent fn.
if tcx.is_typeck_child(def_id.to_def_id()) {
return;
}

Expand Down
11 changes: 11 additions & 0 deletions src/test/ui/inline-const/expr-unsafe-err.mir.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
--> $DIR/expr-unsafe-err.rs:8:9
|
LL | require_unsafe();
| ^^^^^^^^^^^^^^^^ call to unsafe function
|
= note: consult the function's documentation for information on how to avoid undefined behavior

error: aborting due to previous error

For more information about this error, try `rustc --explain E0133`.
11 changes: 11 additions & 0 deletions src/test/ui/inline-const/expr-unsafe-err.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// revisions: mir thir
// [thir]compile-flags: -Z thir-unsafeck
#![feature(inline_const)]
const unsafe fn require_unsafe() -> usize { 1 }

fn main() {
const {
require_unsafe();
//~^ ERROR [E0133]
}
}
11 changes: 11 additions & 0 deletions src/test/ui/inline-const/expr-unsafe-err.thir.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0133]: call to unsafe function `require_unsafe` is unsafe and requires unsafe function or block
--> $DIR/expr-unsafe-err.rs:8:9
|
LL | require_unsafe();
| ^^^^^^^^^^^^^^^^ call to unsafe function
|
= note: consult the function's documentation for information on how to avoid undefined behavior

error: aborting due to previous error

For more information about this error, try `rustc --explain E0133`.
14 changes: 14 additions & 0 deletions src/test/ui/inline-const/expr-unsafe.mir.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
warning: unnecessary `unsafe` block
--> $DIR/expr-unsafe.rs:12:13
|
LL | unsafe {}
| ^^^^^^ unnecessary `unsafe` block
|
note: the lint level is defined here
--> $DIR/expr-unsafe.rs:4:9
|
LL | #![warn(unused_unsafe)]
| ^^^^^^^^^^^^^

warning: 1 warning emitted

16 changes: 16 additions & 0 deletions src/test/ui/inline-const/expr-unsafe.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// check-pass
// revisions: mir thir
// [thir]compile-flags: -Z thir-unsafeck
#![warn(unused_unsafe)]
#![feature(inline_const)]
const unsafe fn require_unsafe() -> usize { 1 }

fn main() {
unsafe {
const {
require_unsafe();
unsafe {}
//~^ WARNING unnecessary `unsafe` block
}
}
}
17 changes: 17 additions & 0 deletions src/test/ui/inline-const/expr-unsafe.thir.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
warning: unnecessary `unsafe` block
--> $DIR/expr-unsafe.rs:12:13
|
LL | unsafe {
| ------ because it's nested under this `unsafe` block
...
LL | unsafe {}
| ^^^^^^ unnecessary `unsafe` block
|
note: the lint level is defined here
--> $DIR/expr-unsafe.rs:4:9
|
LL | #![warn(unused_unsafe)]
| ^^^^^^^^^^^^^

warning: 1 warning emitted

17 changes: 17 additions & 0 deletions src/test/ui/inline-const/pat-unsafe-err.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// ignore-test This is currently broken
// revisions: mir thir
// [thir]compile-flags: -Z thir-unsafeck

#![allow(incomplete_features)]
#![feature(inline_const_pat)]

const unsafe fn require_unsafe() -> usize { 1 }

fn main() {
match () {
const {
require_unsafe();
//~^ ERROR [E0133]
} => (),
}
}
22 changes: 22 additions & 0 deletions src/test/ui/inline-const/pat-unsafe.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// ignore-test This is currently broken
// check-pass
// revisions: mir thir
// [thir]compile-flags: -Z thir-unsafeck

#![allow(incomplete_features)]
#![warn(unused_unsafe)]
#![feature(inline_const_pat)]

const unsafe fn require_unsafe() -> usize { 1 }

fn main() {
unsafe {
match () {
const {
require_unsafe();
unsafe {}
//~^ WARNING unnecessary `unsafe` block
} => (),
}
}
}

0 comments on commit 7357cfb

Please sign in to comment.