Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit bb67915

Browse files
LeSeulArtichautLeSeulArtichaut
committed
Apply suggestions from code review
1 parent 594c499 commit bb67915

File tree

8 files changed

+125
-50
lines changed

8 files changed

+125
-50
lines changed

‎src/librustc_lint/levels.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ use rustc_middle::ty::TyCtxt;
1717
use rustc_session::lint::{builtin, Level, Lint};
1818
use rustc_session::parse::feature_err;
1919
use rustc_session::Session;
20-
use rustc_span::source_map::MultiSpan;
2120
use rustc_span::symbol::{sym, Symbol};
21+
use rustc_span::{source_map::MultiSpan, Span, DUMMY_SP};
2222

2323
use std::cmp;
2424

@@ -80,6 +80,8 @@ impl<'s> LintLevelsBuilder<'s> {
8080
let level = cmp::min(level, self.sets.lint_cap);
8181

8282
let lint_flag_val = Symbol::intern(lint_name);
83+
self.check_gated_lint(lint_flag_val, DUMMY_SP);
84+
8385
let ids = match store.find_lints(&lint_name) {
8486
Ok(ids) => ids,
8587
Err(_) => continue, // errors handled in check_lint_name_cmdline above
@@ -211,6 +213,7 @@ impl<'s> LintLevelsBuilder<'s> {
211213
let name = meta_item.path.segments.last().expect("empty lint name").ident.name;
212214
match store.check_lint_name(&name.as_str(), tool_name) {
213215
CheckLintNameResult::Ok(ids) => {
216+
self.check_gated_lint(name, attr.span);
214217
let src = LintSource::Node(name, li.span(), reason);
215218
for id in ids {
216219
specs.insert(*id, (level, src));
@@ -383,6 +386,20 @@ impl<'s> LintLevelsBuilder<'s> {
383386
BuilderPush { prev, changed: prev != self.cur }
384387
}
385388

389+
fn check_gated_lint(&self, name: Symbol, span: Span) {
390+
if name.as_str() == "unsafe_op_in_unsafe_fn"
391+
&& !self.sess.features_untracked().unsafe_block_in_unsafe_fn
392+
{
393+
feature_err(
394+
&self.sess.parse_sess,
395+
sym::unsafe_block_in_unsafe_fn,
396+
span,
397+
"the `unsafe_op_in_unsafe_fn` lint is unstable",
398+
)
399+
.emit();
400+
}
401+
}
402+
386403
/// Called after `push` when the scope of a set of attributes are exited.
387404
pub fn pop(&mut self, push: BuilderPush) {
388405
self.cur = push.prev;

‎src/librustc_middle/mir/query.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,17 @@ pub enum UnsafetyViolationKind {
2121
GeneralAndConstFn,
2222
/// Borrow of packed field.
2323
/// Has to be handled as a lint for backwards compatibility.
24-
BorrowPacked(hir::HirId),
24+
BorrowPacked,
2525
/// Unsafe operation in an `unsafe fn` but outside an `unsafe` block.
2626
/// Has to be handled as a lint for backwards compatibility.
2727
/// Should stay gated under `#![feature(unsafe_block_in_unsafe_fn)]`.
28-
UnsafeFn(hir::HirId),
28+
UnsafeFn,
2929
}
3030

3131
#[derive(Copy, Clone, PartialEq, RustcEncodable, RustcDecodable, HashStable)]
3232
pub struct UnsafetyViolation {
3333
pub source_info: SourceInfo,
34+
pub lint_root: hir::HirId,
3435
pub description: Symbol,
3536
pub details: Symbol,
3637
pub kind: UnsafetyViolationKind,

‎src/librustc_mir/transform/check_unsafety.rs

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use rustc_data_structures::fx::FxHashSet;
22
use rustc_errors::struct_span_err;
33
use rustc_hir as hir;
44
use rustc_hir::def_id::{DefId, LocalDefId};
5+
use rustc_hir::hir_id::HirId;
56
use rustc_hir::intravisit;
67
use rustc_hir::Node;
78
use rustc_middle::mir::visit::{MutatingUseContext, PlaceContext, Visitor};
@@ -10,6 +11,7 @@ use rustc_middle::ty::cast::CastTy;
1011
use rustc_middle::ty::query::Providers;
1112
use rustc_middle::ty::{self, TyCtxt};
1213
use rustc_session::lint::builtin::{SAFE_PACKED_BORROWS, UNSAFE_OP_IN_UNSAFE_FN, UNUSED_UNSAFE};
14+
use rustc_session::lint::Level;
1315
use rustc_span::symbol::{sym, Symbol};
1416

1517
use std::ops::Bound;
@@ -220,7 +222,18 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
220222

221223
for (i, elem) in place.projection.iter().enumerate() {
222224
let proj_base = &place.projection[..i];
223-
let old_source_info = self.source_info;
225+
if context.is_borrow() {
226+
if util::is_disaligned(self.tcx, self.body, self.param_env, *place) {
227+
self.require_unsafe(
228+
"borrow of packed field",
229+
"fields of packed structs might be misaligned: dereferencing a \
230+
misaligned pointer or even just creating a misaligned reference \
231+
is undefined behavior",
232+
UnsafetyViolationKind::BorrowPacked,
233+
);
234+
}
235+
}
236+
let source_info = self.source_info;
224237
if let [] = proj_base {
225238
let decl = &self.body.local_decls[place.local];
226239
if decl.internal {
@@ -301,7 +314,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
301314
}
302315
_ => {}
303316
}
304-
self.source_info = old_source_info;
317+
self.source_info = source_info;
305318
}
306319
}
307320
}
@@ -314,9 +327,15 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
314327
kind: UnsafetyViolationKind,
315328
) {
316329
let source_info = self.source_info;
330+
let lint_root = self.body.source_scopes[self.source_info.scope]
331+
.local_data
332+
.as_ref()
333+
.assert_crate_local()
334+
.lint_root;
317335
self.register_violations(
318336
&[UnsafetyViolation {
319337
source_info,
338+
lint_root,
320339
description: Symbol::intern(description),
321340
details: Symbol::intern(details),
322341
kind,
@@ -343,15 +362,15 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
343362
match violation.kind {
344363
UnsafetyViolationKind::GeneralAndConstFn
345364
| UnsafetyViolationKind::General => {}
346-
UnsafetyViolationKind::BorrowPacked(_) => {
365+
UnsafetyViolationKind::BorrowPacked => {
347366
if self.min_const_fn {
348367
// const fns don't need to be backwards compatible and can
349368
// emit these violations as a hard error instead of a backwards
350369
// compat lint
351370
violation.kind = UnsafetyViolationKind::General;
352371
}
353372
}
354-
UnsafetyViolationKind::UnsafeFn(_) => {
373+
UnsafetyViolationKind::UnsafeFn => {
355374
bug!("`UnsafetyViolationKind::UnsafeFn` in an `Safe` context")
356375
}
357376
}
@@ -365,14 +384,9 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
365384
Safety::FnUnsafe if self.tcx.features().unsafe_block_in_unsafe_fn => {
366385
for violation in violations {
367386
let mut violation = *violation;
368-
let lint_root = self.body.source_scopes[self.source_info.scope]
369-
.local_data
370-
.as_ref()
371-
.assert_crate_local()
372-
.lint_root;
373387

374388
// FIXME(LeSeulArtichaut): what to do with `UnsafetyViolationKind::BorrowPacked`?
375-
violation.kind = UnsafetyViolationKind::UnsafeFn(lint_root);
389+
violation.kind = UnsafetyViolationKind::UnsafeFn;
376390
if !self.violations.contains(&violation) {
377391
self.violations.push(violation)
378392
}
@@ -394,7 +408,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
394408
UnsafetyViolationKind::GeneralAndConstFn => {}
395409
// these things are forbidden in const fns
396410
UnsafetyViolationKind::General
397-
| UnsafetyViolationKind::BorrowPacked(_) => {
411+
| UnsafetyViolationKind::BorrowPacked => {
398412
let mut violation = *violation;
399413
// const fns don't need to be backwards compatible and can
400414
// emit these violations as a hard error instead of a backwards
@@ -404,7 +418,7 @@ impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
404418
self.violations.push(violation)
405419
}
406420
}
407-
UnsafetyViolationKind::UnsafeFn(_) => bug!(
421+
UnsafetyViolationKind::UnsafeFn => bug!(
408422
"`UnsafetyViolationKind::UnsafeFn` in an `ExplicitUnsafe` context"
409423
),
410424
}
@@ -657,47 +671,50 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: DefId) {
657671
let UnsafetyCheckResult { violations, unsafe_blocks } =
658672
tcx.unsafety_check_result(def_id.expect_local());
659673

660-
let or_block_msg = if tcx.features().unsafe_block_in_unsafe_fn { "" } else { " or block" };
661-
662-
for &UnsafetyViolation { source_info, description, details, kind } in violations.iter() {
674+
for &UnsafetyViolation { source_info, lint_root, description, details, kind } in
675+
violations.iter()
676+
{
663677
// Report an error.
678+
let unsafe_fn_msg =
679+
if unsafe_op_in_unsafe_fn_allowed(tcx, lint_root) { "" } else { " function or" };
680+
664681
match kind {
665682
UnsafetyViolationKind::GeneralAndConstFn | UnsafetyViolationKind::General => {
666683
// once
667684
struct_span_err!(
668685
tcx.sess,
669686
source_info.span,
670687
E0133,
671-
"{} is unsafe and requires unsafe function{}",
688+
"{} is unsafe and requires unsafe{} block",
672689
description,
673-
or_block_msg,
690+
unsafe_fn_msg,
674691
)
675692
.span_label(source_info.span, &*description.as_str())
676693
.note(&details.as_str())
677694
.emit();
678695
}
679-
UnsafetyViolationKind::BorrowPacked(lint_hir_id) => {
696+
UnsafetyViolationKind::BorrowPacked => {
680697
if let Some(impl_def_id) = builtin_derive_def_id(tcx, def_id) {
681698
tcx.ensure().unsafe_derive_on_repr_packed(impl_def_id);
682699
} else {
683700
tcx.struct_span_lint_hir(
684701
SAFE_PACKED_BORROWS,
685-
lint_hir_id,
702+
lint_root,
686703
source_info.span,
687704
|lint| {
688705
lint.build(&format!(
689-
"{} is unsafe and requires unsafe function{} (error E0133)",
690-
description, or_block_msg,
706+
"{} is unsafe and requires unsafe{} block (error E0133)",
707+
description, unsafe_fn_msg,
691708
))
692709
.note(&details.as_str())
693710
.emit()
694711
},
695712
)
696713
}
697714
}
698-
UnsafetyViolationKind::UnsafeFn(lint_hir_id) => tcx.struct_span_lint_hir(
715+
UnsafetyViolationKind::UnsafeFn => tcx.struct_span_lint_hir(
699716
UNSAFE_OP_IN_UNSAFE_FN,
700-
lint_hir_id,
717+
lint_root,
701718
source_info.span,
702719
|lint| {
703720
lint.build(&format!(
@@ -728,3 +745,7 @@ pub fn check_unsafety(tcx: TyCtxt<'_>, def_id: DefId) {
728745
report_unused_unsafe(tcx, &unsafe_used, block_id);
729746
}
730747
}
748+
749+
fn unsafe_op_in_unsafe_fn_allowed(tcx: TyCtxt<'_>, id: HirId) -> bool {
750+
tcx.lint_level_at_node(UNSAFE_OP_IN_UNSAFE_FN, id).0 == Level::Allow
751+
}

‎src/librustc_mir_build/build/block.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder};
44
use crate::hair::*;
55
use rustc_hir as hir;
66
use rustc_middle::mir::*;
7+
use rustc_session::lint::builtin::UNSAFE_OP_IN_UNSAFE_FN;
8+
use rustc_session::lint::Level;
79
use rustc_span::Span;
810

911
impl<'a, 'tcx> Builder<'a, 'tcx> {
@@ -218,7 +220,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
218220
match self.unpushed_unsafe {
219221
Safety::Safe => {}
220222
// no longer treat `unsafe fn`s as `unsafe` contexts (see RFC #2585)
221-
Safety::FnUnsafe if self.hir.tcx().features().unsafe_block_in_unsafe_fn => {}
223+
Safety::FnUnsafe
224+
if self.hir.tcx().lint_level_at_node(UNSAFE_OP_IN_UNSAFE_FN, hir_id).0
225+
!= Level::Allow => {}
222226
_ => return,
223227
}
224228
self.unpushed_unsafe = Safety::ExplicitUnsafe(hir_id);
@@ -233,7 +237,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
233237
.push_unsafe_count
234238
.checked_sub(1)
235239
.unwrap_or_else(|| span_bug!(span, "unsafe count underflow"));
236-
if self.push_unsafe_count == 0 { Some(self.unpushed_unsafe) } else { None }
240+
if self.push_unsafe_count == 0 {
241+
Some(self.unpushed_unsafe)
242+
} else {
243+
None
244+
}
237245
}
238246
};
239247

‎src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.rs

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,4 @@
1-
#![deny(unused_unsafe)]
2-
3-
unsafe fn unsf() {}
4-
5-
unsafe fn foo() {
6-
unsafe { //~ ERROR unnecessary `unsafe` block
7-
unsf()
8-
}
9-
}
1+
#![deny(unsafe_op_in_unsafe_fn)]
2+
//~^ ERROR the `unsafe_op_in_unsafe_fn` lint is unstable
103

114
fn main() {}

‎src/test/ui/feature-gates/feature-gate-unsafe_block_in_unsafe_fn.stderr

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,30 @@
1-
error: unnecessary `unsafe` block
2-
--> $DIR/feature-gate-unsafe_block_in_unsafe_fn.rs:6:5
1+
error[E0658]: the `unsafe_op_in_unsafe_fn` lint is unstable
2+
--> $DIR/feature-gate-unsafe_block_in_unsafe_fn.rs:1:1
33
|
4-
LL | unsafe fn foo() {
5-
| --------------- because it's nested under this `unsafe` fn
6-
LL | unsafe {
7-
| ^^^^^^ unnecessary `unsafe` block
4+
LL | #![deny(unsafe_op_in_unsafe_fn)]
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
86
|
9-
note: the lint level is defined here
10-
--> $DIR/feature-gate-unsafe_block_in_unsafe_fn.rs:1:9
7+
= note: see issue #71668 <https://github.com/rust-lang/rust/issues/71668> for more information
8+
= help: add `#![feature(unsafe_block_in_unsafe_fn)]` to the crate attributes to enable
9+
10+
error[E0658]: the `unsafe_op_in_unsafe_fn` lint is unstable
11+
--> $DIR/feature-gate-unsafe_block_in_unsafe_fn.rs:1:1
12+
|
13+
LL | #![deny(unsafe_op_in_unsafe_fn)]
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
15+
|
16+
= note: see issue #71668 <https://github.com/rust-lang/rust/issues/71668> for more information
17+
= help: add `#![feature(unsafe_block_in_unsafe_fn)]` to the crate attributes to enable
18+
19+
error[E0658]: the `unsafe_op_in_unsafe_fn` lint is unstable
20+
--> $DIR/feature-gate-unsafe_block_in_unsafe_fn.rs:1:1
21+
|
22+
LL | #![deny(unsafe_op_in_unsafe_fn)]
23+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1124
|
12-
LL | #![deny(unused_unsafe)]
13-
| ^^^^^^^^^^^^^
25+
= note: see issue #71668 <https://github.com/rust-lang/rust/issues/71668> for more information
26+
= help: add `#![feature(unsafe_block_in_unsafe_fn)]` to the crate attributes to enable
1427

15-
error: aborting due to previous error
28+
error: aborting due to 3 previous errors
1629

30+
For more information about this error, try `rustc --explain E0658`.

‎src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ unsafe fn baz() {
2121
#[allow(unsafe_op_in_unsafe_fn)]
2222
unsafe fn qux() {
2323
unsf(); // no error
24+
25+
unsafe { unsf() }
26+
//~^ ERROR unnecessary `unsafe` block
2427
}
2528

26-
fn main() {}
29+
fn main() {
30+
unsf()
31+
//~^ ERROR call to unsafe function is unsafe and requires unsafe function or block
32+
}

‎src/test/ui/unsafe/rfc-2585-unsafe_op_in_unsafe_fn.stderr

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,20 @@ note: the lint level is defined here
2525
LL | #![deny(unused_unsafe)]
2626
| ^^^^^^^^^^^^^
2727

28-
error: aborting due to previous error; 1 warning emitted
28+
error: unnecessary `unsafe` block
29+
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:25:5
30+
|
31+
LL | unsafe { unsf() }
32+
| ^^^^^^ unnecessary `unsafe` block
33+
34+
error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
35+
--> $DIR/rfc-2585-unsafe_op_in_unsafe_fn.rs:30:5
36+
|
37+
LL | unsf()
38+
| ^^^^^^ call to unsafe function
39+
|
40+
= note: consult the function's documentation for information on how to avoid undefined behavior
41+
42+
error: aborting due to 3 previous errors; 1 warning emitted
2943

44+
For more information about this error, try `rustc --explain E0133`.

0 commit comments

Comments
 (0)