[libc-commits] [PATCH] D85103: [libc] Add strspn implementation.

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/src/string/strspn.cpp:21
+
+static void toggle_bit(size_t *bitset, const size_t ch) {
+  const size_t mask = size_t{1} << (ch % exploded_unsigned_char_bit);
----------------
I would prefer a `bool` specialization of https://github.com/llvm/llvm-project/blob/master/libc/utils/CPP/Array.h for which the internal implementation is a bit array like `std::vector<bool>`.

Also, why not use `uintptr_t` array instead of `size_t` array?

```
static constexpr size_t BitsPerUnit = 8 * sizeof(uintptr_t);
uintptr_t Data[size / BitsPerUnit + size % BitsPerUnit]; // size is of type size_t

// Setter; x is of type size_t
Data[x / BitsPerUnit] |= (uintptr_t(1) << (x % BitsPerUnit));

// Getter; x is of type size_t
return Data[x / BitsPerUnit] & (uintptr_t(1) << (x % BitsPerUnit));
```


================
Comment at: libc/src/string/strspn.cpp:32-40
+  if (!segment[0])
+    return 0;
+
+  const char *initial = src;
+  if (!segment[1]) { // The segment is a single character.
+    for (; *src == *segment; ++src)
+      ;
----------------
cgyurgyik wrote:
> I believe I accidentally resolved this, but
> @abrachet 's comment from earlier: 
> 
> 
> > The difference in speed here is going to be negligible, the question is whether you think it is worth complicating the code for these edge cases.
> 
> 
I agree with @abrachet that this has almost no benefit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85103