From faeafea8743ab9069b78c8f6fd403714488fcfa6 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Mon, 18 Mar 2024 11:21:17 -0400 Subject: [PATCH] Use macro for wrapping += and -= The current auto-generated += and -= implementation is hard to read, and should be replaced with += where possible. That said, we cannot auto-replace it just yet because Rust behaves differently in debug mode, therefore we should use second best approach - a macro that clearly shows intention without all the boilerplate code. The only manual code are two macros in the src/enc/mod.rs Use this replacement file as described in other recent PRs to replace boilerplate code.
replacement file content ```diff @@ expression cond, expr; @@ if cond { - let _rhs = 1; - let _lhs = &mut expr; - *_lhs = (*_lhs).wrapping_add(_rhs as u32); +::wrapping_add!(expr, 1); } @@ expression expr; @@ -{ - let _rhs = 1; - let _lhs = &mut expr; - *_lhs = (*_lhs).wrapping_add(_rhs as u32); +::wrapping_add!(expr, 1); -} @@ expression expr; @@ -{ - let _rhs = 1u32; - let _lhs = &mut expr; - *_lhs = (*_lhs).wrapping_add(_rhs); +::wrapping_add!(expr, 1); -} @@ expression expr; @@ -{ - let _rhs = 1i32; - let _lhs = &mut expr; - *_lhs = (*_lhs).wrapping_add(_rhs as u32); +::wrapping_add!(expr, 1); -} @@ expression expr; @@ -{ - let _rhs = 1; - let _lhs = &mut expr; - *_lhs = (*_lhs).wrapping_add(_rhs as usize); +::wrapping_add!(expr, 1); -} @@ expression inc, expr; @@ -{ - let _rhs = inc; - let _lhs = &mut expr; - *_lhs = (*_lhs).wrapping_add(_rhs as u32); +::wrapping_add!(expr, inc as u32); -} @@ expression inc, expr; @@ -{ - let _rhs = inc; - let _lhs = &mut expr; - *_lhs = (*_lhs).wrapping_add(_rhs); +::wrapping_add!(expr, inc); -} @@ expression inc, expr; @@ -{ - let _rhs = inc; - let _lhs = &mut expr; - *_lhs = (*_lhs).wrapping_add(_rhs as u32); +::wrapping_add!(expr, inc as u32); -} @@ expression inc, expr; @@ -{ - let _rhs = inc; - let _lhs = &mut expr; - *_lhs = (*_lhs).wrapping_add(_rhs as usize); +::wrapping_add!(expr, inc as usize); -} @@ expression expr; @@ -{ - let _rhs = 1; - let _lhs = &mut expr; - *_lhs = (*_lhs).wrapping_sub(_rhs as usize); +::wrapping_sub!(expr, 1); -} ```
--- src/enc/block_splitter.rs | 6 +----- src/enc/cluster.rs | 9 ++++----- src/enc/encode.rs | 18 +++--------------- src/enc/metablock.rs | 9 ++++----- src/enc/mod.rs | 18 ++++++++++++++++++ 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/enc/block_splitter.rs b/src/enc/block_splitter.rs index ffdfa5f7..991aac72 100755 --- a/src/enc/block_splitter.rs +++ b/src/enc/block_splitter.rs @@ -508,11 +508,7 @@ fn ClusterBlocks< while i < length { { 0i32; - { - let _rhs = 1; - let _lhs = &mut block_lengths.slice_mut()[block_idx]; - *_lhs = (*_lhs).wrapping_add(_rhs as u32); - } + ::wrapping_add!(block_lengths.slice_mut()[block_idx], 1); if i.wrapping_add(1) == length || block_ids[i] as i32 != block_ids[i.wrapping_add(1)] as i32 { diff --git a/src/enc/cluster.rs b/src/enc/cluster.rs index 432f3098..eca925c3 100755 --- a/src/enc/cluster.rs +++ b/src/enc/cluster.rs @@ -181,11 +181,10 @@ pub fn BrotliHistogramCombine< let best_idx2: u32 = (pairs[0]).idx2; HistogramSelfAddHistogram(out, (best_idx1 as usize), (best_idx2 as usize)); (out[(best_idx1 as usize)]).set_bit_cost((pairs[0]).cost_combo); - { - let _rhs = cluster_size[(best_idx2 as usize)]; - let _lhs = &mut cluster_size[(best_idx1 as usize)]; - *_lhs = (*_lhs).wrapping_add(_rhs); - } + ::wrapping_add!( + cluster_size[(best_idx1 as usize)], + cluster_size[(best_idx2 as usize)] + ); for i in 0usize..symbols_size { if symbols[i] == best_idx2 { symbols[i] = best_idx1; diff --git a/src/enc/encode.rs b/src/enc/encode.rs index 44b1a789..88d82d7d 100755 --- a/src/enc/encode.rs +++ b/src/enc/encode.rs @@ -1397,11 +1397,7 @@ fn ShouldCompress( i = 0usize; while i < t { { - { - let _rhs = 1; - let _lhs = &mut literal_histo[data[(pos as usize & mask)] as usize]; - *_lhs = (*_lhs).wrapping_add(_rhs as u32); - } + ::wrapping_add!(literal_histo[data[(pos as usize & mask)] as usize], 1); pos = pos.wrapping_add(kSampleRate); } i = i.wrapping_add(1); @@ -1795,16 +1791,8 @@ fn ChooseContextMap( i = 0usize; while i < 9usize { { - { - let _rhs = bigram_histo[i]; - let _lhs = &mut monogram_histo[i.wrapping_rem(3)]; - *_lhs = (*_lhs).wrapping_add(_rhs); - } - { - let _rhs = bigram_histo[i]; - let _lhs = &mut two_prefix_histo[i.wrapping_rem(6)]; - *_lhs = (*_lhs).wrapping_add(_rhs); - } + ::wrapping_add!(monogram_histo[i.wrapping_rem(3)], bigram_histo[i]); + ::wrapping_add!(two_prefix_histo[i.wrapping_rem(6)], bigram_histo[i]); } i = i.wrapping_add(1); } diff --git a/src/enc/metablock.rs b/src/enc/metablock.rs index 95986e10..c2f2f2c7 100755 --- a/src/enc/metablock.rs +++ b/src/enc/metablock.rs @@ -640,11 +640,10 @@ fn BlockSplitterFinishBlock< xself.merge_last_count_ = 0usize; xself.target_block_size_ = xself.min_block_size_; } else { - { - let _rhs = xself.block_size_ as u32; - let _lhs = &mut split.lengths.slice_mut()[xself.num_blocks_.wrapping_sub(1)]; - *_lhs = (*_lhs).wrapping_add(_rhs); - } + ::wrapping_add!( + split.lengths.slice_mut()[xself.num_blocks_.wrapping_sub(1)], + xself.block_size_ as u32 + ); histograms[xself.last_histogram_ix_[0]] = combined_histo[0].clone(); xself.last_entropy_[0] = combined_entropy[0]; if split.num_types == 1 { diff --git a/src/enc/mod.rs b/src/enc/mod.rs index a4fe0d0f..1d0d639c 100755 --- a/src/enc/mod.rs +++ b/src/enc/mod.rs @@ -347,3 +347,21 @@ where read_err?; Ok(total_out.unwrap()) } + +#[macro_export] +macro_rules! wrapping_add { + ($expr:expr, $increment:expr) => {{ + let _rhs = $increment; + let _lhs = &mut $expr; + *_lhs = (*_lhs).wrapping_add(_rhs); + }}; +} + +#[macro_export] +macro_rules! wrapping_sub { + ($expr:expr, $increment:expr) => {{ + let _rhs = $increment; + let _lhs = &mut $expr; + *_lhs = (*_lhs).wrapping_sub(_rhs); + }}; +}