[libc-commits] [PATCH] D124959: [libc] add uint128 implementation
Michael Jones via Phabricator via libc-commits
libc-commits at lists.llvm.org
Wed May 11 15:20:26 PDT 2022
michaelrj marked an inline comment as done.
michaelrj added inline comments.
================
Comment at: libc/src/__support/FPUtil/UInt.h:60
public:
- UInt() { kind = Valid; }
+ constexpr UInt() {}
----------------
lntue wrote:
> If you remove trivial implementations of the default and copy constructors, does it still work correctly?
No, it throws errors in the tests.
================
Comment at: libc/test/src/__support/uint128_test.cpp:13
+
+// while I could use "ASSERT_TRUE(a == b)", this will tell me what the values
+// actually are.
----------------
sivachandra wrote:
> michaelrj wrote:
> > lntue wrote:
> > > You should be able to add specialization for UInt128 in `LibcTest.cpp` and use ASSERT_EQ, EXPECT_EQ as usual. Also disable the `__uint128_t` variant over there when it's not defined.
> > I could make `ASSERT_EQ` support my custom UInt128, but that would add a dependency for effectively no gain, since UInt128 shouldn't be called by name in any other test file. Also, the `__uint128_t` code is already disabled for `LibcTest.cpp`
> By "disabled" do you mean the conditional on `__SIZEOF_INT128__`? If yes, you should add the `else` branches for it with this specialization. To keep everything more logically organized, you should probably move the `UInt.h` header file to `src/__support/CPP`.
I've moved `UInt.h` into `CPP`, and I've added specializations to `LibcTest`, but they're not conditional since this class is always available.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124959/new/
https://reviews.llvm.org/D124959
More information about the libc-commits
mailing list