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

[Siva Chandra via Phabricator via libc-commits]

sivachandra 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;
----------------
Not sure if there is a need to initialize from a bigger array. If not, can we restrict this constructor with `enable_if`?


================
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;
----------------
Is the ternary operation correct?


================
Comment at: libc/src/__support/CPP/UInt.h:81
+  template <size_t SmallerBits>
+  UInt<Bits> &operator=(const UInt<SmallerBits> &other) {
+    static_assert(SmallerBits < Bits);
----------------
Instead of a `static_assert`, can we use `enable_if` to restrict this overload?


================
Comment at: libc/src/__support/CPP/UInt.h:279
+    }
+    if (other == UInt<Bits>(0)) { // TODO: Make this error correctly.
+      return remainder;
----------------
`cpp::optional`?


================
Comment at: libc/src/__support/CPP/UInt.h:283
+
+    int curBit = 0;
+    UInt<Bits> quotient(0);
----------------
Naming style.


================
Comment at: libc/src/__support/CPP/UInt.h:286-287
+    UInt<Bits> subtractor = other;
+    for (; static_cast<unsigned>(curBit) < Bits && subtractor < *this &&
+           subtractor << 1 > subtractor;
+         ++curBit, subtractor.shift_left(1)) {
----------------
This expression is a bit complicated to parse. Can you put it in a lambda function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132184