EricWF requested changes to this revision.
EricWF added a comment.
This revision now requires changes to proceed.
I think the test could be improved. First could you add the test within
`test/SemaCXX/coroutines.cpp`? Second could you add some negative tests that
check the diagnostics generated when you don't provide a specialization of
coroutine traits (ie that the old behaviour of not including the class type now
produces diagnostics). Could you also add tests for const/volatile and
lvalue/rvalue qualified member functions?
================
Comment at: lib/Sema/SemaCoroutine.cpp:85
+ if (MD->isInstance()) {
+ QualType T = MD->getThisType(S.Context);
+ Args.addArgument(TemplateArgumentLoc(
----------------
This seems wrong to me.
`getThisType` returns the type of the `this` parameter as specified under
[class.this] but according to the coroutines spec the type of the parameter
should be the type of the `implicit object parameter`, which is specified under
[[http://eel.is/c++draft/over.match.funcs#4 | (over.match.funcs) p4 ]].
================
Comment at: lib/Sema/SemaCoroutine.cpp:441
+ return false;
+ }();
----------------
Huh, I've never seen lambdas used like this before but I really like it.
https://reviews.llvm.org/D35046
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
