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

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: llvm/include/llvm/ADT/Bitfields.h:204
+  // In case sizeof(bool) != 1, replace `void` by an additionnal
+  // std::conditionnal.
+  using type = std::conditional<sizeof(bool) == 1, uint8_t, void>::type;
----------------
typo: conditionnal


================
Comment at: llvm/include/llvm/ADT/Bitfields.h:212
+struct Bitfield {
+
+  /// Describes an element of a Bitfield. This type is then used with
----------------
nit: Why the newline here?


================
Comment at: llvm/include/llvm/ADT/Bitfields.h:214
+  /// Describes an element of a Bitfield. This type is then used with
+  /// Bitfield::get \param T, the type of the field once in unpacked form,
+  /// \param Offset, the position of the first bit,
----------------
Did you intend for `\param T` to be on the next line?


================
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
----------------
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;
};
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82454