CXX-1169 Minor refactoring

[koraa]

Initially I intended to remove the explicit clearing of the stack in the builder::core _impl destructor, but when I did that the tests had a memory corruption.

I am not sure why this is, possibly a double free maybe? (Maybe the stack needs to be cleared before bson_destroy?)

[xdg]

This has been assigned CXX-1169 for tracking.

[jrassi]

Apologies for the delay in responding! Thanks so much for submitting this.

I can explain why you were seeing memory corruption when you attempted to remove the logic to clear _stack in the core::impl destructor. This was causing _root to be destroyed before _stack (since the core::impl destructor body run before the core::impl member destructors), which is invalid: the destructor for the frame at the bottom of _stack dereferences its bson_t parent pointer, which in this case would point to the already destroyed bson_t _root.

The existing core::impl logic is messy/subtle and your commit improves on it, but I think we can do even better and remove all of the core::impl destruction logic, by changing the declaration order of the core::impl members to properly express the members' dependencies. This requires wrapping the bson_t member in an RAII class mongocxx::libbson::scoped_bson_t (which is currently private to mongocxx, so we can't use it here).

I'm going to merge your "Prefix the CITER macro with BSONCXX_" and "Avoid false positives in the clang static analyzer" commits as-is, and take the above alternative approach to your core::impl fixes.

Thanks again!

[koraa]

sounds good!

[jrassi]

Merged "Avoid false positives in the clang static analyzer" in 7bacc73, and merged "Prefix the CITER macro with BSONCXX_" in d642e29. I've also merged in the above-mentioned refactor of core::impl in cc73e18.

Thanks again. :)