[Coroutine] Warn deprecated 'std::experimental::coro' uses

[ChuanqiXu]

Summary

Since we've decided the to not support std::experimental::coroutine*, we should tell the user they need to update.
We could emit warning on the compiler side or in libc++'s side by #warning directives.
I choose to warn on the compiler side since some people might use libstdc++ + self-defined coroutine header (Seastar is an example: https://github.com/scylladb/seastar/blob/master/include/seastar/core/std-coroutine.hh).
Since new warning might break the libcxx's CI system, I add -Wno-coroutine for the legacy tests. I guess it would be OK since the legacy test would be removed.

[ldionne]

LGTM with some suggestions, thanks for adding this notice!

clang/include/clang/Basic/DiagnosticGroups.td

57–60

clang/include/clang/Basic/DiagnosticSemaKinds.td

11015–11017

I would simply reword as "Please move from std::experimental::%0 to std::%0. Support for std::experimental::%0 will be removed in LLVM 15."

IMO the set of users defining their own coroutines header is small and it's not worth making the warning more obtuse for that corner case.

libcxx/test/std/experimental/language.support/support.coroutines/lit.local.cfg

8

[Mordante]

Thanks for working on this and landing the coroutines. LGTM after applying @ldionne's suggestions.

[• Quuxplusone]

clang/include/clang/Basic/DiagnosticSemaKinds.td

11015–11017

+1; people who write their own "coroutine.h" header are also operating outside of standard C++ and outside of the Clang/libc++ ecosystem, so they should be quite used to figuring things out for themselves. If they want friendly help from the toolchain, they should use the toolchain's headers.

clang/test/SemaCXX/co_await-range-for-exp-namespace.cpp

53

I'd like to see the next word in this diagnostic. (Ditto throughout. I'd also like to make sure that the diagnostic is not being printed with single-quotes in the wrong place, e.g. Found deprecated std::experimental::'coroutine_handle'; if it is, please fix that.)

This code is also misindented; please fix it up, as long as you're touching it. Should be

for co_await (auto i : arr) {}

[ChuanqiXu]

Address comments.

[ChuanqiXu]

clang/test/SemaCXX/co_await-range-for-exp-namespace.cpp

53

Oh, clang-format couldn't recognize the new grammar. I need to be careful to use it to format codes.

libcxx/test/std/experimental/language.support/support.coroutines/lit.local.cfg

8

Since the option is new added, I wonder if the CI would break if it didn't update compiler in time. (Or if it would run CI for previous compiler). Let's see if the CI would be green.

[ChuanqiXu]

Use -Wno-coroutine instead of -Wno-deprectated-experimental-coroutine to keep CI stable.

[ChuanqiXu]

The CI fails. But I guess it would be irrelevant with this diff:

/var/lib/buildkite-agent/builds/llvm-project/libcxx/.clang-format:6:17: error: invalid boolean
SpacesInAngles: Leave
                ^~~~~
Error reading /var/lib/buildkite-agent/builds/llvm-project/libcxx/.clang-format: Invalid argument

[• Quuxplusone]

FWIW, the problem with

SpacesInAngles: Leave
                ^~~~~

is that the debian builder has an outdated (or maybe just 13.x?) clang-format; this option is super new. I don't know what the appropriate course of action is, there.

clang/include/clang/Basic/DiagnosticSemaKinds.td

11015–11017

Nit: s/Warning </Warning</

clang/test/SemaCXX/co_await-range-for-exp-namespace.cpp

53

FYI: I'd prefer a space before the (. Compare for (~~~), for co_await (~~~). (I've left a comment in the clang-format Discord channel, too.)

libcxx/test/libcxx/experimental/language.support/support.coroutines/dialect_support.pass.cpp

10

Doesn't the change in lit.local.cfg make this change redundant?

[ChuanqiXu]

clang/test/SemaCXX/co_await-range-for-exp-namespace.cpp

53

Thanks for issuing this!

libcxx/test/libcxx/experimental/language.support/support.coroutines/dialect_support.pass.cpp

10

Yeah, this test isn't covered by lit.local.cfg below.

clang/include/clang/Basic/DiagnosticGroups.td

 def BitFieldWidth : DiagGroup<"bitfield-width">;
 def CompoundTokenSplitByMacro : DiagGroup<"compound-token-split-by-macro">;
 def CompoundTokenSplitBySpace : DiagGroup<"compound-token-split-by-space">;
 def CompoundTokenSplit : DiagGroup<"compound-token-split",
                                    [CompoundTokenSplitByMacro,
                                     CompoundTokenSplitBySpace]>;
 def CoroutineMissingUnhandledException :
   DiagGroup<"coroutine-missing-unhandled-exception">;
-def Coroutine : DiagGroup<"coroutine", [CoroutineMissingUnhandledException]>;
+def DeprecatedExperimentalCoroutine :
+  DiagGroup<"deprecated-experimental-coroutine">;
+def Coroutine : DiagGroup<"coroutine", [CoroutineMissingUnhandledException, DeprecatedExperimentalCoroutine]>;
 def ObjCBoolConstantConversion : DiagGroup<"objc-bool-constant-conversion">;
 def ConstantConversion : DiagGroup<"constant-conversion",
                                    [BitFieldConstantConversion,
                                     ObjCBoolConstantConversion]>;
 def LiteralConversion : DiagGroup<"literal-conversion">;
 def StringConversion : DiagGroup<"string-conversion">;
 def SignConversion : DiagGroup<"sign-conversion">;
 def PointerBoolConversion : DiagGroup<"pointer-bool-conversion">;
 def UndefinedBoolConversion : DiagGroup<"undefined-bool-conversion">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

def err_coroutine_invalid_func_context : Error<
   "'%1' cannot be used in %select{a constructor|a destructor"
   "|the 'main' function|a constexpr function"
   "|a function with a deduced return type|a varargs function"
   "|a consteval function}0">;
 def err_implied_coroutine_type_not_found : Error<
   "%0 type was not found; include <coroutine> before defining "
   "a coroutine; include <experimental/coroutine> if your version "
   "of libcxx is less than 14.0">;
+def warn_deprecated_coroutine_namespace : Warning<
+  "Please move from std::experimental::%0 to std::%0. "
+  "Support for std::experimental::%0 will be removed in LLVM 15.">,
+  InGroup<DeprecatedExperimentalCoroutine>;
 def err_mixed_use_std_and_experimental_namespace_for_coroutine : Error <
   "Found mixed use of std namespace and std::experimental namespace for "
   "coroutine, which is disallowed. The coroutine components in "
   "std::experimental namespace is deprecated. Please use coroutine components "
   "under std namespace.">;
 def err_implicit_coroutine_std_nothrow_type_not_found : Error<
   "std::nothrow was not found; include <new> before defining a coroutine which "
   "uses get_return_object_on_allocation_failure()">;

clang/lib/Sema/SemaCoroutine.cpp

if (!CoroNamespace || !LookupQualifiedName(Result, CoroNamespace)) {
       /// TODO: Remove this extra lookup when <experimental/coroutine> is
       /// removed.
       CoroNamespace = lookupStdExperimentalNamespace();
       if (!CoroNamespace || !LookupQualifiedName(Result, CoroNamespace)) {
         Diag(KwLoc, diag::err_implied_coroutine_type_not_found)
             << "std::coroutine_traits";
         return nullptr;
       }
-      /// TODO: Add a warning about not including <experimental/coroutine>
-      /// once we update libcxx.
+      Diag(KwLoc, diag::warn_deprecated_coroutine_namespace)
+          << "coroutine_traits";
     } else {
       /// When we found coroutine_traits in std namespace. Make sure there is no
       /// misleading definition in std::experimental namespace.
       NamespaceDecl *ExpNamespace = lookupStdExperimentalNamespace();
       LookupResult ExpResult(*this,
                              &PP.getIdentifierTable().get("coroutine_traits"),
                              FuncLoc, LookupOrdinaryName);
       if (ExpNamespace && LookupQualifiedName(ExpResult, ExpNamespace)) {

clang/test/SemaCXX/co_await-range-for-exp-namespace.cpp

struct promise_type {
     suspend_never initial_suspend();
     suspend_never final_suspend() noexcept;
     template <class T>
     Awaiter<T *> await_transform(T *) = delete; // expected-note {{explicitly deleted}}
   };
 };
 MyForLoopArrayAwaiter g() {
   int arr[10] = {0};
-  for
-    co_await(auto i : arr) {}
+  for co_await (auto i : arr) {} // expected-warning {{Please move from std::experimental::coroutine_traits to std::coroutine_traits}}
   // expected-error@-1 {{call to deleted member function 'await_transform'}}
   // expected-note@-2 {{'await_transform' implicitly required by 'co_await' here}}
 }
 
 struct ForLoopAwaiterBadBeginTransform {
   struct promise_type {
     ForLoopAwaiterBadBeginTransform get_return_object();
     void return_void();
     void unhandled_exception();
     suspend_never initial_suspend();
     suspend_never final_suspend() noexcept;
 
     template <class T>
     Awaiter<T> await_transform(BeginTag<T>) = delete; // expected-note 1+ {{explicitly deleted}}
 
     template <class T>
     CoawaitTag<T> await_transform(IncTag<T>); // expected-note 1+ {{candidate}}
   };
 };
 ForLoopAwaiterBadBeginTransform bad_begin() {
   Range<int> R;
-  for
-    co_await(auto i : R) {}
+  for co_await (auto i : R) {}
   // expected-error@-1 {{call to deleted member function 'await_transform'}}
   // expected-note@-2 {{'await_transform' implicitly required by 'co_await' here}}
 }
 template <class Dummy>
 ForLoopAwaiterBadBeginTransform bad_begin_template(Dummy) {
   Range<Dummy> R;
-  for
-    co_await(auto i : R) {}
+  for co_await (auto i : R) {}
   // expected-error@-1 {{call to deleted member function 'await_transform'}}
   // expected-note@-2 {{'await_transform' implicitly required by 'co_await' here}}
 }
 template ForLoopAwaiterBadBeginTransform bad_begin_template(int); // expected-note {{requested here}}
 
 template <class Iter>
 Awaiter<Iter> operator co_await(CoawaitTag<Iter, true>) = delete;
 // expected-note@-1 1+ {{explicitly deleted}}
struct promise_type {
     Awaiter<T> await_transform(BeginTag<T> e);
 
     template <class T>
     CoawaitTag<T, true> await_transform(IncTag<T>);
   };
 };
 ForLoopAwaiterBadIncTransform bad_inc_transform() {
   Range<float> R;
-  for
-    co_await(auto i : R) {}
+  for co_await (auto i : R) {}
   // expected-error@-1 {{overload resolution selected deleted operator 'co_await'}}
   // expected-note@-2 {{in implicit call to 'operator++' for iterator of type 'Range<float>'}}
 }
 
 template <class Dummy>
 ForLoopAwaiterBadIncTransform bad_inc_transform_template(Dummy) {
   Range<Dummy> R;
-  for
-    co_await(auto i : R) {}
+  for co_await (auto i : R) {}
   // expected-error@-1 {{overload resolution selected deleted operator 'co_await'}}
   // expected-note@-2 {{in implicit call to 'operator++' for iterator of type 'Range<long>'}}
 }
 template ForLoopAwaiterBadIncTransform bad_inc_transform_template(long); // expected-note {{requested here}}
 
 // Ensure we mark and check the function as a coroutine even if it's
 // never instantiated.
 template <class T>
 constexpr void never_instant(T) {
   static_assert(sizeof(T) != sizeof(T), "function should not be instantiated");
-  for
-    co_await(auto i : foo(T{})) {}
+  for co_await (auto i : foo(T{})) {}
   // expected-error@-1 {{'co_await' cannot be used in a constexpr function}}
 }
 
 namespace NS {
 struct ForLoopAwaiterCoawaitLookup {
   struct promise_type {
     ForLoopAwaiterCoawaitLookup get_return_object();
     void return_void();
     void unhandled_exception();
     suspend_never initial_suspend();
     suspend_never final_suspend() noexcept;
     template <class T>
     CoawaitTag<T, false> await_transform(BeginTag<T> e);
     template <class T>
     Awaiter<T> await_transform(IncTag<T>);
   };
 };
 } // namespace NS
 using NS::ForLoopAwaiterCoawaitLookup;
 
 template <class T>
 ForLoopAwaiterCoawaitLookup test_coawait_lookup(T) {
   Range<T> R;
-  for
-    co_await(auto i : R) {}
+  for co_await (auto i : R) {}
   // expected-error@-1 {{no member named 'await_ready' in 'CoawaitTag<Iter<int>, false>'}}
 }
 template ForLoopAwaiterCoawaitLookup test_coawait_lookup(int); // expected-note {{requested here}}
 
 // FIXME: This test should fail as well since the newly declared operator co_await
 // should not be found by lookup.
 namespace NS2 {
 template <class Iter>
 Awaiter<Iter> operator co_await(CoawaitTag<Iter, false>);
 }
 using NS2::operator co_await;
 template ForLoopAwaiterCoawaitLookup test_coawait_lookup(long);

clang/test/SemaCXX/coreturn-eh-exp-namespace.cpp

 // RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts -fcxx-exceptions -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify -fblocks -Wall -Wextra -Wno-error=unreachable-code
-// expected-no-diagnostics
 
 #include "Inputs/std-coroutine-exp-namespace.h"
 
 using std::experimental::suspend_always;
 using std::experimental::suspend_never;
 
 struct awaitable {
   bool await_ready();

 };
 
 template <typename T1>
 struct std::experimental::coroutine_traits<void, T1> { using promise_type = promise_void_return_value; };
 
 VoidTagReturnValue test() {
   object x = {};
   try {
-    co_return {};
+    co_return {}; // expected-warning {{Please move from std::experimental::coroutine_traits to std::coroutine_traits}}
   } catch (...) {
     throw;
   }
 }

clang/test/SemaCXX/coreturn-exp-namespace.cpp

 struct std::experimental::coroutine_traits<void, T1> { using promise_type = promise_void_return_value; };
 
 template <typename... T>
 struct std::experimental::coroutine_traits<float, T...> { using promise_type = promise_float; };
 
 template <typename... T>
 struct std::experimental::coroutine_traits<int, T...> { using promise_type = promise_int; };
 
-void test0() { co_await a; }
+void test0() { co_await a; } // expected-warning {{Please move from std::experimental::coroutine_traits to std::coroutine_traits}}
 float test1() { co_await a; }
 
 int test2() {
   co_await a;
 } // expected-warning {{non-void coroutine does not return a value}}
 
 int test2a(bool b) {
   if (b)

clang/test/SemaCXX/coroutine-final-suspend-noexcept-exp-namespace.cpp

struct promise_type {
     std::experimental::suspend_always final_suspend(); // expected-note 2 {{must be declared with 'noexcept'}}
     void return_void();
     static void unhandled_exception();
   };
 };
 
 coro_t f(int n) { // expected-error {{the expression 'co_await __promise.final_suspend()' is required to be non-throwing}}
   A a{};
-  co_await a;
+  co_await a; // expected-warning {{Please move from std::experimental::coroutine_traits to std::coroutine_traits}}
 }
 
 template <typename T>
 coro_t f_dep(T n) { // expected-error {{the expression 'co_await __promise.final_suspend()' is required to be non-throwing}}
   A a{};
   co_await a;
 }
 
 void foo() {
   f_dep<int>(5); // expected-note {{in instantiation of function template specialization 'f_dep<int>' requested here}}
 }

clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp

-// This file is to test the mixed use of `std::experimental::coroutine*` and `std::coroutine*`
+// This file is to test the mixed use of `std::experimental::coroutine_traits` and `std::coroutine_traits`
 // which is similar to coroutine-mixed-exp-namesapce. This file tests the relative order of
 // included header wouldn't affect the diagnostic messages.
 // RUN: %clang_cc1 -verify -std=c++20 -fsyntax-only %s
 
 #include "Inputs/std-coroutine-exp-namespace.h"
 #include "Inputs/std-coroutine.h"
 
 struct my_awaitable {

clang/test/SemaCXX/coroutine-rvo-exp-namespace.cpp

struct promise_type {
     auto get_return_object() { return task{}; }
     static void unhandled_exception() {}
     void return_value(T &&value) {} // expected-note 4{{passing argument}}
   };
 };
 
 task<NoCopyNoMove> local2val() {
   NoCopyNoMove value;
-  co_return value;
+  co_return value; // expected-warning {{Please move from std::experimental::coroutine_traits to std::coroutine_traits}}
 }
 
 task<NoCopyNoMove &> local2ref() {
   NoCopyNoMove value;
   co_return value; // expected-error {{non-const lvalue reference to type 'NoCopyNoMove' cannot bind to a temporary of type 'NoCopyNoMove'}}
 }
 
 // We need the move constructor for construction of the coroutine.

clang/test/SemaCXX/coroutine-seh-exp-namespace.cpp

struct promise_type {
     void return_void() noexcept;
     void unhandled_exception() noexcept;
   };
 };
 
 void SEH_used() {
   __try {      // expected-error {{cannot use SEH '__try' in a coroutine when C++ exceptions are enabled}}
     co_return; // expected-note {{function is a coroutine due to use of 'co_return' here}}
+               // expected-warning@-1 {{Please move from std::experimental::coroutine_traits to std::coroutine_traits}}
   } __except (0) {
   }
 }

clang/test/SemaCXX/coroutine-traits-undefined-template-exp-namespace.cpp

 };
 
 template <> struct coroutine_traits<void>; // expected-note {{forward declaration of 'std::experimental::coroutine_traits<void>'}}
 } // namespace experimental
 } // namespace std
 
 void uses_forward_declaration() {
   co_return; // expected-error {{this function cannot be a coroutine: missing definition of specialization 'coroutine_traits<void>'}}
+             // expected-warning@-1 {{Please move from std::experimental::coroutine_traits to std::coroutine_traits}}
 }

clang/test/SemaCXX/coroutine-unhandled_exception-warning-exp-namespace.cpp

 // RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts \
 // RUN:    -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify \
 // RUN:    -fblocks -Wno-unreachable-code -Wno-unused-value
 
 // RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts \
 // RUN:    -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify \
 // RUN:    -fblocks -Wno-unreachable-code -Wno-unused-value \
-// RUN:    -DDISABLE_WARNING -Wno-coroutine-missing-unhandled-exception
+// RUN:    -DDISABLE_WARNING -Wno-deprecated-experimental-coroutine -Wno-coroutine-missing-unhandled-exception
 
 #if __has_feature(cxx_exceptions)
 #error This test requires exceptions be disabled
 #endif
 
 #include "Inputs/std-coroutine-exp-namespace.h"
 
 using std::experimental::suspend_always;
#endif
   void return_void();
 };
 
 template <typename... T>
 struct std::experimental::coroutine_traits<void, T...> { using promise_type = promise_void; };
 
 #ifndef DISABLE_WARNING
 void test0() { // expected-warning {{'promise_void' is required to declare the member 'unhandled_exception()' when exceptions are enabled}}
-  co_return;
+  co_return;   // expected-warning {{Please move from std::experimental::coroutine_traits to std::coroutine_traits}}
 }
 #else
 void test0() { // expected-no-diagnostics
   co_return;
 }
 #endif

clang/test/SemaCXX/coroutine_handle-address-return-type-exp-namespace.cpp

struct awaitable {
   bool await_ready();
 
   std::experimental::coroutine_handle<task::promise_type>
   await_suspend(std::experimental::coroutine_handle<> handle);
   void await_resume();
 } a;
 
 task f() {
-  co_await a;
+  co_await a; // expected-warning {{Please move from std::experimental::coroutine_traits to std::coroutine_traits}}
 }
 
 int main() {
   f();
   return 0;
 }

clang/test/SemaCXX/coroutines-exp-namespace.cpp

 struct auto_await_suspend {
   bool await_ready();
   template <typename F> auto await_suspend(F) {}
   void await_resume();
 };
 
 struct DummyVoidTag {};
 DummyVoidTag no_specialization() { // expected-error {{this function cannot be a coroutine: 'std::experimental::coroutine_traits<DummyVoidTag>' has no member named 'promise_type'}}
-  co_await a;
+  co_await a;                      // expected-warning {{Please move from std::experimental::coroutine_traits to std::coroutine_traits}}
 }
 
 template <typename... T>
 struct std::experimental::coroutine_traits<int, T...> {};
 
 int no_promise_type() { // expected-error {{this function cannot be a coroutine: 'std::experimental::coroutine_traits<int>' has no member named 'promise_type'}}
   co_await a;
 }

libcxx/test/libcxx/experimental/language.support/support.coroutines/dialect_support.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // REQUIRES: fcoroutines-ts
-// ADDITIONAL_COMPILE_FLAGS: -fcoroutines-ts
+// ADDITIONAL_COMPILE_FLAGS: -fcoroutines-ts -Wno-coroutine
 
 // A simple "breathing" test that checks that <experimental/coroutine>
 // can be parsed and used in all dialects, including C++03 in order to match
 // Clang's behavior.
 
 #include <experimental/coroutine>
 
 #include "test_macros.h"

libcxx/test/std/experimental/language.support/support.coroutines/lit.local.cfg

 # If the compiler doesn't support coroutines mark all of the tests under
 # this directory as unsupported. Otherwise add the required `-fcoroutines-ts`
 # flag.
 if 'fcoroutines-ts' not in config.available_features:
   config.unsupported = True
 else:
   config.test_format.addCompileFlags(config, '-fcoroutines-ts')
+  config.test_format.addCompileFlags(config, '-Wno-coroutine')