Skip to content

CXX-2109 Deprecate all references to utf8 #744

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 14, 2020
Merged

CXX-2109 Deprecate all references to utf8 #744

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a9c728c
CXX-2109 Deprecate all references to utf8
bazile-clyde Nov 9, 2020
312345f
Merge branch 'master' into CXX-2109
bazile-clyde Nov 13, 2020
52fc441
CXX-2109 Deprecate all references to utf8
bazile-clyde Nov 9, 2020
53a45e7
Merge branch 'CXX-2109' of github.com:bazile-clyde/mongo-cxx-driver i…
bazile-clyde Nov 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/content/mongocxx-v3/tutorial.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ value is a string:

```c++
bsoncxx::document::element element = view["name"];
if(element.type() != bsoncxx::type::k_utf8) {
if(element.type() != bsoncxx::type::k_string) {
// Error
}
std::string name = element.get_string().value.to_string();
Expand Down
2 changes: 1 addition & 1 deletion examples/bsoncxx/builder_basic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ int main(int, char**) {
using bsoncxx::builder::basic::kvp;

doc.append(
kvp("foo", "bar")); // string literal value will be converted to b_utf8 automatically
kvp("foo", "bar")); // string literal value will be converted to b_string automatically
doc.append(kvp("baz", types::b_bool{false}));
doc.append(kvp("garply", types::b_double{3.14159}));

Expand Down
6 changes: 3 additions & 3 deletions examples/bsoncxx/builder_core.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ int main(int, char**) {
// Just call append() and pass a value wrapped in the corresponding BSON type.
auto array = builder::core{true}; // we are building an array

array.append(types::b_utf8{"hello"}); // append a UTF-8 string
array.append(types::b_double{1.234}); // append a double
array.append(types::b_int32{1234}); // append an int32
array.append(types::b_string{"hello"}); // append a UTF-8 string
array.append(types::b_double{1.234}); // append a double
array.append(types::b_int32{1234}); // append an int32
// ... etc.
}
2 changes: 1 addition & 1 deletion examples/bsoncxx/view_and_value.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ int main(int, char**) {

// we can use type() to get the type of the value.
switch (ele.type()) {
case type::k_utf8:
case type::k_string:
std::cout << "Got String!" << std::endl;
break;
case type::k_oid:
Expand Down
2 changes: 1 addition & 1 deletion examples/mongocxx/get_values_from_documents.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ void iterate_messagelist(const bsoncxx::document::element& ele) {
if (status && status.type() == type::k_int32) {
std::cout << "status: " << status.get_int32().value << std::endl;
}
if (msg && msg.type() == type::k_utf8) {
if (msg && msg.type() == type::k_string) {
std::cout << "msg: " << msg.get_string().value << std::endl;
}
} else {
Expand Down
11 changes: 4 additions & 7 deletions src/bsoncxx/builder/core.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,15 +288,15 @@ core& core::append(const types::b_double& value) {
return *this;
}

core& core::append(const types::b_utf8& value) {
core& core::append(const types::b_string& value) {
stdx::string_view key = _impl->next_key();

if (!bson_append_utf8(_impl->back(),
key.data(),
static_cast<std::int32_t>(key.length()),
value.value.data(),
static_cast<std::int32_t>(value.value.length()))) {
throw bsoncxx::exception{error_code::k_cannot_append_utf8};
throw bsoncxx::exception{error_code::k_cannot_append_string};
}

return *this;
Expand Down Expand Up @@ -545,13 +545,13 @@ core& core::append(const types::b_maxkey&) {
}

core& core::append(std::string str) {
append(types::b_utf8{std::move(str)});
append(types::b_string{std::move(str)});

return *this;
}

core& core::append(stdx::string_view str) {
append(types::b_utf8{std::move(str)});
append(types::b_string{std::move(str)});

return *this;
}
Expand Down Expand Up @@ -647,15 +647,12 @@ core& core::concatenate(const bsoncxx::document::view& view) {

core& core::append(const bsoncxx::types::bson_value::view& value) {
switch (static_cast<int>(value.type())) {
// CXX-1817; deprecation warning suppressed for get_utf8()
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_BEGIN
#define BSONCXX_ENUM(type, val) \
case val: \
append(value.get_##type()); \
break;
#include <bsoncxx/enums/type.hpp>
#undef BSONCXX_ENUM
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_END
}

return *this;
Expand Down
6 changes: 3 additions & 3 deletions src/bsoncxx/builder/core.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,9 @@ class BSONCXX_API core {
/// @throws
/// bsoncxx::exception if the current BSON datum is a document that is waiting for a key to be
/// appended to start a new key/value pair.
/// bsoncxx::exception if the utf8 fails to append.
/// bsoncxx::exception if the string fails to append.
///
core& append(const types::b_utf8& value);
core& append(const types::b_string& value);

///
/// Appends a BSON document.
Expand Down Expand Up @@ -510,7 +510,7 @@ class BSONCXX_API core {
BSONCXX_INLINE core& append(T* v) {
static_assert(std::is_same<typename std::remove_const<T>::type, char>::value,
"append is disabled for non-char pointer types");
append(types::b_utf8{v});
append(types::b_string{v});

return *this;
}
Expand Down
8 changes: 1 addition & 7 deletions src/bsoncxx/document/element.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,32 +78,26 @@ stdx::string_view element::key() const {
return stdx::string_view{key};
}

// CXX-1817; deprecation warning suppressed for get_utf8()
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_BEGIN
#define BSONCXX_ENUM(name, val) \
types::b_##name element::get_##name() const { \
types::bson_value::view v{_raw, _length, _offset, _keylen}; \
return v.get_##name(); \
}
#include <bsoncxx/enums/type.hpp>
#undef BSONCXX_ENUM
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_END

types::b_utf8 element::get_string() const {
types::b_string element::get_utf8() const {
types::bson_value::view v{_raw, _length, _offset, _keylen};
return v.get_string();
}

types::bson_value::view element::get_value() const {
switch (static_cast<int>(type())) {
// CXX-1817; deprecation warning suppressed for get_utf8()
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_BEGIN
#define BSONCXX_ENUM(type, val) \
case val: \
return types::bson_value::view{get_##type()};
#include <bsoncxx/enums/type.hpp>
#undef BSONCXX_ENUM
BSONCXX_SUPPRESS_DEPRECATION_WARNINGS_END
}

BSONCXX_UNREACHABLE;
Expand Down
15 changes: 7 additions & 8 deletions src/bsoncxx/document/element.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ enum class binary_sub_type : std::uint8_t;
namespace types {
struct b_eod;
struct b_double;
struct b_utf8;
struct b_string;
struct b_document;
struct b_array;
struct b_binary;
Expand Down Expand Up @@ -145,25 +145,24 @@ class BSONCXX_API element {
types::b_double get_double() const;

///
/// Getter for elements of the b_utf8 type.
/// Getter for elements of the b_string type.
///
/// @deprecated use document::element::get_string() instead.
///
/// @throws bsoncxx::exception if this element is not a b_utf8.
/// @throws bsoncxx::exception if this element is not a b_string.
///
/// @return the element's value.
///
BSONCXX_DEPRECATED types::b_utf8 get_utf8() const;
BSONCXX_DEPRECATED types::b_string get_utf8() const;

///
/// Getter for elements of the b_utf8, or string, type. This function acts as a
/// wrapper for get_utf8().
/// Getter for elements of the b_string type.
///
/// @throws bsoncxx::exception if this element is not a b_utf8.
/// @throws bsoncxx::exception if this element is not a b_string.
///
/// @return the element's value.
///
types::b_utf8 get_string() const;
types::b_string get_string() const;

///
/// Getter for elements of the b_document type.
Expand Down
2 changes: 1 addition & 1 deletion src/bsoncxx/enums/type.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
#endif

BSONCXX_ENUM(double, 0x01)
BSONCXX_ENUM(utf8, 0x02)
BSONCXX_ENUM(string, 0x02)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this renaming causes some API removal.

grepping through all .hpp files for #include <bsoncxx/enums/type.hpp>, I think these would be removed.

  1. type::k_utf8 is defined in src/bsoncxx/types.hpp.
  2. error_code::k_need_element_type_k_utf8 and error_code::k_cannot_append_utf8 are defined. These are integer error codes which I don't believe we can change.

Both cases are enum values.

For those cases, we may need to redefine each at the same integer value.

Ideally we can deprecate the old utf8 value. There may not be a completely portable way of doing so, but #pragma deprecated would at least deprecate for compilers that recognize the pragma. Ideally, users are informed by a compiler warning. But at the very least, we can give them notice via release notes.

Here's what I'm thinking.

///
/// An enumeration of each BSON type.
/// These x-macros will expand to be of the form:
///    k_double = 0x01,
///    k_string = 0x02,
///    k_document = 0x03,
///    k_array = 0x04 ...
///
enum class type : std::uint8_t {
#define BSONCXX_ENUM(name, val) k_##name = val,
#include <bsoncxx/enums/type.hpp>
#undef BSONCXX_ENUM
    k_utf8 = 0x02
};

#pragma deprecated(k_utf8)

I think this could be a motivation for doing CXX-641 before the next release as well. That way we can double check ABI changes before we release. I moved that to "Needs Triage".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I added k_utf8 and deprecated it using BSONCXX_DEPRECATED. For the error codes, I added them back to error_code.hpp. They're at the end of the enum to avoid changing other error code values. I set them to the _string variants since they share the same values. I did not add them to the switch statements in error_code.cpp. You can't add a new case for the same values.

Copy link
Contributor Author

@bazile-clyde bazile-clyde Nov 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it appears that the ability deprecated enum values didn't appear until GCC-6 (source):

The C and C++ compilers now support attributes on enumerators. For instance, it is now possible to mark enumerators as deprecated:

enum {
  newval,
  oldval __attribute__ ((deprecated ("too old")))
};

The code snippet above (#pragma deprecated(k_utf8)) doesn't even compile locally so I'm not sure if it's possible to deprecate this.

Local compilers:

➜  g++ --version
g++ (Ubuntu 10.2.0-13ubuntu1) 10.2.0
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

➜  clang --version   
Ubuntu clang version 11.0.0-2
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might just have to leave this and error code as is until the next major release. @samantharitter any ideas?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into it. If there's isn't a straightforward way to deprecate it, then I agree we'll have to leave it for now. We can note the deprecation in release notes.

I verified #pragma deprecated will warn on compilers that don't recognize it: https://godbolt.org/z/xPhvoe so I agree that's not a viable solution unless we #ifdef it with compilers we know support the pragma.

BSONCXX_ENUM(document, 0x03)
BSONCXX_ENUM(array, 0x04)
BSONCXX_ENUM(binary, 0x05)
Expand Down
2 changes: 2 additions & 0 deletions src/bsoncxx/exception/error_code.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ enum class error_code : std::int32_t {
#define BSONCXX_ENUM(name, value) k_cannot_append_##name,
#include <bsoncxx/enums/type.hpp>
#undef BSONCXX_ENUM
k_cannot_append_utf8 = k_cannot_append_string,
k_need_element_type_k_utf8 = k_need_element_type_k_string,
// Add new constant string message to error_code.cpp as well!
};

Expand Down
14 changes: 7 additions & 7 deletions src/bsoncxx/test/bson_builder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ void bson_eq_object(const bson_t* bson, const T actual) {
REQUIRE(std::memcmp(expected.data(), actual.data(), expected.length()) == 0);
}

TEST_CASE("builder appends utf8", "[bsoncxx::builder::stream]") {
TEST_CASE("builder appends string", "[bsoncxx::builder::stream]") {
bson_t expected;
bson_init(&expected);

Expand All @@ -83,8 +83,8 @@ TEST_CASE("builder appends utf8", "[bsoncxx::builder::stream]") {
bson_eq_stream(&expected, b);
}

SECTION("works with b_utf8") {
b << "hello" << types::b_utf8{"world"};
SECTION("works with b_string") {
b << "hello" << types::b_string{"world"};

bson_eq_stream(&expected, b);
}
Expand Down Expand Up @@ -822,7 +822,7 @@ TEST_CASE("core method chaining to build array works", "[bsoncxx::builder::core]
auto array_view = array.view();

REQUIRE(std::distance(array_view.begin(), array_view.end()) == 3);
REQUIRE(array_view[0].type() == type::k_utf8);
REQUIRE(array_view[0].type() == type::k_string);
REQUIRE(string::to_string(array_view[0].get_string().value) == "foo");
REQUIRE(array_view[1].type() == type::k_int32);
REQUIRE(array_view[1].get_int32().value == 1);
Expand Down Expand Up @@ -1207,7 +1207,7 @@ TEST_CASE("builder::basic::make_array works", "[bsoncxx::builder::basic::make_ar
auto array_view = array.view();

REQUIRE(std::distance(array_view.begin(), array_view.end()) == 3);
REQUIRE(array_view[0].type() == type::k_utf8);
REQUIRE(array_view[0].type() == type::k_string);
REQUIRE(string::to_string(array_view[0].get_string().value) == "foo");
REQUIRE(array_view[1].type() == type::k_int32);
REQUIRE(array_view[1].get_int32().value == 1);
Expand Down Expand Up @@ -1263,8 +1263,8 @@ TEST_CASE("list builder appends utf8", "[bsoncxx::builder::list]") {
bson_eq_object(&expected, b.view().get_document().value);
}

SECTION("works with b_utf8") {
builder::list b{"hello", types::b_utf8{"world"}};
SECTION("works with b_string") {
builder::list b{"hello", types::b_string{"world"}};

bson_eq_object(&expected, b.view().get_document().value);
}
Expand Down
14 changes: 7 additions & 7 deletions src/bsoncxx/test/bson_types.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ TEST_CASE("b_double", "[bsoncxx::type::b_double]") {
REQUIRE(!(a == c));
}

TEST_CASE("b_utf8", "[bsoncxx::type::b_utf8]") {
b_utf8 a{"hello"};
b_utf8 b{a};
b_utf8 c{"world"};
TEST_CASE("b_string", "[bsoncxx::type::b_string]") {
b_string a{"hello"};
b_string b{a};
b_string c{"world"};
REQUIRE(a == b);
REQUIRE(!(a == c));
}
Expand Down Expand Up @@ -196,9 +196,9 @@ TEST_CASE("bson_value::view with b_double", "[bsoncxx::types::bson_value::view]"
REQUIRE(bson_value::view{double_val}.get_double() == double_val);
}

TEST_CASE("bson_value::view with b_utf8", "[bsoncxx::types::bson_value::view]") {
b_utf8 utf8_val{"hello"};
REQUIRE(bson_value::view{utf8_val}.get_string() == utf8_val);
TEST_CASE("bson_value::view with b_string", "[bsoncxx::types::bson_value::view]") {
b_string string_val{"hello"};
REQUIRE(bson_value::view{string_val}.get_string() == string_val);
}

TEST_CASE("bson_value::view with b_document", "[bsoncxx::types::bson_value::view]") {
Expand Down
4 changes: 2 additions & 2 deletions src/bsoncxx/test/bson_value.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,15 @@ TEST_CASE("types::bson_value::value", "[bsoncxx::types::bson_value::value]") {
coverting_construction_test(b_bool{true}, test_value);
}

SECTION("utf8") {
SECTION("string") {
std::string value = "super duper";

auto test_doc = bson_value::make_value(value);
value_construction_test(test_doc.view());

coverting_construction_test(value, test_doc);
coverting_construction_test(value.c_str(), test_doc);
coverting_construction_test(b_utf8{value}, test_doc);
coverting_construction_test(b_string{value}, test_doc);
coverting_construction_test(stdx::string_view{value}, test_doc);

const char raw_data[]{'s', 'u', 'p', 'e', 'r', ' ', 'd', 'u', 'p', 'e', 'r'};
Expand Down
26 changes: 17 additions & 9 deletions src/bsoncxx/types.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,15 @@ BSONCXX_INLINE_NAMESPACE_BEGIN
/// An enumeration of each BSON type.
/// These x-macros will expand to be of the form:
/// k_double = 0x01,
/// k_utf8 = 0x02,
/// k_string = 0x02,
/// k_document = 0x03,
/// k_array = 0x04 ...
///
enum class type : std::uint8_t {
#define BSONCXX_ENUM(name, val) k_##name = val,
#include <bsoncxx/enums/type.hpp>
#undef BSONCXX_ENUM
k_utf8 = 0x02,
};

///
Expand Down Expand Up @@ -110,19 +111,19 @@ BSONCXX_INLINE bool operator==(const b_double& lhs, const b_double& rhs) {
///
/// A BSON UTF-8 encoded string value.
///
struct BSONCXX_API b_utf8 {
static constexpr auto type_id = type::k_utf8;
struct BSONCXX_API b_string {
static constexpr auto type_id = type::k_string;

///
/// Constructor for b_utf8.
/// Constructor for b_string.
///
/// @param t
/// The value to wrap.
///
template <typename T,
typename std::enable_if<!std::is_same<b_utf8, typename std::decay<T>::type>::value,
typename std::enable_if<!std::is_same<b_string, typename std::decay<T>::type>::value,
int>::type = 0>
BSONCXX_INLINE explicit b_utf8(T&& t) : value(std::forward<T>(t)) {}
BSONCXX_INLINE explicit b_string(T&& t) : value(std::forward<T>(t)) {}

stdx::string_view value;

Expand All @@ -135,14 +136,21 @@ struct BSONCXX_API b_utf8 {
};

///
/// free function comparator for b_utf8
/// free function comparator for b_string
///
/// @relatesalso b_utf8
/// @relatesalso b_string
///
BSONCXX_INLINE bool operator==(const b_utf8& lhs, const b_utf8& rhs) {
BSONCXX_INLINE bool operator==(const b_string& lhs, const b_string& rhs) {
return lhs.value == rhs.value;
}

///
/// This class has been renamed to b_string
///
/// @deprecated use b_string instead.
///
BSONCXX_DEPRECATED typedef b_string b_utf8;

///
/// A BSON document value.
///
Expand Down
2 changes: 1 addition & 1 deletion src/bsoncxx/types/bson_value/value.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ value::value(int64_t v) : _impl{stdx::make_unique<impl>()} {

value::value(const char* v) : value(stdx::string_view{v}) {}
value::value(std::string v) : value(stdx::string_view{v}) {}
value::value(b_utf8 v) : value(v.value) {}
value::value(b_string v) : value(v.value) {}
value::value(stdx::string_view v) : _impl{stdx::make_unique<impl>()} {
_impl->_value.value_type = BSON_TYPE_UTF8;
_impl->_value.value.v_utf8.str = make_copy_for_libbson(v);
Expand Down
2 changes: 1 addition & 1 deletion src/bsoncxx/types/bson_value/value.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class BSONCXX_API value {
///
/// These x-macros will expand to:
/// value(b_double v);
/// value(b_utf8 v);
/// value(b_string v);
/// value(b_document v);
/// value(b_array v); ...
///
Expand Down
Loading