[PATCH] Implement std::experimental::sample
Eric Fiselier
eric at efcs.ca
Thu Apr 30 11:00:27 PDT 2015
More comments. The `__sample` that takes an input iterator and a random access iterator LGTM.
================
Comment at: include/experimental/algorithm:58
@@ +57,3 @@
+ for (; __first != __last; ++__first, (void)++__k) {
+ _Distance __r = _VSTD::uniform_int_distribution<_Distance>(0, __k)(__g);
+ if (__r < __sz)
----------------
`_Distance` is the wrong type to be using here. It's possible that `_Distance` is a type that cannot represent the value of `__k`. The LFTS spec seems misleading about how distance should be used. I'll have to file a standard defect to clarify the wording.
I think `__k` should be the `distance_type` of the `_PopulationIterator`.
Example Code (This has UB and exits with -6 on my machine).
```
std::vector<int> pop(1024, 42);
std::vector<int> out(128, 0);
std::minstd_rand g;
std::experimental::sample(pop.begin(), pop.end(), out.begin(), (signed char)127, g);
```
================
Comment at: include/experimental/algorithm:74
@@ +73,3 @@
+ _Distance __r =
+ _VSTD::uniform_int_distribution<_Distance>(0, --__unsampled_sz)(__g);
+ if (__r < __n) {
----------------
Do we need to re-construct the `uniform_int_distribution` every iteration?
http://reviews.llvm.org/D9044
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the cfe-commits
mailing list