crc32c icon indicating copy to clipboard operation
crc32c copied to clipboard

fix(build.rs): fix MaybeUninit usage & uninitialized integers (UB)

Open nissaofthesea opened this issue 2 years ago • 0 comments

fixes #32

i changed Matrix to have zero-initialized integers because it was used in so many methods, and would make things more convoluted to write correctly that way. i believe this is fine because Matrix is only constructed once (in create_zero_operator), so cost of initializing 128 bytes to zero should be entirely negligible (especially for a build script).

on that note, it may be simpler to just zero-initialize everything. i can only really see benefits to tightly optimizing initialization if it's /very/ hot code, but this is a build script, and we would be at most zero-initializing 8KiB.

(copy pasted from commit message)

recap & questions

  • should the build script try to optimize array initialization (by initializing lazily w/ MaybeUninit, like this pr implements)?
    • it would be much simpler to zero-initialize, with very little perfomance loss
    • this is a build script, i don't think such a performance loss is critical
  • would it be preferrable for CrcTable to have a const ctor/generator instead of a build script generator?

nissaofthesea avatar Oct 07 '22 09:10 nissaofthesea