Make set_error use member function instead of tag_invoke
#1369
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aurianer
force-pushed
the
set_error_member_function
branch
from
8677164 to
2dfdcbf
Compare
6 tasks
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
aurianer
force-pushed
the
set_error_member_function
branch
from
2dfdcbf to
2007442
Compare
aurianer
changed the title
set_error use member function instead of tag_invoke
aurianer
changed the title
set_error use member function instead of tag_invokeset_error use member function instead of tag_invoke
aurianer
force-pushed
the
set_error_member_function
branch
3 times, most recently
from
eef0497 to
0058ce4
Compare
One static_assert on non_receiver_6 was checking if a receiver with a set_error without noexcept is a receiver or not. non_receiver_6 without noexcept on set_error is recognized as a receiver when we add the SFINAE to the cpo in receiver.hpp. If we remove this SFINAE, the static_assert inside the CPO that the set_error impl should be noexcept is triggered (what we expect). Trying with stdexec (adding the PIKA_STDEXEC_RECEIVER_CONCEPT to the non_receiver_6, so that we test the noexceptness of set_error has the same behaviour as non stdexec code. It's recognized as a receiver even without the noexceptness. Since the behaviours are similar both with stdexec and without, I'm removing this specific test case for now. It could be interesting to figure out why the SFINAE discards the candidate, or to add a stronger condition to satisfy the is_receiver trait.
aurianer
force-pushed
the
set_error_member_function
branch
from
0058ce4 to
edb2a4e
Compare
msimberg
reviewed
Jan 21, 2025
| @@ -447,7 +449,7 @@ function(pika_check_for_cxx23_static_call_operator) | |||
| pika_add_config_test( | |||
| PIKA_WITH_CXX23_STATIC_CALL_OPERATOR | |||
| SOURCE cmake/tests/cxx23_static_call_operator.cpp | |||
| FILE ${ARGN} | |||
| FILE ${ARGN} CXXFLAGS -pedantic-errors | |||
There was a problem hiding this comment.
Do you know what the failure mode is if this flag isn't supported? Does the test fail or does configuration fail? I'm thinking e.g. about nvc++ which may not support the flag, but if it does it's perhaps not such a big concern.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #1204
I guess the main change worth to mention here is the removal of non_receiver_6 (and renaming of non_receiver_7 to non_receiver_6).
decription of the corresponding commit:
One static_assert on non_receiver_6 was checking if a receiver with a set_error without noexcept is a receiver or not. The standard says that it should not. non_receiver_6 without noexcept on set_error is recognized as a receiver. If we remove the SFINAE to the set_error cpo in receiver.hpp, the static_assert inside the CPO that the set_error impl should be noexcept is triggered (what we expect). Trying with stdexec (adding the PIKA_STDEXEC_RECEIVER_CONCEPT to the non_receiver_6 so that it doesn't get discarded at the concept but we can actually test the noexceptness of set_error) has the same behaviour as non stdexec code. It's recognized as a receiver even without the noexceptness.
Since the behaviours are similar both with stdexec and without, I'm removing this specific test case for now. It could be interesting to figure out why the SFINAE discards the candidate, or to add a stronger condition to satisfy the is_receiver trait.