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

Fixed wrong configuration for ASIO_HAS_CO_AWAIT #1145

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Febbe
Copy link

@Febbe Febbe commented Oct 18, 2022

This adds a more general approach, to check for c++20 coroutines, which takes the feature test macros into account, defined since c++20: Proposal: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p0912r5.html cppreference: https://en.cppreference.com/w/User:D41D8CD98F/feature_testing_macros#C.2B.2B20

I've kept the old macros, in the case compilers + stl-impl add the support, even, when they did not yet set the feature testing macros.
I've also made them more robust: The #elif were not mutually exclusive, so I've changed them to if !defined(ASIO_HAS_CO_AWAIT) && ...

This adds a more general approach, to check for c++20 coroutines, which takes the feature test macros into account, defined since c++20:   
Proposal: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p0912r5.html   
cppreference: https://en.cppreference.com/w/User:D41D8CD98F/feature_testing_macros#C.2B.2B20  

I've kept the old macros, in the case compilers + stl-impl add the support, even, when they did not yet set the feature testing macros.
I've also made them more robust: The `#elif` were not mutually exclusive, so I've changed them to `if !defined(ASIO_HAS_CO_AWAIT) && ...`
@Febbe
Copy link
Author

Febbe commented Oct 18, 2022

Fixes #1138

Febbe added 3 commits October 18, 2022 16:37
Todo: squash it (fixup mode)
More general
ASIO_HAS_CO_AWAIT  => checks for compiler support
ASIO_HAS_STD_COROUTINE => checks for std::coroutine support
@Febbe
Copy link
Author

Febbe commented Oct 18, 2022

Waiting for feedback, pipeline and approval. I will rebase / squash (fixup) it then.

@madhur4127
Copy link

Interesting test case

@Febbe
Copy link
Author

Febbe commented Nov 27, 2022

Interesting test case

Can you elaborate?
Btw., to use coroutines with clang13 you must use: -Wall -Wextra -std=c++20 -stdlib=libc++. The libstdc++ does not have the header.

@madhur4127
Copy link

madhur4127 commented Nov 28, 2022

The problem is that using clang13 with libstdc++ will fail in a weird way.

HAS_CO_AWAIT will be true but HAS_COROUTINE is false so if you look at any header say awaitable.hpp it'll fail to find the experimental/coroutine.

there shouldn't be the missing header error.

@madhur4127
Copy link

I guess what I am trying to hint is that you should also change HAS_STD_COROUTINE to pick the right header.

@Febbe
Copy link
Author

Febbe commented Nov 28, 2022

I guess what I am trying to hint is that you should also change HAS_STD_COROUTINE to pick the right header.

This would require another variable like ASIO_COROUTINE_HEADER, which is then set to either the plane one or the one in experimental.

@Febbe
Copy link
Author

Febbe commented Nov 28, 2022

But in my opinion, as long as a compiler + library does not fully implement a feature (clang and coroutines), 3rd parties using the feature should concentrate on the newest compiler version, implementing the partial feature.

For example: No one actually want to use clang13 with a not fully implemented coroutine support, when there is clang16 / clang17 which fully support it (hopefully).

@madhur4127
Copy link

madhur4127 commented Nov 28, 2022

I'm afraid to say this but not every firm can use bleeding edge compilers.

Some firms have a rigorous process to update the compiler and they might be stuck with clang 13.

in this case we can simply check for has_co_await and then if any of coroutine or experimental/coroutine header exists we should pick the right one.

This will handle case when using clang13 with exp and non exp coro header

@Febbe
Copy link
Author

Febbe commented Nov 28, 2022

I'm afraid to say this but not every firm can use bleeding edge compilers.

Doesn't the same apply to bleeding edge / incomplete compiler features? With that logic, no dev must be allowed to use unfinished language features of the current compiler.

I can take a look into this, but if @chriskohlhoff doesn't like it at all, it does not make sense to put more effort into this patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants