Skip to content

Expand std::string_view support to str, bytes, memoryview #3521

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

Merged
merged 7 commits into from
Dec 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 0 additions & 17 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,6 @@
#include <utility>
#include <vector>

#if defined(PYBIND11_CPP17)
# if defined(__has_include)
# if __has_include(<string_view>)
# define PYBIND11_HAS_STRING_VIEW
# endif
# elif defined(_MSC_VER)
# define PYBIND11_HAS_STRING_VIEW
# endif
#endif
#ifdef PYBIND11_HAS_STRING_VIEW
#include <string_view>
#endif

#if defined(__cpp_lib_char8_t) && __cpp_lib_char8_t >= 201811L
# define PYBIND11_HAS_U8STRING
#endif

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
PYBIND11_NAMESPACE_BEGIN(detail)

Expand Down
15 changes: 15 additions & 0 deletions include/pybind11/detail/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,21 @@
# define PYBIND11_HAS_VARIANT 1
#endif

#if defined(PYBIND11_CPP17)
# if defined(__has_include)
# if __has_include(<string_view>)
# define PYBIND11_HAS_STRING_VIEW
# endif
# elif defined(_MSC_VER)
# define PYBIND11_HAS_STRING_VIEW
# endif
#endif

#if defined(__cpp_lib_char8_t) && __cpp_lib_char8_t >= 201811L
# define PYBIND11_HAS_U8STRING
#endif


#include <Python.h>
#include <frameobject.h>
#include <pythread.h>
Expand Down
45 changes: 45 additions & 0 deletions include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
# include <optional>
#endif

#ifdef PYBIND11_HAS_STRING_VIEW
# include <string_view>
#endif

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)

/* A few forward declarations */
Expand Down Expand Up @@ -1085,6 +1089,20 @@ class str : public object {
// NOLINTNEXTLINE(google-explicit-constructor)
str(const std::string &s) : str(s.data(), s.size()) { }

#ifdef PYBIND11_HAS_STRING_VIEW
// enable_if is needed to avoid "ambiguous conversion" errors (see PR #3521).
template <typename T, detail::enable_if_t<std::is_same<T, std::string_view>::value, int> = 0>
// NOLINTNEXTLINE(google-explicit-constructor)
str(T s) : str(s.data(), s.size()) { }

# ifdef PYBIND11_HAS_U8STRING
// reinterpret_cast here is safe (C++20 guarantees char8_t has the same size/alignment as char)
// NOLINTNEXTLINE(google-explicit-constructor)
str(std::u8string_view s) : str(reinterpret_cast<const char*>(s.data()), s.size()) { }
# endif

#endif

explicit str(const bytes &b);

/** \rst
Expand Down Expand Up @@ -1167,6 +1185,26 @@ class bytes : public object {
pybind11_fail("Unable to extract bytes contents!");
return std::string(buffer, (size_t) length);
}

#ifdef PYBIND11_HAS_STRING_VIEW
// enable_if is needed to avoid "ambiguous conversion" errors (see PR #3521).
template <typename T, detail::enable_if_t<std::is_same<T, std::string_view>::value, int> = 0>
// NOLINTNEXTLINE(google-explicit-constructor)
bytes(T s) : bytes(s.data(), s.size()) { }

// Obtain a string view that views the current `bytes` buffer value. Note that this is only
// valid so long as the `bytes` instance remains alive and so generally should not outlive the
// lifetime of the `bytes` instance.
// NOLINTNEXTLINE(google-explicit-constructor)
operator std::string_view() const {
char *buffer = nullptr;
ssize_t length = 0;
if (PYBIND11_BYTES_AS_STRING_AND_SIZE(m_ptr, &buffer, &length))
pybind11_fail("Unable to extract bytes contents!");
return {buffer, static_cast<size_t>(length)};
}
#endif

};
// Note: breathe >= 4.17.0 will fail to build docs if the below two constructors
// are included in the doxygen group; close here and reopen after as a workaround
Expand Down Expand Up @@ -1714,6 +1752,13 @@ class memoryview : public object {
static memoryview from_memory(const void *mem, ssize_t size) {
return memoryview::from_memory(const_cast<void*>(mem), size, true);
}

#ifdef PYBIND11_HAS_STRING_VIEW
static memoryview from_memory(std::string_view mem) {
return from_memory(const_cast<char*>(mem.data()), static_cast<ssize_t>(mem.size()), true);
}
#endif

#endif
};

Expand Down
24 changes: 24 additions & 0 deletions tests/test_builtin_casters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,35 @@ TEST_SUBMODULE(builtin_casters, m) {
m.def("string_view16_return", []() { return std::u16string_view(u"utf16 secret \U0001f382"); });
m.def("string_view32_return", []() { return std::u32string_view(U"utf32 secret \U0001f382"); });

// The inner lambdas here are to also test implicit conversion
using namespace std::literals;
m.def("string_view_bytes", []() { return [](py::bytes b) { return b; }("abc \x80\x80 def"sv); });
m.def("string_view_str", []() { return [](py::str s) { return s; }("abc \342\200\275 def"sv); });
m.def("string_view_from_bytes", [](const py::bytes &b) { return [](std::string_view s) { return s; }(b); });
#if PY_MAJOR_VERSION >= 3
m.def("string_view_memoryview", []() {
static constexpr auto val = "Have some \360\237\216\202"sv;
return py::memoryview::from_memory(val);
});
#endif

# ifdef PYBIND11_HAS_U8STRING
m.def("string_view8_print", [](std::u8string_view s) { py::print(s, s.size()); });
m.def("string_view8_chars", [](std::u8string_view s) { py::list l; for (auto c : s) l.append((std::uint8_t) c); return l; });
m.def("string_view8_return", []() { return std::u8string_view(u8"utf8 secret \U0001f382"); });
m.def("string_view8_str", []() { return py::str{std::u8string_view{u8"abc ‽ def"}}; });
# endif

struct TypeWithBothOperatorStringAndStringView {
// NOLINTNEXTLINE(google-explicit-constructor)
operator std::string() const { return "success"; }
// NOLINTNEXTLINE(google-explicit-constructor)
operator std::string_view() const { return "failure"; }
};
m.def("bytes_from_type_with_both_operator_string_and_string_view",
[]() { return py::bytes(TypeWithBothOperatorStringAndStringView()); });
m.def("str_from_type_with_both_operator_string_and_string_view",
[]() { return py::str(TypeWithBothOperatorStringAndStringView()); });
#endif

// test_integer_casting
Expand Down
11 changes: 11 additions & 0 deletions tests/test_builtin_casters.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,17 @@ def test_string_view(capture):
"""
)

assert m.string_view_bytes() == b"abc \x80\x80 def"
assert m.string_view_str() == u"abc ‽ def"
assert m.string_view_from_bytes(u"abc ‽ def".encode("utf-8")) == u"abc ‽ def"
if hasattr(m, "has_u8string"):
assert m.string_view8_str() == u"abc ‽ def"
if not env.PY2:
assert m.string_view_memoryview() == "Have some 🎂".encode()

assert m.bytes_from_type_with_both_operator_string_and_string_view() == b"success"
assert m.str_from_type_with_both_operator_string_and_string_view() == "success"


def test_integer_casting():
"""Issue #929 - out-of-range integer values shouldn't be accepted"""
Expand Down