simple_enum icon indicating copy to clipboard operation
simple_enum copied to clipboard

make enum bounds declaration required for any use with simple enum functions

Open vilchanskyio-work opened this issue 3 months ago • 9 comments

Hi, thanks for wring this library.

Unfortunately, it suffers from the same issue as magic_enum, trying to static_cast non-existing enum values, albeit it doesn't spam 120+ warnings, but only 3:

<redacted>/simple_enum/simple_enum.hpp:339:89:   in 'constexpr' expansion of 'simple_enum::v0_9::detail::prepare_enum_meta_info<QCanBusDevice::CanBusError, 0, 10>()'
<redacted>/simple_enum/simple_enum.hpp:303:50: warning: the result of the conversion is unspecified because '(1 + 7)' is outside the range of type 'QCanBusDevice::CanBusError' [-Wconversion]
  303 |     (..., cont_pass<static_cast<enum_type>(first + utype(indices))>(meta[indices + 1], enum_beg));
      |                                            ~~~~~~^~~~~~~~~~~~~~~~
<redacted>/simple_enum/simple_enum.hpp:303:50: warning: the result of the conversion is unspecified because '(1 + 8)' is outside the range of type 'QCanBusDevice::CanBusError' [-Wconversion]
<redacted>/simple_enum/simple_enum.hpp:303:50: warning: the result of the conversion is unspecified because '(1 + 9)' is outside the range of type 'QCanBusDevice::CanBusError' [-Wconversion]

For reference QCanBusDevice::CanBusError is defined as:

    enum CanBusError {
        NoError,
        ReadError,
        WriteError,
        ConnectionError,
        ConfigurationError,
        UnknownError,
        OperationError,
        TimeoutError
    };

Is it possible to address this?

vilchanskyio-work avatar Sep 18 '25 11:09 vilchanskyio-work

The library is meant to be used with "last" defined.

enum CanBusError {
        NoError,
        ReadError,
        WriteError,
        ConnectionError,
        ConfigurationError,
        UnknownError,
        OperationError,
        TimeoutError,
        last = TimeoutError   // <--- add this line
    };

so you don't get out of range casts, and possibly incorrect behavior as the default reflection range is [0-10] which is quite small, and if you had added 3 more error enums the last one wouldn't have been reflected and you would get silent runtime errors.

another workaround to just silence the warning (not recommended) is mentioning the underlying type.

enum CanBusError : int {
//                 ^^^

if you cannot edit the enum definition (because it is 3rd party) then you can use an external definition (recommended solution)

template<>
struct simple_enum::info<CanBusError>
{
    static constexpr auto first = CanBusError::NoError;
    static constexpr auto last = CanBusError::TimeoutError;
};

ZXShady avatar Sep 19 '25 02:09 ZXShady

Thanks! It didn't occur to me that I must define the range for every enum.

vilchanskyio-work avatar Sep 19 '25 09:09 vilchanskyio-work

Thanks! It didn't occur to me that I must define the range for every enum.

It is the main difference between this library and others, if you don't want to do that then you could use the magic_enum library or others like it like (enchantum and conjure enum)

there is also this line of code inside simple_enum

#ifndef SIMPLE_ENUM_CUSTOM_UNBOUNDED_RANGE
#define SIMPLE_ENUM_CUSTOM_UNBOUNDED_RANGE
inline constexpr auto default_unbounded_upper_range = 10;
#endif

Which could allow you to edit the default range. by doing

// before including any headers from simple enum

#define SIMPLE_ENUM_CUSTOM_UNBOUNDED_RANGE

namespace simple_enum::inline v0_9
{
inline constexpr auto default_unbounded_upper_range = XXX; 
}

This changes the default range from [0,10] to [0,XXX].

ZXShady avatar Sep 19 '25 12:09 ZXShady

    enum CanBusError {
        NoError,
        ReadError,
        WriteError,
        ConnectionError,
        ConfigurationError,
        UnknownError,
        OperationError,
        TimeoutError
    };

Is it possible to address this?

I wrote it mainly for bounded use add

consteval auto adl_enum_bounds(CanBusError)
  {
  using enum CanBusError;
  return simple_enum::adl_info{NoError, TimeoutError};
  }

arturbac avatar Oct 15 '25 20:10 arturbac

if you cannot edit the enum definition (because it is 3rd party) then you can use an external definition (recommended solution)

template<> struct simple_enum::info<CanBusError> { static constexpr auto first = CanBusError::NoError; static constexpr auto last = CanBusError::TimeoutError; };

yep, but more comfortable as I mentioned here is to use adl definition, and I would suggest using it instead, because You can do it in the namespace of CanBusError

consteval auto adl_enum_bounds(CanBusError)
  {
  using enum CanBusError;
  return simple_enum::adl_info{NoError, TimeoutError};
  }

I always define bounds for enum as it gives the fastest compile time processing and allows me to control enums what part of them is valid, and it doesn't matter then if it starts at negative number, 0 or positive. such case will compile with same speed as when a is 0 when You declare bounds.

enum struct X : uint32_t
{
 a = 115678,
 b,
 c
};

consteval auto adl_enum_bounds(X)
  {
  using enum X;
  return simple_enum::adl_info{a, c};
  }

arturbac avatar Oct 15 '25 21:10 arturbac

if you cannot edit the enum definition (because it is 3rd party) then you can use an external definition (recommended solution)

template<> struct simple_enum::info<CanBusError> { static constexpr auto first = CanBusError::NoError; static constexpr auto last = CanBusError::TimeoutError; };

yep, but more comfortable as I mentioned here is to use adl definition, and I would suggest using it instead, because You can do it in the namespace of CanBusError

consteval auto adl_enum_bounds(CanBusError)
  {
  using enum CanBusError;
  return simple_enum::adl_info{NoError, TimeoutError};
  }

I always define bounds for enum as it gives the fastest compile time processing and allows me to control enums what part of them is valid, and it doesn't matter then if it starts at negative number, 0 or positive. such case will compile with same speed as when a is 0 when You declare bounds.

enum struct X : uint32_t
{
 a = 115678,
 b,
 c
};

consteval auto adl_enum_bounds(X)
  {
  using enum X;
  return simple_enum::adl_info{a, c};
  }

I am myself against adl approaches because you can have a silent bug easily

catch the mistake

consteval auto adl_enum_buonds(X)
{
using enum X;
return simple_enum::adl_info{a, c};
}

The bug is

.

. . . . . . . . . . . . . . . . . . . . . . .

I mispelled the function name it should be "bounds" not "buonds"

and you won't get any errors at all.

ZXShady avatar Oct 15 '25 21:10 ZXShady

and you won't get any errors at all.

Thanks for pointing that, but that`s not always correct. But I think adding cmake option of strict build mode that will require for most things in simple enum bounds to be declared with removal of auto scan range at same time would be a good idea.

There are functionalities already in simple enum which require enum to be bounded and You will get compile time error in such case like generic_error_category, enum_enumerations, enum_names, min, max.

When You use adl declaration for bounds and error decl

enum class some_error
  {
  success = 0,
  failed_other_reason,
  unknown
  };

consteval auto adl_enum_buonds(some_error)
  {
  using enum some_error;
  return simple_enum::adl_info{success, unknown, true};
  }
template class simple_enum::generic_error_category<some_error>;
error: constraints not satisfied for class template 'generic_error_category' [with ErrorEnum = some_error]

And You can always add

static_assert(simple_enum::bounded_enum<some_error>);

even inside adl_enum_bounds

consteval auto adl_enum_bounds(some_error)
  {
  using enum some_error;
  return simple_enum::adl_info{success, unknown, true};
  static_assert(simple_enum::bounded_enum<some_error>);
  }

arturbac avatar Oct 16 '25 05:10 arturbac

and you won't get any errors at all.

Thanks for pointing that, but that`s not always correct. But I think adding cmake option of strict build mode that will require for most things in simple enum bounds to be declared with removal of auto scan range at same time would be a good idea.

That's my idea as well. disable auto scan range since it is so tiny anyways.

and force to set the range

this is a breaking change but I argue your code is already broken if you don't set the ranges

ZXShady avatar Oct 16 '25 11:10 ZXShady

I totally agree, this is my way of doing things, find errors early by compiler not at runtime on production.

arturbac avatar Oct 16 '25 16:10 arturbac