[PATCH] D36423: [libc++] Introsort based sorting function

Ben Craig via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 15 07:09:01 PDT 2017


bcraig added a comment.

The test headers should not be in the production include folder.  They should probably be in the benchmark folder.

If possible, follow the style and conventions of the existing tests.  When you can't follow the style and convention of the existing tests, try to make it obvious (or leave a comment) as to why the new test is special.

For example, one way to follow the existing conventions would be to have a test that looks like this...

BENCHMARK_CAPTURE(BM_Sort, **qsort_worst_uint32**, **getQSortKiller<uint32_t>**)->Arg(TestNumInputs);

That test would be called qsort_worst_uint32.  You would need to author the sequence generator so that it had a signature like this...
template <class T> std::vector<T> getQSortKiller(size_t N)

On an encouraging note, I don't see anything wrong with the production code.  I'm optimistic about getting this in once we iron out the test issues.


https://reviews.llvm.org/D36423





More information about the cfe-commits mailing list