[libcxx-commits] [PATCH] D114136: [libc++] Test that our algorithms never copy a user-provided comparator.

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/test/libcxx/algorithms/robust_against_copying_comparators.pass.cpp:9
+
+// <algorithm>
+
----------------
Same comment re. `<numeric>` as the other patch.


================
Comment at: libcxx/test/libcxx/algorithms/robust_against_copying_comparators.pass.cpp:16
+
+struct Less {
+    int *copies_;
----------------
Optional: consider using a mixin to reduce boilerplate:
```
struct CopyTracker {
  int *copies_;
  TEST_CONSTEXPR explicit CopyTracker(int *copies) : copies_(copies) {}
  TEST_CONSTEXPR CopyTracker(const CopyTracker& rhs) : copies_(rhs.copies_) { *copies_ += 1; }
};

struct Less : CopyTracker {
  TEST_CONSTEXPR explicit Less(int *copies) : CopyTracker(copies) {}
  TEST_CONSTEXPR bool operator()(void*, void*) const { return false; }
};

// ...
```



================
Comment at: libcxx/test/libcxx/algorithms/robust_against_copying_comparators.pass.cpp:170
+    (void)std::sort_heap(first, last, Less(&copies)); assert(copies == 0);
+    if (!TEST_IS_CONSTANT_EVALUATED) (void)std::stable_partition(first, last, UnaryTrue(&copies)); assert(copies == 0);
+    if (!TEST_IS_CONSTANT_EVALUATED) (void)std::stable_sort(first, last, Less(&copies)); assert(copies == 0);
----------------
Nit: I think LLVM style guide is 80 columns. Also, this would make the `assert` unconditional (applies below as well).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114136