[libc-commits] [PATCH] D132184: [libc] add division to UInt

[Michael Jones via Phabricator via libc-commits]

michaelrj added inline comments.


================
Comment at: libc/src/__support/CPP/UInt.h:41
+  // Construct a UInt from a C array.
+  template <size_t N> constexpr UInt(const uint64_t (&nums)[N]) {
+    size_t min_wordcount = N > WordCount ? N : WordCount;
----------------
lntue wrote:
> sivachandra wrote:
> > Not sure if there is a need to initialize from a bigger array. If not, can we restrict this constructor with `enable_if`?
> I think a better option that you're looking for is an implicit constructors / assignment from `UInt<M>` to `UInt<N>` with `M <= N`
Re: sivachandra, we can't use `enable_if` here because `typetraits.h` depends on `UInt.h`.
Re: lntue: I've added that as well, since it's also useful, but I need the ability to initialize from a C array as well.


================
Comment at: libc/src/__support/CPP/UInt.h:42
+  template <size_t N> constexpr UInt(const uint64_t (&nums)[N]) {
+    size_t min_wordcount = N > WordCount ? N : WordCount;
+    size_t i = 0;
----------------
sivachandra wrote:
> Is the ternary operation correct?
oops, flipped.


================
Comment at: libc/src/__support/CPP/UInt.h:81
+  template <size_t SmallerBits>
+  UInt<Bits> &operator=(const UInt<SmallerBits> &other) {
+    static_assert(SmallerBits < Bits);
----------------
sivachandra wrote:
> Instead of a `static_assert`, can we use `enable_if` to restrict this overload?
No. TypeTraits depends on UInt, so UInt can't use anything from TypeTraits. This includes `enable_if`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132184