bson_atomic_int32_fetch_add uses a header declared type for the int32_t argument

[Downchuck]

bson_atomic_int32_fetch_add is generated with the type DECL_ATOMIC_INTEGRAL_INT32, but is coded as int32_t in two places. This results in an incompatible pointer types error when MSVC is true from the bson-atomic.h file.

This patch resolves the error, tested and resolved with clang-cl for compatibility.

[vector-of-bool]

After looking at the problem and the code in question, I'm thinking there may be a better solution pending more refactoring, but I think this is sufficient to keep things working for now. Thank you for your contribution!

I'll let CI run its checks, then it should be good to merge.

[vector-of-bool]

It looks like the macro is not defined on many compilation environments. This may require further refactoring. I will investigate some more possible changes to the API.

[Downchuck]

Yeah I'd just pop it in around line 375 of bson atomic around that else statement, but I'm worried about that emulate 32 but statement.

Should I update the PR to do it so we can at least see how many more builds pass?

[vector-of-bool]

Sure, give it a try.

[Downchuck]

I was so uncomfortable with this tweak, I made a typo in the commit message. I agree there's good reason to do a little refactoring. Hopefully the failed checks give a little more signal.

[Downchuck]

@vector-of-bool - checking in - looks like it needs an approve to run the autotests.

[vector-of-bool]

Sorry for the delay. Been quite distracted.

Taking a quick look, I suspect you meant to define to "int32_t", instead of "int32"? If you can confirm/fix then I'll get this running again.

[Downchuck]

@vector-of-bool I'm really not sure: there's that note of "Other compilers that we support provide generic intrinsics" above, which uses int32 - though it also has that emulate flag. The fall-through statements in this section seem to suggest that's where it'll land for these other compilers (which I haven't used).

int32_t certainly makes more sense at first glance, it's just the surrounding code I took a hint from.

[vector-of-bool]

In those other contexts, the int32 is token-pasted to form other identifiers, including int32_t, so you probably want int32_t here.

[Downchuck]

Seemed a mostly OK build check.

[vector-of-bool]

This looks like to be a safe solution for the time being. I'll probably refactor the atomic.h header in the future to make things cleaner. Thank you for finding this issue! Apologies for the long delay, though.

[Downchuck]

@vector-of-bool You have an idea when this libbson update will go out in a release zip? Hoping to change a cmake file to point to it.