Skip to content

Fix some code smells from initial Sonar report #94

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 6 commits into from
Apr 26, 2023
Merged

Fix some code smells from initial Sonar report #94

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6dc0c82
Fix some code smells
kunitoki Apr 2, 2023
6629eb3
Merge branch 'master' into dev/fix_code_smells
kunitoki Apr 2, 2023
263a771
Fixes
kunitoki Apr 2, 2023
e60d16c
Defaulted constructors
kunitoki Apr 2, 2023
98f711d
More tweaks to expected
kunitoki Apr 2, 2023
fdd6a96
Merge branch 'master' into dev/fix_code_smells
kunitoki Apr 2, 2023
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
101 changes: 49 additions & 52 deletions Source/LuaBridge/detail/Expected.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,6 @@ union expected_storage<T, E, false, false>
E error_;
};


template <class E>
union expected_storage<void, E, true, false>
{
Expand All @@ -273,9 +272,7 @@ union expected_storage<void, E, true, false>
{
}

~expected_storage()
{
}
~expected_storage() = default;

constexpr const E& error() const noexcept
{
Expand All @@ -293,33 +290,33 @@ union expected_storage<void, E, true, false>
};

template <class T, class E, bool IsCopyConstructible, bool IsMoveConstructible>
class ExpectedBaseTrivial
class expected_base_trivial
{
using this_type = ExpectedBaseTrivial<T, E, IsCopyConstructible, IsMoveConstructible>;
using this_type = expected_base_trivial<T, E, IsCopyConstructible, IsMoveConstructible>;

protected:
using storage_type = expected_storage<T, E>;

constexpr ExpectedBaseTrivial() noexcept
constexpr expected_base_trivial() noexcept
: valid_(true)
{
}

template <class... Args>
constexpr ExpectedBaseTrivial(std::in_place_t, Args&&... args) noexcept
constexpr expected_base_trivial(std::in_place_t, Args&&... args) noexcept
: storage_(std::in_place, std::forward<Args>(args)...)
, valid_(true)
{
}

template <class... Args>
constexpr ExpectedBaseTrivial(UnexpectType, Args&&... args) noexcept
constexpr expected_base_trivial(UnexpectType, Args&&... args) noexcept
: storage_(unexpect, std::forward<Args>(args)...)
, valid_(false)
{
}

ExpectedBaseTrivial(const ExpectedBaseTrivial& other) noexcept
expected_base_trivial(const expected_base_trivial& other) noexcept
{
if (other.valid_)
{
Expand All @@ -331,7 +328,7 @@ class ExpectedBaseTrivial
}
}

ExpectedBaseTrivial(ExpectedBaseTrivial&& other) noexcept
expected_base_trivial(expected_base_trivial&& other) noexcept
{
if (other.valid_)
{
Expand All @@ -343,7 +340,7 @@ class ExpectedBaseTrivial
}
}

~ExpectedBaseTrivial() noexcept = default;
~expected_base_trivial() noexcept = default;

constexpr const T& value() const noexcept
{
Expand Down Expand Up @@ -414,33 +411,33 @@ class ExpectedBaseTrivial
};

template <class T, class E, bool IsCopyConstructible, bool IsMoveConstructible>
class ExpectedBaseNonTrivial
class expected_base_non_trivial
{
using this_type = ExpectedBaseNonTrivial<T, E, IsCopyConstructible, IsMoveConstructible>;
using this_type = expected_base_non_trivial<T, E, IsCopyConstructible, IsMoveConstructible>;

protected:
using storage_type = expected_storage<T, E>;

constexpr ExpectedBaseNonTrivial() noexcept(std::is_nothrow_default_constructible_v<storage_type>)
constexpr expected_base_non_trivial() noexcept(std::is_nothrow_default_constructible_v<storage_type>)
: valid_(true)
{
}

template <class... Args>
constexpr ExpectedBaseNonTrivial(std::in_place_t, Args&&... args) noexcept(std::is_nothrow_constructible_v<storage_type, std::in_place_t, Args...>)
constexpr expected_base_non_trivial(std::in_place_t, Args&&... args) noexcept(std::is_nothrow_constructible_v<storage_type, std::in_place_t, Args...>)
: storage_(std::in_place, std::forward<Args>(args)...)
, valid_(true)
{
}

template <class... Args>
constexpr ExpectedBaseNonTrivial(UnexpectType, Args&&... args) noexcept(std::is_nothrow_constructible_v<storage_type, UnexpectType, Args...>)
constexpr expected_base_non_trivial(UnexpectType, Args&&... args) noexcept(std::is_nothrow_constructible_v<storage_type, UnexpectType, Args...>)
: storage_(unexpect, std::forward<Args>(args)...)
, valid_(false)
{
}

ExpectedBaseNonTrivial(const ExpectedBaseNonTrivial& other)
expected_base_non_trivial(const expected_base_non_trivial& other)
{
if (other.valid_)
{
Expand All @@ -452,7 +449,7 @@ class ExpectedBaseNonTrivial
}
}

ExpectedBaseNonTrivial(ExpectedBaseNonTrivial&& other)
expected_base_non_trivial(expected_base_non_trivial&& other) noexcept
{
if (other.valid_)
{
Expand All @@ -464,7 +461,7 @@ class ExpectedBaseNonTrivial
}
}

~ExpectedBaseNonTrivial() noexcept(noexcept(std::declval<this_type>().destroy()))
~expected_base_non_trivial()
{
destroy();
}
Expand Down Expand Up @@ -546,35 +543,35 @@ class ExpectedBaseNonTrivial
};

template <class T, class E, bool IsMoveConstructible>
class ExpectedBaseNonTrivial<T, E, false, IsMoveConstructible>
class expected_base_non_trivial<T, E, false, IsMoveConstructible>
{
using this_type = ExpectedBaseNonTrivial<T, E, false, IsMoveConstructible>;
using this_type = expected_base_non_trivial<T, E, false, IsMoveConstructible>;

protected:
using storage_type = expected_storage<T, E>;

constexpr ExpectedBaseNonTrivial() noexcept(std::is_nothrow_default_constructible_v<storage_type>)
constexpr expected_base_non_trivial() noexcept(std::is_nothrow_default_constructible_v<storage_type>)
: valid_(true)
{
}

template <class... Args>
constexpr ExpectedBaseNonTrivial(std::in_place_t, Args&&... args) noexcept(std::is_nothrow_constructible_v<storage_type, std::in_place_t, Args...>)
constexpr expected_base_non_trivial(std::in_place_t, Args&&... args) noexcept(std::is_nothrow_constructible_v<storage_type, std::in_place_t, Args...>)
: storage_(std::in_place, std::forward<Args>(args)...)
, valid_(true)
{
}

template <class... Args>
constexpr ExpectedBaseNonTrivial(UnexpectType, Args&&... args) noexcept(std::is_nothrow_constructible_v<storage_type, UnexpectType, Args...>)
constexpr expected_base_non_trivial(UnexpectType, Args&&... args) noexcept(std::is_nothrow_constructible_v<storage_type, UnexpectType, Args...>)
: storage_(unexpect, std::forward<Args>(args)...)
, valid_(false)
{
}

ExpectedBaseNonTrivial(const ExpectedBaseNonTrivial& other) = delete;
expected_base_non_trivial(const expected_base_non_trivial& other) = delete;

ExpectedBaseNonTrivial(ExpectedBaseNonTrivial&& other)
expected_base_non_trivial(expected_base_non_trivial&& other) noexcept
{
if (other.valid_)
{
Expand All @@ -586,7 +583,7 @@ class ExpectedBaseNonTrivial<T, E, false, IsMoveConstructible>
}
}

~ExpectedBaseNonTrivial() noexcept(noexcept(std::declval<this_type>().destroy()))
~expected_base_non_trivial()
{
destroy();
}
Expand Down Expand Up @@ -668,34 +665,34 @@ class ExpectedBaseNonTrivial<T, E, false, IsMoveConstructible>
};

template <class T, class E, bool IsCopyConstructible>
class ExpectedBaseNonTrivial<T, E, IsCopyConstructible, false>
class expected_base_non_trivial<T, E, IsCopyConstructible, false>
{
using this_type = ExpectedBaseNonTrivial<T, E, IsCopyConstructible, false>;
using this_type = expected_base_non_trivial<T, E, IsCopyConstructible, false>;

protected:
using storage_type = expected_storage<T, E>;

template <class U = storage_type, class = std::enable_if_t<std::is_default_constructible_v<U>>>
constexpr ExpectedBaseNonTrivial() noexcept(std::is_nothrow_default_constructible_v<storage_type>)
constexpr expected_base_non_trivial() noexcept(std::is_nothrow_default_constructible_v<storage_type>)
: valid_(true)
{
}

template <class... Args>
constexpr ExpectedBaseNonTrivial(std::in_place_t, Args&&... args) noexcept(std::is_nothrow_constructible_v<storage_type, std::in_place_t, Args...>)
constexpr expected_base_non_trivial(std::in_place_t, Args&&... args) noexcept(std::is_nothrow_constructible_v<storage_type, std::in_place_t, Args...>)
: storage_(std::in_place, std::forward<Args>(args)...)
, valid_(true)
{
}

template <class... Args>
constexpr ExpectedBaseNonTrivial(UnexpectType, Args&&... args) noexcept(std::is_nothrow_constructible_v<storage_type, UnexpectType, Args...>)
constexpr expected_base_non_trivial(UnexpectType, Args&&... args) noexcept(std::is_nothrow_constructible_v<storage_type, UnexpectType, Args...>)
: storage_(unexpect, std::forward<Args>(args)...)
, valid_(false)
{
}

ExpectedBaseNonTrivial(const ExpectedBaseNonTrivial& other)
expected_base_non_trivial(const expected_base_non_trivial& other)
{
if (other.valid_)
{
Expand All @@ -707,9 +704,9 @@ class ExpectedBaseNonTrivial<T, E, IsCopyConstructible, false>
}
}

ExpectedBaseNonTrivial(ExpectedBaseNonTrivial&& other) = delete;
expected_base_non_trivial(expected_base_non_trivial&& other) = delete;

~ExpectedBaseNonTrivial() noexcept(noexcept(std::declval<this_type>().destroy()))
~expected_base_non_trivial()
{
destroy();
}
Expand Down Expand Up @@ -791,38 +788,38 @@ class ExpectedBaseNonTrivial<T, E, IsCopyConstructible, false>
};

template <class T, class E>
class ExpectedBaseNonTrivial<T, E, false, false>
class expected_base_non_trivial<T, E, false, false>
{
using this_type = ExpectedBaseNonTrivial<T, E, false, false>;
using this_type = expected_base_non_trivial<T, E, false, false>;

protected:
using storage_type = expected_storage<T, E>;

template <class U = storage_type, class = std::enable_if_t<std::is_default_constructible_v<U>>>
constexpr ExpectedBaseNonTrivial() noexcept(std::is_nothrow_default_constructible_v<storage_type>)
constexpr expected_base_non_trivial() noexcept(std::is_nothrow_default_constructible_v<storage_type>)
: valid_(true)
{
}

template <class... Args>
constexpr ExpectedBaseNonTrivial(std::in_place_t, Args&&... args) noexcept(std::is_nothrow_constructible_v<storage_type, std::in_place_t, Args...>)
constexpr expected_base_non_trivial(std::in_place_t, Args&&... args) noexcept(std::is_nothrow_constructible_v<storage_type, std::in_place_t, Args...>)
: storage_(std::in_place, std::forward<Args>(args)...)
, valid_(true)
{
}

template <class... Args>
constexpr ExpectedBaseNonTrivial(UnexpectType, Args&&... args) noexcept(std::is_nothrow_constructible_v<storage_type, UnexpectType, Args...>)
constexpr expected_base_non_trivial(UnexpectType, Args&&... args) noexcept(std::is_nothrow_constructible_v<storage_type, UnexpectType, Args...>)
: storage_(unexpect, std::forward<Args>(args)...)
, valid_(false)
{
}

ExpectedBaseNonTrivial(const ExpectedBaseNonTrivial& other) = delete;
expected_base_non_trivial(const expected_base_non_trivial& other) = delete;

ExpectedBaseNonTrivial(ExpectedBaseNonTrivial&& other) = delete;
expected_base_non_trivial(expected_base_non_trivial&& other) = delete;

~ExpectedBaseNonTrivial() noexcept(noexcept(std::declval<this_type>().destroy()))
~expected_base_non_trivial()
{
destroy();
}
Expand Down Expand Up @@ -904,10 +901,10 @@ class ExpectedBaseNonTrivial<T, E, false, false>
};

template <class T, class E, bool IsCopyConstructible, bool IsMoveConstructible>
using ExpectedBase = std::conditional_t<
using expected_base = std::conditional_t<
(std::is_void_v<T> || std::is_trivially_destructible_v<T>) && std::is_trivially_destructible_v<E>,
ExpectedBaseTrivial<T, E, IsCopyConstructible, IsMoveConstructible>,
ExpectedBaseNonTrivial<T, E, IsCopyConstructible, IsMoveConstructible>>;
expected_base_trivial<T, E, IsCopyConstructible, IsMoveConstructible>,
expected_base_non_trivial<T, E, IsCopyConstructible, IsMoveConstructible>>;

} // namespace detail

Expand Down Expand Up @@ -1032,11 +1029,11 @@ struct is_unexpected<Unexpected<E>> : std::true_type
};

template <class T, class E>
class Expected : public detail::ExpectedBase<T, E, std::is_copy_constructible_v<T>, std::is_move_constructible_v<T>>
class Expected : public detail::expected_base<T, E, std::is_copy_constructible_v<T>, std::is_move_constructible_v<T>>
{
static_assert(!std::is_reference_v<E> && !std::is_void_v<E>, "Unexpected type can't be a reference or void");

using base_type = detail::ExpectedBase<T, E, std::is_copy_constructible_v<T>, std::is_move_constructible_v<T>>;
using base_type = detail::expected_base<T, E, std::is_copy_constructible_v<T>, std::is_move_constructible_v<T>>;
using this_type = Expected<T, E>;

public:
Expand Down Expand Up @@ -1144,7 +1141,7 @@ class Expected : public detail::ExpectedBase<T, E, std::is_copy_constructible_v<
return *this;
}

Expected& operator=(Expected&& other)
Expected& operator=(Expected&& other) noexcept
{
if (other.hasValue())
{
Expand Down Expand Up @@ -1344,11 +1341,11 @@ class Expected : public detail::ExpectedBase<T, E, std::is_copy_constructible_v<
};

template <class E>
class Expected<void, E> : public detail::ExpectedBase<void, E, std::is_copy_constructible_v<E>, std::is_move_constructible_v<E>>
class Expected<void, E> : public detail::expected_base<void, E, std::is_copy_constructible_v<E>, std::is_move_constructible_v<E>>
{
static_assert(!std::is_reference_v<E> && !std::is_void_v<E>, "Unexpected type can't be a reference or void");

using base_type = detail::ExpectedBase<void, E, std::is_copy_constructible_v<E>, std::is_move_constructible_v<E>>;
using base_type = detail::expected_base<void, E, std::is_copy_constructible_v<E>, std::is_move_constructible_v<E>>;
using this_type = Expected<void, E>;

public:
Expand Down
12 changes: 6 additions & 6 deletions Source/LuaBridge/detail/LuaRef.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class LuaRefBase
{
};

LuaRefBase(lua_State* L)
LuaRefBase(lua_State* L) noexcept
: m_L(L)
{
}
Expand Down Expand Up @@ -827,7 +827,7 @@ class LuaRef : public LuaRefBase<LuaRef, LuaRef>
*
* @note The object is popped.
*/
LuaRef(lua_State* L, FromStack)
LuaRef(lua_State* L, FromStack) noexcept
: LuaRefBase(L)
, m_ref(luaL_ref(m_L, LUA_REGISTRYINDEX))
{
Expand Down Expand Up @@ -867,7 +867,7 @@ class LuaRef : public LuaRefBase<LuaRef, LuaRef>
*
* @param L A Lua state.
*/
LuaRef(lua_State* L)
LuaRef(lua_State* L) noexcept
: LuaRefBase(L)
, m_ref(LUA_NOREF)
{
Expand Down Expand Up @@ -921,7 +921,7 @@ class LuaRef : public LuaRefBase<LuaRef, LuaRef>
*
* @param other An existing reference.
*/
LuaRef(LuaRef&& other)
LuaRef(LuaRef&& other) noexcept
: LuaRefBase(other.m_L)
, m_ref(std::exchange(other.m_ref, LUA_NOREF))
{
Expand Down Expand Up @@ -1046,7 +1046,7 @@ class LuaRef : public LuaRefBase<LuaRef, LuaRef>
*
* @returns This reference.
*/
LuaRef& operator=(LuaRef&& rhs)
LuaRef& operator=(LuaRef&& rhs) noexcept
{
if (m_ref != LUA_NOREF)
luaL_unref(m_L, LUA_REGISTRYINDEX, m_ref);
Expand Down Expand Up @@ -1230,7 +1230,7 @@ class LuaRef : public LuaRefBase<LuaRef, LuaRef>
}

private:
void swap(LuaRef& other)
void swap(LuaRef& other) noexcept
{
using std::swap;

Expand Down
Loading