[libc-commits] [libc] [llvm] [libc] Improve qsort (PR #120450)

[Michael Jones via libc-commits]

================
@@ -19,13 +19,11 @@ LLVM_LIBC_FUNCTION(void, qsort_r,
                    (void *array, size_t array_size, size_t elem_size,
                     int (*compare)(const void *, const void *, void *),
                     void *arg)) {
-  if (array == nullptr || array_size == 0 || elem_size == 0)
-    return;
-  internal::Comparator c(compare, arg);
-  auto arr = internal::Array(reinterpret_cast<uint8_t *>(array), array_size,
-                             elem_size, c);
 
-  internal::sort(arr);
+  internal::unstable_sort(array, array_size, elem_size,
+                          [compare, arg](const void *a, const void *b) -> bool {
+                            return compare(a, b, arg) < 0;
+                          });
----------------
michaelrj-google wrote:

My concern is mainly around readability. I found it hard to determine what `is_less` did in the primary qsort files since it's always passed with a templated type, and I had a hard time finding where it came from because it's never declared as `is_less`, it's just an anonymous lambda. I don't think we can easily fix the template issue, since the lambdas for qsort and qsort_r have different types, but we might be able to solve the lack of a declaration.
```suggestion
  const auto is_less = [compare, arg](const void *a, const void *b) -> bool {
    return compare(a, b, arg) < 0;
  };
  internal::unstable_sort(array, array_size, elem_size, is_less);
```
This would make it much easier to find the source of `is_less`, without materially changing the code. Does this seem like a good solution to you?

https://github.com/llvm/llvm-project/pull/120450