blastrock created this revision. blastrock added reviewers: GorNishanov, EricWF, lewissbaker, tks2103, modocache, rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. blastrock edited the summary of this revision.
This started with bug https://bugs.llvm.org/show_bug.cgi?id=41909 which was fixed by https://reviews.llvm.org/D62550 . In the last comment of the bug report <https://bugs.llvm.org/show_bug.cgi?id=41909#c3>, Pavel A. Lebedev gives a code snippet that now makes clang trigger an ICE. This code was broken by that very commit. I am no expert, but I tried to follow this comment <https://reviews.llvm.org/D62550#1526987>. We now build the dependent statements only if the promise type is not dependent anymore. If it still is, I guess dependent statements are built in a later pass when all templates are instantiated. I have added the second code snippet from the bug and the godbolt snippet from modocache's answer on the review, this patch seems to fix them all. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D71542 Files: clang/lib/Sema/TreeTransform.h clang/test/SemaCXX/coroutines.cpp Index: clang/test/SemaCXX/coroutines.cpp =================================================================== --- clang/test/SemaCXX/coroutines.cpp +++ clang/test/SemaCXX/coroutines.cpp @@ -734,6 +734,26 @@ } template void ok_generic_lambda_coawait_PR41909<int>(); // expected-note {{in instantiation of function template specialization 'ok_generic_lambda_coawait_PR41909<int>' requested here}} +void ok_generic_lambda_coawait_PR41909_2() { + [](auto &arg) -> coro<good_promise_1> { // expected-warning {{expression result unused}} + co_await 12; + }; + [](auto &arg) -> coro<good_promise_1> { + co_await 24; + }("argument"); +} + +template <typename T> +void ok_generic_lambda_coawait_PR41909_3() { + [](auto &arg) -> coro<good_promise_1> { // expected-warning {{expression result unused}} + []() -> coro<good_promise_1> { + co_await 12; + }; + co_await 24; + }; +} +template void ok_generic_lambda_coawait_PR41909_3<int>(); // expected-note {{in instantiation of function template specialization 'ok_generic_lambda_coawait_PR41909_3<int>' requested here}} + template<> struct std::experimental::coroutine_traits<int, int, const char**> { using promise_type = promise; }; Index: clang/lib/Sema/TreeTransform.h =================================================================== --- clang/lib/Sema/TreeTransform.h +++ clang/lib/Sema/TreeTransform.h @@ -7229,16 +7229,7 @@ Builder.ReturnValue = Res.get(); if (S->hasDependentPromiseType()) { - // PR41909: We may find a generic coroutine lambda definition within a - // template function that is being instantiated. In this case, the lambda - // will have a dependent promise type, until it is used in an expression - // that creates an instantiation with a non-dependent promise type. We - // should not assert or build coroutine dependent statements for such a - // generic lambda. - auto *MD = dyn_cast_or_null<CXXMethodDecl>(FD); - if (!MD || !MD->getParent()->isGenericLambda()) { - assert(!Promise->getType()->isDependentType() && - "the promise type must no longer be dependent"); + if (!Promise->getType()->isDependentType()) { assert(!S->getFallthroughHandler() && !S->getExceptionHandler() && !S->getReturnStmtOnAllocFailure() && !S->getDeallocate() && "these nodes should not have been built yet");
Index: clang/test/SemaCXX/coroutines.cpp =================================================================== --- clang/test/SemaCXX/coroutines.cpp +++ clang/test/SemaCXX/coroutines.cpp @@ -734,6 +734,26 @@ } template void ok_generic_lambda_coawait_PR41909<int>(); // expected-note {{in instantiation of function template specialization 'ok_generic_lambda_coawait_PR41909<int>' requested here}} +void ok_generic_lambda_coawait_PR41909_2() { + [](auto &arg) -> coro<good_promise_1> { // expected-warning {{expression result unused}} + co_await 12; + }; + [](auto &arg) -> coro<good_promise_1> { + co_await 24; + }("argument"); +} + +template <typename T> +void ok_generic_lambda_coawait_PR41909_3() { + [](auto &arg) -> coro<good_promise_1> { // expected-warning {{expression result unused}} + []() -> coro<good_promise_1> { + co_await 12; + }; + co_await 24; + }; +} +template void ok_generic_lambda_coawait_PR41909_3<int>(); // expected-note {{in instantiation of function template specialization 'ok_generic_lambda_coawait_PR41909_3<int>' requested here}} + template<> struct std::experimental::coroutine_traits<int, int, const char**> { using promise_type = promise; }; Index: clang/lib/Sema/TreeTransform.h =================================================================== --- clang/lib/Sema/TreeTransform.h +++ clang/lib/Sema/TreeTransform.h @@ -7229,16 +7229,7 @@ Builder.ReturnValue = Res.get(); if (S->hasDependentPromiseType()) { - // PR41909: We may find a generic coroutine lambda definition within a - // template function that is being instantiated. In this case, the lambda - // will have a dependent promise type, until it is used in an expression - // that creates an instantiation with a non-dependent promise type. We - // should not assert or build coroutine dependent statements for such a - // generic lambda. - auto *MD = dyn_cast_or_null<CXXMethodDecl>(FD); - if (!MD || !MD->getParent()->isGenericLambda()) { - assert(!Promise->getType()->isDependentType() && - "the promise type must no longer be dependent"); + if (!Promise->getType()->isDependentType()) { assert(!S->getFallthroughHandler() && !S->getExceptionHandler() && !S->getReturnStmtOnAllocFailure() && !S->getDeallocate() && "these nodes should not have been built yet");
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits