This is an archive of the discontinued LLVM Phabricator instance.

[coroutines][PR41909] Generalize fix from D62550
ClosedPublic

Authored by modocache on Nov 21 2019, 5:13 PM.

Details

Summary

In https://reviews.llvm.org/D62550 @rsmith pointed out that there are
many situations in which a coroutine body statement may be
transformed/rebuilt as part of a template instantiation, and my naive
check whether the coroutine was a generic lambda was insufficient.

This is indeed true, as I've learned by reading more of the
TreeTransform code. Most transformations are written in a way that
doesn't assume the resulting types are not dependent types. So the
assertion in 'TransformCoroutineBodyStmt', that the promise type must no
longer be dependent, is out of place.

This patch removes the assertion, spruces up some code comments, and
adds a test that would have failed with my naive check from
https://reviews.llvm.org/D62550.

Diff Detail

Event Timeline

modocache created this revision.Nov 21 2019, 5:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 21 2019, 5:13 PM
Herald added a subscriber: EricWF. · View Herald Transcript

This also fix same ice when build cppcoro with current trunk. FYI

@GorNishanov, @rsmith, friendly ping! @rsmith this patch addresses your requests from https://reviews.llvm.org/D62550.

rsmith accepted this revision.Dec 14 2019, 3:44 PM
This revision is now accepted and ready to land.Dec 14 2019, 3:44 PM
This revision was automatically updated to reflect the committed changes.

Thanks for the review! Much appreciated :)