[PATCH] D82058: [ADT] Add Bitfield utilities - alternative design

[Clement Courbet via Phabricator via llvm-commits]

courbet added inline comments.


================
Comment at: llvm/unittests/ADT/BitFieldsTest.cpp:21
+  using Bool = Bitfield<bool, 0, 1>;
+  bitfield<Bool>(Storage) = true;
+  EXPECT_EQ(Storage, 0b00000001);
----------------
gchatelet wrote:
> courbet wrote:
> > I think @serge-sans-paille was suggesting to go one step further : do away with the `bitfield` function, and replace it with the ctor directly: `Bool(Storage) = true`.
> Yes unfortunately this is not possible as `Storage` type needs to be deduced somehow.
> e.g.
> ```
> template<typename UserType, unsigned Offset, unsigned Size, UserType MaxValue = ...>
> struct Bitfield {
>   Bitfield(StorageType &Storage) // where does StorageType comes from?
> };
> ```
> 
> The type would need to be part of the template arguments.
> e.g.
> ```
> template<typename StorageType, typename UserType, unsigned Offset, unsigned Size, UserType MaxValue = ...>
> struct Bitfield {
>   Bitfield(StorageType &Storage) //OK
> };
> ```
> But I believe this is confusing to declare (e.g. `Bitfield<uint64_t, uint_8t, 0, 4>`) and would require a lot of editing in case the `StorageType` changes.
> 
> The function is the trick to deduce `StorageType` automatically.
That makes sense. Then maybe at least make the function static and part of the class ?

```
using Bool = Bitfield<bool, 0, 1>;
Bool::fieldOf(Storage) = true;
```

That's a midpoint between the current proposal:

```
using Bool = Bitfield<bool, 0, 1>;
bitfield<Bool>(Storage) = true;
```

And the previous one:

```
using Bool = Bitfield<bool, 0, 1>;
Bool::setField(Storage, true);
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82058