Repository navigation
Clippy gives unit_arg warning when using black_box #12707
Description
Summary
Consider the following program:
use std::{hint, thread, time};
fn main() {
hint::black_box(thread::sleep(time::Duration::from_millis(10)));
}The black_box() call is used to request that the function call is not removed. In this case, sleep() does not return anything. Even so, in order to give the black_box hint, it must be passed to the function.
The suggestion is to remove the black box, which is not an option.
Lint Name
unit_arg
Reproducer
I tried this code:
use std::{hint, thread, time};
fn main() {
hint::black_box(thread::sleep(time::Duration::from_millis(10)));
}I saw this happen:
Checking playground v0.0.1 (/playground)
warning: passing a unit value to a function
--> src/main.rs:3:5
|
3 | hint::black_box(thread::sleep(time::Duration::from_millis(10)));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg
= note: `#[warn(clippy::unit_arg)]` on by default
help: move the expression in front of the call and replace it with the unit literal `()`
|
3 ~ thread::sleep(time::Duration::from_millis(10));
4 ~ hint::black_box(());
|
I expected to see this happen:
No warning, because the point of black_box is to be a hint.
Version
0.1.79 (2024-04-22 7f2fc33)
From https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=dbf98b9c0525acfe5280671a77fdf304
Additional Labels
No response
Activity
Alexendoo commentedon Apr 24, 2024
black_boxapplies to the value you pass into it, I don't think applying it to a unit would achieve anything significant as there's only one possible value of unitWhat is the intended effect of the
black_boxhere?xobs commentedon Apr 25, 2024
I'm not sure that
black_boxapplies to the value you pass into it.The intended effect is for the function to not be optimized out or inlined. In this case, it's obvious that
thread::sleep()won't get optmized out, but there may be cases where the compiler will decide to optimize or otherwise inline a call to a function.For example, in the documentation, if
black_boxonly applied to the value, then it would only apply to aboolrather than the functioncontains: https://doc.rust-lang.org/std/hint/fn.black_box.html#when-is-this-usefulThe warning won't fire if you apply
black_boxto a function that returns a value, but it will fire if you try to black_box a function that returns no value.y21 commentedon Apr 25, 2024
std::hint::black_boxis an ordinary function, so it's treated like any other function. The argument expression is still evaluated normally and optimized as per the usual rules.black_box(42 * 42)is still constant-folded toblack_box(1764), so it does not prevent optimizations on its own in that regard.Only the actual value produced by the expression in the function call is assumed to be used in arbitrary ways.
To my knowledge,
black_boxhas no effect on expressions producing ZSTs (e.g.()), because zero-sized types carry no (runtime) information that needs to evaluate at runtime.The example in the documentation is different:
black_box(contains(...))means "theboolvalue can be observed and might be shown to the user, so you cannot optimize out thisbool". It does not say anything aboutcontains-- it is still free to optimize out thecontains()call if it could and simply end up withblack_box(true). The other nestedblack_boxes ensure that this doesn't happen.For
thread::sleepthis argument can't be made because no computation is needed to create a(). The compiler can make up()out of thin air. In practice, I don't thinksleepwill ever be optimized out, but for a different reason, theblack_boxdoes not change that.So, IMO this lint is correct, even for
black_box.xobs commentedon Apr 25, 2024
You're right, I appear to have a fundamental misunderstanding of the
black_boxdocumentation, and the lint here is, in fact, correct.Closing, and perhaps this ticket's presence will help someone else in the future should they run into the same problem.
Rollup merge of rust-lang#133942 - BD103:black-box-docs, r=saethlin
Rollup merge of rust-lang#133942 - BD103:black-box-docs, r=saethlin
Rollup merge of rust-lang#133942 - BD103:black-box-docs, r=saethlin
Rollup merge of rust-lang#133942 - BD103:black-box-docs, r=saethlin
Rollup merge of rust-lang#133942 - BD103:black-box-docs, r=saethlin
Rollup merge of rust-lang#133942 - BD103:black-box-docs, r=saethlin
Rollup merge of rust-lang#133942 - BD103:black-box-docs, r=saethlin
Rollup merge of rust-lang#133942 - BD103:black-box-docs, r=saethlin
Rollup merge of rust-lang#133942 - BD103:black-box-docs, r=saethlin
Unrolled build for rust-lang#133942
Rollup merge of #133942 - BD103:black-box-docs, r=saethlin
Rollup merge of rust-lang#133942 - BD103:black-box-docs, r=saethlin