[PATCH] D23035: [Support] Add LLVM_BITFIELD_WIDTH and LLVM_CHECKED_BITFIELD_ASSIGN macros.

[Reid Kleckner via llvm-commits]

rnk added a comment.

In https://reviews.llvm.org/D23035#502871, @jlebar wrote:

> > I think we should just encourage this pattern
>
>
> Do you think it would be an improvement to rewrite clang's Expr.h to this pattern?  Or, more to my point, https://reviews.llvm.org/D23036?  It seems kind of wordy to me, but I'm open to thoughts.


You only end up using LLVM_CHECKED_BITFIELD_ASSIGN four times in that patch, so I'm not sure it's worth factoring the assertion out into a macro is worth it.

> I don't like that this macro obfuscates the code, but I do like that we don't have to rely on people writing correct check-in-range code.  Like, it wasn't at all obvious to me that `foo.x = y; assert(foo.x == y);` was correct, or that it *wouldn't* be correct if the bitfield were signed.  The macro gets that stuff right, which I like.


Either way we have an education problem: either remember this library thing, or remember that round-tripping through the bitfield is the least fragile way to range check it. It's a *little* verbose, but there's enough less magic that I like writing it out a bit more.


https://reviews.llvm.org/D23035