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

Support building against libstdc++ with Clang #38

Merged
merged 4 commits into from
Jan 6, 2022

Conversation

danvratil
Copy link
Owner

By providing custom header for Clang, we can now build QCoro
with clang against libstdc++.

Fixes #22.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2022

Unit Test Results

  6 files  ±0    6 suites  ±0   2m 52s ⏱️ +2s
13 tests ±0  13 ✔️ ±0  0 💤 ±0  0 ±0 
74 runs  +1  74 ✔️ +1  0 💤 ±0  0 ±0 

Results for commit d51a971. ± Comparison against base commit 24fa9ac.

♻️ This comment has been updated with latest results.

@suy
Copy link

suy commented Jan 6, 2022

Anything that improves clang support, which is so pervasive as code model/language server (and caused me many headaches with Qt Creator), is veeeeeery welcome. :)

That said, and just for completeness sake, I think it's worth mentioning that adding things inside the std namespace is undefined behavior. Since it works here, it's probably the right call. I don't know if it can be added with a narrower condition (not just clang, but clang under some specific version?).

Clang looks for coroutine-related classes in std::experimental namespace.
We provide a custom implementation of those classes in std namespace, and
we also expose those into the std::experimental namespace. This allows
using std::coroutine_handle<> everywhere regardless of compiler and also
satisfies Clang, because it finds what it needs in std::experimental
namespace, without actually including <experimental/coroutine>.
Since we now provide our own implementaion of the standard <coroutine>
header for Clang, we are no longer limited by Clang looking for
coroutines in std::experimental namespace, which was only provided by
libc++.

libc++ is still supported with Clang, but we no longer provide the hack
to support mixing libc++-built QCoro with libstd++-built Qt.
The macro is marked as deprecated now, so it generates fair amount of
warnings when building QCoro.
@danvratil
Copy link
Owner Author

I know, but unfortunately, as far as I know, there's currently no other way/trick how to make coroutines in clang work with libstdc++, without implementing the necessary types in std ourselves.

Once there's a clang version that does not need this "hack", I'll add a version check and make sure the standard header is used.

@danvratil
Copy link
Owner Author

Since the check is based on __cpp_lib_coroutine macro, the moment Clang starts fully supporting coroutines (i.e. moving the from std::experimental to std namespace), it should start defining __cpp_impl_coroutine. When building against libstdc++, this will enable __cpp_lib_coroutine and that version of Clang will automatically start using the standard header from libstdc++. Should Clang be used with a standard library that won't support the standard header (libc++ as of now), then it will automatically fall back to using the implementation provided by QCoro.

For GCC, it does not officially support switching standard library implementations, so recent-enough GCC with recent-enough libstdc++ will always satisfy __cpp_lib_coroutine and use the standard header. Older versions will trigger the error message and fail compilation.

MSVC uses its own standard library and the coroutine support the is self-contained, so it will always fall to the same cases like GCC.

Thus I think global check for _clang_ should be good enough...I hope :-)

@danvratil danvratil merged commit 50224c3 into main Jan 6, 2022
@danvratil danvratil deleted the clang-noexperimental branch January 6, 2022 19:26
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.

Can't find Threads if compiled with Clang
2 participants