[PATCH] D82454: [ADT] Add Bitfield utilities - design #3

[River Riddle via Phabricator via llvm-commits]

rriddle marked an inline comment as done.
rriddle added inline comments.


================
Comment at: llvm/include/llvm/ADT/Bitfields.h:219
+  template <typename T, unsigned Offset, unsigned Size,
+            T MaxValue = std::is_enum<T>::value
+                             ? T(0) // coupled with static_assert below
----------------
gchatelet wrote:
> rriddle wrote:
> > You could likely simplify this with SFINAE:
> > 
> > ```
> > template <typename T, unsigned Offset, unsigned Size,
> >         T MaxValue = std::numeric_limits<T>::max(),
> >         bool IsEnum = std::is_enum<T>::value>
> > struct Element {
> > };
> > 
> > template <typename T, unsigned Offset, unsigned Size, T MaxValue>
> > struct Element<T, Offset, Size, MaxValue, true>
> >    : public Element<typename std::underlying_type<T>::type, Offset, Size, MaxValue> {
> >   using Type = T;
> > };
> > ```
> I've tried that but we need a few more tweaks to make it compile:
>  - cast `MaxValue` to the underlying type in the enum specialization,
>  - add a `using` directive to be able to access `IntegerType`at instantiation time.
> 
> With this, it's not that much simpler. Especially with the fact that you need to consider more code to understand the whole thing. WDYT?
> 
> ```
>   template <typename T, unsigned Offset, unsigned Size,
>             T MaxValue = std::numeric_limits<T>::max(),
>             bool IsEnum = std::is_enum<T>::value>
>   struct Element {
>   };
> 
>   template <typename T, unsigned Offset, unsigned Size, T MaxValue>
>   struct Element<T, Offset, Size, MaxValue, true>
>       : public Element<typename std::underlying_type<T>::type, Offset, Size,
>                        static_cast<typename std::underlying_type<T>::type>(
>                            MaxValue)> {
>     using Type = T;
> 
>   private:
>     using BaseT =
>         Element<typename std::underlying_type<T>::type, Offset, Size,
>                 static_cast<typename std::underlying_type<T>::type>(MaxValue)>;
>     static_assert(std::is_unsigned<typename BaseT::IntegerType>::value,
>                   "Enum must be unsigned");
>   };
> ```
SGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82454/new/

https://reviews.llvm.org/D82454