[libcxx-commits] [PATCH] D62719: A hot fix for exclusive_scan

Wed Jun 5 13:39:39 PDT 2019

rodgert added a comment.

In D62719#1527360 <https://reviews.llvm.org/D62719#1527360>, @mclow.lists wrote:

> In D62719#1527089 <https://reviews.llvm.org/D62719#1527089>, @MikeDvorskiy wrote:
>
> > In D62719#1525450 <https://reviews.llvm.org/D62719#1525450>, @rodgert wrote:
> >
> > > Actually, we should probably get a test of some sort here before accepting the change. Yes, it's fairly obviously wrong and the fix is fairly obviously correct, but it's a regression and we should provide tests for regressions, lest certain standard library maintainers ask uncomfortable questions.
> >
> >
> > Do you mean a test should we added which will coverage the fixed error? In other words we need a test which checks following thing:
> >
> > "Parallel algorithms shall not participate in overload resolution unless is_execution_policy_v<decay_-
> >  t<ExecutionPolicy>> is true."
> >
> > Right?
>
>
> Right.
>
> > As far as I understand we should one call pear each algorithm with "fake" policy for  is_execution_policy_v<fake_policy> is false and get a compilation error like "exclusive_scan(....).... is not found".
> >  Otherwise, test "fail". So, it is "negative" test, right?
> >  So, I have two questions:
> > 
> > 1. Does LLVM test system support a negative test?
>
> Yes it does. See (for example) "test/std/iterators/iterator.container/empty.array.fail.cpp"
>
> > 2. If yes, to cover the all cases (to be sure that "enable_if is present") we have to add more 80 negative test units.
>
> Agreed.

I don't think we need all 80 tests to proceed with this commit, but here is as good a starting point as any on the process of adding such tests.

Repository:
  rPSTL pstl

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

https://reviews.llvm.org/D62719