Lint in a safe pub fn when a trivially unsafe operation is used on an its arguments by rperier · Pull Request #163203 · rust-lang/rust

rperier · 2026-09-23T08:40:56Z

fixes #162090

This is a first proposal.

A simpler lint name would probably be better
We should probably add a span on the function signature itself (or at least on its args in the signature)

I am opened to suggestions.

rustbot · 2026-09-23T08:41:00Z

Some changes occurred in integer formatting

cc @tgross35

stdarch is developed in its own repository. If possible, consider making this change to rust-lang/stdarch instead.

cc @Amanieu, @folkertdev, @sayantn

rustbot · 2026-09-23T08:41:03Z

r? @adwinwhite

rustbot has assigned @adwinwhite.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 77 candidates
Random selection from 19 candidates

tgross35

So this is just checking if any argument is passed to any unsafe function? I think that's going to be far too noisy and a better fit for Clippy, it looks like a lot of false positives.

Maybe there's more of a case for uplifting clippy::not_unsafe_ptr_arg_deref? I think that's more along the lines of what Josh was suggesting at #162090 (comment).

Also there are no tests or examples, this is hard to evaluate.

View changes since this review

tgross35 · 2026-09-23T08:54:39Z

+declare_lint! {
+    /// The `safe_fn_direct_use_of_unsafe_op_on_args` lint detects when unsafe operations
+    /// are trivially used on a function's arguments, and this function is safe and pub.
+    /// which might cause [undefined behavior].
+    pub SAFE_FN_DIRECT_USE_OF_UNSAFE_OP_ON_ARGS,
+    Warn,
+    "detects when a trivial unsafe operation is used on arguments, from a safe fn"
+}


Please always include examples and a more thorough explanation with declare_lint. In particular, what makes something "trivial" could be clarified.

tgross35 · 2026-09-23T08:57:08Z

+    #[allow(safe_fn_direct_use_of_unsafe_op_on_args)]
    pub fn insert_mut(&mut self, index: usize, value: T) -> &mut T {


Always use expect rather than allow unless it's unavoidable, so we don't keep around allows that we no longer need. The reason would be nice too

tgross35 · 2026-09-23T08:59:25Z

+pub fn for_each_expr<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    node: &'tcx hir::Expr<'tcx>,
+    f: impl FnMut(&'tcx hir::Expr<'tcx>),
+) {


Docs please. Could this be pub(crate) or not pub at all?

tgross35 · 2026-09-23T09:00:53Z

+#[derive(Diagnostic)]
+#[diag("call to an unsafe function on an argument of a safe function")]
+pub(crate) struct BuiltinSafeFnDirectUseOfUnsafeOpOnArgs {
+    #[label("this code might cause undefined behavior when executed")]
+    pub label: Span,
+}


This sort of thing could use a help

bjorn3 · 2026-09-23T09:20:36Z

    /// ```
    #[stable(feature = "push_mut", since = "1.95.0")]
    #[must_use = "if you don't need a reference to the value, use `VecDeque::insert` instead"]
+    #[allow(safe_fn_direct_use_of_unsafe_op_on_args)]


Why does this lint trigger here? This function an assertion and early exit to catch the unsafe cases, ensuring that the function is actually safe. I think the lint should only trigger when the safety obligations of the unsafe operation are obviously not discharged by an assertion or early-out. If the standard library already has this many allows, I can't imagine how many false-positives it has for user code.

View changes since the review

rperier · 2026-09-23T10:01:49Z

So this is just checking if any argument is passed to any unsafe function?

Yep, it is.

I think that's going to be far too noisy and a better fit for Clippy, it looks like a lot of false positives.

Maybe there's more of a case for uplifting clippy::not_unsafe_ptr_arg_deref? I think that's more along the lines of what Josh was suggesting at #162090 (comment).

Yeah, this is also what I wanted to discuss in this PR.

Initially, as the issue was still labeled T-compiler, I thought that the lint had to be implemented in the compiler anyway and clippy::not_unsafe_ptr_arg_deref was mentioned more as an example or a reference implementation.

What do you want to do for this lint ? If all of you agree to move this to clippy instead, I can propose another PR for improving the existing lint in clippy, then.

Also there are no tests or examples, this is hard to evaluate.

I totally agree, depending on what we decide here, I am going to fix it.

…ument This adds a new lint for emitting a warning when unsafe ops are directly used on a public safe function's arguments, which might cause undefined behaviors.

rperier · 2026-09-23T11:55:06Z

So this is just checking if any argument is passed to any unsafe function?

We could also reduce the scope of this lint to warn only about direct use of ptr::write, ptr::read, unchecked_* on arguments of types raw pointers and integers (and not any kind of unsafe op on arguments, like it is currently the case).

Also there are no tests or examples, this is hard to evaluate.

I have just added a test for this, which shows most of the supported cases.

rust-log-analyzer · 2026-09-23T12:23:41Z

The job test-pr-check-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

#15 3.194 Building wheels for collected packages: reuse
#15 3.195   Building wheel for reuse (pyproject.toml): started
#15 3.414   Building wheel for reuse (pyproject.toml): finished with status 'done'
#15 3.416   Created wheel for reuse: filename=reuse-4.0.3-cp310-cp310-linux_x86_64.whl size=132855 sha256=78b18759d7868af836e6c653035275b9564ead1e102ae69b3b788a82f243d193
#15 3.416   Stored in directory: /tmp/pip-ephem-wheel-cache-v1x55yp6/wheels/3d/8d/0a/e0fc6aba4494b28a967ab5eaf951c121d9c677958714e34532
#15 3.418 Successfully built reuse
#15 3.419 Installing collected packages: boolean-py, binaryornot, tomlkit, reuse, python-debian, markupsafe, license-expression, jinja2, chardet, attrs
#15 3.822 Successfully installed attrs-23.2.0 binaryornot-0.4.4 boolean-py-4.0 chardet-5.2.0 jinja2-3.1.4 license-expression-30.3.0 markupsafe-2.1.5 python-debian-0.1.49 reuse-4.0.3 tomlkit-0.13.0
#15 3.822 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
#15 DONE 3.9s
---
    |
 49 | / macro_rules! intrinsics {
 50 | |     () => ();
...   |
125 | |         intrinsics!($($rest)*);
    | |         ---------------------- in this macro invocation (#2)
...   |
155 | |                 $name($($argname),*)
    | |                         ^^^^^^^^ this code might cause undefined behavior when executed
...   |
---
   ::: library/compiler-builtins/compiler-builtins/src/int/sdiv.rs:47:1
    |
 47 | /  macro_rules! sdiv {
 48 | |      (
 49 | |          $unsigned_fn:ident, // name of the unsigned division function
 50 | |          $signed_fn:ident, // name of the signed division function
...   |
 55 | |/         intrinsics! {
 56 | ||             $(
 57 | ||                 #[$attr]
 58 | ||             )*
...   ||
 78 | ||         }
    | ||_________- in this macro invocation (#2)
 79 | |      }
 80 | |  }
    | |__- in this expansion of `sdiv!` (#1)
...
193 |    sdiv!(__udivdi3, __divdi3, u64, i64, maybe_use_optimized_c_shim);
    |    ---------------------------------------------------------------- in this macro invocation (#1)

warning: call to an unsafe function on an argument of a safe function
   --> library/compiler-builtins/compiler-builtins/src/macros.rs:155:25
    |
---
   ::: library/compiler-builtins/compiler-builtins/src/int/sdiv.rs:82:1
    |
 82 | /  macro_rules! smod {
 83 | |      (
 84 | |          $unsigned_fn:ident, // name of the unsigned division function
 85 | |          $signed_fn:ident, // name of the signed division function
...   |
 90 | |/         intrinsics! {
 91 | ||             $(
 92 | ||                 #[$attr]
 93 | ||             )*
...   ||
113 | ||         }
    | ||_________- in this macro invocation (#2)
114 | |      }
115 | |  }
    | |__- in this expansion of `smod!` (#1)
...
194 |    smod!(__umoddi3, __moddi3, u64, i64, maybe_use_optimized_c_shim);
    |    ---------------------------------------------------------------- in this macro invocation (#1)

warning: call to an unsafe function on an argument of a safe function
   --> library/compiler-builtins/compiler-builtins/src/macros.rs:155:25
    |
 49 | / macro_rules! intrinsics {
 50 | |     () => ();
...   |
155 | |                 $name($($argname),*)
    | |                         ^^^^^^^^ this code might cause undefined behavior when executed
...   |
167 | |         intrinsics!($($rest)*);
    | |         ---------------------- in this macro invocation (#2)
...   |
440 | |     );
441 | | }
    | | -
    | | |
    | |_in this expansion of `intrinsics!` (#1)
    |   in this expansion of `intrinsics!` (#2)
    |
   ::: library/compiler-builtins/compiler-builtins/src/int/shift.rs:71:1
    |
 71 | / intrinsics! {
 72 | |     #[maybe_use_optimized_c_shim]
 73 | |     pub extern "C" fn __ashlsi3(a: u32, b: u32) -> u32 {
 74 | |         a.ashl(b)
...   |
116 | | }
    | |_- in this macro invocation (#1)

warning: call to an unsafe function on an argument of a safe function
---
...   |
155 | |                 $name($($argname),*)
    | |                         ^^^^^^^^ this code might cause undefined behavior when executed
...   |
167 | |         intrinsics!($($rest)*);
    | |         ----------------------
    | |         |
    | |         in this macro invocation (#2)
    | |         in this macro invocation (#4)
...   |
439 | |         intrinsics!($($rest)*);
    | |         ---------------------- in this macro invocation (#3)
440 | |     );
441 | | }
    | | -
    | | |
    | | in this expansion of `intrinsics!` (#1)
    | | in this expansion of `intrinsics!` (#2)
    | |_in this expansion of `intrinsics!` (#3)
    |   in this expansion of `intrinsics!` (#4)
    |
   ::: library/compiler-builtins/compiler-builtins/src/int/shift.rs:71:1
    |
 71 | / intrinsics! {
 72 | |     #[maybe_use_optimized_c_shim]
 73 | |     pub extern "C" fn __ashlsi3(a: u32, b: u32) -> u32 {
 74 | |         a.ashl(b)
...   |
116 | | }
    | |_- in this macro invocation (#1)

warning: call to an unsafe function on an argument of a safe function
---
...   |
155 | |                 $name($($argname),*)
    | |                         ^^^^^^^^ this code might cause undefined behavior when executed
...   |
167 | |         intrinsics!($($rest)*);
    | |         ----------------------
    | |         |
    | |         in this macro invocation (#2)
    | |         in this macro invocation (#4)
    | |         in this macro invocation (#6)
...   |
439 | |         intrinsics!($($rest)*);
    | |         ----------------------
    | |         |
    | |         in this macro invocation (#3)
    | |         in this macro invocation (#5)
440 | |     );
---
    |   in this expansion of `intrinsics!` (#6)
    |
   ::: library/compiler-builtins/compiler-builtins/src/int/shift.rs:71:1
    |
 71 | / intrinsics! {
 72 | |     #[maybe_use_optimized_c_shim]
 73 | |     pub extern "C" fn __ashlsi3(a: u32, b: u32) -> u32 {
 74 | |         a.ashl(b)
...   |
116 | | }
    | |_- in this macro invocation (#1)

warning: call to an unsafe function on an argument of a safe function
---
    | |_- in this expansion of `intrinsics!`
    |
   ::: library/compiler-builtins/compiler-builtins/src/int/udiv.rs:133:1
    |
133 | / intrinsics! {
134 | |     #[maybe_use_optimized_c_shim]
135 | |     /// Returns `n / d`
136 | |     pub extern "C" fn __udivdi3(n: u64, d: u64) -> u64 {
...   |
199 | | }
    | |_- in this macro invocation

warning: call to an unsafe function on an argument of a safe function
---
...   |
155 | |                 $name($($argname),*)
    | |                         ^^^^^^^^ this code might cause undefined behavior when executed
...   |
167 | |         intrinsics!($($rest)*);
    | |         ---------------------- in this macro invocation (#2)
...   |
440 | |     );
441 | | }
    | | -
    | | |
    | |_in this expansion of `intrinsics!` (#1)
    |   in this expansion of `intrinsics!` (#2)
    |
   ::: library/compiler-builtins/compiler-builtins/src/int/udiv.rs:133:1
    |
133 | / intrinsics! {
134 | |     #[maybe_use_optimized_c_shim]
135 | |     /// Returns `n / d`
136 | |     pub extern "C" fn __udivdi3(n: u64, d: u64) -> u64 {
...   |
199 | | }
    | |_- in this macro invocation (#1)

[RUSTC-TIMING] compiler_builtins test:false 2.800

tgross35 · 2026-09-23T15:01:51Z

The way to get a concrete answer is to nominate for lang, but it feels a bit early for that. Perhaps start with a discussion at #t-lang to figure out an exact proposal?

rust-bors · 2026-09-23T22:32:57Z

☔ The latest upstream changes (presumably #163227) made this pull request unmergeable. Please resolve the merge conflicts by rebasing.

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 23, 2026

rustbot assigned adwinwhite Sep 23, 2026

rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Sep 23, 2026

rperier force-pushed the warn_direct_use_of_unsafe_op_on_args branch from fd65c7b to 2eebb0a Compare September 23, 2026 08:58

tgross35 reviewed Sep 23, 2026

View reviewed changes

bjorn3 reviewed Sep 23, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

Warn in a safe fn when a trivially unsafe operation is used on an arg…

d9299af

…ument This adds a new lint for emitting a warning when unsafe ops are directly used on a public safe function's arguments, which might cause undefined behaviors.

rperier force-pushed the warn_direct_use_of_unsafe_op_on_args branch from 2eebb0a to d9299af Compare September 23, 2026 11:51

		#[allow(safe_fn_direct_use_of_unsafe_op_on_args)]
		pub fn insert_mut(&mut self, index: usize, value: T) -> &mut T {

Uh oh!

Conversation

rperier commented Sep 23, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Sep 23, 2026

Uh oh!

rustbot commented Sep 23, 2026

Uh oh!

tgross35 left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgross35 Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

tgross35 Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

tgross35 Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

tgross35 Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

bjorn3 Sep 23, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

rperier commented Sep 23, 2026

Uh oh!

rperier commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-log-analyzer commented Sep 23, 2026

Uh oh!

tgross35 commented Sep 23, 2026

Uh oh!

rust-bors Bot commented Sep 23, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rperier commented Sep 23, 2026 •

edited by rustbot

Loading

tgross35 left a comment •

edited by rustbot

Loading

bjorn3 Sep 23, 2026 •

edited by rustbot

Loading

rperier commented Sep 23, 2026 •

edited

Loading