[libc++] Don't warn that coroutines aren't supported when including <experimental/coroutine>

[philnik]

Summary

This change makes the behavior of <experimental/coroutine> consistent with other headers that only work conditionally.

[• Quuxplusone]

No, that definitely shouldn't be the case. There's no need to #include <__config> before #include <algorithm>, because <algorithm> itself includes <__config>.
(And there's no need for <algorithm> to include <__config> before including <__algorithm/adjacent_find.h>, because <__algorithm/adjacent_find.h> itself includes <__config>! And so on.)

[philnik]

In D119964#3327177, @Quuxplusone wrote:

No, that definitely shouldn't be the case. There's no need to #include <__config> before #include <algorithm>, because <algorithm> itself includes <__config>.
(And there's no need for <algorithm> to include <__config> before including <__algorithm/adjacent_find.h>, because <__algorithm/adjacent_find.h> itself includes <__config>! And so on.)

The problem is that we use macros from <__config>. This means that we might include a header that shouldn't be included. Just as an example, maybe we want to remove <experimental/algorithm>. That means that the first include in the list is conditionally included, but the header where the condition is set isn't included before the condition is checked. That means <experimental/coroutine> is always included, but might not be available in a configuration.

[• Quuxplusone]

libcxx/test/libcxx/clang_tidy.sh.cpp

208–212 ↗

(On Diff #409347)

In D119964#3327193, @philnik wrote:

In D119964#3327177, @Quuxplusone wrote:

No, that definitely shouldn't be the case. There's no need to #include <__config> before #include <algorithm>, because <algorithm> itself includes <__config>.
(And there's no need for <algorithm> to include <__config> before including <__algorithm/adjacent_find.h>, because <__algorithm/adjacent_find.h> itself includes <__config>! And so on.)

The problem is that we use macros from <__config>. This means that we might include a header that shouldn't be included. Just as an example, maybe we want to remove <experimental/algorithm>. That means that the first include in the list is conditionally included, but the header where the condition is set isn't included before the condition is checked. That means <experimental/coroutine> is always included, but might not be available in a configuration.

Aha, I see. So there's no problem today, but there will be a problem as soon as you git rm experimental/algorithm, because then the first header in the list of experimental headers (alphabetically) will be one that it's not always safe to include.

I guess my next question is, why is it unsafe to unconditionally include <experimental/coroutine>? So I looked at the top of that file and found

#ifdef _LIBCPP_HAS_NO_EXPERIMENTAL_COROUTINES
# if defined(_LIBCPP_WARNING)
    _LIBCPP_WARNING("<experimental/coroutine> cannot be used with this compiler")
# else
#   warning <experimental/coroutine> cannot be used with this compiler
# endif
#endif

Perhaps we should replace this with our usual convention, which is just to make the whole file a no-op if it's not supported. (<coroutine> uses our usual convention, because it was written more recently.) @ldionne @ChuanqiXu, thoughts?

[Mordante]

libcxx/test/libcxx/clang_tidy.sh.cpp

208–212 ↗

(On Diff #409347)

In D119964#3327193, @philnik wrote:

In D119964#3327177, @Quuxplusone wrote:

Perhaps we should replace this with our usual convention, which is just to make the whole file a no-op if it's not supported. (<coroutine> uses our usual convention, because it was written more recently.) @ldionne @ChuanqiXu, thoughts?

In the release notes we mentioned <experimental/coroutine> will be removed in LLVM 15. So maybe removing this header now will be a better solution.

[philnik]

libcxx/test/libcxx/clang_tidy.sh.cpp

208–212 ↗

(On Diff #409347)

It would probably be a good idea. I'll put it on my TODO-list. I still think it's a good idea to add the <__config> includes to make sure we have all the macros before we check if they are defined.

[ChuanqiXu]

libcxx/test/libcxx/clang_tidy.sh.cpp

208–212 ↗

(On Diff #409347)

Personally, I prefer to diagnostic early if we detected user are doing the wrong behavior. But given that consistency is very important in projects like libcxx, I agree we should follow the usual convention to make it a no-op.

[ldionne]

I believe we should make <experimental/coroutine> empty instead of issuing a #warning when coroutines are disabled. That will solve this problem and be consistent with what we do everywhere else.

libcxx/test/libcxx/clang_tidy.sh.cpp

208–212 ↗

(On Diff #409347)

In the release notes we mentioned <experimental/coroutine> will be removed in LLVM 15. So maybe removing this header now will be a better solution.

The note I have locally says LLVM 16. We shipped <coroutine> in LLVM 14, which means that we remove <experimental/coroutines> in LLVM 16. Please correct me if I'm mistaken.

[philnik]

• Don't warn in <experimental/coroutine>

[philnik]

Only upload the new diff

[ldionne]

Please make sure your commit message explains why you're doing this -- otherwise someone looking at the history would be rather puzzled (at first glance, this appears to be removing something useful).

[Mordante]

libcxx/test/libcxx/clang_tidy.sh.cpp

208–212 ↗

(On Diff #409347)

As discussed on Discord I thought it was LLVM 15 and added that version in the release notes. The release notes have been updated in D120935.

libcxx/include/experimental/coroutine

 #include <memory> // for hash<T*>
 #include <new>
 #include <type_traits>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
 #endif
 
-#ifdef _LIBCPP_HAS_NO_EXPERIMENTAL_COROUTINES
-# if defined(_LIBCPP_WARNING)
-    _LIBCPP_WARNING("<experimental/coroutine> cannot be used with this compiler")
-# else
-#   warning <experimental/coroutine> cannot be used with this compiler
-# endif
-#endif
-
 #ifndef _LIBCPP_HAS_NO_EXPERIMENTAL_COROUTINES
 
 _LIBCPP_BEGIN_NAMESPACE_EXPERIMENTAL_COROUTINES
 
 template <class _Tp, class = void>
 struct __coroutine_traits_sfinae {};
 
 template <class _Tp>