Author: ericwf Date: Mon Mar 6 16:52:28 2017 New Revision: 297089 URL: http://llvm.org/viewvc/llvm-project?rev=297089&view=rev Log: [coroutines] Improve diagnostics when building implicit constructs.
Previously when a coroutine was building the implicit setup/destroy constructs it would emit diagostics about failures on the first co_await/co_return/co_yield it encountered. This was confusing because that construct may not itself be ill-formed. This patch moves the diagnostics to the function start instead. Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Sema/SemaCoroutine.cpp cfe/trunk/test/SemaCXX/coroutines.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=297089&r1=297088&r2=297089&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Mar 6 16:52:28 2017 @@ -8837,6 +8837,8 @@ def err_implied_std_coroutine_traits_pro "this function cannot be a coroutine: %q0 has no member named 'promise_type'">; def err_implied_std_coroutine_traits_promise_type_not_class : Error< "this function cannot be a coroutine: %0 is not a class">; +def err_coroutine_promise_type_incomplete : Error< + "this function cannot be a coroutine: %0 is an incomplete type">; def err_coroutine_traits_missing_specialization : Error< "this function cannot be a coroutine: missing definition of " "specialization %q0">; Modified: cfe/trunk/lib/Sema/SemaCoroutine.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCoroutine.cpp?rev=297089&r1=297088&r2=297089&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaCoroutine.cpp (original) +++ cfe/trunk/lib/Sema/SemaCoroutine.cpp Mon Mar 6 16:52:28 2017 @@ -24,18 +24,19 @@ using namespace sema; /// Look up the std::coroutine_traits<...>::promise_type for the given /// function type. static QualType lookupPromiseType(Sema &S, const FunctionProtoType *FnType, - SourceLocation Loc) { + SourceLocation KwLoc, + SourceLocation FuncLoc) { // FIXME: Cache std::coroutine_traits once we've found it. NamespaceDecl *StdExp = S.lookupStdExperimentalNamespace(); if (!StdExp) { - S.Diag(Loc, diag::err_implied_std_coroutine_traits_not_found); + S.Diag(KwLoc, diag::err_implied_std_coroutine_traits_not_found); return QualType(); } LookupResult Result(S, &S.PP.getIdentifierTable().get("coroutine_traits"), - Loc, Sema::LookupOrdinaryName); + FuncLoc, Sema::LookupOrdinaryName); if (!S.LookupQualifiedName(Result, StdExp)) { - S.Diag(Loc, diag::err_implied_std_coroutine_traits_not_found); + S.Diag(KwLoc, diag::err_implied_std_coroutine_traits_not_found); return QualType(); } @@ -49,52 +50,58 @@ static QualType lookupPromiseType(Sema & } // Form template argument list for coroutine_traits<R, P1, P2, ...>. - TemplateArgumentListInfo Args(Loc, Loc); + TemplateArgumentListInfo Args(KwLoc, KwLoc); Args.addArgument(TemplateArgumentLoc( TemplateArgument(FnType->getReturnType()), - S.Context.getTrivialTypeSourceInfo(FnType->getReturnType(), Loc))); + S.Context.getTrivialTypeSourceInfo(FnType->getReturnType(), KwLoc))); // FIXME: If the function is a non-static member function, add the type // of the implicit object parameter before the formal parameters. for (QualType T : FnType->getParamTypes()) Args.addArgument(TemplateArgumentLoc( - TemplateArgument(T), S.Context.getTrivialTypeSourceInfo(T, Loc))); + TemplateArgument(T), S.Context.getTrivialTypeSourceInfo(T, KwLoc))); // Build the template-id. QualType CoroTrait = - S.CheckTemplateIdType(TemplateName(CoroTraits), Loc, Args); + S.CheckTemplateIdType(TemplateName(CoroTraits), KwLoc, Args); if (CoroTrait.isNull()) return QualType(); - if (S.RequireCompleteType(Loc, CoroTrait, + if (S.RequireCompleteType(KwLoc, CoroTrait, diag::err_coroutine_traits_missing_specialization)) return QualType(); - CXXRecordDecl *RD = CoroTrait->getAsCXXRecordDecl(); + auto *RD = CoroTrait->getAsCXXRecordDecl(); assert(RD && "specialization of class template is not a class?"); // Look up the ::promise_type member. - LookupResult R(S, &S.PP.getIdentifierTable().get("promise_type"), Loc, + LookupResult R(S, &S.PP.getIdentifierTable().get("promise_type"), KwLoc, Sema::LookupOrdinaryName); S.LookupQualifiedName(R, RD); auto *Promise = R.getAsSingle<TypeDecl>(); if (!Promise) { - S.Diag(Loc, diag::err_implied_std_coroutine_traits_promise_type_not_found) + S.Diag(FuncLoc, + diag::err_implied_std_coroutine_traits_promise_type_not_found) << RD; return QualType(); } - // The promise type is required to be a class type. QualType PromiseType = S.Context.getTypeDeclType(Promise); - if (!PromiseType->getAsCXXRecordDecl()) { - // Use the fully-qualified name of the type. + + auto buildElaboratedType = [&]() { auto *NNS = NestedNameSpecifier::Create(S.Context, nullptr, StdExp); NNS = NestedNameSpecifier::Create(S.Context, NNS, false, CoroTrait.getTypePtr()); - PromiseType = S.Context.getElaboratedType(ETK_None, NNS, PromiseType); + return S.Context.getElaboratedType(ETK_None, NNS, PromiseType); + }; - S.Diag(Loc, diag::err_implied_std_coroutine_traits_promise_type_not_class) - << PromiseType; + if (!PromiseType->getAsCXXRecordDecl()) { + S.Diag(FuncLoc, + diag::err_implied_std_coroutine_traits_promise_type_not_class) + << buildElaboratedType(); return QualType(); } + if (S.RequireCompleteType(FuncLoc, buildElaboratedType(), + diag::err_coroutine_promise_type_incomplete)) + return QualType(); return PromiseType; } @@ -176,7 +183,8 @@ static FunctionScopeInfo *checkCoroutine QualType T = FD->getType()->isDependentType() ? S.Context.DependentTy : lookupPromiseType( - S, FD->getType()->castAs<FunctionProtoType>(), Loc); + S, FD->getType()->castAs<FunctionProtoType>(), + Loc, FD->getLocation()); if (T.isNull()) return nullptr; Modified: cfe/trunk/test/SemaCXX/coroutines.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/coroutines.cpp?rev=297089&r1=297088&r2=297089&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/coroutines.cpp (original) +++ cfe/trunk/test/SemaCXX/coroutines.cpp Mon Mar 6 16:52:28 2017 @@ -59,14 +59,14 @@ void no_specialization() { template <typename... T> struct std::experimental::coroutine_traits<int, T...> {}; -int no_promise_type() { - co_await a; // expected-error {{this function cannot be a coroutine: 'std::experimental::coroutine_traits<int>' has no member named 'promise_type'}} +int no_promise_type() { // expected-error {{this function cannot be a coroutine: 'std::experimental::coroutine_traits<int>' has no member named 'promise_type'}} + co_await a; } template <> struct std::experimental::coroutine_traits<double, double> { typedef int promise_type; }; -double bad_promise_type(double) { - co_await a; // expected-error {{this function cannot be a coroutine: 'experimental::coroutine_traits<double, double>::promise_type' (aka 'int') is not a class}} +double bad_promise_type(double) { // expected-error {{this function cannot be a coroutine: 'experimental::coroutine_traits<double, double>::promise_type' (aka 'int') is not a class}} + co_await a; } template <> @@ -77,7 +77,7 @@ double bad_promise_type_2(int) { co_yield 0; // expected-error {{no member named 'yield_value' in 'std::experimental::coroutine_traits<double, int>::promise_type'}} } -struct promise; // expected-note 2{{forward declaration}} +struct promise; // expected-note {{forward declaration}} struct promise_void; struct void_tag {}; template <typename... T> @@ -93,10 +93,7 @@ struct coroutine_handle; } } -// FIXME: This diagnostic is terrible. -void undefined_promise() { // expected-error {{variable has incomplete type 'promise_type'}} - // FIXME: This diagnostic doesn't make any sense. - // expected-error@-2 {{incomplete definition of type 'promise'}} +void undefined_promise() { // expected-error {{this function cannot be a coroutine: 'experimental::coroutine_traits<void>::promise_type' (aka 'promise') is an incomplete type}} co_await a; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits