[PATCH] D69022: [coroutines] Remove assert on CoroutineParameterMoves in Sema::buildCoroutineParameterMoves

[JunMa via Phabricator via cfe-commits]

junparser added a comment.

In D69022#1755636 <https://reviews.llvm.org/D69022#1755636>, @modocache wrote:

> 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.)


Thanks so much for reviewing the patch and giving the helpful advise.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69022/new/

https://reviews.llvm.org/D69022