[PATCH] D48863: [Sema] Explain coroutine_traits template in diag

[Brian Gesiak via Phabricator via cfe-commits]

modocache added inline comments.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9053
   "a coroutine">;
+def note_coroutine_types_for_traits_here : Note<
+  "the coroutine traits class template is being instantiated using the return "
----------------
GorNishanov wrote:
> I am wondering what is the use case here.
> Is it to guard against badly designed standard library?
> 
> I played around a little and tried to see how I could get to trigger this error with a user-code, but, failed. If I did not specify enough parameters it will default to primary template defined in the <experimental/coroutine>
That's fair. Here's my rationale: SemaCoroutune.cpp contains diagnostics, even before this change, in case of a "badly designed standard library". There's a diagnostic for if coroutine_traits is defined as non-template class, for example.

A user would not encounter that diagnostic under any normal circumstance. If they follow the instructions the compiler gives them, they'll include `<experimental/coroutine>` and be done with it. But since we have the code to emit the (unlikely to ever be seen) diagnostic anyway, why not make it as helpful as we can?

If a user for some reason defines their own `coroutine_traits`, and their definition only takes a single template argument, and if they define a coroutine with a single return type and a single parameter, then they'll see the diagnostic "too many template arguments for class template". At this point to learn why the heck a `co_return` statement is instantiating a template, they'd have to either read the compiler source code or the standard. My rationale is that we might as well save them this step.

All that being said, this change is just a suggestion, I'm not insisting we put it in! I drew some inspiration for this change from your Naked Coroutines talk, in which you used the `co_return` keyword and then followed the instructions the compiler gave you until you had a working program. Since the Clang diagnostic tells me in that case that "std::experimental::coroutine_traits is not defined, you must include <experimental/coroutine>", I wondered what would happen if a user decided to just define their own `std::experimental::coroutine_traits` instead. If they do that, and correct the errors that `coroutine_traits` must be a class and it must be a template, eventually they could find their way to this diagnostic. I agree that it's an unlikely case, so feel free to reject this and I'll close it :)


Repository:
  rC Clang

https://reviews.llvm.org/D48863