[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–59

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

54

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

54

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

54

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

54

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 DeprecatedCorotuine :
+  DiagGroup<"deprecated-coroutine">;
+def Coroutine : DiagGroup<"coroutine", [CoroutineMissingUnhandledException, DeprecatedCorotuine]>;
 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">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

   "'%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 <
+  "Found deprecated std::experimental::%0. Consider to update your libc++ "
+  "or move coroutine components into std namespace in case you are using "
+  "self-defined coroutine components">,
+  InGroup<DeprecatedCorotuine>;
 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 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) {}
+    co_await(auto i : arr) {} // expected-warning {{Found deprecated std::experimental}}
   // 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();

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 {{Found deprecated std::experimental}}
   } 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 {{Found deprecated std::experimental}}
 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 {{Found deprecated std::experimental}}
 }
 
 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-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 {{Found deprecated std::experimental}}
 }
 
 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 {{Found deprecated std::experimental}}
   } __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 {{Found deprecated std::experimental}}
 }

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-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 {{Found deprecated std::experimental}}
 }
 #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 {{Found deprecated std::experimental}}
 }
 
 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 {{Found deprecated std::experimental}}
 }
 
 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')