Skip to content

Commit b515add

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 c35494b commit b515add

File tree

5 files changed

+29
-25
lines changed

5 files changed

+29
-25
lines changed

src/enc/block_splitter.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -448,11 +448,7 @@ fn ClusterBlocks<
448448
i = 0usize;
449449
while i < length {
450450
{
451-
{
452-
let _rhs = 1;
453-
let _lhs = &mut block_lengths.slice_mut()[block_idx];
454-
*_lhs = (*_lhs).wrapping_add(_rhs as u32);
455-
}
451+
::wrapping_add!(block_lengths.slice_mut()[block_idx], 1);
456452
if i.wrapping_add(1) == length
457453
|| block_ids[i] as i32 != block_ids[i.wrapping_add(1)] as i32
458454
{

src/enc/cluster.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -171,11 +171,10 @@ pub fn BrotliHistogramCombine<
171171
let best_idx2: u32 = (pairs[0]).idx2;
172172
HistogramSelfAddHistogram(out, (best_idx1 as usize), (best_idx2 as usize));
173173
(out[(best_idx1 as usize)]).set_bit_cost((pairs[0]).cost_combo);
174-
{
175-
let _rhs = cluster_size[(best_idx2 as usize)];
176-
let _lhs = &mut cluster_size[(best_idx1 as usize)];
177-
*_lhs = (*_lhs).wrapping_add(_rhs);
178-
}
174+
::wrapping_add!(
175+
cluster_size[(best_idx1 as usize)],
176+
cluster_size[(best_idx2 as usize)]
177+
);
179178
for i in 0usize..symbols_size {
180179
if symbols[i] == best_idx2 {
181180
symbols[i] = best_idx1;

src/enc/encode.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1737,16 +1737,8 @@ fn ChooseContextMap(
17371737
i = 0usize;
17381738
while i < 9usize {
17391739
{
1740-
{
1741-
let _rhs = bigram_histo[i];
1742-
let _lhs = &mut monogram_histo[i.wrapping_rem(3)];
1743-
*_lhs = (*_lhs).wrapping_add(_rhs);
1744-
}
1745-
{
1746-
let _rhs = bigram_histo[i];
1747-
let _lhs = &mut two_prefix_histo[i.wrapping_rem(6)];
1748-
*_lhs = (*_lhs).wrapping_add(_rhs);
1749-
}
1740+
::wrapping_add!(monogram_histo[i.wrapping_rem(3)], bigram_histo[i]);
1741+
::wrapping_add!(two_prefix_histo[i.wrapping_rem(6)], bigram_histo[i]);
17501742
}
17511743
i = i.wrapping_add(1);
17521744
}

src/enc/metablock.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -630,11 +630,10 @@ fn BlockSplitterFinishBlock<
630630
xself.merge_last_count_ = 0usize;
631631
xself.target_block_size_ = xself.min_block_size_;
632632
} else {
633-
{
634-
let _rhs = xself.block_size_ as u32;
635-
let _lhs = &mut split.lengths.slice_mut()[xself.num_blocks_.wrapping_sub(1)];
636-
*_lhs = (*_lhs).wrapping_add(_rhs);
637-
}
633+
::wrapping_add!(
634+
split.lengths.slice_mut()[xself.num_blocks_.wrapping_sub(1)],
635+
xself.block_size_ as u32
636+
);
638637
histograms[xself.last_histogram_ix_[0]] = combined_histo[0].clone();
639638
xself.last_entropy_[0] = combined_entropy[0];
640639
if split.num_types == 1 {

src/enc/mod.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,3 +342,21 @@ where
342342
read_err?;
343343
Ok(total_out.unwrap())
344344
}
345+
346+
#[macro_export]
347+
macro_rules! wrapping_add {
348+
($expr:expr, $increment:expr) => {{
349+
let _rhs = $increment;
350+
let _lhs = &mut $expr;
351+
*_lhs = (*_lhs).wrapping_add(_rhs);
352+
}};
353+
}
354+
355+
#[macro_export]
356+
macro_rules! wrapping_sub {
357+
($expr:expr, $increment:expr) => {{
358+
let _rhs = $increment;
359+
let _lhs = &mut $expr;
360+
*_lhs = (*_lhs).wrapping_sub(_rhs);
361+
}};
362+
}

0 commit comments

Comments
 (0)