Skip to content

Commit faeafea

Browse files
committed
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. <details> <summary>replacement file content</summary> ```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); -} ``` </details>
1 parent 6fae4d3 commit faeafea

File tree

5 files changed

+30
-30
lines changed

5 files changed

+30
-30
lines changed

src/enc/block_splitter.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -508,11 +508,7 @@ fn ClusterBlocks<
508508
while i < length {
509509
{
510510
0i32;
511-
{
512-
let _rhs = 1;
513-
let _lhs = &mut block_lengths.slice_mut()[block_idx];
514-
*_lhs = (*_lhs).wrapping_add(_rhs as u32);
515-
}
511+
::wrapping_add!(block_lengths.slice_mut()[block_idx], 1);
516512
if i.wrapping_add(1) == length
517513
|| block_ids[i] as i32 != block_ids[i.wrapping_add(1)] as i32
518514
{

src/enc/cluster.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,10 @@ pub fn BrotliHistogramCombine<
181181
let best_idx2: u32 = (pairs[0]).idx2;
182182
HistogramSelfAddHistogram(out, (best_idx1 as usize), (best_idx2 as usize));
183183
(out[(best_idx1 as usize)]).set_bit_cost((pairs[0]).cost_combo);
184-
{
185-
let _rhs = cluster_size[(best_idx2 as usize)];
186-
let _lhs = &mut cluster_size[(best_idx1 as usize)];
187-
*_lhs = (*_lhs).wrapping_add(_rhs);
188-
}
184+
::wrapping_add!(
185+
cluster_size[(best_idx1 as usize)],
186+
cluster_size[(best_idx2 as usize)]
187+
);
189188
for i in 0usize..symbols_size {
190189
if symbols[i] == best_idx2 {
191190
symbols[i] = best_idx1;

src/enc/encode.rs

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1397,11 +1397,7 @@ fn ShouldCompress(
13971397
i = 0usize;
13981398
while i < t {
13991399
{
1400-
{
1401-
let _rhs = 1;
1402-
let _lhs = &mut literal_histo[data[(pos as usize & mask)] as usize];
1403-
*_lhs = (*_lhs).wrapping_add(_rhs as u32);
1404-
}
1400+
::wrapping_add!(literal_histo[data[(pos as usize & mask)] as usize], 1);
14051401
pos = pos.wrapping_add(kSampleRate);
14061402
}
14071403
i = i.wrapping_add(1);
@@ -1795,16 +1791,8 @@ fn ChooseContextMap(
17951791
i = 0usize;
17961792
while i < 9usize {
17971793
{
1798-
{
1799-
let _rhs = bigram_histo[i];
1800-
let _lhs = &mut monogram_histo[i.wrapping_rem(3)];
1801-
*_lhs = (*_lhs).wrapping_add(_rhs);
1802-
}
1803-
{
1804-
let _rhs = bigram_histo[i];
1805-
let _lhs = &mut two_prefix_histo[i.wrapping_rem(6)];
1806-
*_lhs = (*_lhs).wrapping_add(_rhs);
1807-
}
1794+
::wrapping_add!(monogram_histo[i.wrapping_rem(3)], bigram_histo[i]);
1795+
::wrapping_add!(two_prefix_histo[i.wrapping_rem(6)], bigram_histo[i]);
18081796
}
18091797
i = i.wrapping_add(1);
18101798
}

src/enc/metablock.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -640,11 +640,10 @@ fn BlockSplitterFinishBlock<
640640
xself.merge_last_count_ = 0usize;
641641
xself.target_block_size_ = xself.min_block_size_;
642642
} else {
643-
{
644-
let _rhs = xself.block_size_ as u32;
645-
let _lhs = &mut split.lengths.slice_mut()[xself.num_blocks_.wrapping_sub(1)];
646-
*_lhs = (*_lhs).wrapping_add(_rhs);
647-
}
643+
::wrapping_add!(
644+
split.lengths.slice_mut()[xself.num_blocks_.wrapping_sub(1)],
645+
xself.block_size_ as u32
646+
);
648647
histograms[xself.last_histogram_ix_[0]] = combined_histo[0].clone();
649648
xself.last_entropy_[0] = combined_entropy[0];
650649
if split.num_types == 1 {

src/enc/mod.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,3 +347,21 @@ where
347347
read_err?;
348348
Ok(total_out.unwrap())
349349
}
350+
351+
#[macro_export]
352+
macro_rules! wrapping_add {
353+
($expr:expr, $increment:expr) => {{
354+
let _rhs = $increment;
355+
let _lhs = &mut $expr;
356+
*_lhs = (*_lhs).wrapping_add(_rhs);
357+
}};
358+
}
359+
360+
#[macro_export]
361+
macro_rules! wrapping_sub {
362+
($expr:expr, $increment:expr) => {{
363+
let _rhs = $increment;
364+
let _lhs = &mut $expr;
365+
*_lhs = (*_lhs).wrapping_sub(_rhs);
366+
}};
367+
}

0 commit comments

Comments
 (0)