[clang] 3784e8c - [Clang] Fix Unevaluated Lambdas

Corentin Jabot via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 25 11:16:51 PDT 2022


Author: Corentin Jabot
Date: 2022-03-25T19:16:45+01:00
New Revision: 3784e8ccfbdaaab31f9e9c221daa59a218279999

URL: https://github.com/llvm/llvm-project/commit/3784e8ccfbdaaab31f9e9c221daa59a218279999
DIFF: https://github.com/llvm/llvm-project/commit/3784e8ccfbdaaab31f9e9c221daa59a218279999.diff

LOG: [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

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/AST/DeclCXX.h
    clang/include/clang/Sema/Sema.h
    clang/lib/AST/ASTImporter.cpp
    clang/lib/AST/DeclBase.cpp
    clang/lib/AST/DeclCXX.cpp
    clang/lib/Sema/SemaLambda.cpp
    clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
    clang/lib/Sema/TreeTransform.h
    clang/lib/Serialization/ASTReaderDecl.cpp
    clang/lib/Serialization/ASTWriter.cpp
    clang/test/SemaCXX/lambda-unevaluated.cpp
    clang/unittests/AST/ASTImporterTest.cpp
    clang/www/cxx_status.html

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 58523c2569283..39499b79e34d4 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -83,6 +83,14 @@ Bug Fixes
   per identifier.
   Fixes `Issue 28985 <https://github.com/llvm/llvm-project/issues/28985>`_.
 
+- 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
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 - ``-Wliteral-range`` will warn on floating-point equality comparisons with

diff  --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index 2833df0505dac..545f29975fd89 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 5573621f6120e..817df4e108433 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6783,7 +6783,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 063fe2bfdf5e3..9b421c7dec7c9 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -2874,7 +2874,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 9ee1cc0830867..ccadbdd6db0d4 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<FunctionDecl>(this)) {

diff  --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 0cf6e60b2a6c3..400709d172932 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 b05e0b5cc0f12..c0b2fad530c5c 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 8c7f5a4f19658..e6e6b3bc3d588 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1853,7 +1853,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 1ee457b0566ae..4b9b48293b629 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12982,14 +12982,24 @@ TreeTransform<Derived>::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<std::tuple<bool, unsigned, unsigned, Decl *>> Mangling;

diff  --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 29bef2aa20897..0889f5ae6750b 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<CXXRecordDecl::LambdaDefinitionData &>(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 16c25a00b4461..c06cbb251c408 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -5733,7 +5733,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 1a06528a1c063..07a4db78ac311 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 at -2 {{substitution failure}}
 
+template <typename T>
+auto foo(decltype([] {
+  return [] { return T(); }();
+})) {}
+
+void test() {
+  foo<int>({});
+}
+
+template <typename T>
+struct C {
+  template <typename U>
+  auto foo(decltype([] {
+    return [] { return T(); }();
+  })) {}
+};
+
+void test2() {
+  C<int>{}.foo<long>({});
+}
+
 namespace PR52073 {
 // OK, these are distinct functions not redefinitions.
 template<typename> void f(decltype([]{})) {} // expected-note {{candidate}}
@@ -40,6 +61,62 @@ void use_f() { f<int>({}); } // expected-error {{ambiguous}}
 template<int N> void g(const char (*)[([]{ return N; })()]) {} // expected-note {{candidate}}
 template<int N> 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 <class T>
+struct A {
+  void spam(decltype([] {}));
+};
+
+template <class T>
+void A<T>::spam(decltype([] {})) // expected-error{{out-of-line definition of 'spam' does not match}}
+{}
+
+struct B {
+  template <class T>
+  void spam(decltype([] {}));
+};
+
+template <class T>
+void B::spam(decltype([] {})) {} // expected-error{{out-of-line definition of 'spam' does not match}}
+
+} // namespace GH51416
+
+namespace GH50376 {
+
+template <typename T, typename Fn>
+struct foo_t {    // expected-note 2{{candidate constructor}}
+  foo_t(T ptr) {} // expected-note{{candidate constructor}}
+};
+
+template <typename T>
+using alias = foo_t<T, decltype([](int) { return 0; })>;
+
+template <typename T>
+auto fun(T const &t) -> alias<T> {
+  return alias<T>{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 <class T> void spam(decltype([] {}) (*s)[sizeof(T)] = nullptr) {}
+void foo() {
+  spam<int>();
+}
+} // namespace GH51414
+
+namespace GH51641 {
+template <class T>
+void foo(decltype(+[](T) {}) lambda, T param);
+static_assert(!__is_same(decltype(foo<int>), void));
+} // namespace GH51641

diff  --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index 77c6d6ad2e19d..b52e0b5d18e39 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 79da75dee0d23..bd863d0f2fc50 100755
--- a/clang/www/cxx_status.html
+++ b/clang/www/cxx_status.html
@@ -1024,7 +1024,11 @@ <h2 id="cxx20">C++20 implementation status</h2>
     <tr>
       <td>Lambdas in unevaluated contexts</td>
       <td><a href="https://wg21.link/p0315r4">P0315R4</a></td>
-      <td class="partial" align="center">Partial</td>
+      <td class="partial" align="center">
+        <details><summary>Partial</summary>
+          temp.deduct/9 is not implemented yet.
+        </details>
+      </td>
     </tr>
     <!-- Jacksonville papers -->
     <tr>


        


More information about the cfe-commits mailing list