Skip to content

Commit

Permalink
nothrow assertions
Browse files Browse the repository at this point in the history
Radiant originally only enforced that destructors
should not throw. Radiant is now adopting a policy
that move operations should not throw too.

- cleans up existing nothrow dtor assertions
- institutes nothrow move assertions
  • Loading branch information
jxy-s committed May 2, 2024
1 parent 6e87fba commit 8ca1239
Show file tree
Hide file tree
Showing 12 changed files with 105 additions and 24 deletions.
4 changes: 2 additions & 2 deletions MODULE.bazel.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 23 additions & 0 deletions radiant/Locks.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,15 @@ class RAD_NODISCARD LockExclusive final
LockExclusive(LockType& lock) noexcept(noexcept(lock.LockExclusive()))
: m_lock(lock)
{
RAD_S_ASSERT_NOTHROW(noexcept(lock.LockExclusive()));

m_lock.LockExclusive();
}

~LockExclusive()
{
RAD_S_ASSERT_NOTHROW_DTOR(noexcept(m_lock.Unlock()));

m_lock.Unlock();
}

Expand All @@ -76,11 +80,15 @@ class RAD_NODISCARD LockShared final
LockShared(LockType& lock) noexcept(noexcept(lock.LockShared()))
: m_lock(lock)
{
RAD_S_ASSERT_NOTHROW(noexcept(lock.LockShared()));

m_lock.LockShared();
}

~LockShared()
{
RAD_S_ASSERT_NOTHROW_DTOR(noexcept(m_lock.Unlock()));

m_lock.Unlock();
}

Expand Down Expand Up @@ -116,6 +124,7 @@ class RAD_NODISCARD RelockableExclusive final
: m_lock(lock),
m_acquired(true)
{
RAD_S_ASSERT_NOTHROW(noexcept(lock.LockExclusive()));
m_lock.LockExclusive();
}

Expand All @@ -128,6 +137,8 @@ class RAD_NODISCARD RelockableExclusive final

~RelockableExclusive()
{
RAD_S_ASSERT_NOTHROW_DTOR(noexcept(m_lock.Unlock()));

if (m_acquired)
{
m_lock.Unlock();
Expand All @@ -138,6 +149,8 @@ class RAD_NODISCARD RelockableExclusive final
/// @warning It is not allowed to call Unlock() if the lock is not acquired.
void Unlock() noexcept(noexcept(DeclVal<LockType>().Unlock()))
{
RAD_S_ASSERT_NOTHROW(noexcept(DeclVal<LockType>().Unlock()));

RAD_ASSERT(m_acquired);
m_acquired = false;
m_lock.Unlock();
Expand All @@ -149,6 +162,8 @@ class RAD_NODISCARD RelockableExclusive final
/// acquired.
void Lock() noexcept(noexcept(DeclVal<LockType>().LockExclusive()))
{
RAD_S_ASSERT_NOTHROW(noexcept(DeclVal<LockType>().LockExclusive()));

RAD_ASSERT(!m_acquired);
m_lock.LockExclusive();
m_acquired = true;
Expand Down Expand Up @@ -179,6 +194,8 @@ class RAD_NODISCARD RelockableShared final
: m_lock(lock),
m_acquired(true)
{
RAD_S_ASSERT_NOTHROW(noexcept(lock.LockShared()));

m_lock.LockShared();
}

Expand All @@ -192,6 +209,8 @@ class RAD_NODISCARD RelockableShared final
/// @brief Defer acquiring the lock to a manual call to Lock()
~RelockableShared()
{
RAD_S_ASSERT_NOTHROW_DTOR(noexcept(m_lock.Unlock()));

if (m_acquired)
{
m_lock.Unlock();
Expand All @@ -202,6 +221,8 @@ class RAD_NODISCARD RelockableShared final
/// @warning It is not allowed to call Unlock() if the lock is not acquired.
void Unlock() noexcept(noexcept(DeclVal<LockType>().Unlock()))
{
RAD_S_ASSERT_NOTHROW(noexcept(DeclVal<LockType>().Unlock()));

RAD_ASSERT(m_acquired);
m_acquired = false;
m_lock.Unlock();
Expand All @@ -213,6 +234,8 @@ class RAD_NODISCARD RelockableShared final
/// acquired.
void Lock() noexcept(noexcept(DeclVal<LockType>().LockShared()))
{
RAD_S_ASSERT_NOTHROW(noexcept(DeclVal<LockType>().Unlock()));

RAD_ASSERT(!m_acquired);
m_lock.LockShared();
m_acquired = true;
Expand Down
6 changes: 4 additions & 2 deletions radiant/Result.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,7 @@ struct ResultStorage<T, E, false>

~ResultStorage() noexcept
{
RAD_S_ASSERTMSG(IsNoThrowDtor<T> && IsNoThrowDtor<E>,
"Destructors should not throw!");
RAD_S_ASSERT_NOTHROW_DTOR(IsNoThrowDtor<T> && IsNoThrowDtor<E>);

Destruct();
}
Expand Down Expand Up @@ -327,6 +326,9 @@ class RAD_NODISCARD Result final : private detail::ResultStorage<T, E>
using typename StorageType::OkType;
using typename StorageType::ErrType;

RAD_S_ASSERT_NOTHROW_MOVE((IsNoThrowMoveCtor<T> && IsNoThrowMoveAssign<T> &&
IsNoThrowMoveCtor<E> && IsNoThrowMoveAssign<E>));

constexpr Result() noexcept
: StorageType(ResultEmptyTag)
{
Expand Down
8 changes: 4 additions & 4 deletions radiant/ScopeExit.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ class ScopeExit

~ScopeExit()
{
RAD_S_ASSERTMSG(IsNoThrowDtor<T>, "Destructors should not throw!");
RAD_S_ASSERTMSG(noexcept(m_fn()), "Destructors should not throw!");
RAD_S_ASSERT_NOTHROW_DTOR(IsNoThrowDtor<T>);
RAD_S_ASSERT_NOTHROW_DTOR(noexcept(m_fn()));

if (m_call)
{
Expand Down Expand Up @@ -76,8 +76,8 @@ class ScopeGuard

~ScopeGuard()
{
RAD_S_ASSERTMSG(IsNoThrowDtor<T>, "Destructors should not throw!");
RAD_S_ASSERTMSG(noexcept(m_fn()), "Destructors should not throw!");
RAD_S_ASSERT_NOTHROW_DTOR(IsNoThrowDtor<T>);
RAD_S_ASSERT_NOTHROW_DTOR(noexcept(m_fn()));

m_fn();
}
Expand Down
44 changes: 41 additions & 3 deletions radiant/TotallyRad.h
Original file line number Diff line number Diff line change
Expand Up @@ -229,16 +229,54 @@ inline bool DoAssert(const char* Assertion, const char* File, int Line)
#define RAD_S_ASSERT(x) static_assert(x, #x)
#define RAD_S_ASSERTMSG(x, m) static_assert(x, m)

//
// Enables broad assertions that objects do not throw exceptions.
//
#ifndef RAD_ENABLE_NOTHROW_ASSERTIONS
#define RAD_ENABLE_NOTHROW_ASSERTIONS 1
#endif

#if RAD_ENABLE_NOTHROW_ASSERTIONS
#define RAD_S_ASSERT_NOTHROW(x) RAD_S_ASSERT(x)
#define RAD_S_ASSERT_NOTHROWMSG(x, m) RAD_S_ASSERTMSG(x, m)
#else
#define RAD_S_ASSERT_NOTHROW(x) ((void)0)
#define RAD_S_ASSERT_NOTHROWMSG(x, m) ((void)0)
#define RAD_S_ASSERT_NOTHROW(x) RAD_S_ASSERT(true)
#define RAD_S_ASSERT_NOTHROWMSG(x, m) RAD_S_ASSERT(true)
#endif

//
// Enables assertions that destructors do not throw exceptions.
//
// Core Guideline: A destructor must not fail. If a destructor tries to exit
// with an exception, it’s a bad design error and the program had better
// terminate.
//
#ifndef RAD_ENABLE_NOTHROW_DTOR_ASSERTIONS
#define RAD_ENABLE_NOTHROW_DTOR_ASSERTIONS 1
#endif
#if RAD_ENABLE_NOTHROW_DTOR_ASSERTIONS
#define RAD_S_ASSERT_NOTHROW_DTOR(x) \
RAD_S_ASSERTMSG(x, "destructors should not throw")
#define RAD_S_ASSERT_NOTHROW_DTOR_T(x) \
RAD_S_ASSERTMSG(IsNoThrowDtor<x>, "destructors should not throw")
#else
#define RAD_S_ASSERT_NOTHROW_DTOR(x) RAD_S_ASSERT(true)
#endif

//
// Enables assertions the move operations do not throw exceptions.
//
// Core Guideline: A throwing move violates most people’s reasonable
// assumptions. A non-throwing move will be used more efficiently by
// standard-library and language facilities.
//
#ifndef RAD_ENABLE_NOTHROW_MOVE_ASSERTIONS
#define RAD_ENABLE_NOTHROW_MOVE_ASSERTIONS 1
#endif
#if RAD_ENABLE_NOTHROW_MOVE_ASSERTIONS
#define RAD_S_ASSERT_NOTHROW_MOVE(x) \
RAD_S_ASSERTMSG(x, "move operations should not throw")
#else
#define RAD_S_ASSERT_NOTHROW_MOVE(x) RAD_S_ASSERT(true)
#endif

#define RAD_NOT_COPYABLE(x) \
Expand Down
2 changes: 2 additions & 0 deletions radiant/TypeWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class TypeWrapper<T, false>

using Type = T;

RAD_S_ASSERT_NOTHROW_MOVE((IsNoThrowMoveCtor<T> && IsNoThrowMoveAssign<T>));

template <typename U = T, EnIf<IsDefaultCtor<U>, int> = 0>
constexpr TypeWrapper() noexcept(IsNoThrowDefaultCtor<T>)
: m_value()
Expand Down
11 changes: 7 additions & 4 deletions radiant/Vector.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ class Vector final
static constexpr uint16_t InlineCount = TInlineCount;
using AllocatorType = TAllocator;

RAD_S_ASSERT_NOTHROW_MOVE((IsNoThrowMoveCtor<T> && IsNoThrowMoveAssign<T> &&
IsNoThrowMoveCtor<TAllocator> &&
IsNoThrowMoveAssign<TAllocator>));

//
// TODO - rad::Vector does not yet support types that might throw exceptions
// when manipulating it. It is the intention of the radiant authors for the
Expand All @@ -60,14 +64,13 @@ class Vector final
// itself should be in a valid state when this happens.
//
RAD_S_ASSERTMSG((IsNoThrowDtor<T> && IsNoThrowDefaultCtor<T> &&
IsNoThrowCopyCtor<T> && IsNoThrowMoveCtor<T> &&
IsNoThrowCopyAssign<T> && IsNoThrowMoveAssign<T>),
IsNoThrowCopyCtor<T> && IsNoThrowCopyAssign<T>),
"rad::Vector does not yet support types that might throw.");

~Vector()
{
RAD_S_ASSERTMSG(IsNoThrowDtor<T> && IsNoThrowDtor<TAllocator>,
"Destructors should not throw!");
RAD_S_ASSERT_NOTHROW_DTOR(IsNoThrowDtor<T> &&
IsNoThrowDtor<TAllocator>);

Clear();
Storage().Free(Allocator());
Expand Down
12 changes: 6 additions & 6 deletions radiant/detail/VectorOperations.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ struct VectorManipulation
template <typename U = T, EnIf<!IsTrivDtor<U>, int> = 0>
inline void Dtor(T* item) noexcept
{
RAD_S_ASSERTMSG(IsNoThrowDtor<T>, "Destructors should not throw!");
RAD_S_ASSERT_NOTHROW_DTOR(IsNoThrowDtor<T>);

item->~T();
}
Expand All @@ -60,7 +60,7 @@ struct VectorManipulation
template <typename U = T, EnIf<!IsTrivDtor<U>, int> = 0>
inline void DtorRange(T* start, T* end) noexcept
{
RAD_S_ASSERTMSG(IsNoThrowDtor<T>, "Destructors should not throw!");
RAD_S_ASSERT_NOTHROW_DTOR(IsNoThrowDtor<T>);

for (T* p = start; p != end; p++)
{
Expand Down Expand Up @@ -121,7 +121,7 @@ struct VectorManipulation
uint32_t count,
const T& value) noexcept(IsNoThrowCopyCtor<T>)
{
RAD_S_ASSERTMSG(IsNoThrowDtor<T>, "Destructors should not throw!");
RAD_S_ASSERT_NOTHROW_DTOR(IsNoThrowDtor<T>);

for (uint32_t i = 0; i < count; i++)
{
Expand All @@ -142,7 +142,7 @@ struct VectorManipulation
inline void CopyCtorDtorDestRange(T* dest, T* src, uint32_t count) noexcept(
IsNoThrowCopyCtor<T>)
{
RAD_S_ASSERTMSG(IsNoThrowDtor<T>, "Destructors should not throw!");
RAD_S_ASSERT_NOTHROW_DTOR(IsNoThrowDtor<T>);

for (uint32_t i = 0; i < count; i++)
{
Expand All @@ -165,7 +165,7 @@ struct VectorManipulation
T* src,
uint32_t count) noexcept(IsNoThrowMoveCtor<T>)
{
RAD_S_ASSERTMSG(IsNoThrowDtor<T>, "Destructors should not throw!");
RAD_S_ASSERT_NOTHROW_DTOR(IsNoThrowDtor<T>);

for (uint32_t i = 0; i < count; i++)
{
Expand Down Expand Up @@ -197,7 +197,7 @@ struct VectorManipulation
inline void MoveCtorDtorSrcRange(T* dest, T* src, uint32_t count) noexcept(
IsNoThrowMoveCtor<T>)
{
RAD_S_ASSERTMSG(IsNoThrowDtor<T>, "Destructors should not throw!");
RAD_S_ASSERT_NOTHROW_DTOR(IsNoThrowDtor<T>);

for (uint32_t i = 0; i < count; i++)
{
Expand Down
4 changes: 3 additions & 1 deletion test/test_Result.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
//
// Disable nothrow assertions for unit testing
//
#define RAD_ENABLE_NOTHROW_ASSERTIONS 0
#define RAD_ENABLE_NOTHROW_ASSERTIONS 0
#define RAD_ENABLE_NOTHROW_DTOR_ASSERTIONS 0
#define RAD_ENABLE_NOTHROW_MOVE_ASSERTIONS 0

#include "gtest/gtest.h"

Expand Down
4 changes: 3 additions & 1 deletion test/test_TypeWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
//
// Disable nothrow assertions for unit testing
//
#define RAD_ENABLE_NOTHROW_ASSERTIONS 0
#define RAD_ENABLE_NOTHROW_ASSERTIONS 0
#define RAD_ENABLE_NOTHROW_DTOR_ASSERTIONS 0
#define RAD_ENABLE_NOTHROW_MOVE_ASSERTIONS 0

#include "gtest/gtest.h"

Expand Down
4 changes: 3 additions & 1 deletion test/test_Utility.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
//
// Disable nothrow assertions for unit testing
//
#define RAD_ENABLE_NOTHROW_ASSERTIONS 0
#define RAD_ENABLE_NOTHROW_ASSERTIONS 0
#define RAD_ENABLE_NOTHROW_DTOR_ASSERTIONS 0
#define RAD_ENABLE_NOTHROW_MOVE_ASSERTIONS 0

#include "gtest/gtest.h"

Expand Down
7 changes: 7 additions & 0 deletions test/test_Vector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.

//
// Disable nothrow assertions for unit testing
//
#define RAD_ENABLE_NOTHROW_ASSERTIONS 0
#define RAD_ENABLE_NOTHROW_DTOR_ASSERTIONS 0
#define RAD_ENABLE_NOTHROW_MOVE_ASSERTIONS 0

#include "gtest/gtest.h"

#include "test/TestAlloc.h"
Expand Down

0 comments on commit 8ca1239

Please sign in to comment.