Improve enum conversions (#4397)

* Simplify enum conversions

* Make stats and parse status enums compatible

* Simplify conversion

* Move `parse_status` and auxiliary functions to more a appropriate namespace

* More simplifications

* Use ADL for `to_stat_detail (...)` calls

* Update magic_enum version

* Customize enum ranges for reflection

* Add enum safeguards and test accordingly

CMakeLists.txt

@@ -526,7 +526,7 @@ add_subdirectory(submodules/rocksdb EXCLUDE_FROM_ALL)
 include_directories(submodules/cpptoml/include)
 
 # magic_enum
-include_directories(submodules/magic_enum/include)
+include_directories(submodules/magic_enum/include/magic_enum)
 
 add_subdirectory(crypto/ed25519-donna)
 

nano/core_test/enums.cpp

@@ -7,17 +7,59 @@
 TEST (enums, stat_type)
 {
 	ASSERT_FALSE (nano::to_string (static_cast<nano::stat::type> (0)).empty ());
+	ASSERT_NO_THROW (std::string{ nano::to_string (static_cast<nano::stat::type> (0)) });
+
 	ASSERT_FALSE (nano::to_string (nano::stat::type::_last).empty ());
+	ASSERT_NO_THROW (std::string{ nano::to_string (nano::stat::type::_last) });
+	ASSERT_EQ (nano::to_string (nano::stat::type::_last), "_last");
 }
 
 TEST (enums, stat_detail)
 {
 	ASSERT_FALSE (nano::to_string (static_cast<nano::stat::detail> (0)).empty ());
+	ASSERT_NO_THROW (std::string{ nano::to_string (static_cast<nano::stat::detail> (0)) });
+
 	ASSERT_FALSE (nano::to_string (nano::stat::detail::_last).empty ());
+	ASSERT_NO_THROW (std::string{ nano::to_string (nano::stat::detail::_last) });
+	ASSERT_EQ (nano::to_string (nano::stat::detail::_last), "_last");
 }
 
 TEST (enums, stat_dir)
 {
 	ASSERT_FALSE (nano::to_string (static_cast<nano::stat::dir> (0)).empty ());
+	ASSERT_NO_THROW (std::string{ nano::to_string (static_cast<nano::stat::dir> (0)) });
+
 	ASSERT_FALSE (nano::to_string (nano::stat::dir::_last).empty ());
+	ASSERT_NO_THROW (std::string{ nano::to_string (nano::stat::dir::_last) });
+	ASSERT_EQ (nano::to_string (nano::stat::dir::_last), "_last");
+}
+
+TEST (enums, log_type)
+{
+	ASSERT_FALSE (to_string (static_cast<nano::log::type> (0)).empty ());
+	ASSERT_NO_THROW (std::string{ to_string (static_cast<nano::log::type> (0)) });
+
+	ASSERT_FALSE (to_string (nano::log::type::_last).empty ());
+	ASSERT_NO_THROW (std::string{ to_string (nano::log::type::_last) });
+	ASSERT_EQ (to_string (nano::log::type::_last), "_last");
+}
+
+TEST (enums, log_detail)
+{
+	ASSERT_FALSE (to_string (static_cast<nano::log::detail> (0)).empty ());
+	ASSERT_NO_THROW (std::string{ to_string (static_cast<nano::log::detail> (0)) });
+
+	ASSERT_FALSE (to_string (nano::log::detail::_last).empty ());
+	ASSERT_NO_THROW (std::string{ to_string (nano::log::detail::_last) });
+	ASSERT_EQ (to_string (nano::log::detail::_last), "_last");
+}
+
+TEST (enums, log_category)
+{
+	ASSERT_FALSE (to_string (static_cast<nano::log::type> (0)).empty ());
+	ASSERT_NO_THROW (std::string{ to_string (static_cast<nano::log::type> (0)) });
+
+	ASSERT_FALSE (to_string (nano::log::type::_last).empty ());
+	ASSERT_NO_THROW (std::string{ to_string (nano::log::type::_last) });
+	ASSERT_EQ (to_string (nano::log::type::_last), "_last");
 }

nano/core_test/message_deserializer.cpp

@@ -51,7 +51,7 @@ auto message_deserializer_success_checker (message_type & message_original) -> v
 		ASSERT_EQ (*deserialized_bytes, *original_bytes);
 	});
 	// This is a sanity test, to ensure the successful deserialization case passes.
-	ASSERT_EQ (message_deserializer->status, nano::transport::message_deserializer::parse_status::success);
+	ASSERT_EQ (message_deserializer->status, nano::transport::parse_status::success);
 }
 
 TEST (message_deserializer, exact_confirm_ack)

nano/core_test/network.cpp

@@ -796,16 +796,16 @@ TEST (network, duplicate_detection)
 	auto & node1 = *system.add_node (node_flags);
 	nano::publish publish{ nano::dev::network_params.network, nano::dev::genesis };
 
-	ASSERT_EQ (0, node1.stats.count (nano::stat::type::filter, nano::stat::detail::duplicate_publish));
+	ASSERT_EQ (0, node1.stats.count (nano::stat::type::filter, nano::stat::detail::duplicate_publish_message));
 
 	// Publish duplicate detection through TCP
 	auto tcp_channel = node0.network.tcp_channels.find_node_id (node1.get_node_id ());
 	ASSERT_NE (nullptr, tcp_channel);
-	ASSERT_EQ (0, node1.stats.count (nano::stat::type::filter, nano::stat::detail::duplicate_publish));
+	ASSERT_EQ (0, node1.stats.count (nano::stat::type::filter, nano::stat::detail::duplicate_publish_message));
 	tcp_channel->send (publish);
-	ASSERT_TIMELY_EQ (2s, node1.stats.count (nano::stat::type::filter, nano::stat::detail::duplicate_publish), 0);
+	ASSERT_TIMELY_EQ (2s, node1.stats.count (nano::stat::type::filter, nano::stat::detail::duplicate_publish_message), 0);
 	tcp_channel->send (publish);
-	ASSERT_TIMELY_EQ (2s, node1.stats.count (nano::stat::type::filter, nano::stat::detail::duplicate_publish), 1);
+	ASSERT_TIMELY_EQ (2s, node1.stats.count (nano::stat::type::filter, nano::stat::detail::duplicate_publish_message), 1);
 }
 
 TEST (network, duplicate_revert_publish)

nano/lib/logging_enums.cpp

@@ -1,6 +1,3 @@
-#define MAGIC_ENUM_RANGE_MIN 0
-#define MAGIC_ENUM_RANGE_MAX 256
-
 #include <nano/lib/logging_enums.hpp>
 #include <nano/lib/utility.hpp>
 
@@ -24,32 +21,22 @@ std::string_view nano::log::to_string (nano::log::level level)
 const std::vector<nano::log::level> & nano::log::all_levels ()
 {
 	static std::vector<nano::log::level> all = [] () {
-		std::vector<nano::log::level> result;
-		for (auto const & lvl : magic_enum::enum_values<nano::log::level> ())
-		{
-			result.push_back (lvl);
-		}
-		return result;
+		return nano::util::enum_values<nano::log::level> ();
 	}();
 	return all;
 }
 
 const std::vector<nano::log::type> & nano::log::all_types ()
 {
 	static std::vector<nano::log::type> all = [] () {
-		std::vector<nano::log::type> result;
-		for (auto const & lvl : magic_enum::enum_values<nano::log::type> ())
-		{
-			result.push_back (lvl);
-		}
-		return result;
+		return nano::util::enum_values<nano::log::type> ();
 	}();
 	return all;
 }
 
 nano::log::level nano::log::parse_level (std::string_view name)
 {
-	auto value = magic_enum::enum_cast<nano::log::level> (name);
+	auto value = nano::util::parse_enum<nano::log::level> (name);
 	if (value.has_value ())
 	{
 		return value.value ();
@@ -66,7 +53,7 @@ nano::log::level nano::log::parse_level (std::string_view name)
 
 nano::log::type nano::log::parse_type (std::string_view name)
 {
-	auto value = magic_enum::enum_cast<nano::log::type> (name);
+	auto value = nano::util::parse_enum<nano::log::type> (name);
 	if (value.has_value ())
 	{
 		return value.value ();
@@ -79,7 +66,7 @@ nano::log::type nano::log::parse_type (std::string_view name)
 
 nano::log::detail nano::log::parse_detail (std::string_view name)
 {
-	auto value = magic_enum::enum_cast<nano::log::detail> (name);
+	auto value = nano::util::parse_enum<nano::log::detail> (name);
 	if (value.has_value ())
 	{
 		return value.value ();

nano/lib/logging_enums.hpp

@@ -4,6 +4,8 @@
 #include <string_view>
 #include <vector>
 
+#include <magic_enum.hpp>
+
 namespace nano::log
 {
 enum class level
@@ -74,6 +76,8 @@ enum class type
 	bootstrap,
 	bootstrap_lazy,
 	bootstrap_legacy,
+
+	_last // Must be the last enum
 };
 
 enum class detail
@@ -110,6 +114,7 @@ enum class detail
 	requesting_account_or_head,
 	requesting_pending,
 
+	_last // Must be the last enum
 };
 
 // TODO: Additionally categorize logs by categories which can be enabled/disabled independently
@@ -119,6 +124,8 @@ enum class category
 
 	work_generation,
 	// ...
+
+	_last // Must be the last enum
 };
 }
 
@@ -140,3 +147,19 @@ nano::log::detail parse_detail (std::string_view);
 std::vector<nano::log::level> const & all_levels ();
 std::vector<nano::log::type> const & all_types ();
 }
+
+// Ensure that the enum_range is large enough to hold all values (including future ones)
+template <>
+struct magic_enum::customize::enum_range<nano::log::type>
+{
+	static constexpr int min = 0;
+	static constexpr int max = 128;
+};
+
+// Ensure that the enum_range is large enough to hold all values (including future ones)
+template <>
+struct magic_enum::customize::enum_range<nano::log::detail>
+{
+	static constexpr int min = 0;
+	static constexpr int max = 512;
+};

nano/lib/stats_enums.cpp

@@ -1,8 +1,5 @@
 #include <nano/lib/stats_enums.hpp>
 
-#define MAGIC_ENUM_RANGE_MIN 0
-#define MAGIC_ENUM_RANGE_MAX 256
-
 #include <magic_enum.hpp>
 
 std::string_view nano::to_string (nano::stat::type type)

nano/lib/stats_enums.hpp

@@ -3,6 +3,8 @@
 #include <cstdint>
 #include <string_view>
 
+#include <magic_enum.hpp>
+
 namespace nano::stat
 {
 /** Primary statistics type */
@@ -67,6 +69,8 @@ enum class detail : uint8_t
 	broadcast,
 	cleanup,
 	top,
+	none,
+	success,
 
 	// processing queue
 	queue,
@@ -172,7 +176,7 @@ enum class detail : uint8_t
 	invalid_frontier_req_message,
 	invalid_asc_pull_req_message,
 	invalid_asc_pull_ack_message,
-	message_too_big,
+	message_size_too_big,
 	outdated_version,
 
 	// tcp
@@ -210,7 +214,7 @@ enum class detail : uint8_t
 	requests_unknown,
 
 	// duplicate
-	duplicate_publish,
+	duplicate_publish_message,
 
 	// telemetry
 	invalid_signature,
@@ -317,3 +321,19 @@ std::string_view to_string (stat::type);
 std::string_view to_string (stat::detail);
 std::string_view to_string (stat::dir);
 }
+
+// Ensure that the enum_range is large enough to hold all values (including future ones)
+template <>
+struct magic_enum::customize::enum_range<nano::stat::type>
+{
+	static constexpr int min = 0;
+	static constexpr int max = 128;
+};
+
+// Ensure that the enum_range is large enough to hold all values (including future ones)
+template <>
+struct magic_enum::customize::enum_range<nano::stat::detail>
+{
+	static constexpr int min = 0;
+	static constexpr int max = 512;
+};

nano/lib/utility.hpp

@@ -14,6 +14,7 @@
 #include <sstream>
 #include <vector>
 
+#include <magic_enum.hpp>
 #include <magic_enum_containers.hpp>
 
 namespace boost
@@ -262,4 +263,38 @@ std::string to_str (T const & val)
 {
 	return boost::lexical_cast<std::string> (val);
 }
+
+/**
+ * Same as `magic_enum::enum_values (...)` but ignores reserved values (starting with underscore)
+ */
+template <class E>
+std::vector<E> enum_values ()
+{
+	std::vector<E> result;
+	for (auto const & [val, name] : magic_enum::enum_entries<E> ())
+	{
+		if (!name.starts_with ('_'))
+		{
+			result.push_back (val);
+		}
+	}
+	return result;
+}
+
+/**
+ * Same as `magic_enum::enum_cast (...)` but ignores reserved values (starting with underscore).
+ * Case insensitive.
+ */
+template <class E>
+std::optional<E> parse_enum (std::string_view name)
+{
+	if (name.starts_with ('_'))
+	{
+		return std::nullopt;
+	}
+	else
+	{
+		return magic_enum::enum_cast<E> (name, magic_enum::case_insensitive);
+	}
+}
 }

nano/node/active_transactions.cpp

@@ -324,7 +324,7 @@ void nano::active_transactions::cleanup_election (nano::unique_lock<nano::mutex>
 
 	lock_a.unlock ();
 
-	node.stats.inc (completion_type (*election), nano::to_stat_detail (election->behavior ()));
+	node.stats.inc (completion_type (*election), to_stat_detail (election->behavior ()));
 
 	vacancy_update ();
 
@@ -461,7 +461,7 @@ nano::election_insertion_result nano::active_transactions::insert (std::shared_p
 		{
 			cache->fill (result.election);
 		}
-		node.stats.inc (nano::stat::type::active_started, nano::to_stat_detail (election_behavior_a));
+		node.stats.inc (nano::stat::type::active_started, to_stat_detail (election_behavior_a));
 		node.observers.active_started.notify (hash);
 		vacancy_update ();
 	}

nano/node/election.cpp

@@ -5,6 +5,8 @@
 
 #include <boost/format.hpp>
 
+#include <magic_enum.hpp>
+
 using namespace std::chrono;
 
 std::chrono::milliseconds nano::election::base_latency () const
@@ -693,24 +695,9 @@ std::vector<nano::vote_with_weight_info> nano::election::votes_with_weight () co
 
 nano::stat::detail nano::to_stat_detail (nano::election_behavior behavior)
 {
-	switch (behavior)
-	{
-		case nano::election_behavior::normal:
-		{
-			return nano::stat::detail::normal;
-		}
-		case nano::election_behavior::hinted:
-		{
-			return nano::stat::detail::hinted;
-		}
-		case nano::election_behavior::optimistic:
-		{
-			return nano::stat::detail::optimistic;
-		}
-	}
-
-	debug_assert (false, "unknown election behavior");
-	return {};
+	auto value = magic_enum::enum_cast<nano::stat::detail> (magic_enum::enum_name (behavior));
+	debug_assert (value);
+	return value.value_or (nano::stat::detail{});
 }
 
 nano::election_behavior nano::election::behavior () const