From ae01c74ba875f6f90ca01d2a9b3183139ae0b538 Mon Sep 17 00:00:00 2001 From: Corentin Jabot Date: Sat, 12 Mar 2022 20:49:01 +0100 Subject: [PATCH] [Clang] Fix Unevaluated Lambdas Unlike other types, when lambdas are instanciated, they are recreated from scratch. When an unevaluated lambdas appear in the type of a function, parameter it is instanciated in the wrong declaration context, as parameters are transformed before the function. To support lambda in function parameters, we try to compute whether they are dependant without looking at the declaration context. This is a short term stopgap solution to avoid clang iceing. A better fix might be to inject some kind of transparent declaration with correctly computed dependency for function parameters, variable templates, etc. Fixes https://github.com/llvm/llvm-project/issues/50376 Fixes https://github.com/llvm/llvm-project/issues/51414 Fixes https://github.com/llvm/llvm-project/issues/51416 Fixes https://github.com/llvm/llvm-project/issues/51641 Fixes https://github.com/llvm/llvm-project/issues/54296 Reviewed By: aaron.ballman Differential Revision: https://reviews.llvm.org/D121532 (cherry picked from commit 3784e8ccfbdaaab31f9e9c221daa59a218279999) --- clang/docs/ReleaseNotes.rst | 8 ++ clang/include/clang/AST/DeclCXX.h | 28 +++++-- clang/include/clang/Sema/Sema.h | 2 +- clang/lib/AST/ASTImporter.cpp | 2 +- clang/lib/AST/DeclBase.cpp | 2 + clang/lib/AST/DeclCXX.cpp | 8 +- clang/lib/Sema/SemaLambda.cpp | 31 ++++---- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 2 +- clang/lib/Sema/TreeTransform.h | 24 ++++-- clang/lib/Serialization/ASTReaderDecl.cpp | 6 +- clang/lib/Serialization/ASTWriter.cpp | 2 +- clang/test/SemaCXX/lambda-unevaluated.cpp | 79 ++++++++++++++++++- clang/unittests/AST/ASTImporterTest.cpp | 2 + clang/www/cxx_status.html | 6 +- 14 files changed, 161 insertions(+), 41 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 5821e41fc73320..6075bfb6f410ed 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -67,6 +67,14 @@ Major New Features - ``float.h`` now exposes (in hosted mode) extensions made available from the AIX system header. +- Unevaluated lambdas in dependant contexts no longer result in clang crashing. + This fixes Issues `50376 < https://github.com/llvm/llvm-project/issues/50376>`_, + `54296 < https://github.com/llvm/llvm-project/issues/54296>`_, + `51414 < https://github.com/llvm/llvm-project/issues/51414>`_, + `51416 < https://github.com/llvm/llvm-project/issues/51416>`_, + and `51641 < https://github.com/llvm/llvm-project/issues/51641>`_. + + Improvements to Clang's diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index 2833df0505daca..545f29975fd89c 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -275,6 +275,14 @@ class CXXRecordDecl : public RecordDecl { SMF_All = 0x3f }; +public: + enum LambdaDependencyKind { + LDK_Unknown = 0, + LDK_AlwaysDependent, + LDK_NeverDependent, + }; + +private: struct DefinitionData { #define FIELD(Name, Width, Merge) \ unsigned Name : Width; @@ -374,7 +382,7 @@ class CXXRecordDecl : public RecordDecl { /// lambda will have been created with the enclosing context as its /// declaration context, rather than function. This is an unfortunate /// artifact of having to parse the default arguments before. - unsigned Dependent : 1; + unsigned DependencyKind : 2; /// Whether this lambda is a generic lambda. unsigned IsGenericLambda : 1; @@ -408,9 +416,9 @@ class CXXRecordDecl : public RecordDecl { /// The type of the call method. TypeSourceInfo *MethodTyInfo; - LambdaDefinitionData(CXXRecordDecl *D, TypeSourceInfo *Info, bool Dependent, + LambdaDefinitionData(CXXRecordDecl *D, TypeSourceInfo *Info, unsigned DK, bool IsGeneric, LambdaCaptureDefault CaptureDefault) - : DefinitionData(D), Dependent(Dependent), IsGenericLambda(IsGeneric), + : DefinitionData(D), DependencyKind(DK), IsGenericLambda(IsGeneric), CaptureDefault(CaptureDefault), NumCaptures(0), NumExplicitCaptures(0), HasKnownInternalLinkage(0), ManglingNumber(0), MethodTyInfo(Info) { @@ -547,7 +555,7 @@ class CXXRecordDecl : public RecordDecl { bool DelayTypeCreation = false); static CXXRecordDecl *CreateLambda(const ASTContext &C, DeclContext *DC, TypeSourceInfo *Info, SourceLocation Loc, - bool DependentLambda, bool IsGeneric, + unsigned DependencyKind, bool IsGeneric, LambdaCaptureDefault CaptureDefault); static CXXRecordDecl *CreateDeserialized(const ASTContext &C, unsigned ID); @@ -1774,7 +1782,17 @@ class CXXRecordDecl : public RecordDecl { /// function declaration itself is dependent. This flag indicates when we /// know that the lambda is dependent despite that. bool isDependentLambda() const { - return isLambda() && getLambdaData().Dependent; + return isLambda() && getLambdaData().DependencyKind == LDK_AlwaysDependent; + } + + bool isNeverDependentLambda() const { + return isLambda() && getLambdaData().DependencyKind == LDK_NeverDependent; + } + + unsigned getLambdaDependencyKind() const { + if (!isLambda()) + return LDK_Unknown; + return getLambdaData().DependencyKind; } TypeSourceInfo *getLambdaTypeInfo() const { diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 4b609f4b1477c3..43772f2cd735e7 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -6737,7 +6737,7 @@ class Sema final { /// Create a new lambda closure type. CXXRecordDecl *createLambdaClosureType(SourceRange IntroducerRange, TypeSourceInfo *Info, - bool KnownDependent, + unsigned LambdaDependencyKind, LambdaCaptureDefault CaptureDefault); /// Start the definition of a lambda expression. diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 457465e87d9353..455aa15153b25b 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -2865,7 +2865,7 @@ ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl *D) { return TInfoOrErr.takeError(); if (GetImportedOrCreateSpecialDecl( D2CXX, CXXRecordDecl::CreateLambda, D, Importer.getToContext(), - DC, *TInfoOrErr, Loc, DCXX->isDependentLambda(), + DC, *TInfoOrErr, Loc, DCXX->getLambdaDependencyKind(), DCXX->isGenericLambda(), DCXX->getLambdaCaptureDefault())) return D2CXX; ExpectedDecl CDeclOrErr = import(DCXX->getLambdaContextDecl()); diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index 9ee1cc08308673..ccadbdd6db0d45 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -1161,6 +1161,8 @@ bool DeclContext::isDependentContext() const { if (Record->isDependentLambda()) return true; + if (Record->isNeverDependentLambda()) + return false; } if (const auto *Function = dyn_cast(this)) { diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index 0cf6e60b2a6c35..400709d1729324 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -146,16 +146,16 @@ CXXRecordDecl *CXXRecordDecl::Create(const ASTContext &C, TagKind TK, CXXRecordDecl * CXXRecordDecl::CreateLambda(const ASTContext &C, DeclContext *DC, TypeSourceInfo *Info, SourceLocation Loc, - bool Dependent, bool IsGeneric, + unsigned DependencyKind, bool IsGeneric, LambdaCaptureDefault CaptureDefault) { auto *R = new (C, DC) CXXRecordDecl(CXXRecord, TTK_Class, C, DC, Loc, Loc, nullptr, nullptr); R->setBeingDefined(true); - R->DefinitionData = - new (C) struct LambdaDefinitionData(R, Info, Dependent, IsGeneric, - CaptureDefault); + R->DefinitionData = new (C) struct LambdaDefinitionData( + R, Info, DependencyKind, IsGeneric, CaptureDefault); R->setMayHaveOutOfDateDef(false); R->setImplicit(true); + C.getTypeDeclType(R, /*PrevDecl=*/nullptr); return R; } diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp index b05e0b5cc0f12f..c0b2fad530c5c7 100644 --- a/clang/lib/Sema/SemaLambda.cpp +++ b/clang/lib/Sema/SemaLambda.cpp @@ -238,21 +238,19 @@ getGenericLambdaTemplateParameterList(LambdaScopeInfo *LSI, Sema &SemaRef) { return LSI->GLTemplateParameterList; } -CXXRecordDecl *Sema::createLambdaClosureType(SourceRange IntroducerRange, - TypeSourceInfo *Info, - bool KnownDependent, - LambdaCaptureDefault CaptureDefault) { +CXXRecordDecl * +Sema::createLambdaClosureType(SourceRange IntroducerRange, TypeSourceInfo *Info, + unsigned LambdaDependencyKind, + LambdaCaptureDefault CaptureDefault) { DeclContext *DC = CurContext; while (!(DC->isFunctionOrMethod() || DC->isRecord() || DC->isFileContext())) DC = DC->getParent(); bool IsGenericLambda = getGenericLambdaTemplateParameterList(getCurLambda(), *this); // Start constructing the lambda class. - CXXRecordDecl *Class = CXXRecordDecl::CreateLambda(Context, DC, Info, - IntroducerRange.getBegin(), - KnownDependent, - IsGenericLambda, - CaptureDefault); + CXXRecordDecl *Class = CXXRecordDecl::CreateLambda( + Context, DC, Info, IntroducerRange.getBegin(), LambdaDependencyKind, + IsGenericLambda, CaptureDefault); DC->addDecl(Class); return Class; @@ -898,17 +896,18 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro, // Determine if we're within a context where we know that the lambda will // be dependent, because there are template parameters in scope. - bool KnownDependent; + CXXRecordDecl::LambdaDependencyKind LambdaDependencyKind = + CXXRecordDecl::LDK_Unknown; if (LSI->NumExplicitTemplateParams > 0) { auto *TemplateParamScope = CurScope->getTemplateParamParent(); assert(TemplateParamScope && "Lambda with explicit template param list should establish a " "template param scope"); assert(TemplateParamScope->getParent()); - KnownDependent = TemplateParamScope->getParent() - ->getTemplateParamParent() != nullptr; - } else { - KnownDependent = CurScope->getTemplateParamParent() != nullptr; + if (TemplateParamScope->getParent()->getTemplateParamParent() != nullptr) + LambdaDependencyKind = CXXRecordDecl::LDK_AlwaysDependent; + } else if (CurScope->getTemplateParamParent() != nullptr) { + LambdaDependencyKind = CXXRecordDecl::LDK_AlwaysDependent; } // Determine the signature of the call operator. @@ -977,8 +976,8 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro, UPPC_DeclarationType); } - CXXRecordDecl *Class = createLambdaClosureType(Intro.Range, MethodTyInfo, - KnownDependent, Intro.Default); + CXXRecordDecl *Class = createLambdaClosureType( + Intro.Range, MethodTyInfo, LambdaDependencyKind, Intro.Default); CXXMethodDecl *Method = startLambdaDefinition(Class, Intro.Range, MethodTyInfo, EndLoc, Params, ParamInfo.getDeclSpec().getConstexprSpecifier(), diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 467372c71496a1..d64c0969cee124 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -1831,7 +1831,7 @@ Decl *TemplateDeclInstantiator::VisitCXXRecordDecl(CXXRecordDecl *D) { if (D->isLambda()) Record = CXXRecordDecl::CreateLambda( SemaRef.Context, Owner, D->getLambdaTypeInfo(), D->getLocation(), - D->isDependentLambda(), D->isGenericLambda(), + D->getLambdaDependencyKind(), D->isGenericLambda(), D->getLambdaCaptureDefault()); else Record = CXXRecordDecl::Create(SemaRef.Context, D->getTagKind(), Owner, diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 5c37fcaaea13d9..ff8ca76a7e7601 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -12922,14 +12922,24 @@ TreeTransform::TransformLambdaExpr(LambdaExpr *E) { NewTrailingRequiresClause = getDerived().TransformExpr(TRC); // Create the local class that will describe the lambda. - // FIXME: KnownDependent below is wrong when substituting inside a templated - // context that isn't a DeclContext (such as a variable template). + + // FIXME: DependencyKind below is wrong when substituting inside a templated + // context that isn't a DeclContext (such as a variable template), or when + // substituting an unevaluated lambda inside of a function's parameter's type + // - as parameter types are not instantiated from within a function's DC. We + // use isUnevaluatedContext() to distinguish the function parameter case. + CXXRecordDecl::LambdaDependencyKind DependencyKind = + CXXRecordDecl::LDK_Unknown; + if (getSema().isUnevaluatedContext() && + (getSema().CurContext->isFileContext() || + !getSema().CurContext->getParent()->isDependentContext())) + DependencyKind = CXXRecordDecl::LDK_NeverDependent; + CXXRecordDecl *OldClass = E->getLambdaClass(); - CXXRecordDecl *Class - = getSema().createLambdaClosureType(E->getIntroducerRange(), - NewCallOpTSI, - /*KnownDependent=*/false, - E->getCaptureDefault()); + CXXRecordDecl *Class = + getSema().createLambdaClosureType(E->getIntroducerRange(), NewCallOpTSI, + DependencyKind, E->getCaptureDefault()); + getDerived().transformedLocalDecl(OldClass, {Class}); Optional> Mangling; diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 5d63a26132b738..3322daf434f670 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -1801,7 +1801,7 @@ void ASTDeclReader::ReadCXXDefinitionData( using Capture = LambdaCapture; auto &Lambda = static_cast(Data); - Lambda.Dependent = Record.readInt(); + Lambda.DependencyKind = Record.readInt(); Lambda.IsGenericLambda = Record.readInt(); Lambda.CaptureDefault = Record.readInt(); Lambda.NumCaptures = Record.readInt(); @@ -1917,8 +1917,8 @@ void ASTDeclReader::ReadCXXRecordDefinition(CXXRecordDecl *D, bool Update) { // allocate the appropriate DefinitionData structure. bool IsLambda = Record.readInt(); if (IsLambda) - DD = new (C) CXXRecordDecl::LambdaDefinitionData(D, nullptr, false, false, - LCD_None); + DD = new (C) CXXRecordDecl::LambdaDefinitionData( + D, nullptr, CXXRecordDecl::LDK_Unknown, false, LCD_None); else DD = new (C) struct CXXRecordDecl::DefinitionData(D); diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 763fc9537c04b7..422fc9e2b8594c 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -5726,7 +5726,7 @@ void ASTRecordWriter::AddCXXDefinitionData(const CXXRecordDecl *D) { // Add lambda-specific data. if (Data.IsLambda) { auto &Lambda = D->getLambdaData(); - Record->push_back(Lambda.Dependent); + Record->push_back(Lambda.DependencyKind); Record->push_back(Lambda.IsGenericLambda); Record->push_back(Lambda.CaptureDefault); Record->push_back(Lambda.NumCaptures); diff --git a/clang/test/SemaCXX/lambda-unevaluated.cpp b/clang/test/SemaCXX/lambda-unevaluated.cpp index 1a06528a1c0632..07a4db78ac311a 100644 --- a/clang/test/SemaCXX/lambda-unevaluated.cpp +++ b/clang/test/SemaCXX/lambda-unevaluated.cpp @@ -30,6 +30,27 @@ auto g(T) -> decltype([]() { T::invalid; } ()); auto e = g(0); // expected-error{{no matching function for call}} // expected-note@-2 {{substitution failure}} +template +auto foo(decltype([] { + return [] { return T(); }(); +})) {} + +void test() { + foo({}); +} + +template +struct C { + template + auto foo(decltype([] { + return [] { return T(); }(); + })) {} +}; + +void test2() { + C{}.foo({}); +} + namespace PR52073 { // OK, these are distinct functions not redefinitions. template void f(decltype([]{})) {} // expected-note {{candidate}} @@ -40,6 +61,62 @@ void use_f() { f({}); } // expected-error {{ambiguous}} template void g(const char (*)[([]{ return N; })()]) {} // expected-note {{candidate}} template void g(const char (*)[([]{ return N; })()]) {} // expected-note {{candidate}} // FIXME: We instantiate the lambdas into the context of the function template, -// so we think they're dependent and can't evaluate a call to them. +// so we think they're dependent and can't evaluate a call to them. void use_g() { g<6>(&"hello"); } // expected-error {{no matching function}} } + +namespace GH51416 { + +template +struct A { + void spam(decltype([] {})); +}; + +template +void A::spam(decltype([] {})) // expected-error{{out-of-line definition of 'spam' does not match}} +{} + +struct B { + template + void spam(decltype([] {})); +}; + +template +void B::spam(decltype([] {})) {} // expected-error{{out-of-line definition of 'spam' does not match}} + +} // namespace GH51416 + +namespace GH50376 { + +template +struct foo_t { // expected-note 2{{candidate constructor}} + foo_t(T ptr) {} // expected-note{{candidate constructor}} +}; + +template +using alias = foo_t; + +template +auto fun(T const &t) -> alias { + return alias{t}; // expected-error{{no viable conversion from returned value of type 'alias<...>'}} +} + +void f() { + int i; + auto const error = fun(i); // expected-note{{in instantiation}} +} + +} // namespace GH50376 + +namespace GH51414 { +template void spam(decltype([] {}) (*s)[sizeof(T)] = nullptr) {} +void foo() { + spam(); +} +} // namespace GH51414 + +namespace GH51641 { +template +void foo(decltype(+[](T) {}) lambda, T param); +static_assert(!__is_same(decltype(foo), void)); +} // namespace GH51641 diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp index 04da7988fc308f..1b6bcc34e48bf6 100644 --- a/clang/unittests/AST/ASTImporterTest.cpp +++ b/clang/unittests/AST/ASTImporterTest.cpp @@ -5824,6 +5824,7 @@ TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionBody) { std::distance(FromL->decls().begin(), FromL->decls().end()); EXPECT_NE(ToLSize, 0u); EXPECT_EQ(ToLSize, FromLSize); + EXPECT_FALSE(FromL->isDependentLambda()); } TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionParam) { @@ -5843,6 +5844,7 @@ TEST_P(ASTImporterOptionSpecificTestBase, LambdaInFunctionParam) { std::distance(FromL->decls().begin(), FromL->decls().end()); EXPECT_NE(ToLSize, 0u); EXPECT_EQ(ToLSize, FromLSize); + EXPECT_TRUE(FromL->isDependentLambda()); } TEST_P(ASTImporterOptionSpecificTestBase, LambdaInGlobalScope) { diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html index 5005ec6c08e4c6..49dad05ccf9ac9 100755 --- a/clang/www/cxx_status.html +++ b/clang/www/cxx_status.html @@ -1021,7 +1021,11 @@

C++20 implementation status

Lambdas in unevaluated contexts P0315R4 - Partial + +
Partial + temp.deduct/9 is not implemented yet. +
+