CXX-3233 add bsoncxx v1 implementations and tests #1430

eramongodb · 2025-07-28T19:44:42Z

Resolves CXX-3233. Followup to #1412.

This is 3 out of an estimated 7 major PRs which in total are expected to resolve CXX-2745.

Top-Level Dependencies (CXX-3239)
bsoncxx v1 Interfaces (CXX-3232)
bsoncxx v1 Implementations (CXX-3233) (<-- you are here)
bsoncxx v_noabi Refactor (CXX-3234)
mongocxx v1 Interfaces (CXX-3236)
mongocxx v1 Implementations (CXX-3237)
mongocxx v_noabi Refactor (CXX-3238)

This PR provides implementations and tests for the v1 API introduced by CXX-3232 in #1412.

Important

This PR deliberately leaves the v_noabi API interfaces, implementations, and tests unchanged to facilitate direct, side-by-side comparisons between v1 and v_noabi. Redundancy and duplication between v1 and v_noabi will be addressed by the bsoncxx v_noabi refactor (CXX-3234).

In summary, notable changes (relative to the v_noabi API's implementation and tests) include:

Smaller ABI footprint: 136 (unique) exported v1 symbols vs. 250 (approximately equivalent) exported v_noabi symbols.
- Note: v1 is still missing equivalents for builder::*, validate::*, and vector::*, which bring the v_noabi symbol count up to 325.
- The biggest ABI footprint savings come from:
  - Nearly all array::* function defined in terms of document::* except v1::array::view::find(std::uint32_t) const.
  - Defining types::value constructors in terms of b_<type> overloads only.
  - Inlined definitions of comparison operator overloads and many other trivial functions.
"Internal" component headers lib/**/*.hh.
- A pattern that is more common for the mongocxx v_noabi implementations, but unlike mongocxx, v1 keeps them lightweight (definitions in the .cpp to minimize recompilations).
- Internal-only API within internal classes instead of friend declarations to access "private" interfaces or data.
- The public static member functions within internal classes declare the exact set of private data being exposed for use by dependent components (including tests).
- Invocations of specific internal public static member functions explicitly documents-as-code the dependents that require corresponding private data.
An error category is consistently defined as a static local immortal object within the error category function.
- The name of the category is the fully-qualified name of the associated class (e.g. bsoncxx::v1::decimal128) or the error code enumeration otherwise (e.g. bsoncxx::v1::source_errc).
- Every error code defines an equivalent() mapping to the source_errc and type_errc error condition enumerations.
More applications of the new BSONCXX_V1_TYPES_XMACRO X-macro pattern in place of #include <bsoncxx/enums/type.hpp> X-macro headers.
Implementation of the "inline PIMPL" idiom referenced in CXX-3232 (placement new of impl object into reserved data member storage rather than dynamic allocation).
A new internal and undocumented "invalid" state for v1::document::element used to implement the alternative approach to CXX-2120. Note the deliberate lack of any mention of this state in the public API: it is meant to be entirely unobservable except in service of corresponding exception error messages.
"Test" component headers (test/**/*.hh) defining Catch stringification for tested entities.
- Alternative to the include-everything bsoncxx/test/catch.hh header to minimize recompilations.
- Open to alternative design proposals which are not susceptible to ODR violation of StringMaker<T> specializations. 🫠
- bsoncxx::test::stringify() as equivalent to Catch::Detail::Stringify() (deduces T in call to StringMaker<T>::convert()).
- New "stringify" test cases for Catch expression decomposition behavior (implicit "to string" test coverage).
- Catch stringification validated by running tests with the --success flag.
Catch test suite quality improvements.
- Nearly 100% test coverage of bsoncxx v1 API (note: don't expect this for mongocxx...), including copy+move semantics (note: do expect this for mongocxx) to avoid problems like this, this, etc.
  - Especially important for inline PIMPL classes and v1::document::value to ensure correct impl and custom deleter behavior.
  - CHECK_NOFAIL used to document behavior which may vary across standard library implementations (surprising!).
- [namespace][to][component] for better test case filtering.
  - [error] tag for testing error-code-specific behavior (e.g. mapping codes to conditions).
  - [test] tag for test-suite-specific behavior (e.g. Catch stringification).
- All documented exception-throwing code paths are explicitly tested with CHECK_THROWS_WITH_CODE (excluding v1::oid::oid()...).
- Static assertions of important compile-time properties (e.g. overload resolution behavior).
  - Defined in test components instead of library components to avoid impacting library-only build performance.

Note

To avoid v1 test components depending on the v_noabi API (no v1 library or test component includes v_noabi headers), BSON data are hand-written as binary data (e.g. std::uint8_t data[] = {...}) instead of using the bsoncxx::v_noabi::builder::* API. These may eventually be substituted with a v1 builder API, but I think there is also merit to keeping them independent of any builder API, such that the test cases are not dependent on unrelated behavior and minimizes cross-component test behavior dependencies. 🤔

eramongodb · 2025-07-28T19:45:06Z

src/bsoncxx/lib/bsoncxx/v1/array/view.cpp

+    if (!_view) {
+        return this->cend();
+    }


This check is not present in v_noabi). This is a consequence of the new, well-defined "invalid" state of an array view.

eramongodb · 2025-07-28T19:46:30Z

src/bsoncxx/lib/bsoncxx/v1/array/view.cpp

+    if (!bson_init_static(&bson, _view.data(), _view.length())) {
+        throw v1::exception{code::invalid_data};
+    }
+
+    bson_iter_t iter;
+
+    if (!bson_iter_init_find_w_len(&iter, &bson, key.c_str(), static_cast<int>(key.length()))) {
+        return this->end();
+    }


This is slightly different to v_noabi's bson_init_static -> bson_iter_init -> bson_iter_init_find implementation, but should still be equivalent in observable behavior.

bson_iter_find functions may return false if they fail to find the element, but also may return false if they encounter a parse error (and set iter.err_off). Should this also throw invalid_data in that case?

Good catch. Yes, it should throw invalid_data.

src/bsoncxx/lib/bsoncxx/v1/array/view.cpp

eramongodb · 2025-07-28T19:47:19Z

src/bsoncxx/lib/bsoncxx/v1/decimal128.cpp

+    if (sv.empty()) {
+        throw v1::exception{code::empty_string};
+    }
+
+    if (sv.size() > std::size_t{INT_MAX}) {
+        throw v1::exception{code::invalid_string_length};
+    }


These checks are not present in v_noabi.

eramongodb · 2025-07-28T19:47:48Z

src/bsoncxx/lib/bsoncxx/v1/document/view.cpp


+namespace {
+
+constexpr std::uint8_t k_default_view[5] = {5u, 0u, 0u, 0u, 0u};


This is the special empty document representation that is used to avoid v1::document::value allocation (with a no-op deleter).

Minor: Can this be a std::array<u8, 5>? (clang-tidy nags about arrays)

eramongodb · 2025-07-28T19:57:43Z

src/bsoncxx/lib/bsoncxx/v1/oid.cpp

+    if (!bytes) {
+        throw v1::exception{code::null_bytes_ptr};
+    }


This check is not present in v_noabi, which may lead to library-level undefined behavior when invoking memcpy().

eramongodb · 2025-07-28T19:58:22Z

src/bsoncxx/lib/bsoncxx/v1/types/value.cpp

+// For backward compatibility, do not prematurely truncate strings in bsoncxx API. Instead, defer handling of potential
+// embedded null bytes to the bson library.


This comment documents an observation that v_noabi's make_copy_for_libbson (which in hindsight should really be moved out of private and into v_noabi along with a few more current private headers) uses memcpy rather than str(n)cpy to create owning BSON strings. The v1 implementation preserves this behavior, avoiding truncation and deferring null byte validation to the bson library.

eramongodb · 2025-07-28T19:59:01Z

src/bsoncxx/lib/bsoncxx/v1/types/value.cpp

+#pragma pop_macro("X")
+}
+
+// BSONCXX_V1_TYPES_XMACRO: update below.


The overloads below implement the behavior currently handled by v_noabi's convert_to_libbson() in convert.hh, ~~but without exposing bson_value_t to dependent components~~.

eramongodb · 2025-07-28T19:59:15Z

src/bsoncxx/lib/bsoncxx/v1/types/value.cpp

+    if (v.value.size() > UINT32_MAX) {
+        throw v1::exception{code::invalid_length_u32};
+    }


These UINT32_MAX checks are not in v_noabi and guard the subsequent casts to std::uint32_t.

eramongodb · 2025-07-28T19:59:31Z

src/bsoncxx/lib/bsoncxx/v1/types/view.cpp

+
+} // namespace
+
+view view::internal::make(bson_value_t const& v) {


This implements the behavior currently handled by v_noabi's convert_from_libbson() in convert.hh, ~~but without exposing bson_value_t to dependent components~~.

src/bsoncxx/lib/bsoncxx/v1/element/view.cpp

kevinAlbs · 2025-07-31T20:13:13Z

src/bsoncxx/lib/bsoncxx/v1/element/view.hh

+    static v1::stdx::optional<bson_iter_t> to_bson_iter(view const& v);
+
+   private:
+    friend view;


Consider moving the private static methods of element::view::internal to element::view::impl:

// In bsoncxx::v1::element::view::impl: static view::impl const& to_impl(view const& self) { return *reinterpret_cast<view::impl const*>(self._storage.data()); } static view::impl const* to_impl(view const* self) { return reinterpret_cast<view::impl const*>(self->_storage.data()); } static view::impl* to_impl(view* self) { return reinterpret_cast<view::impl*>(self->_storage.data()); }

That appears to allow removing the friend declaration. IIUC the internal classes are intended as a controlled internal view into private API from other components. In this case, no other components need to (or can) call these private static methods.

Comment similarly applies to types::value::internal.

src/bsoncxx/test/v1/array/value.cpp

src/bsoncxx/test/v1/types/value.cpp

vector-of-bool

Mostly looks good, but leaving open for a behavioral question regarding iteration exception-throwing

src/bsoncxx/lib/bsoncxx/v1/array/view.cpp

vector-of-bool · 2025-08-13T15:06:13Z

src/bsoncxx/lib/bsoncxx/v1/array/view.cpp

+    if (!bson_init_static(&bson, _view.data(), _view.length())) {
+        throw v1::exception{code::invalid_data};
+    }
+
+    bson_iter_t iter;
+
+    if (!bson_iter_init_find_w_len(&iter, &bson, key.c_str(), static_cast<int>(key.length()))) {
+        return this->end();
+    }


bson_iter_find functions may return false if they fail to find the element, but also may return false if they encounter a parse error (and set iter.err_off). Should this also throw invalid_data in that case?

vector-of-bool · 2025-08-13T15:08:06Z

src/bsoncxx/lib/bsoncxx/v1/document/view.cpp


+namespace {
+
+constexpr std::uint8_t k_default_view[5] = {5u, 0u, 0u, 0u, 0u};


Minor: Can this be a std::array<u8, 5>? (clang-tidy nags about arrays)

src/bsoncxx/lib/bsoncxx/v1/document/view.cpp

src/bsoncxx/lib/bsoncxx/v1/element/view.cpp

src/bsoncxx/test/v1/types/id.cpp

src/bsoncxx/test/v1/types/value.cpp

src/bsoncxx/test/v1/types/view.cpp

src/bsoncxx/lib/bsoncxx/v1/exception.cpp

src/bsoncxx/test/v1/oid.cpp

vector-of-bool

LGTM

eramongodb added 2 commits July 28, 2025 14:40

v1: impls

9fc87c6

v1: tests

b034251

eramongodb self-assigned this Jul 28, 2025

eramongodb commented Jul 28, 2025

View reviewed changes

eramongodb marked this pull request as ready for review July 28, 2025 20:00

eramongodb requested a review from a team as a code owner July 28, 2025 20:00

eramongodb requested review from kevinAlbs, mdb-ad and vector-of-bool and removed request for mdb-ad July 28, 2025 20:00

Merge remote-tracking branch 'upstream/master' into cxx-abi-v1-impls

e51551b

kevinAlbs reviewed Aug 6, 2025

View reviewed changes

eramongodb added 7 commits August 6, 2025 10:47

Add some basic find() and subscript assertions

f258f76

Fix doc comments for BSON array data

25354e4

Do not violate precondition in our own test cases

b2a4393

Assert resulting document length in invalid_length assertions

1bc5302

Remove unused values array

877cd50

Fix view vs. value in btype_vs_value static assertions

9e3d254

Move T -> T::impl conversion functions into T::impl

2cfc2d7

eramongodb requested a review from kevinAlbs August 6, 2025 16:01

ClangFormat

8de4ee1

kevinAlbs approved these changes Aug 6, 2025

View reviewed changes

Merge remote-tracking branch 'upstream/master' into cxx-abi-v1-impls

a3fd459

vector-of-bool reviewed Aug 13, 2025

View reviewed changes

eramongodb added 6 commits August 14, 2025 10:51

Merge remote-tracking branch 'upstream/master' into HEAD

1a94bf7

Distinguish "missing" vs. "invalid" after calling bson_iter_*

2b76d2c

Use std::array instead of raw arrays

4c6b735

BSON binary subtype values 128-255 are "user defined"

81fa5c0

Minor mem-list-init improvement

22c67c2

Make WSAGuard Windows-only and static local

4258c04

eramongodb added 3 commits August 14, 2025 10:51

Use bsoncxx::make_unique_for_overwrite

116890e

Fix and cleanup include directives

3de63ff

Use common format for unknown error codes

e5c7fa9

eramongodb requested a review from vector-of-bool August 14, 2025 16:20

vector-of-bool approved these changes Aug 14, 2025

View reviewed changes

eramongodb merged commit 54f58f4 into mongodb:master Aug 14, 2025
20 of 22 checks passed

eramongodb deleted the cxx-abi-v1-impls branch August 14, 2025 18:02

This was referenced Aug 15, 2025

Partially revert "CXX-2120 handle nonexistent document fields (#984)" #1445

Merged

CXX-3234 refactor bsoncxx::v_noabi to use bsoncxx::v1 #1447

Merged

CXX-3233 use common format for unknown error codes #1449

Merged

eramongodb mentioned this pull request Nov 14, 2025

Reuse bsoncxx::v1 out-of-line definitions of Catch::StringMaker<T>::convert() #1500

Merged


		namespace {

		constexpr std::uint8_t k_default_view[5] = {5u, 0u, 0u, 0u, 0u};

		// For backward compatibility, do not prematurely truncate strings in bsoncxx API. Instead, defer handling of potential
		// embedded null bytes to the bson library.


		} // namespace

		view view::internal::make(bson_value_t const& v) {

CXX-3233 add bsoncxx v1 implementations and tests #1430

CXX-3233 add bsoncxx v1 implementations and tests #1430

Uh oh!

Conversation

eramongodb commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eramongodb Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eramongodb Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vector-of-bool left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vector-of-bool left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eramongodb commented Jul 28, 2025 •

edited

Loading

eramongodb Jul 28, 2025 •

edited

Loading

eramongodb Jul 28, 2025 •

edited

Loading