search

gpakosz / PackedArray

Random access array of tightly packed unsigned integers
Other
160 stars 32 forks source link

A question about __PackedArray_pack_ code #4

Open NanXiao opened 5 years ago

NanXiao commented 5 years ago

Hi @gpakosz ,

Greeting from me!

I am reading PackedArray's code, and come across the following statement: 

packed |= *out & ~((uint32_t)(1ULL << ((((uint64_t)count * (uint64_t)PACKEDARRAY_IMPL_BITS_PER_ITEM + startBit - 1) % 32) + 1)) - 1);

Why not just use: 

packed |= *out & ~((uint32_t)(1ULL << (((uint64_t)count * (uint64_t)PACKEDARRAY_IMPL_BITS_PER_ITEM + startBit) % 32)) - 1)

What is the difference between these two methods?

Thanks very much in advance!

Bets Regards Nan Xiao

gpakosz commented 5 years ago

Hello @NanXiao 👋

Right off the bat, I don't remember why it's written this way. If you replace the existing code by your suggestion, do all the tests still pass?

NanXiao commented 5 years ago

@gpakosz

Thanks very much for your quick response!

I modify the code, and at least PackedArraySelfTest passes the tests.

When you have time, could you double-check?

Thanks!

gpakosz commented 5 years ago

@NanXiao

I had a second quick look. I can't remember why it's written this way but your suggestion looks good to me. I may have goofed the night I wrote the original code 🤷‍♂️