Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

P0202R3 constexpr For <algorithm> And exchange() #6

Closed
StephanTLavavej opened this issue Sep 5, 2019 · 6 comments · Fixed by #425
Closed

P0202R3 constexpr For <algorithm> And exchange() #6

StephanTLavavej opened this issue Sep 5, 2019 · 6 comments · Fixed by #425
Assignees
Labels
cxx20 C++20 feature fixed Something works now, yay!
Milestone

Comments

@StephanTLavavej
Copy link
Member

StephanTLavavej commented Sep 5, 2019

P0202R3 constexpr For <algorithm> And exchange()

LWG-3256 Feature testing macro for constexpr algorithms

This needs #156 is_constant_evaluated().

Feature-test macro as of WG21-N4842:

@BillyONeal
Copy link
Member

I'm starting this and hope to get the <xutility> family constexprized shortly.

@BillyONeal
Copy link
Member

Should we turn on the feature test macro for this even though some algorithms will break due to DevCom-883631 ?

@BillyONeal
Copy link
Member

(e.g. so far some calls to is_permutation will trigger it)

@CaseyCarter
Copy link
Member

Should we turn on the feature test macro for this even though some algorithms will break due to DevCom-883631 ?

Yes, I believe we should. I wouldn't say that we don't support is_permutation at runtime if there were argument types for which the compiler generates bogus diagnostics or values for which it generates bad code, and this seems like the constexpr flavor of such cases.

@BillyONeal
Copy link
Member

BillyONeal commented Jan 15, 2020

p0202r3, those in bold have libc++ tests already present we can use:

  • all_of
  • any_of
  • none_of
  • for_each
  • for_each_n
  • find
  • find_if
  • find_if_not
  • find_end
  • find_first_of
  • adjacent_find
  • count
  • count_if
  • mismatch
  • equal
  • is_permutation
  • search
  • search_n
  • copy
  • copy_n
  • copy_if
  • copy_backward
  • move
  • move_backward
  • transform
  • replace
  • replace_if
  • replace_copy
  • replace_copy_if
  • fill
  • fill_n
  • generate
  • generate_n
  • remove
  • remove_if
  • remove_copy
  • remove_copy_if
  • unique
  • unique_copy
  • reverse_copy
  • rotate_copy
  • is_partitioned
  • partition_copy
  • partition_point
  • is_sorted
  • is_sorted_until
  • lower_bound
  • upper_bound
  • equal_range
  • binary_search
  • merge
  • includes
  • set_union
  • set_intersection
  • set_difference
  • set_symmetric_difference
  • is_heap
  • is_heap_until
  • lexicographical_compare
  • exchange

BillyONeal added a commit to BillyONeal/STL that referenced this issue Jan 16, 2020
Resolves microsoftGH-6, microsoftGH-38, and drive-by fixes microsoftGH-414.

Constexprizes the following algorithms and enables all relevant libc++ tests:

* adjacent_find
* all_of
* any_of
* binary_search
* copy
* copy_backward
* copy_if
* copy_n
* count
* count_if
* equal
* equal_range
* exchange
* fill
* fill_n
* find
* find_end
* find_first_of
* find_if
* find_if_not
* for_each
* for_each_n
* generate
* generate_n
* includes
* is_heap
* is_heap
* is_heap_until
* is_partitioned
* is_permutation
* is_sorted
* is_sorted_until
* iter_swap
* lexicographical_compare
* lower_bound
* make_heap
* merge
* mismatch
* move
* move_backward
* next_permutation
* none_of
* nth_element
* partial_sort
* partial_sort_copy
* partition
* partition_copy
* partition_point
* pop_heap
* prev_permutation
* push_heap
* remove
* remove_copy
* remove_copy_if
* remove_if
* replace
* replace_copy
* replace_copy_if
* replace_if
* reverse_copy
* revese
* rotate
* rotate_copy
* search
* search_n
* set_difference
* set_intersection
* set_symmetric_difference
* set_union
* sort
* sort_heap
* swap
* swap_ranges
* transform
* unique
* unique_copy
* upper_bound

This commit also contains the contents of microsoft#423 to workaround DevCom-883631, it will be rebased and that content removed once that is merged (the other one needs to be merged first).

Specific notes:

`<xutility>`:
* The `_Ptr_copy_cat` family are moved down next to std::copy as that is their first consumer.
* The core language loop for `copy_n` is fairly long and so it was extracted into its own function, `_Copy_n_core` (note similar name schema to `_Copy_memmove`)
* `reverse` was changed to use early-returns for its optimization passes; this removes the nice thing of having if constexpr not instantiate the loop. However, this form allows the loop to not be duplicated.
@BillyONeal
Copy link
Member

I asked libc++ folks how they would like submitted tests for this since they would fail, and didn't get a reasonable answer, so I'm going to go ahead and do an internal-only test for the missing algorithms, as well as the ones that needed explicit is_constant_evaluated tests.

BillyONeal added a commit to BillyONeal/STL that referenced this issue Jan 21, 2020
Resolves microsoftGH-6 ( P0202R3 ), microsoftGH-38 ( P0879R0 ), and drive-by fixes microsoftGH-414.

Everywhere: Add constexpr, _CONSTEXPR20, and _CONSTEXPR20_ICE to things.

skipped_tests.txt: Turn on all tests previously blocked by missing constexpr algorithms (and exchange and swap). Mark those algorithms that cannot be turned on that we have outstanding PRs for with their associated PRs.
yvals_core.h: Turn on feature test macros.
xutility:
* Move the _Ptr_cat family down to copy, and fix associated SHOUTY comments to indicate that this is really an implementation detail of copy, not something the rest of the standard library intends to use directly. Removed and clarified some of the comments as requested by Casey Carter.
* Extract _Copy_n_core which implements copy_n using only the core language (rather than memcpy-as-an-intrinsic). Note that we cannot use __builtin_memcpy or similar to avoid the is_constant_evaluated check here; builtin_memcpy only works in constexpr contexts when the inputs are of type char.
numeric: Refactor as suggested by microsoftGH-414.
StephanTLavavej pushed a commit that referenced this issue Jan 23, 2020
* Implement constexpr algorithms.

Resolves GH-6 ( P0202R3 ), resolves GH-38 ( P0879R0 ), and drive-by fixes GH-414.

Everywhere: Add constexpr, _CONSTEXPR20, and _CONSTEXPR20_ICE to things.

skipped_tests.txt: Turn on all tests previously blocked by missing constexpr algorithms (and exchange and swap). Mark those algorithms that cannot be turned on that we have outstanding PRs for with their associated PRs.
yvals_core.h: Turn on feature test macros.
xutility:
* Move the _Ptr_cat family down to copy, and fix associated SHOUTY comments to indicate that this is really an implementation detail of copy, not something the rest of the standard library intends to use directly. Removed and clarified some of the comments as requested by Casey Carter.
* Extract _Copy_n_core which implements copy_n using only the core language (rather than memcpy-as-an-intrinsic). Note that we cannot use __builtin_memcpy or similar to avoid the is_constant_evaluated check here; builtin_memcpy only works in constexpr contexts when the inputs are of type char.
numeric: Refactor as suggested by GH-414.

* Attempt alternate fix of GH-414 suggested by Stephan.

* Stephan product code PR comments:

* _Swap_ranges_unchecked => _CONSTEXPR20
* _Idl_dist_add => _NODISCARD (and remove comments)
* is_permutation => _NODISCARD
* Add yvals_core.h comments.

* Delete unused _Copy_n_core and TRANSITION, DevCom-889321 comment.

* Put the comments in the right place and remove phantom braces.
@StephanTLavavej StephanTLavavej added the fixed Something works now, yay! label Jan 23, 2020
@cbezault cbezault added this to the Conformance milestone May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cxx20 C++20 feature fixed Something works now, yay!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants