[libcxx-commits] [PATCH] D125329: Replace modulus operations in std::seed_seq::generate with conditional checks.

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

This looks pretty good, but I'd like to see some benchmark results before approving. BTW you can ignore clang-format currently. (I assume it's the reason for your last two diffs). If you don't have commit access please provide "Your Name" <your.email at address.com> for attribution.



================
Comment at: libcxx/benchmarks/random.bench.cpp:10-14
+#include <cstddef>
+#include <random>
+#include <algorithm>
+#include <array>
+#include <functional>
----------------
Could you order these alphabetically?


================
Comment at: libcxx/benchmarks/random.bench.cpp:1
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
----------------
This shouldn't be in a `.cpp` file, only in `.h` files.


================
Comment at: libcxx/include/__random/seed_seq.h:119-126
+          if (++__kmodn == __n)
+            __kmodn = 0;
+          if (++__km1modn == __n)
+            __km1modn = 0;
+          if (++__kpmodn == __n)
+            __kpmodn = 0;
+          if (++__kqmodn == __n)
----------------
This would be just perfect for a lambda. Why do we have to support C++03?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125329