-
Notifications
You must be signed in to change notification settings - Fork 547
CXX-3234 refactor bsoncxx::v_noabi to use bsoncxx::v1 #1447
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 15 commits
df2c385
c9ee565
5e8e749
bcf655c
7ed8a60
4b5bc26
d6da139
ecb298a
b5ebba8
225bff9
d076881
aba1dca
5372020
ab3256a
ee1043b
e385adc
baef8b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,10 +14,15 @@ | |
|
||
#pragma once | ||
|
||
#include <bsoncxx/array/element-fwd.hpp> | ||
|
||
// | ||
|
||
#include <bsoncxx/v1/element/view.hpp> | ||
|
||
#include <cstddef> | ||
#include <cstdint> | ||
|
||
#include <bsoncxx/array/element-fwd.hpp> | ||
#include <bsoncxx/array/view-fwd.hpp> | ||
#include <bsoncxx/types/bson_value/view-fwd.hpp> | ||
|
||
|
@@ -36,50 +41,41 @@ namespace array { | |
/// interrogated by calling type() and a specific value can be extracted through | ||
/// get_X() accessors. | ||
/// | ||
class element : private document::element { | ||
class element : private v_noabi::document::element { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For backward compatibility, the existing structure where " As far as I can tell, CXX-1830 can be resolved per its current description: there is no difference between the API for |
||
public: | ||
BSONCXX_ABI_EXPORT_CDECL() element(); | ||
|
||
using document::element::operator bool; | ||
|
||
using document::element::type; | ||
|
||
using document::element::get_array; | ||
using document::element::get_binary; | ||
using document::element::get_bool; | ||
using document::element::get_code; | ||
using document::element::get_codewscope; | ||
using document::element::get_date; | ||
using document::element::get_dbpointer; | ||
using document::element::get_decimal128; | ||
using document::element::get_document; | ||
using document::element::get_double; | ||
using document::element::get_int32; | ||
using document::element::get_int64; | ||
using document::element::get_maxkey; | ||
using document::element::get_minkey; | ||
using document::element::get_null; | ||
using document::element::get_oid; | ||
using document::element::get_regex; | ||
using document::element::get_string; | ||
using document::element::get_symbol; | ||
using document::element::get_timestamp; | ||
using document::element::get_undefined; | ||
|
||
using document::element::get_value; | ||
|
||
using document::element::operator[]; | ||
|
||
using document::element::key; | ||
using document::element::keylen; | ||
using document::element::length; | ||
using document::element::offset; | ||
using document::element::raw; | ||
|
||
private: | ||
friend ::bsoncxx::v_noabi::array::view; | ||
|
||
explicit element(std::uint8_t const* raw, std::uint32_t length, std::uint32_t offset, std::uint32_t keylen); | ||
element() : v_noabi::document::element() {} | ||
|
||
/* explicit(false) */ element(v1::element::view const& v) : v_noabi::document::element{v} {} | ||
|
||
using v_noabi::document::element::operator v1::element::view; | ||
|
||
using v_noabi::document::element::operator bool; | ||
|
||
using v_noabi::document::element::raw; | ||
|
||
using v_noabi::document::element::length; | ||
|
||
using v_noabi::document::element::offset; | ||
|
||
using v_noabi::document::element::keylen; | ||
|
||
using v_noabi::document::element::type; | ||
|
||
using v_noabi::document::element::key; | ||
|
||
#pragma push_macro("X") | ||
#undef X | ||
#define X(_name, _value) using v_noabi::document::element::get_##_name; | ||
BSONCXX_V1_TYPES_XMACRO(X) | ||
#pragma pop_macro("X") | ||
|
||
using v_noabi::document::element::get_value; | ||
|
||
using v_noabi::document::element::get_owning_value; | ||
|
||
using v_noabi::document::element::operator[]; | ||
|
||
friend bool operator==(element const& lhs, v_noabi::types::bson_value::view const& rhs); | ||
}; | ||
|
||
/// | ||
|
@@ -90,16 +86,24 @@ class element : private document::element { | |
/// @{ | ||
|
||
/// @relatesalso bsoncxx::v_noabi::array::element | ||
BSONCXX_ABI_EXPORT_CDECL(bool) operator==(element const& elem, types::bson_value::view const& v); | ||
inline bool operator==(element const& lhs, v_noabi::types::bson_value::view const& rhs) { | ||
return static_cast<v_noabi::document::element const&>(lhs) == rhs; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Take advantage of (private) inheritance to defer implementation to |
||
} | ||
|
||
/// @relatesalso bsoncxx::v_noabi::array::element | ||
BSONCXX_ABI_EXPORT_CDECL(bool) operator==(types::bson_value::view const& v, element const& elem); | ||
inline bool operator==(v_noabi::types::bson_value::view const& lhs, element const& rhs) { | ||
return rhs == lhs; | ||
} | ||
|
||
/// @relatesalso bsoncxx::v_noabi::array::element | ||
BSONCXX_ABI_EXPORT_CDECL(bool) operator!=(element const& elem, types::bson_value::view const& v); | ||
inline bool operator!=(element const& lhs, v_noabi::types::bson_value::view const& rhs) { | ||
return !(lhs == rhs); | ||
} | ||
|
||
/// @relatesalso bsoncxx::v_noabi::array::element | ||
BSONCXX_ABI_EXPORT_CDECL(bool) operator!=(types::bson_value::view const& v, element const& elem); | ||
inline bool operator!=(v_noabi::types::bson_value::view const& lhs, element const& rhs) { | ||
return !(lhs == rhs); | ||
} | ||
|
||
/// @} | ||
/// | ||
|
@@ -108,11 +112,29 @@ BSONCXX_ABI_EXPORT_CDECL(bool) operator!=(types::bson_value::view const& v, elem | |
} // namespace v_noabi | ||
} // namespace bsoncxx | ||
|
||
namespace bsoncxx { | ||
namespace v_noabi { | ||
|
||
// Ambiguous whether `v1::element::view` should be converted to `v_noabi::array::element` or | ||
// `v_noabi::document::element.` Require users to explicitly cast to the expected type instead. | ||
// | ||
// v_noabi::array::element from_v1(v1::element::view const& v); | ||
|
||
/// | ||
/// Convert to the @ref bsoncxx::v1 equivalent of `v`. | ||
/// | ||
inline v1::element::view to_v1(v_noabi::array::element const& v) { | ||
return v1::element::view{v}; | ||
} | ||
|
||
} // namespace v_noabi | ||
} // namespace bsoncxx | ||
|
||
namespace bsoncxx { | ||
namespace array { | ||
|
||
using ::bsoncxx::v_noabi::array::operator==; | ||
using ::bsoncxx::v_noabi::array::operator!=; | ||
using v_noabi::array::operator==; | ||
using v_noabi::array::operator!=; | ||
|
||
} // namespace array | ||
} // namespace bsoncxx | ||
|
@@ -123,3 +145,7 @@ using ::bsoncxx::v_noabi::array::operator!=; | |
/// @file | ||
/// Provides @ref bsoncxx::v_noabi::array::element. | ||
/// | ||
/// @par Includes | ||
/// - @ref bsoncxx/document/element.hpp | ||
/// - @ref bsoncxx/v1/element/view.hpp | ||
/// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: rename to
zeroinit_type
? This value-initiailizes the_bytes
to all zeroes.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The zero-initialization is meant to be an implementation detail that avoids language-level undefined behavior (in the spirit of
-ftrivial-auto-var-init=zero
and to avoid triggering static analysis warnings for uninitialized data members), but it's still meant to be library-level undefined behavior to access_bytes
after initializing withuninit_type
, since the elements are expected to be unconditionally overwritten byv_noabi::oid::oid()
anyways. I expect/hope the compiler should be able to see this unconditional write-without-read and optimize accordingly (ignoring exception paths):There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought, to avoid the "uninit" confusion, adopted the
*_for_overwrite
suffix fromstd::make_unique_for_overwrite
, which hopefully makes the intention much clearer.