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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to