Add warning for coroutines without any `co_*` keywords in the definition

[JankoDedic]

This is a feature request for a Clang C++ compiler warning.

A function without any co_* keywords in its body is not a coroutine, even if it returns an awaitable type. What is intended to be a coroutine by the developer can accidentally become a normal function, which has entirely different behavior and can result in surprising bugs and undefined behavior. Some of these mistakes will be caught by -Wreturn-type, but not all of them.

Example: https://godbolt.org/z/8EfeTYdvq

#include <coroutine>

struct coroutine {
    struct promise_type {
        coroutine get_return_object() { return {}; }
        std::suspend_always initial_suspend() { return {}; }
        void return_void() {}
        void unhandled_exception() {}
        std::suspend_always final_suspend() noexcept { return {}; }
    };
};

coroutine example() {
    throw 67;
}

A compiler warning diagnosing functions with awaitable return types that don't have any co_* keywords in the definition could help entirely eliminate this class of bugs from codebases using coroutines.

[llvmbot]

@llvm/issue-subscribers-coroutines

Author: Janko Dedic (JankoDedic)

Details

This is a feature request for a Clang C++ compiler warning.

A function without any co_* keywords in its body is not a coroutine, even if it returns an awaitable type. What is intended to be a coroutine by the developer can accidentally become a normal function, which has entirely different behavior and can result in surprising bugs and undefined behavior. Some of these mistakes will be caught by -Wreturn-type, but not all of them.

Example: https://godbolt.org/z/8EfeTYdvq

#include <coroutine>

struct coroutine {
    struct promise_type {
        coroutine get_return_object() { return {}; }
        std::suspend_always initial_suspend() { return {}; }
        void return_void() {}
        void unhandled_exception() {}
        std::suspend_always final_suspend() noexcept { return {}; }
    };
};

coroutine example() {
    throw 67;
}

A compiler warning diagnosing functions with awaitable return types that don't have any co_* keywords in the definition could help entirely eliminate this class of bugs from codebases using coroutines.

[ChuanqiXu9]

In our real code, we have a "future" class which can be used as a return type for a coroutine and a normal function. Because future is a classic async class in "future.then" manner. And it is not easy to convert all old style "future.then" to co_await style.
And I feel this is not specific to our internal code. This is general.

Another point is, for classic Task type, the RVO may help the performance, e.g.,

Task (...) {
     return another_task(...);
}

So I am not in favor of this warning.

[JankoDedic]

There are definitely valid use cases for this, as those you've mentioned. However, some awaitables are designed to only be co_awaited and nothing else, and they can only be constructed as a return object from a coroutine and in no other way.

One use case I've found especially error-prone is passing coroutine callbacks:

co_await foo([](auto x) -> task<void> {
    /* co_ keywords missing */
});

This is an easy mistake to make IME.

Your example is actually another potential pitfall that this warning could help avoid that I didn't think of initially.

task foo(const std::string& s);

task bar(std::string s) {
  return foo(s); // ref to s will dangle
}

task baz() {
  co_await bar("hello"_s);
}

Dangling reference parameters are a common pitfall with coroutines that is usually solved by making awaitables immovable and allowing the user to only await prvalues. For example, this is done at Google (https://m.youtube.com/watch?v=k-A12dpMYHo&t=498s). However, the example above is one that cannot be fixed on the library side because copy elision makes it valid even if awaitable is immovable.

That being said, I wouldn't want to impose this onto anyone if they designed their types in a different way. I'd be totally fine if this warning was optional and I could just flip it on. I would personally find this to be a very helpful diagnostic that would have caught many mistakes I've made in the past.