[PATCH] D117114: [llvm][ADT] Implement `BitVector(std::initializer_list)`

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D117114#3238494 <https://reviews.llvm.org/D117114#3238494>, @dexonsmith wrote:

> It might be awkward to add this new ambiguity. I wonder if we really want to copy that from `std::vector<bool>`, which doesn't have a great API for bitsets anyway. @dblaikie, any thoughts?

Re: the ambiguity: We could pre-empt that in a separate commit by making the ctor "explicit" - honestly I think we shouldn't be using {} to call user-defined ctors that aren't representative of/"Feel like" conversions. (wouldn't mind that as a broad style statement for LLVM and/or in the form of "mark multi-arg ctors explicit unless they're intentionally converting/structural constructors".

What aspect of "not greatness" API design do you mean - having both the `(unsigned bool)` and init list ctors at the same time? I think with the former being marked as explicit it's /probably/ OK to me? () does "interesting" things, {} represents the value in some sense.

I wouldn't object to a named ctor either, though. (for either case - though I sort of feel like the existing `(unsigned, bool)` is the one that could use a name moreso than the new one (if the new one is only via init list, at least - if you could spell the new one as "SV(true, false, true, true, ...)" yeah, maybe having a name would be good))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117114