From f26d551387f032e05e5e6551605b150f38c3f5b2 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Tue, 15 Aug 2017 22:58:45 +0000 Subject: [PATCH] Do not look through pack expansions when looking for unexpanded parameter packs. Fixes a selection of rejects-valids when pack-expanding a lambda that itself contains a pack expansion. llvm-svn: 310972 --- clang/include/clang/AST/RecursiveASTVisitor.h | 15 ++- clang/lib/Sema/SemaTemplateVariadic.cpp | 82 ++++++++++++---- .../CXX/temp/temp.decls/temp.variadic/p4.cpp | 98 +++++++++++++++++++ clang/utils/TableGen/ClangAttrEmitter.cpp | 21 ++++ 4 files changed, 197 insertions(+), 19 deletions(-) diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h index b696a032853a..d86099190715 100644 --- a/clang/include/clang/AST/RecursiveASTVisitor.h +++ b/clang/include/clang/AST/RecursiveASTVisitor.h @@ -267,6 +267,12 @@ public: bool TraverseTemplateArguments(const TemplateArgument *Args, unsigned NumArgs); + /// \brief Recursively visit a base specifier. This can be overridden by a + /// subclass. + /// + /// \returns false if the visitation was terminated early, true otherwise. + bool TraverseCXXBaseSpecifier(const CXXBaseSpecifier &Base); + /// \brief Recursively visit a constructor initializer. This /// automatically dispatches to another visitor for the initializer /// expression, but not for the name of the initializer, so may @@ -1768,13 +1774,20 @@ bool RecursiveASTVisitor::TraverseRecordHelper(RecordDecl *D) { return true; } +template +bool RecursiveASTVisitor::TraverseCXXBaseSpecifier( + const CXXBaseSpecifier &Base) { + TRY_TO(TraverseTypeLoc(Base.getTypeSourceInfo()->getTypeLoc())); + return true; +} + template bool RecursiveASTVisitor::TraverseCXXRecordHelper(CXXRecordDecl *D) { if (!TraverseRecordHelper(D)) return false; if (D->isCompleteDefinition()) { for (const auto &I : D->bases()) { - TRY_TO(TraverseTypeLoc(I.getTypeSourceInfo()->getTypeLoc())); + TRY_TO(TraverseCXXBaseSpecifier(I)); } // We don't traverse the friends or the conversions, as they are // already in decls_begin()/decls_end(). diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp index e3925e812ac0..14b8981f7998 100644 --- a/clang/lib/Sema/SemaTemplateVariadic.cpp +++ b/clang/lib/Sema/SemaTemplateVariadic.cpp @@ -161,7 +161,7 @@ namespace { return true; } - /// \brief Suppress traversel into types with location information + /// \brief Suppress traversal into types with location information /// that do not contain unexpanded parameter packs. bool TraverseTypeLoc(TypeLoc TL) { if ((!TL.getType().isNull() && @@ -172,19 +172,48 @@ namespace { return true; } - /// \brief Suppress traversal of non-parameter declarations, since - /// they cannot contain unexpanded parameter packs. + /// \brief Suppress traversal of parameter packs. bool TraverseDecl(Decl *D) { - auto *PVD = dyn_cast_or_null(D); // A function parameter pack is a pack expansion, so cannot contain - // an unexpanded parameter pack. - if (PVD && PVD->isParameterPack()) + // an unexpanded parameter pack. Likewise for a template parameter + // pack that contains any references to other packs. + if (D->isParameterPack()) return true; - if (PVD || InLambda) - return inherited::TraverseDecl(D); + return inherited::TraverseDecl(D); + } - return true; + /// \brief Suppress traversal of pack-expanded attributes. + bool TraverseAttr(Attr *A) { + if (A->isPackExpansion()) + return true; + + return inherited::TraverseAttr(A); + } + + /// \brief Suppress traversal of pack expansion expressions and types. + ///@{ + bool TraversePackExpansionType(PackExpansionType *T) { return true; } + bool TraversePackExpansionTypeLoc(PackExpansionTypeLoc TL) { return true; } + bool TraversePackExpansionExpr(PackExpansionExpr *E) { return true; } + bool TraverseCXXFoldExpr(CXXFoldExpr *E) { return true; } + + ///@} + + /// \brief Suppress traversal of using-declaration pack expansion. + bool TraverseUnresolvedUsingValueDecl(UnresolvedUsingValueDecl *D) { + if (D->isPackExpansion()) + return true; + + return inherited::TraverseUnresolvedUsingValueDecl(D); + } + + /// \brief Suppress traversal of using-declaration pack expansion. + bool TraverseUnresolvedUsingTypenameDecl(UnresolvedUsingTypenameDecl *D) { + if (D->isPackExpansion()) + return true; + + return inherited::TraverseUnresolvedUsingTypenameDecl(D); } /// \brief Suppress traversal of template argument pack expansions. @@ -203,6 +232,22 @@ namespace { return inherited::TraverseTemplateArgumentLoc(ArgLoc); } + /// \brief Suppress traversal of base specifier pack expansions. + bool TraverseCXXBaseSpecifier(const CXXBaseSpecifier &Base) { + if (Base.isPackExpansion()) + return true; + + return inherited::TraverseCXXBaseSpecifier(Base); + } + + /// \brief Suppress traversal of mem-initializer pack expansions. + bool TraverseConstructorInitializer(CXXCtorInitializer *Init) { + if (Init->isPackExpansion()) + return true; + + return inherited::TraverseConstructorInitializer(Init); + } + /// \brief Note whether we're traversing a lambda containing an unexpanded /// parameter pack. In this case, the unexpanded pack can occur anywhere, /// including all the places where we normally wouldn't look. Within a @@ -233,6 +278,7 @@ namespace { Expr *Init) { if (C->isPackExpansion()) return true; + return inherited::TraverseLambdaCapture(Lambda, C, Init); } }; @@ -266,25 +312,25 @@ Sema::DiagnoseUnexpandedParameterPacks(SourceLocation Loc, // parameter pack, and we are done. // FIXME: Store 'Unexpanded' on the lambda so we don't need to recompute it // later. - SmallVector GenericLambdaParamReferences; + SmallVector LambdaParamPackReferences; for (unsigned N = FunctionScopes.size(); N; --N) { if (sema::LambdaScopeInfo *LSI = dyn_cast(FunctionScopes[N-1])) { - if (LSI->isGenericLambda()) { + if (N == FunctionScopes.size()) { for (auto &Param : Unexpanded) { auto *PD = dyn_cast_or_null( Param.first.dyn_cast()); if (PD && PD->getDeclContext() == LSI->CallOperator) - GenericLambdaParamReferences.push_back(Param); + LambdaParamPackReferences.push_back(Param); } } - // If we have references to a parameter of a generic lambda, only - // diagnose those ones. We don't know whether any other unexpanded - // parameters referenced herein are actually unexpanded; they might - // be expanded at an outer level. - if (!GenericLambdaParamReferences.empty()) { - Unexpanded = GenericLambdaParamReferences; + // If we have references to a parameter pack of the innermost enclosing + // lambda, only diagnose those ones. We don't know whether any other + // unexpanded parameters referenced herein are actually unexpanded; + // they might be expanded at an outer level. + if (!LambdaParamPackReferences.empty()) { + Unexpanded = LambdaParamPackReferences; break; } diff --git a/clang/test/CXX/temp/temp.decls/temp.variadic/p4.cpp b/clang/test/CXX/temp/temp.decls/temp.variadic/p4.cpp index d8294a1f154a..1681325f2e6c 100644 --- a/clang/test/CXX/temp/temp.decls/temp.variadic/p4.cpp +++ b/clang/test/CXX/temp/temp.decls/temp.variadic/p4.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -std=c++11 -fsyntax-only -fexceptions -fcxx-exceptions -verify %s +// RUN: %clang_cc1 -std=c++2a -fsyntax-only -fexceptions -fcxx-exceptions -verify %s template struct tuple; template struct int_c; @@ -174,6 +175,7 @@ int check_temp_arg_1[is_same::type, tuple, int_c<2>, int_c<3>, int_c<4>, int_c<5>>>::value? 1 : -1]; +#if __cplusplus < 201703L // In a dynamic-exception-specification (15.4); the pattern is a type-id. template struct f_with_except { @@ -191,3 +193,99 @@ struct check_f_with_except_2 : f_with_except { struct check_f_with_except_3 : f_with_except { virtual void f() throw(int, float, double); // expected-error{{exception specification of overriding function is more lax than base version}} }; +#endif + +namespace PackExpansionWithinLambda { + void swallow(...); + template void f(U ...u) { + swallow([=] { + // C++17 [temp.variadic]p4: + // Pack expansions can occur in the following contexts: + + // - in a function parameter pack + void g(T...); + +#if __cplusplus >= 201703L + struct A : T... { + // - in a using-declaration + using T::x...; + using typename T::U...; + }; +#endif + + // - in a template parameter pack that is a pack expansion + // FIXME: We do not support any way to reach this case yet. + + // - in an initializer-list + int arr[] = {T().x...}; + + // - in a base-specifier-list + struct B : T... { + // - in a mem-initializer-list + B() : T{0}... {} + }; + + // - in a template-argument-list + f(); + + // - in an attribute-list + // FIXME: We do not support any such attributes yet. + + // - in an alignment-specifier + alignas(T...) int y; + + // - in a capture-list + [](T ...t) { [t...]{}(); } (T()...); + + // - in a sizeof... expression + const int k1 = sizeof...(T); + +#if __cplusplus >= 201703L + // - in a fold-expression + const int k2 = ((sizeof(T)/sizeof(T)) + ...); + + static_assert(k1 == k2); +#endif + + // Trigger clang to look in here for unexpanded packs. + U u; + } ...); + } + + template void nested() { + swallow([=] { + [](T ...t) { [t]{}(); } (T()...); // expected-error {{unexpanded parameter pack 't'}} + }...); // expected-error {{does not contain any unexpanded}} + } + + template void g() { + // Check that we do detect the above cases when the pack is not expanded. + swallow([=] { void h(T); }); // expected-error {{unexpanded parameter pack 'T'}} + swallow([=] { struct A : T {}; }); // expected-error {{unexpanded parameter pack 'T'}} +#if __cplusplus >= 201703L + swallow([=] { struct A : T... { using T::x; }; }); // expected-error {{unexpanded parameter pack 'T'}} + swallow([=] { struct A : T... { using typename T::U; }; }); // expected-error {{unexpanded parameter pack 'T'}} +#endif + + swallow([=] { int arr[] = {T().x}; }); // expected-error {{unexpanded parameter pack 'T'}} + swallow([=] { struct B : T... { B() : T{0} {} }; }); // expected-error {{unexpanded parameter pack 'T'}} + swallow([=] { f(); }); // expected-error {{unexpanded parameter pack 'T'}} + swallow([=] { alignas(T) int y; }); // expected-error {{unexpanded parameter pack 'T'}} + swallow([=] { [](T ...t) { + [t]{}(); // expected-error {{unexpanded parameter pack 't'}} + } (T()...); }); + } + + struct T { int x; using U = int; }; + void g() { f(1, 2, 3); } + + template void pack_in_lambda(U ...u) { // expected-note {{here}} + // FIXME: Move this test into 'f' above once we support this syntax. + [] typename ...U>(U ...uv) {}; // expected-error {{expected body of lambda}} expected-error {{does not refer to a value}} + } + + template void pack_expand_attr() { + // FIXME: Move this test into 'f' above once we support this. + [[gnu::aligned(alignof(T))...]] int x; // expected-error {{cannot be used as an attribute pack}} expected-error {{unexpanded}} + } +} diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp index b6d2988964b4..abbef617bb1d 100644 --- a/clang/utils/TableGen/ClangAttrEmitter.cpp +++ b/clang/utils/TableGen/ClangAttrEmitter.cpp @@ -490,6 +490,17 @@ namespace { OS << "}\n"; } + void writeASTVisitorTraversal(raw_ostream &OS) const override { + StringRef Name = getUpperName(); + OS << " if (A->is" << Name << "Expr()) {\n" + << " if (!getDerived().TraverseStmt(A->get" << Name << "Expr()))\n" + << " return false;\n" + << " } else if (auto *TSI = A->get" << Name << "Type()) {\n" + << " if (!getDerived().TraverseTypeLoc(TSI->getTypeLoc()))\n" + << " return false;\n" + << " }\n"; + } + void writeCloneArgs(raw_ostream &OS) const override { OS << "is" << getLowerName() << "Expr, is" << getLowerName() << "Expr ? static_cast(" << getLowerName() @@ -630,6 +641,10 @@ namespace { << "A->" << getLowerName() << "_size()"; } + void writeASTVisitorTraversal(raw_ostream &OS) const override { + // FIXME: Traverse the elements. + } + void writeCtorBody(raw_ostream &OS) const override { OS << " std::copy(" << getUpperName() << ", " << getUpperName() << " + " << ArgSizeName << ", " << ArgName << ");\n"; @@ -1153,6 +1168,12 @@ namespace { OS << " }"; } + void writeASTVisitorTraversal(raw_ostream &OS) const override { + OS << " if (auto *TSI = A->get" << getUpperName() << "Loc())\n"; + OS << " if (!getDerived().TraverseTypeLoc(TSI->getTypeLoc()))\n"; + OS << " return false;\n"; + } + void writeTemplateInstantiationArgs(raw_ostream &OS) const override { OS << "A->get" << getUpperName() << "Loc()"; }