Skip to content

Commit

Permalink
Refactor ustring and ustringhash
Browse files Browse the repository at this point in the history
* Use default implementations of copy ctr, destructor, and assignment
  from ustring. This allows ustring & ustringhash to be recognized as
  being a variety of flavors of trivially constructible, copyable, and
  destructible.

* It seems that the assignment operators were returning `const &`
  whereas it's more typical to return `&`, so now we do.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
  • Loading branch information
lgritz committed Jan 16, 2024
1 parent d3c9edc commit 98fca20
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 36 deletions.
54 changes: 21 additions & 33 deletions src/include/OpenImageIO/ustring.h
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ class OIIO_UTIL_API ustring {
#endif

/// ustring destructor.
~ustring() noexcept {}
~ustring() noexcept = default;

/// Conversion to an OIIO::string_view.
operator string_view() const noexcept { return { c_str(), length() }; }
Expand All @@ -209,79 +209,75 @@ class OIIO_UTIL_API ustring {
explicit operator std::string() const noexcept { return string(); }

/// Assign a ustring to *this.
const ustring& assign(const ustring& str)
ustring& assign(const ustring& str)
{
m_chars = str.m_chars;
return *this;
}

/// Assign a substring of a ustring to *this.
const ustring& assign(const ustring& str, size_type pos, size_type n = npos)
ustring& assign(const ustring& str, size_type pos, size_type n = npos)
{
*this = ustring(str, pos, n);
return *this;
}

/// Assign a std::string to *this.
const ustring& assign(const std::string& str)
ustring& assign(const std::string& str)
{
assign(str.c_str());
return *this;
}

/// Assign a substring of a std::string to *this.
const ustring& assign(const std::string& str, size_type pos,
size_type n = npos)
ustring& assign(const std::string& str, size_type pos, size_type n = npos)
{
*this = ustring(str, pos, n);
return *this;
}

/// Assign a null-terminated C string (char*) to *this.
const ustring& assign(const char* str)
ustring& assign(const char* str)
{
m_chars = str ? make_unique(str) : nullptr;
return *this;
}

/// Assign the first n characters of str to *this.
const ustring& assign(const char* str, size_type n)
ustring& assign(const char* str, size_type n)
{
*this = ustring(str, n);
return *this;
}

/// Assign n copies of c to *this.
const ustring& assign(size_type n, char c)
ustring& assign(size_type n, char c)
{
*this = ustring(n, c);
return *this;
}

/// Assign a string_view to *this.
const ustring& assign(string_view str)
ustring& assign(string_view str)
{
m_chars = str.length() ? make_unique(str) : nullptr;
return *this;
}

/// Assign a ustring to another ustring.
const ustring& operator=(const ustring& str) { return assign(str); }
ustring& operator=(const ustring& str) noexcept = default;

/// Assign a null-terminated C string (char *) to a ustring.
const ustring& operator=(const char* str) { return assign(str); }
ustring& operator=(const char* str) { return assign(str); }

/// Assign a C++ std::string to a ustring.
const ustring& operator=(const std::string& str) { return assign(str); }
ustring& operator=(const std::string& str) { return assign(str); }

/// Assign a string_view to a ustring.
const ustring& operator=(string_view str) { return assign(str); }
ustring& operator=(string_view str) { return assign(str); }

/// Assign a single char to a ustring.
const ustring& operator=(char c)
{
return *this = ustring(string_view(&c, 1));
}
ustring& operator=(char c) { return *this = ustring(string_view(&c, 1)); }

/// Return a C string representation of a ustring.
const char* c_str() const noexcept { return m_chars; }
Expand Down Expand Up @@ -803,6 +799,9 @@ class OIIO_UTIL_API ustringhash {
OIIO_HOSTDEVICE constexpr ustringhash(const ustringhash& str) noexcept
= default;

/// Move construct a ustringhash from another ustringhash.
OIIO_HOSTDEVICE ustringhash(ustringhash&& str) noexcept = default;

/// Construct from a ustring
ustringhash(const ustring& str) noexcept
: m_hash(str.hash())
Expand Down Expand Up @@ -865,15 +864,12 @@ class OIIO_UTIL_API ustringhash {
}

/// Assign from a ustringhash
OIIO_HOSTDEVICE constexpr const ustringhash&
operator=(const ustringhash& str)
{
m_hash = str.m_hash;
return *this;
}
OIIO_HOSTDEVICE constexpr ustringhash& operator=(const ustringhash& str)
= default;
OIIO_HOSTDEVICE ustringhash& operator=(ustringhash&& str) = default;

/// Assign from a ustring
const ustringhash& operator=(const ustring& str)
ustringhash& operator=(const ustring& str)
{
m_hash = str.hash();
return *this;
Expand Down Expand Up @@ -1132,7 +1128,6 @@ OIIO_NAMESPACE_END


namespace std { // not necessary in C++17, then we can just say std::hash

// std::hash specialization for ustring
template<> struct hash<OIIO::ustring> {
std::size_t operator()(OIIO::ustring u) const noexcept
Expand All @@ -1150,13 +1145,6 @@ template<> struct hash<OIIO::ustringhash> {
return static_cast<std::size_t>(u.hash());
}
};


// Force C++ to understand that ustring and ustringhash are trivially copyable
// (i.e. memcpy from one ustring to another is fine).
template<> struct is_trivially_copyable<OIIO::ustring> : std::true_type {};
template<> struct is_trivially_copyable<OIIO::ustringhash> : std::true_type {};

} // namespace std


Expand Down
18 changes: 15 additions & 3 deletions src/libutil/ustring_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,12 @@ test_ustring()
ustring foo("foo"), bar("bar"), empty(""), uninit;
ustring foobarbaz("foobarbaz");

OIIO_CHECK_ASSERT(std::is_trivially_copyable<ustring>::value);
OIIO_CHECK_ASSERT(std::is_default_constructible<OIIO::ustring>());
OIIO_CHECK_ASSERT(std::is_trivially_copyable<OIIO::ustring>());
OIIO_CHECK_ASSERT(std::is_trivially_destructible<OIIO::ustring>());
OIIO_CHECK_ASSERT(std::is_trivially_move_constructible<OIIO::ustring>());
OIIO_CHECK_ASSERT(std::is_trivially_copy_constructible<OIIO::ustring>());
OIIO_CHECK_ASSERT(std::is_trivially_move_assignable<OIIO::ustring>());

// Size of a ustring is just a pointer
OIIO_CHECK_EQUAL(sizeof(ustring), sizeof(const char*));
Expand Down Expand Up @@ -174,7 +179,14 @@ test_ustringhash()
// Two ustrings
ustring foo("foo"), bar("bar");

OIIO_CHECK_ASSERT(std::is_trivially_copyable<ustringhash>::value);
OIIO_CHECK_ASSERT(std::is_default_constructible<OIIO::ustringhash>());
OIIO_CHECK_ASSERT(std::is_trivially_copyable<OIIO::ustringhash>());
OIIO_CHECK_ASSERT(std::is_trivially_destructible<OIIO::ustringhash>());
OIIO_CHECK_ASSERT(
std::is_trivially_move_constructible<OIIO::ustringhash>());
OIIO_CHECK_ASSERT(
std::is_trivially_copy_constructible<OIIO::ustringhash>());
OIIO_CHECK_ASSERT(std::is_trivially_move_assignable<OIIO::ustringhash>());

OIIO_CHECK_EQUAL(sizeof(ustringhash), sizeof(size_t));

Expand Down Expand Up @@ -293,7 +305,7 @@ verify_no_collisions()
{
// Try to force a hash collision
parallel_for(int64_t(0), int64_t(1000000LL * int64_t(collide)),
[](int64_t i) { ustring u = ustring::fmtformat("{:x}", i); });
[](int64_t i) { (void)ustring::fmtformat("{:x}", i); });
std::vector<ustring> collisions;
size_t ncollisions = ustring::hash_collisions(&collisions);
OIIO_CHECK_ASSERT(ncollisions == 0);
Expand Down

0 comments on commit 98fca20

Please sign in to comment.