Skip to content

feat: split unsafe_code lint into lint group #108975

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 7 additions & 140 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@
//! `late_lint_methods!` invocation in `lib.rs`.
use crate::fluent_generated as fluent;
use crate::r#unsafe::{UNSAFE_OBLIGATION_DEFINE, UNSAFE_OBLIGATION_DISCHARGE};
use crate::{
errors::BuiltinEllpisisInclusiveRangePatterns,
lints::{
@@ -36,22 +37,21 @@ use crate::{
BuiltinTypeAliasGenericBoundsSuggestion, BuiltinTypeAliasWhereClause,
BuiltinUnexpectedCliConfigName, BuiltinUnexpectedCliConfigValue,
BuiltinUngatedAsyncFnTrackCaller, BuiltinUnnameableTestItems, BuiltinUnpermittedTypeInit,
BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub, BuiltinUnsafe,
BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
BuiltinWhileTrue, SuggestChangingAssocTypes,
BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub, BuiltinUnstableFeatures,
BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub, BuiltinWhileTrue,
SuggestChangingAssocTypes,
},
types::{transparent_newtype_field, CItemKind},
EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext,
};
use hir::IsAsync;
use rustc_ast::attr;
use rustc_ast::tokenstream::{TokenStream, TokenTree};
use rustc_ast::visit::{FnCtxt, FnKind};
use rustc_ast::{self as ast, *};
use rustc_ast_pretty::pprust::{self, expr_to_string};
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_errors::{Applicability, DecorateLint, MultiSpan};
use rustc_errors::{Applicability, MultiSpan};
use rustc_feature::{deprecated_attributes, AttributeGate, BuiltinAttribute, GateIssue, Stability};
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
@@ -287,140 +287,6 @@ impl<'tcx> LateLintPass<'tcx> for NonShorthandFieldPatterns {
}
}

declare_lint! {
/// The `unsafe_code` lint catches usage of `unsafe` code.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(unsafe_code)]
/// fn main() {
/// unsafe {
///
/// }
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// This lint is intended to restrict the usage of `unsafe`, which can be
/// difficult to use correctly.
UNSAFE_CODE,
Allow,
"usage of `unsafe` code"
}

declare_lint_pass!(UnsafeCode => [UNSAFE_CODE]);

impl UnsafeCode {
fn report_unsafe(
&self,
cx: &EarlyContext<'_>,
span: Span,
decorate: impl for<'a> DecorateLint<'a, ()>,
) {
// This comes from a macro that has `#[allow_internal_unsafe]`.
if span.allows_unsafe() {
return;
}

cx.emit_spanned_lint(UNSAFE_CODE, span, decorate);
}
}

impl EarlyLintPass for UnsafeCode {
fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &ast::Attribute) {
if attr.has_name(sym::allow_internal_unsafe) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::AllowInternalUnsafe);
}
}

#[inline]
fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
if let ast::ExprKind::Block(ref blk, _) = e.kind {
// Don't warn about generated blocks; that'll just pollute the output.
if blk.rules == ast::BlockCheckMode::Unsafe(ast::UserProvided) {
self.report_unsafe(cx, blk.span, BuiltinUnsafe::UnsafeBlock);
}
}
}

fn check_item(&mut self, cx: &EarlyContext<'_>, it: &ast::Item) {
match it.kind {
ast::ItemKind::Trait(box ast::Trait { unsafety: ast::Unsafe::Yes(_), .. }) => {
self.report_unsafe(cx, it.span, BuiltinUnsafe::UnsafeTrait);
}

ast::ItemKind::Impl(box ast::Impl { unsafety: ast::Unsafe::Yes(_), .. }) => {
self.report_unsafe(cx, it.span, BuiltinUnsafe::UnsafeImpl);
}

ast::ItemKind::Fn(..) => {
if let Some(attr) = cx.sess().find_by_name(&it.attrs, sym::no_mangle) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::NoMangleFn);
}

if let Some(attr) = cx.sess().find_by_name(&it.attrs, sym::export_name) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::ExportNameFn);
}

if let Some(attr) = cx.sess().find_by_name(&it.attrs, sym::link_section) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::LinkSectionFn);
}
}

ast::ItemKind::Static(..) => {
if let Some(attr) = cx.sess().find_by_name(&it.attrs, sym::no_mangle) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::NoMangleStatic);
}

if let Some(attr) = cx.sess().find_by_name(&it.attrs, sym::export_name) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::ExportNameStatic);
}

if let Some(attr) = cx.sess().find_by_name(&it.attrs, sym::link_section) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::LinkSectionStatic);
}
}

_ => {}
}
}

fn check_impl_item(&mut self, cx: &EarlyContext<'_>, it: &ast::AssocItem) {
if let ast::AssocItemKind::Fn(..) = it.kind {
if let Some(attr) = cx.sess().find_by_name(&it.attrs, sym::no_mangle) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::NoMangleMethod);
}
if let Some(attr) = cx.sess().find_by_name(&it.attrs, sym::export_name) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::ExportNameMethod);
}
}
}

fn check_fn(&mut self, cx: &EarlyContext<'_>, fk: FnKind<'_>, span: Span, _: ast::NodeId) {
if let FnKind::Fn(
ctxt,
_,
ast::FnSig { header: ast::FnHeader { unsafety: ast::Unsafe::Yes(_), .. }, .. },
_,
_,
body,
) = fk
{
let decorator = match ctxt {
FnCtxt::Foreign => return,
FnCtxt::Free => BuiltinUnsafe::DeclUnsafeFn,
FnCtxt::Assoc(_) if body.is_none() => BuiltinUnsafe::DeclUnsafeMethod,
FnCtxt::Assoc(_) => BuiltinUnsafe::ImplUnsafeMethod,
};
self.report_unsafe(cx, span, decorator);
}
}
}

declare_lint! {
/// The `missing_docs` lint detects missing documentation for public items.
///
@@ -1633,7 +1499,8 @@ declare_lint_pass!(
WHILE_TRUE,
BOX_POINTERS,
NON_SHORTHAND_FIELD_PATTERNS,
UNSAFE_CODE,
UNSAFE_OBLIGATION_DEFINE,
UNSAFE_OBLIGATION_DISCHARGE,
MISSING_DOCS,
MISSING_COPY_IMPLEMENTATIONS,
MISSING_DEBUG_IMPLEMENTATIONS,
6 changes: 6 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
@@ -76,6 +76,7 @@ mod passes;
mod redundant_semicolon;
mod traits;
mod types;
mod r#unsafe;
mod unused;

pub use array_into_iter::ARRAY_INTO_ITER;
@@ -92,6 +93,7 @@ use rustc_session::lint::builtin::{
};
use rustc_span::symbol::Ident;
use rustc_span::Span;
use r#unsafe::UnsafeCode;

use array_into_iter::ArrayIntoIter;
use builtin::*;
@@ -126,6 +128,8 @@ pub use rustc_session::lint::Level::{self, *};
pub use rustc_session::lint::{BufferedEarlyLint, FutureIncompatibleInfo, Lint, LintId};
pub use rustc_session::lint::{LintArray, LintPass};

use crate::r#unsafe::{UNSAFE_OBLIGATION_DEFINE, UNSAFE_OBLIGATION_DISCHARGE};

fluent_messages! { "../messages.ftl" }

pub fn provide(providers: &mut Providers) {
@@ -272,6 +276,8 @@ fn register_builtins(store: &mut LintStore) {
store.register_lints(&BuiltinCombinedModuleLateLintPass::get_lints());
store.register_lints(&BuiltinCombinedLateLintPass::get_lints());

add_lint_group!("unsafe_code", UNSAFE_OBLIGATION_DEFINE, UNSAFE_OBLIGATION_DISCHARGE);

add_lint_group!(
"nonstandard_style",
NON_CAMEL_CASE_TYPES,
262 changes: 262 additions & 0 deletions compiler/rustc_lint/src/unsafe.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,262 @@
use rustc_ast::{
ast,
visit::{FnCtxt, FnKind},
};
use rustc_errors::DecorateLint;
use rustc_session::lint::builtin::UNSAFE_OP_IN_UNSAFE_FN;
use rustc_span::{sym, Span};

use crate::{lints::BuiltinUnsafe, EarlyContext, EarlyLintPass, LintContext};

declare_lint! {
/// The `unsafe_obligation_define` lint triggers when an "unsafe contract"
/// is defined.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(unsafe_obligation_define)]
/// unsafe trait Foo {}
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// An "unsafe contract" is a set of invariants which must be upheld in
/// order to prevent Undefined Behaviour in unsafe code. This lint triggers
/// when such a contract is defined, for example when defining an
/// `unsafe trait` or unsafe trait method without a body.
pub UNSAFE_OBLIGATION_DEFINE,
Allow,
"definition of unsafe contract"
}

declare_lint! {
/// The `unsafe_obligation_discharge` lint triggers when an
/// "unsafe contract"'s invariants are consumed.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(unsafe_obligation_discharge)]
/// fn main() {
/// unsafe {
///
/// }
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// An "unsafe contract" is a set of invariants which must be upheld in
/// order to prevent Undefined Behaviour in unsafe code. This lint triggers
/// when such a contract's invariants must be upheld. For example, `unsafe`
/// blocks may call functions which have safety variants which must be
/// upheld.
pub UNSAFE_OBLIGATION_DISCHARGE,
Allow,
"discharge of unsafe responsibilities"
}

declare_lint_pass!(UnsafeCode => [UNSAFE_OBLIGATION_DEFINE, UNSAFE_OBLIGATION_DISCHARGE]);

enum ObligationKind {
Discharge,
Define,
}

impl UnsafeCode {
fn report_unsafe(
&self,
cx: &EarlyContext<'_>,
span: Span,
decorate: impl for<'a> DecorateLint<'a, ()>,
kind: ObligationKind,
) {
// This comes from a macro that has `#[allow_internal_unsafe]`.
if span.allows_unsafe() {
return;
}

cx.emit_spanned_lint(
match kind {
ObligationKind::Discharge => UNSAFE_OBLIGATION_DISCHARGE,
ObligationKind::Define => UNSAFE_OBLIGATION_DEFINE,
},
span,
decorate,
);
}
}

impl EarlyLintPass for UnsafeCode {
fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &ast::Attribute) {
if attr.has_name(sym::allow_internal_unsafe) {
self.report_unsafe(
cx,
attr.span,
BuiltinUnsafe::AllowInternalUnsafe,
ObligationKind::Discharge,
);
}
}

#[inline]
fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
if let ast::ExprKind::Block(ref blk, _) = e.kind {
// Don't warn about generated blocks; that'll just pollute the output.
if blk.rules == ast::BlockCheckMode::Unsafe(ast::UserProvided) {
self.report_unsafe(
cx,
blk.span,
BuiltinUnsafe::UnsafeBlock,
ObligationKind::Discharge,
);
}
}
}

fn check_item(&mut self, cx: &EarlyContext<'_>, it: &ast::Item) {
match it.kind {
ast::ItemKind::Trait(box ast::Trait { unsafety: ast::Unsafe::Yes(_), .. }) => {
self.report_unsafe(cx, it.span, BuiltinUnsafe::UnsafeTrait, ObligationKind::Define);
}

ast::ItemKind::Impl(box ast::Impl { unsafety: ast::Unsafe::Yes(_), .. }) => {
self.report_unsafe(
cx,
it.span,
BuiltinUnsafe::UnsafeImpl,
ObligationKind::Discharge,
);
}

ast::ItemKind::Fn(..) => {
if let Some(attr) = cx.sess().find_by_name(&it.attrs, sym::no_mangle) {
self.report_unsafe(
cx,
attr.span,
BuiltinUnsafe::NoMangleFn,
ObligationKind::Discharge,
);
}

if let Some(attr) = cx.sess().find_by_name(&it.attrs, sym::export_name) {
self.report_unsafe(
cx,
attr.span,
BuiltinUnsafe::ExportNameFn,
ObligationKind::Discharge,
);
}

if let Some(attr) = cx.sess().find_by_name(&it.attrs, sym::link_section) {
self.report_unsafe(
cx,
attr.span,
BuiltinUnsafe::LinkSectionFn,
ObligationKind::Discharge,
);
}
}

ast::ItemKind::Static(..) => {
if let Some(attr) = cx.sess().find_by_name(&it.attrs, sym::no_mangle) {
self.report_unsafe(
cx,
attr.span,
BuiltinUnsafe::NoMangleStatic,
ObligationKind::Discharge,
);
}

if let Some(attr) = cx.sess().find_by_name(&it.attrs, sym::export_name) {
self.report_unsafe(
cx,
attr.span,
BuiltinUnsafe::ExportNameStatic,
ObligationKind::Discharge,
);
}

if let Some(attr) = cx.sess().find_by_name(&it.attrs, sym::link_section) {
self.report_unsafe(
cx,
attr.span,
BuiltinUnsafe::LinkSectionStatic,
ObligationKind::Discharge,
);
}
}

_ => {}
}
}

fn check_impl_item(&mut self, cx: &EarlyContext<'_>, it: &ast::AssocItem) {
if let ast::AssocItemKind::Fn(..) = it.kind {
if let Some(attr) = cx.sess().find_by_name(&it.attrs, sym::no_mangle) {
self.report_unsafe(
cx,
attr.span,
BuiltinUnsafe::NoMangleMethod,
ObligationKind::Discharge,
);
}
if let Some(attr) = cx.sess().find_by_name(&it.attrs, sym::export_name) {
self.report_unsafe(
cx,
attr.span,
BuiltinUnsafe::ExportNameMethod,
ObligationKind::Discharge,
);
}
}
}

fn check_fn(&mut self, cx: &EarlyContext<'_>, fk: FnKind<'_>, span: Span, _: ast::NodeId) {
if let FnKind::Fn(
ctxt,
_,
ast::FnSig { header: ast::FnHeader { unsafety: ast::Unsafe::Yes(_), .. }, .. },
_,
_,
body,
) = fk
{
let decorator = match ctxt {
FnCtxt::Foreign => return,
FnCtxt::Free => BuiltinUnsafe::DeclUnsafeFn,
FnCtxt::Assoc(_) if body.is_none() => {
// there is no body, so we know that it cannot contain
// unsafety which does more than simply define an unsafety
// contract, see below.
return self.report_unsafe(
cx,
span,
BuiltinUnsafe::DeclUnsafeMethod,
ObligationKind::Define,
);
}
FnCtxt::Assoc(_) => BuiltinUnsafe::ImplUnsafeMethod,
};

// Unsafe methods can merely define unsafety contracts, but they
// also give free rein for the body of the function to contain
// unsafe code which is not necessarily covered by said contract.
// If unsafe code in the function body is allowed without unsafe
// blocks, then it is just a regular discharge of unsafe
// responibilties.
if cx.get_lint_level(UNSAFE_OP_IN_UNSAFE_FN)
>= cx.get_lint_level(UNSAFE_OBLIGATION_DEFINE)
{
self.report_unsafe(cx, span, decorator, ObligationKind::Define);
} else {
self.report_unsafe(cx, span, decorator, ObligationKind::Discharge);
}
}
}
}
1 change: 1 addition & 0 deletions src/tools/lint-docs/src/groups.rs
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ use std::process::Command;

/// Descriptions of rustc lint groups.
static GROUP_DESCRIPTIONS: &[(&str, &str)] = &[
("unsafe-code", "Lints that detect unsafe code"),
("unused", "Lints that detect things being declared but not used, or excess syntax"),
("let-underscore", "Lints that detect wildcard let bindings that are likely to be invalid"),
("rustdoc", "Rustdoc-specific lints"),
21 changes: 21 additions & 0 deletions tests/ui/lint/issue-108926.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#![forbid(unsafe_code)]

trait Foo {
unsafe fn dangerous();
//~^ ERROR declaration of an `unsafe` method [unsafe_obligation_define]
}

struct ImplOk;
impl Foo for ImplOk {
#[forbid(unsafe_op_in_unsafe_fn)]
unsafe fn dangerous() {}
//~^ ERROR implementation of an `unsafe` method [unsafe_obligation_define]
}

struct ImplBad;
impl Foo for ImplBad {
unsafe fn dangerous() {}
//~^ ERROR implementation of an `unsafe` method [unsafe_obligation_discharge]
}

fn main() {}
29 changes: 29 additions & 0 deletions tests/ui/lint/issue-108926.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
error: declaration of an `unsafe` method
--> $DIR/issue-108926.rs:4:5
|
LL | unsafe fn dangerous();
| ^^^^^^^^^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/issue-108926.rs:1:11
|
LL | #![forbid(unsafe_code)]
| ^^^^^^^^^^^
= note: `#[forbid(unsafe_obligation_define)]` implied by `#[forbid(unsafe_code)]`

error: implementation of an `unsafe` method
--> $DIR/issue-108926.rs:11:5
|
LL | unsafe fn dangerous() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^

error: implementation of an `unsafe` method
--> $DIR/issue-108926.rs:17:5
|
LL | unsafe fn dangerous() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[forbid(unsafe_obligation_discharge)]` implied by `#[forbid(unsafe_code)]`

error: aborting due to 3 previous errors

1 change: 1 addition & 0 deletions tests/ui/lint/lint-attr-everywhere-early.stderr
Original file line number Diff line number Diff line change
@@ -50,6 +50,7 @@ note: the lint level is defined here
|
LL | #![deny(unsafe_code)]
| ^^^^^^^^^^^
= note: `#[deny(unsafe_obligation_discharge)]` implied by `#[deny(unsafe_code)]`

error: usage of an `unsafe` block
--> $DIR/lint-attr-everywhere-early.rs:43:39
1 change: 1 addition & 0 deletions tests/ui/lint/lint-forbid-internal-unsafe.stderr
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ note: the lint level is defined here
|
LL | #![forbid(unsafe_code)]
| ^^^^^^^^^^^
= note: `#[forbid(unsafe_obligation_discharge)]` implied by `#[forbid(unsafe_code)]`

warning: dereferencing a null pointer
--> $DIR/lint-forbid-internal-unsafe.rs:15:26
3 changes: 3 additions & 0 deletions tests/ui/lint/lint-unsafe-code.stderr
Original file line number Diff line number Diff line change
@@ -10,6 +10,7 @@ note: the lint level is defined here
|
LL | #![deny(unsafe_code)]
| ^^^^^^^^^^^
= note: `#[deny(unsafe_obligation_discharge)]` implied by `#[deny(unsafe_code)]`

error: declaration of a `no_mangle` static
--> $DIR/lint-unsafe-code.rs:32:1
@@ -94,6 +95,8 @@ error: declaration of an `unsafe` trait
|
LL | unsafe trait Foo {}
| ^^^^^^^^^^^^^^^^^^^
|
= note: `#[deny(unsafe_obligation_define)]` implied by `#[deny(unsafe_code)]`

error: implementation of an `unsafe` trait
--> $DIR/lint-unsafe-code.rs:66:1
34 changes: 0 additions & 34 deletions tests/ui/lint/reasons-forbidden.rs

This file was deleted.

31 changes: 0 additions & 31 deletions tests/ui/lint/reasons-forbidden.stderr

This file was deleted.