modocache requested changes to this revision. modocache added a comment. This revision now requires changes to proceed.
Sorry for the slow response here, @junparser! The test case you came up with here is great! I can see the issue is that `ScopeInfo->CoroutineParameterMoves` are built up when Clang parses the first `co_await a`, but are not cleared when `lookupPromiseType` results in an error. As a result, when Clang hits the second `co_await a`, it's in a state that the current code didn't anticipate. Your test case does a great job demonstrating this. Your fix for the problem also looks good to me! My only suggestion is to make the test case just a little clearer, as I'll explain... (Also, in the future could you please upload your patches with full context? You can read https://llvm.org/docs/Phabricator.html for more details. I think the section explaining the web interface might be relevant to you, where it suggests `git show HEAD -U999999 > mypatch.patch`. The reason I ask is because on Phabricator I can see what lines you're proposing should be added, but not the surrounding source lines, which made this more difficult to review.) ================ Comment at: test/SemaCXX/coroutines.cpp:93 + co_await a; +} + ---------------- This is a great test case, thanks for coming up with it! However, I think it could be a little clearer, though: right now the `int a` variable is shadowing the `awaitable a` that's declared further up in this file. At first, I wasn't sure whether the shadowing had something to do with the test, but in fact it doesn't. This test case demonstrates the issue but without the confusion, I think: ``` int no_promise_type_multiple_awaits(int) { // expected-error {{this function cannot be a coroutine: 'std::experimental::coroutine_traits<int, int>' has no member named 'promise_type'}} co_await a; co_await a; } ``` Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69022/new/ https://reviews.llvm.org/D69022 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
