criterion.rs icon indicating copy to clipboard operation
criterion.rs copied to clipboard

Recommendation for `black_box`'ing expressions that evaluate to `()`?

Open cmichi opened this issue 3 years ago • 1 comments

We are black_box'ing a number of expressions that evaluate to a unit literal. Clippy complains for all these and we are wondering if there is a best-practice criterion recommendation for getting rid of those warnings while still having the expressions black-boxed?

error: passing a unit value to a function                                                                                                                      
  --> ./bench_lazy.rs:80:9                                                                                                                
   |                                                                                                                                                           
80 |         black_box(*lazy = 13);                                                                                                                            
   |         ^^^^^^^^^^^^^^^^^^^^^                                                                                                                             
   |                                                                                                                                                           
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg                                                    
help: move the expression in front of the call and replace it with the unit literal `()`                                                                       
   |                                                                                                                                                           
80 |         *lazy = 13;                                                                                                                                       
81 |         black_box(());   

cmichi avatar Aug 01 '21 14:08 cmichi

black_box doesn't do what you think it does. In your code you have:

let _ = black_box(|| copy += 1u64);

But that doesn't make sense. It is identical to this code:

let _ = black_box(|| panic!("This closures is never evaluated."));

Generally, you should almost never use black_box. In your case, I think you want this:

b.iter(|| {
             let mut copy = key;
             copy += 1u64;
             copy
         })

lemmih avatar Aug 09 '21 06:08 lemmih