Skip to content

Commit

Permalink
Use macro for wrapping += and -=
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
nyurik committed May 11, 2024
1 parent dac157a commit c318d7c
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 30 deletions.
6 changes: 1 addition & 5 deletions src/enc/block_splitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,11 +448,7 @@ fn ClusterBlocks<
i = 0usize;
while i < length {
{
{
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
{
Expand Down
9 changes: 4 additions & 5 deletions src/enc/cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,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;
Expand Down
18 changes: 3 additions & 15 deletions src/enc/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1309,11 +1309,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);
Expand Down Expand Up @@ -1699,16 +1695,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);
}
Expand Down
9 changes: 4 additions & 5 deletions src/enc/metablock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,11 +638,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 {
Expand Down
18 changes: 18 additions & 0 deletions src/enc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,3 +341,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);
}};
}

0 comments on commit c318d7c

Please sign in to comment.