From a00657737449e7c61cff1355a4cb5677e6d0b7f3 Mon Sep 17 00:00:00 2001 From: Anton Bachin Date: Wed, 29 Apr 2015 03:32:18 -0400 Subject: [PATCH] Interface simplification - collapsed _Internal and Enum and reduced number of names in class scope. --- Enum.h | 70 ++---------- EnumInternal.h | 296 ++++++++++++++++++++++++------------------------- 2 files changed, 158 insertions(+), 208 deletions(-) diff --git a/Enum.h b/Enum.h index 0bf701d..47434a8 100644 --- a/Enum.h +++ b/Enum.h @@ -21,70 +21,20 @@ // operators do, and define the operators in terms of those. // TODO Remove implicit conversion to underlying type. // TODO Rename Underlying so it doesn't clash with any value names. +// TODO _first, _last, _count. +// TODO Try to make names and values into values instead of functions. +// TODO Move the member value into the base class and define most functions +// there. +// TODO Make sure to cast to the underlying type in to_int. #define ENUM(EnumType, UnderlyingType, ...) \ _ENUM_ARRAYS(EnumType, UnderlyingType, __VA_ARGS__); \ \ - class EnumType : public _enum::_Internal { \ - protected: \ - using _Super = _enum::_Internal; \ - \ - public: \ - static constexpr _Value _min = (_Value)_values[_minIndex]; \ - static constexpr _Value _max = (_Value)_values[_maxIndex]; \ - \ - static constexpr size_t _span = _max - _min + 1; \ - \ - static ValueIterable _values() { return _Super::values(); } \ - static NameIterable _names() { return _Super::names(); } \ - \ - const char* to_string() const { return desc(*this); } \ - static EnumType _from_string(const char *name) \ - { return _Super::find(name); } \ - static EnumType _from_string_nocase(const char *name) \ - { return _Super::caseFind(name); } \ - \ - Underlying to_int() const { return _value; } \ - static EnumType _from_int(Underlying value) \ - { return EnumType(value); } \ - static EnumType _from_int_unchecked(Underlying value) \ - { return EnumType(value); } \ - \ - template \ - static bool _is_valid(IntegralType value) \ - { return _Super::valid(value); } \ - static bool _is_valid(const char *name) \ - { return _Super::valid(name); } \ - static bool _is_valid_nocase(const char *name) \ - { return _Super::caseValid(name); } \ - \ - EnumType() = delete; \ - EnumType(_Value value) : _value(value) { } \ - template \ - explicit EnumType(IntegralType value, \ - typename \ - std::enable_if< \ - std::is_integral::value> \ - ::type *dummy = nullptr) : _value(value) \ - { } \ - \ - template \ - EnumType& operator =(IntegralType value) \ - { *this = EnumType(value); return *this; } \ - \ - operator _Value () const { return (_Value)_value; } \ - Underlying toUnderlying() const { return (Underlying)_value; } \ - \ - protected: \ - Underlying _value; \ - \ + class EnumType : public _enum::_Implementation { \ + using _Super = _enum::_Implementation; \ + using _Super::_Super; \ friend _Super; \ - friend ValueIterable; \ + friend _Super::_ValueIterable; \ }; \ \ - static_assert(sizeof(EnumType) == sizeof(UnderlyingType), \ - "internal error: enum type does not have same size as its " \ - "underlying type"); \ - static_assert(alignof(EnumType) == alignof(UnderlyingType), \ - "internal error: enum type does not have same alignment " \ - "requirement as its underlying type"); + _ENUM_STATIC_DEFINITIONS(EnumType); diff --git a/EnumInternal.h b/EnumInternal.h index b4c69a0..a1b63da 100644 --- a/EnumInternal.h +++ b/EnumInternal.h @@ -38,6 +38,8 @@ /// instantiated once for every different underlying type. Underlying types /// are very likely to collide. +// TODO Make it possible for enums to be map keys. + #pragma once @@ -65,8 +67,11 @@ namespace _enum { // Forward declaration of _Internal, for use in a friend declation in _Iterable. -template class _Internal; +template class _Implementation; +// TODO Simplify - names iteration will need a custom iterator if processing +// strings lazily, while values can be done by simply iterating over the values +// array directly. /// Template for iterable objects over enum names and values. /// /// The iterables are intended for use with C++11 `for-each` syntax. They are @@ -97,8 +102,6 @@ template class _Internal; /// expeted. /// /// @todo Consider making `_Iterable` `constexpr`. -/// @todo An iterator over valid values and an iterator over all values should -/// really be different types. /// /// @internal /// @@ -125,7 +128,7 @@ class _Iterable { /// @return A reference to itself. iterator& operator ++() { - if (_index < EnumType::_rawSize) + if (_index < EnumType::_size) ++_index; return *this; @@ -177,7 +180,7 @@ class _Iterable { /// Returns an iterator to the end of the name or value array. iterator end() const { - return iterator(_arrayPointer, EnumType::_rawSize); + return iterator(_arrayPointer, EnumType::_size); } /// Returns the number of valid elements (names or values) in the iterable - @@ -193,7 +196,7 @@ class _Iterable { ArrayType _arrayPointer; /// Permit the enum class itself to create `_Iterable` objects. - friend class _Internal; + friend class _Implementation; }; @@ -329,60 +332,28 @@ constexpr bool _namesMatch(const char *stringizedName, /// maximum declared enum values, and the total number of valid enum values. namespace _range { -/// Type of object returned by `_minMax`. Pair of the minimum and maximum value -/// found. -class _MinMax { - public: - size_t min, max; - - constexpr _MinMax(size_t _min, size_t _max) : - min(_min), max(_max) { } -}; - -/// Compile-time function that finds the default minimum and maximum values of -/// an enum. Note that if the minimum and/or maximum value is overridden using -/// `_min` and `_max`, the corresponding result of this function will be -/// ignored. -/// -/// This function should be called with `bestMin` and `bestMax` set to the first -/// valid (non-special, non-bad) index in the enumeration. One such index is -/// guaranteed to exist by code that runs prior to where this function is -/// called. -/// -/// @tparam UnderlyingType The enum underlying type. Comparisons are done at -/// this type. Note that the signedness of this type affects the -/// comparisons. -/// @param values Enum values array. -/// @param valueCount Number of values. -/// @param specialIndices Special index array. -/// @param specialIndexCount Number of special indices. -/// @param badValue The bad value. -/// @param index Current index in the iteration. This should initially be set to -/// the index after the first valid index (add one to it). -/// @param bestMin Index of the lowest valid value found so far. -/// @param bestMax Index of the highest valid value found so far. template -constexpr _MinMax _minMax(const UnderlyingType *values, size_t valueCount, - size_t index, size_t bestMin, size_t bestMax) +constexpr UnderlyingType _findMin(const UnderlyingType *values, + size_t valueCount, size_t index, + UnderlyingType best) { return - // If the current index is at the end of the array, return the pair of - // the best found minimum and maximum. - index == valueCount ? _MinMax(bestMin, bestMax) : - // If the current value is higher than the best max so far, continue at - // the next index with the best max index updated to the current index. - // Note that it is not necessary to also check if the current value is - // less than the best min - the min and max start at the same value, and - // the min can never go above the max after that. This is an - // optimization that saves a nontrivial amount of time. - values[index] > values[bestMax] ? - _minMax(values, valueCount, index + 1, bestMin, index) : - // Otherwise, if the current value is not higher than the min, continue - // at the next index. If the current value is less than the best min so - // far, then do update the best min for the recursive call. - _minMax(values, valueCount, index + 1, - values[index] < values[bestMin] ? index : bestMin, - bestMax); + index == valueCount ? best : + values[index] < values[valueCount] ? + _findMin(values, valueCount, index + 1, values[index]) : + _findMin(values, valueCount, index + 1, best); +} + +template +constexpr UnderlyingType _findMax(const UnderlyingType *values, + size_t valueCount, size_t index, + UnderlyingType best) +{ + return + index == valueCount ? best : + values[index] > values[valueCount] ? + _findMax(values, valueCount, index + 1, values[index]) : + _findMax(values, valueCount, index + 1, best); } // TODO This can probably now be replaced with a sizeof on the array. @@ -438,8 +409,8 @@ namespace _enum { // TODO Consider reserving memory statically. This will probably entail a great // compile-time slowdown, however. -static const char * const* _processNames(const char * const *rawNames, - size_t count) +static inline const char * const* _processNames(const char * const *rawNames, + size_t count) { // Allocate the replacement names array. const char **processedNames = new const char*[count]; @@ -484,6 +455,8 @@ static const char * const* _processNames(const char * const *rawNames, template class _GeneratedArrays; +// TODO Try to convert values array to _Enumerated[] instead of _Integral[]. + #define _ENUM_ARRAYS(EnumType, UnderlyingType, ...) \ class EnumType; \ \ @@ -491,125 +464,142 @@ template class _GeneratedArrays; \ template <> \ class _GeneratedArrays { \ - public: \ - enum _Value { __VA_ARGS__ }; \ + protected: \ + using _Integral = UnderlyingType; \ \ - using Underlying = UnderlyingType; \ + public: \ + enum _Enumerated : _Integral { __VA_ARGS__ }; \ \ protected: \ - static constexpr Underlying _values[] = \ - { _ENUM_EAT_ASSIGN(UnderlyingType, __VA_ARGS__) }; \ + static constexpr _Integral _value_array[] = \ + { _ENUM_EAT_ASSIGN(_Integral, __VA_ARGS__) }; \ \ - static constexpr const char *_names[] = \ + static constexpr const char *_name_array[] = \ { _ENUM_STRINGIZE(__VA_ARGS__) }; \ \ - static constexpr size_t _rawSize = \ + static constexpr size_t _size = \ _ENUM_PP_COUNT(__VA_ARGS__); \ }; \ \ - constexpr _GeneratedArrays::Underlying _ENUM_WEAK \ - _GeneratedArrays::_values[]; \ - \ - constexpr const char * _ENUM_WEAK _GeneratedArrays::_names[]; \ + constexpr _GeneratedArrays::_Integral _ENUM_WEAK \ + _GeneratedArrays::_value_array[]; \ \ + constexpr const char * _ENUM_WEAK \ + _GeneratedArrays::_name_array[]; \ \ template <> \ - const char * const * _ENUM_WEAK _Internal::_processedNames = \ - nullptr; \ + const char * const * _ENUM_WEAK \ + _Implementation::_processedNames = nullptr; \ \ } -// TODO Compute first index for iteration while computing range properties. -// TODO Eliminate distinction between size and rawSize. +// TODO Make sure _size is public. template -class _Internal : public _GeneratedArrays { +class _Implementation : public _GeneratedArrays { protected: using _arrays = _GeneratedArrays; - using _arrays::_values; - using _arrays::_names; - using _arrays::_rawSize; + using _arrays::_value_array; + using _arrays::_name_array; public: - using typename _arrays::_Value; - using typename _arrays::Underlying; + using _arrays::_size; + + static_assert(_size > 0, "no constants defined in enum type"); + + public: + using typename _arrays::_Enumerated; + using typename _arrays::_Integral; + + static const EnumType _first; + + _Implementation() = delete; + constexpr _Implementation(_Enumerated constant) : _value(constant) { } + + constexpr _Integral to_int() const + { + return _value; + } + + constexpr static EnumType _from_int(_Integral value) + { + return (EnumType)value; + } + + // TODO Use iterables. + const char* to_string() const + { + _processNames(); + + for (size_t index = 0; index < _size; ++index) { + if (_value_array[index] == _value) + return _processedNames[index]; + } + + throw std::domain_error("Enum::_to_string: invalid enum value"); + } + + // TODO Make constexpr. + static EnumType _from_string(const char *name) + { + _processNames(); + + for (size_t index = 0; index < _size; ++index) { + if (strcmp(_processedNames[index], name) == 0) + return (EnumType)_value_array[index]; + } + + throw std::exception(); + } + + // TODO Make constexpr. + static EnumType _from_string_nocase(const char *name) + { + _processNames(); + + for (size_t index = 0; index < _size; ++index) { + if (strcasecmp(_processedNames[index], name) == 0) + return (EnumType)_value_array[index]; + } + + throw std::exception(); + } + + // TODO Eliminate cast inside this function. + operator _Enumerated() { return (_Enumerated)_value; } protected: - static_assert(_rawSize > 0, "no constants defined in enum type"); + _Integral _value; - static constexpr _enum::_range::_MinMax - _minMax = - _enum::_range::_minMax(_values, _rawSize, 1, 0, 0); - - static constexpr size_t _minIndex = _minMax.min; - static constexpr size_t _maxIndex = _minMax.max; - - static constexpr size_t _size = _rawSize; + explicit constexpr _Implementation(_Integral value) : _value(value) { } static const char * const *_processedNames; static void _processNames() { if (_processedNames == nullptr) - _processedNames = _enum::_processNames(_names, _rawSize); + _processedNames = _enum::_processNames(_name_array, _size); } - using ValueIterable = - _Iterable; - using NameIterable = + using _ValueIterable = + _Iterable; + using _NameIterable = _Iterable; - friend ValueIterable; - friend NameIterable; - - static ValueIterable values() + public: + static _ValueIterable _values() { - return ValueIterable(_values); + return _ValueIterable(_value_array); } - static NameIterable names() + static _NameIterable _names() { _processNames(); - return NameIterable(_processedNames); - } - - static const char* desc(EnumType value) - { - _processNames(); - - for (size_t index = 0; index < _rawSize; ++index) { - if (_values[index] == value) - return _processedNames[index]; - } - - throw std::domain_error("Enum::desc: invalid enum value"); - } - - static EnumType find(const char *name) - { - _processNames(); - - for (size_t index = 0; index < _rawSize; ++index) { - if (strcmp(_processedNames[index], name) == 0) - return (EnumType)_values[index]; - } - - throw std::exception(); - } - - static EnumType caseFind(const char *name) - { - _processNames(); - - for (size_t index = 0; index < _rawSize; ++index) { - if (strcasecmp(_processedNames[index], name) == 0) - return (EnumType)_values[index]; - } - - throw std::exception(); + return _NameIterable(_processedNames); } + protected: // See comment by _isIterableIndex. #ifdef __clang__ #pragma GCC diagnostic push @@ -618,12 +608,12 @@ class _Internal : public _GeneratedArrays { // TODO Do a real check here - look it up in the array. template - static bool valid(IntegralType value) + static bool _is_valid(IntegralType value) { static_assert(std::is_integral::value, "argument to EnumType::valid must have integral type"); static_assert(std::is_signed::value == - std::is_signed::value, + std::is_signed<_Integral>::value, "argument to EnumType::valid must be signed if and only " "if underlying type of EnumType is signed"); @@ -634,10 +624,10 @@ class _Internal : public _GeneratedArrays { #pragma GCC diagnostic pop #endif // #ifdef __clang__ - static bool valid(const char *name) + static bool _is_valid(const char *name) { try { - find(name); + _from_string(name); return true; } catch (const std::exception &e) { @@ -645,10 +635,10 @@ class _Internal : public _GeneratedArrays { } } - static bool caseValid(const char *name) + static bool _is_valid_nocase(const char *name) { try { - caseFind(name); + _from_string_nocase(name); return true; } catch (const std::exception &e) { @@ -656,40 +646,41 @@ class _Internal : public _GeneratedArrays { } } + // TODO Remove static casts wherever reasonable. public: bool operator ==(const EnumType &other) const { return static_cast(*this)._value == other._value; } - bool operator ==(const _Value value) const + bool operator ==(const _Enumerated value) const { return static_cast(*this)._value == value; } template bool operator ==(T other) const = delete; bool operator !=(const EnumType &other) const { return !(*this == other); } - bool operator !=(const _Value value) const + bool operator !=(const _Enumerated value) const { return !(*this == value); } template bool operator !=(T other) const = delete; bool operator <(const EnumType &other) const { return static_cast(*this)._value < other._value; } - bool operator <(const _Value value) const + bool operator <(const _Enumerated value) const { return static_cast(*this)._value < value; } template bool operator <(T other) const = delete; bool operator <=(const EnumType &other) const { return static_cast(*this)._value <= other._value; } - bool operator <=(const _Value value) const + bool operator <=(const _Enumerated value) const { return static_cast(*this)._value <= value; } template bool operator <=(T other) const = delete; bool operator >(const EnumType &other) const { return static_cast(*this)._value > other._value; } - bool operator >(const _Value value) const + bool operator >(const _Enumerated value) const { return static_cast(*this)._value > value; } template bool operator >(T other) const = delete; bool operator >=(const EnumType &other) const { return static_cast(*this)._value >= other._value; } - bool operator >=(const _Value value) const + bool operator >=(const _Enumerated value) const { return static_cast(*this)._value >= value; } template bool operator >=(T other) const = delete; @@ -713,4 +704,13 @@ class _Internal : public _GeneratedArrays { template int operator ||(T other) const = delete; }; +#define _ENUM_STATIC_DEFINITIONS(EnumType) \ + namespace _enum { \ + \ + template <> \ + constexpr EnumType _Implementation::_first = \ + EnumType::_from_int(EnumType::_value_array[0]); \ + \ + } + }