[libc-commits] [PATCH] D150211: [libc] Rework of cpp::optional to support more types

[Mikhail Ramalho via Phabricator via libc-commits]

mikhail.ramalho added a comment.

In D150211#4330579 <https://reviews.llvm.org/D150211#4330579>, @michaelrj wrote:

> Before I review this patch, I want to confirm that this is the right strategy. If the main goal is to support `Uint<>` in `optional` is there a way to do that without a complete replacement of `optional`?

We need these changes because of 
`constexpr optional<UInt<Bits>> UInt<T>::div(const UInt<Bits> &other)`, 
used by 
`constexpr UInt<Bits> UInt<T>::operator/(const UInt<Bits> &other) const`.

The optional only seems to be checked here: https://github.com/mikhailramalho/llvm-project/blob/main/libc/test/src/__support/uint_test.cpp#L204, so an alternative would be to remove the optional return from `UInt<T>::div`, delete this test and maybe abort when we divide by zero.

But I would also argue to keep these changes to optional, the main reason being to avoid copies like the fixed test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150211