[PATCH] D74971: Add a llvm::shuffle and use it in lld

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 22 02:51:09 PST 2020


grimar accepted this revision.
grimar added a comment.

LGTM with minor nits.



================
Comment at: llvm/include/llvm/ADT/STLExtras.h:1013
+template <class Iterator, class RNG>
+void shuffle(Iterator first, Iterator last, RNG &&g) {
+  for (auto size = last - first; size > 1; ++first, --size) {
----------------
This file seems has inconsistent variables naming style, but seems
everything that goes below "Extra additions for arrays" uses
an upper case for variable names.


================
Comment at: llvm/include/llvm/ADT/STLExtras.h:1017
+    // but that would be stdlib dependent.
+    auto i = g() % size;
+    std::swap(*first, *(first + i));
----------------
MaskRay wrote:
> `i` can be inlined, too.
Also we often trying to avoid `auto` when the result in not obvious.
(here and above).


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

https://reviews.llvm.org/D74971





More information about the llvm-commits mailing list