Skip to content

Remove ::core::mem:: path to size_of#318

Merged
folkertdev merged 2 commits into
trifectatechfoundation:mainfrom
Rawk:size_of-without-path
Jun 2, 2026
Merged

Remove ::core::mem:: path to size_of#318
folkertdev merged 2 commits into
trifectatechfoundation:mainfrom
Rawk:size_of-without-path

Conversation

@Rawk
Copy link
Copy Markdown
Contributor

@Rawk Rawk commented Jun 2, 2026

size_of is in the Rust prelude, so use that instead.

@folkertdev
Copy link
Copy Markdown
Member

This is kind of hard to review manually, can you instead just make a commit with the result of these steps?

find . -type f -name '*.rs' -exec sed -i 's/::core::mem::size_of/size_of/g' {} +
find . -type f -name '*.rs' -exec sed -i 's/core::mem::size_of/size_of/g' {} +
cargo clippy --fix --allow-dirty
cargo fmt

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 2, 2026

Codecov Report

❌ Patch coverage is 35.89109% with 259 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
lib/compress/zstd_compress.rs 46.42% 75 Missing ⚠️
lib/compress/zstdmt_compress.rs 0.00% 26 Missing ⚠️
programs/benchzstd.rs 0.00% 24 Missing ⚠️
programs/fileio.rs 0.00% 24 Missing ⚠️
programs/zstdcli.rs 0.00% 17 Missing ⚠️
programs/util.rs 0.00% 14 Missing ⚠️
lib/legacy/zstd_v06.rs 56.66% 13 Missing ⚠️
lib/compress/zstd_opt.rs 0.00% 10 Missing ⚠️
lib/decompress/zstd_decompress.rs 16.66% 10 Missing ⚠️
lib/compress/zstd_ldm.rs 11.11% 8 Missing ⚠️
... and 16 more
Flag Coverage Δ
test-aarch64-apple-darwin 33.14% <33.25%> (+0.05%) ⬆️
test-aarch64-unknown-linux-gnu 31.64% <32.75%> (-0.01%) ⬇️
test-i686-unknown-linux-gnu 31.70% <32.75%> (-0.02%) ⬇️
test-x86_64-unknown-linux-gnu 32.92% <35.73%> (-0.87%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
lib/common/entropy_common.rs 94.11% <ø> (-0.54%) ⬇️
lib/common/fse_decompress.rs 95.33% <100.00%> (+0.68%) ⬆️
lib/common/huf.rs 0.00% <ø> (ø)
lib/compress/fse_compress.rs 75.82% <100.00%> (ø)
lib/compress/zstd_compress_internal.rs 62.07% <100.00%> (-0.12%) ⬇️
lib/decompress/huf_decompress.rs 78.58% <100.00%> (ø)
lib/dictBuilder/fastcover.rs 76.22% <100.00%> (-2.64%) ⬇️
lib/dictBuilder/zdict.rs 81.42% <100.00%> (ø)
lib/compress/huf_compress.rs 82.00% <66.66%> (ø)
lib/compress/zstd_compress_literals.rs 70.56% <66.66%> (ø)
... and 24 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Rawk Rawk force-pushed the size_of-without-path branch from caf0dfa to 1e8bdbb Compare June 2, 2026 11:24
@Rawk
Copy link
Copy Markdown
Contributor Author

Rawk commented Jun 2, 2026

This is kind of hard to review manually, can you instead just make a commit with the result of these steps?

That is basically what i did. :-) Except that i also removed some redundant parentheses.

It is now split into two commit, the first from your exact commands, and the second with removed redundant parentheses. I hope it makes it easier to review.

Comment thread lib/legacy/zstd_v05.rs
if (FSEv05_MAX_TABLELOG * 2 + 7) as size_t
> (::core::mem::size_of::<size_t>()).wrapping_mul(8)
{
if (FSEv05_MAX_TABLELOG * 2 + 7) as size_t > size_of::<size_t>().wrapping_mul(8) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a followup PR you could also

Suggested change
if (FSEv05_MAX_TABLELOG * 2 + 7) as size_t > size_of::<size_t>().wrapping_mul(8) {
if (FSEv05_MAX_TABLELOG * 2 + 7) > usize::BITS {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tricky part here is the type conversion which might make this hard to review too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. clippy::manual_bits catches this (disabled by default).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it does catch it with wrapping_mul though, otherwise you could run

RUSTFLAGS="-Aclippy::all -D clippy::manual_bits" cargo clippy --fix --allow-dirty

but that doesn't do anything

@folkertdev folkertdev merged commit 17b10c3 into trifectatechfoundation:main Jun 2, 2026
21 checks passed
@folkertdev folkertdev mentioned this pull request Jun 2, 2026
@Rawk Rawk deleted the size_of-without-path branch June 2, 2026 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants