[libc-commits] [PATCH] D136228: [libc] Implement `hsearch` related facilities.

[Schrodinger ZHU Yifan via Phabricator via libc-commits]

SchrodingerZhu added a comment.

Add some questions in my own review.



================
Comment at: libc/src/__support/swisstable.h:113
+      result = -1;
+    if (unlikely(__builtin_mul_overflow(value, other.value, &result)))
+      result = -1;
----------------
__builtin_mul_overflow seems to be generally available among LLVM and GNU toolchains but not MSVC's. I am not sure current support plan. Is it better to provide a wrapper?


================
Comment at: libc/src/__support/swisstable.h:422
+    } else {
+      inline_memcpy(reinterpret_cast<char *>(&ctrl[num_buckets()]),
+                    reinterpret_cast<const char *>(&ctrl[0]), sizeof(Group));
----------------
There seems to be no wrapper of __builtin_memcpy_inline so I use inline_memcpy instead. will this be a stable interface?


================
Comment at: libc/test/src/__support/swisstable/group_test.cpp:1
+#include <cstddef>
+#if SWISSTABLE_TEST_USE_GENERIC_GROUP
----------------
This line was wrongly added


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136228