Major Clang update for C++/WinRT#492
Conversation
kennykerr
requested review from
BenJKuhn,
DefaultRyan and
Scottj1s
as code owners
| Class::StaticTestReturn(); | ||
|
|
||
| auto discarded = Class::StaticProperty(); discarded; | ||
| [[maybe_unused]] auto discarded = Class::StaticProperty(); |
There was a problem hiding this comment.
Clang isn't fooled...
| @@ -0,0 +1,125 @@ | |||
|
|
|||
| #ifndef __clang__ | |||
There was a problem hiding this comment.
Clang uses a completely different ABI for coroutine support. This means that the VC <experimental/coroutine> header does not support the Clang frontend.
|
|
||
| #else | ||
|
|
||
| namespace std::experimental |
There was a problem hiding this comment.
Since there's a compiler difference that you're smoothing over, it seems likely that projects could accidentally take a dependency on your adapter layer. Would it make sense to put the adapter in a cppwinrt::details namespace, then in the VC version, just do namespace cppwinrt::details { using std::experimental; }, then consume from there so that clang projects using cppwinrt can't easily take an accidental dependency on something that you might want to change later once things standardize a bit more?
There was a problem hiding this comment.
The trick is that everything here is part of the public library support for coroutines. This is intentionally designed for libraries to take a dependency on and is unavoidable. As such, care is taken that this accurately represents the same library surface as the VC header and the C++20 standard - specifically nothing more is provided. The only implementation detail is the implementation of the functions that call the Clang intrinsics.
BenJKuhn
left a comment
BenJKuhn
left a comment
There was a problem hiding this comment.
Basically looks good. A couple minor comments on clang compat & coroutines to consider, though.
|
Thanks Ben! |
2 participants
This turned from a minor update to a pretty major update as I decided to finally get all of the C++/WinRT unit tests to build with Clang. And we have a lot of tests...
Clang support has always been mostly a best effort by smoke testing with simple apps. This was mainly done in support of conformance. Being able to compile all our unit tests means that we can do much much better. Notably with this update, you can finally use C++ coroutines with Clang. This has some interesting possibilities as Clang has a much more advanced optimizer when it comes to coroutines.
Beyond coroutine support, I've fixed a number of conformance issues highlighted by Clang and done a lot of work to clean up the build support to make it simpler and easier to build with either VC or Clang. It's not quite warning-free for Clang just yet, but its a lot closer. I've dealt with all the errors and most of the warnings. More to come as I can now far more easily test with Clang.