lambdas in modules: change storage for LambdaDefinitionData

[elsteveogrande]

Summary

Previously LambdaDefinitionData extended DefinitionData and either might have been used interchangably in a CXXRecordDecl's DefinitionData field.

However there are cases where LambdaDefinitionData ("LDD") aren't copied or moved completely, into this DefinitionData ("DD") field, as it has additional members of its own.

This patch:

• makes LDD a separate structure, no longer using DD as the base class, but otherwise kept the same so as to encapsulate lambda stuff properly
• stores lambda data within an Optional called LambdaData
• replaces IsLambda bit with bool isLambda() const, true IFF lambda data are present in that optional field
• as the formerly-subclass's LDD constructor set some members on DD, this is now done in a new method SetLambdaData; this sets those fields to values sensible for lambdas, and emplaces lambda data in the optional field

This solves one case where lambda data were not handled quite properly.

Note that this doesn't solve all lambda-handling in pcm's (clang modules) but is more of a refactoring pre-step.

Test Plan: ninja check-clang-modules

[elsteveogrande]

Rebase past dependency commit C341499, fix a conflict

[rsmith]

We don't want to allocate storage for the lambda fields for non-lambda classes, which is why we use distinct definition data classes. Is the problem you're trying to solve here that we fake a definition in AST deserialization before we know whether the class is a lambda? If so, that seems solvable by moving the IsLambda flag out of DefinitionData into CXXRecordData (perhaps as a distinct TagTypeKind?).

[elsteveogrande]

lib/Serialization/ASTReaderDecl.cpp

1771

Hi @rsmith, thanks again for looking! This is the part that I was concerned about: that DD could be either a DefinitionData or LambdaDefinitionData reference; similar MergeDD. It didn't seem that move-assign was well-defined in the cases where they were mixed.

If their being different is completely unexpected, though, this whole thing could perhaps be an assertion that they are both lambda, or both not lambda. And an IsLambda field on the CXXRecordDecl` (or TTK_Lambda).

Alternatively, I could make the lambda data a pointer or std::unique_ptr<LambdaDefinitionData> inside DefinitionData (and maybe make the latter final), so as not to allocate much extra space in the non-lambda case.

Depends on whether a new tag type is not too much of a breaking change. Also, whether it's known at this point that this record is a lambda (and DD is already of the correct type).

[elsteveogrande]

Change Optional to unique_ptr instead, to at least save an allocation and keep the same semantics. Reverified with ninja check-clang.

[elsteveogrande]

I did try a copy of D50949 again, locally and without this diff, to see if I can avoid messing with this structure altogether. But then I get a SEGV.

The reason behind this diff is to try to make the logic a little safer, though it's probably still not bulletproof. It at least does seem to fix, or "fix" (i.e. hide) the crash below.

I admit I didn't take the time to really dig into that one, and I'm not yet experienced enough in Clang's code to do so confidently, so it's a bit scary to continue going down rabbit holes :)

But I'm happy to revise (or go fix that problem) if you think that's best. Thanks again!

Stack dump:
0.	Program arguments: /opt/llvm/build/bin/clang -cc1 -internal-isystem /opt/llvm/build/lib/clang/8.0.0/include -nostdsysteminc -fmodules -verify /opt/llvm/llvm/tools/clang/test/Modules/merge-lambdas.cpp -emit-llvm-only
1.	/opt/llvm/llvm/tools/clang/test/Modules/merge-lambdas.cpp:73:1: at annotation token
2.	/opt/llvm/build/tools/clang/test/Modules/PR38531.map:11:14: LLVM IR generation of declaration 'foo(Fun)::(anonymous class)::'
0  clang                    0x00000001032cffe8 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 40
1  clang                    0x00000001032cf165 llvm::sys::RunSignalHandlers() + 85
2  clang                    0x00000001032d05f2 SignalHandler(int) + 258
3  libsystem_platform.dylib 0x00007fff79709f5a _sigtramp + 26
4  libsystem_platform.dylib 0x00007ffeed8dbe3f _sigtramp + 1948065535
5  clang                    0x0000000104b865b1 (anonymous namespace)::CXXNameMangler::manglePrefix(clang::DeclContext const*, bool) + 545
6  clang                    0x0000000104b877d5 (anonymous namespace)::CXXNameMangler::mangleNestedName(clang::NamedDecl const*, clang::DeclContext const*, llvm::SmallVector<llvm::StringRef, 4u> const*, bool) + 453
7  clang                    0x0000000104b8733d (anonymous namespace)::CXXNameMangler::mangleLocalName(clang::Decl const*, llvm::SmallVector<llvm::StringRef, 4u> const*) + 1613
8  clang                    0x0000000104b77bf7 (anonymous namespace)::CXXNameMangler::mangleFunctionEncoding(clang::FunctionDecl const*) + 87
9  clang                    0x0000000104b760c5 (anonymous namespace)::ItaniumMangleContextImpl::mangleCXXCtor(clang::CXXConstructorDecl const*, clang::CXXCtorType, llvm::raw_ostream&) + 245
10 clang                    0x000000010368eb21 getMangledNameImpl(clang::CodeGen::CodeGenModule const&, clang::GlobalDecl, clang::NamedDecl const*, bool) + 305
11 clang                    0x000000010368a83b clang::CodeGen::CodeGenModule::getMangledName(clang::GlobalDecl) + 427
12 clang                    0x0000000103697135 clang::CodeGen::CodeGenModule::EmitGlobal(clang::GlobalDecl) + 1941
13 clang                    0x00000001036f7529 (anonymous namespace)::ItaniumCXXABI::EmitCXXConstructors(clang::CXXConstructorDecl const*) + 41
14 clang                    0x000000010371401f (anonymous namespace)::CodeGeneratorImpl::HandleTopLevelDecl(clang::DeclGroupRef) + 143
15 clang                    0x000000010367a151 clang::BackendConsumer::HandleTopLevelDecl(clang::DeclGroupRef) + 177
16 clang                    0x000000010367a244 clang::BackendConsumer::HandleInterestingDecl(clang::DeclGroupRef) + 20
17 clang                    0x000000010427c2db clang::ASTReader::PassInterestingDeclsToConsumer() + 123
18 clang                    0x0000000104243f8b clang::ASTReader::FinishedDeserializing() + 1867
19 clang                    0x000000010427be48 clang::ASTReader::ReadDeclRecord(unsigned int) + 2808
20 clang                    0x000000010422b219 non-virtual thunk to clang::ASTReader::GetExternalDecl(unsigned int) + 89
21 clang                    0x0000000104afe7a1 clang::RedeclarableTemplateDecl::loadLazySpecializationsImpl() const + 241
22 clang                    0x0000000104aff1dc clang::ClassTemplateDecl::findSpecialization(llvm::ArrayRef<clang::TemplateArgument>, void*&) + 28
23 clang                    0x00000001048096a2 clang::Sema::CheckTemplateIdType(clang::TemplateName, clang::SourceLocation, clang::TemplateArgumentListInfo&) + 1170
24 clang                    0x000000010480c900 clang::Sema::ActOnTemplateIdType(clang::CXXScopeSpec&, clang::SourceLocation, clang::OpaquePtr<clang::TemplateName>, clang::IdentifierInfo*, clang::SourceLocation, clang::SourceLocation, llvm::MutableArrayRef<clang::ParsedTemplateArgument>, clang::SourceLocation, bool, bool) + 1552
25 clang                    0x00000001041df3b1 clang::Parser::AnnotateTemplateIdTokenAsType(bool) + 129
26 clang                    0x000000010416449a clang::Parser::ParseDeclarationSpecifiers(clang::DeclSpec&, clang::Parser::ParsedTemplateInfo const&, clang::AccessSpecifier, clang::Parser::DeclSpecContext, clang::Parser::LateParsedAttrList*) + 10410
27 clang                    0x00000001041eb69b clang::Parser::ParseDeclOrFunctionDefInternal(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec&, clang::AccessSpecifier) + 139
28 clang                    0x00000001041eb367 clang::Parser::ParseDeclarationOrFunctionDefinition(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*, clang::AccessSpecifier) + 343
29 clang                    0x00000001041e9a99 clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) + 2521
30 clang                    0x00000001041e8bd0 clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&) + 688
31 clang                    0x00000001041537a5 clang::ParseAST(clang::Sema&, bool, bool) + 453
32 clang                    0x00000001038a8ed3 clang::FrontendAction::Execute() + 67
33 clang                    0x00000001038650f8 clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) + 1272
34 clang                    0x00000001038f9858 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) + 1688
35 clang                    0x0000000102326339 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) + 1401
36 clang                    0x00000001023243e0 main + 10320
37 libdyld.dylib            0x00007fff793fb015 start + 1
/opt/llvm/build/tools/clang/test/Modules/Output/merge-lambdas.cpp.script: line 1: 62496 Segmentation fault: 11  /opt/llvm/build/bin/clang -cc1 -internal-isystem /opt/llvm/build/lib/clang/8.0.0/include -nostdsysteminc -fmodules -verify /opt/llvm/llvm/tools/clang/test/Modules/merge-lambdas.cpp -emit-llvm-only

include/clang/AST/DeclCXX.h

enum SpecialMemberFlags {
     SMF_CopyConstructor = 0x2,
     SMF_MoveConstructor = 0x4,
     SMF_CopyAssignment = 0x8,
     SMF_MoveAssignment = 0x10,
     SMF_Destructor = 0x20,
     SMF_All = 0x3f
   };
 
-  struct DefinitionData {
+  /// Describes a C++ closure type (generated by a lambda expression).
+  /// Nested within the DefinitionData, see \c LambdaData.
+  struct LambdaDefinitionData {
+    using Capture = LambdaCapture;
+
+    /// Whether this lambda is known to be dependent, even if its
+    /// context isn't dependent.
+    ///
+    /// A lambda with a non-dependent context can be dependent if it occurs
+    /// within the default argument of a function template, because the
+    /// 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;
+
+    /// Whether this lambda is a generic lambda.
+    unsigned IsGenericLambda : 1;
+
+    /// The Default Capture.
+    unsigned CaptureDefault : 2;
+
+    /// The number of captures in this lambda is limited 2^NumCaptures.
+    unsigned NumCaptures : 15;
+
+    /// The number of explicit captures in this lambda.
+    unsigned NumExplicitCaptures : 13;
+
+    /// The number used to indicate this lambda expression for name
+    /// mangling in the Itanium C++ ABI.
+    unsigned ManglingNumber = 0;
+
+    /// The declaration that provides context for this lambda, if the
+    /// actual DeclContext does not suffice. This is used for lambdas that
+    /// occur within default arguments of function parameters within the class
+    /// or within a data member initializer.
+    LazyDeclPtr ContextDecl;
+
+    /// The list of captures, both explicit and implicit, for this
+    /// lambda.
+    Capture *Captures = nullptr;
+
+    /// The type of the call method.
+    TypeSourceInfo *MethodTyInfo;
+
+    LambdaDefinitionData(TypeSourceInfo *Info,
+                         bool Dependent, bool IsGeneric,
+                         LambdaCaptureDefault CaptureDefault)
+      : Dependent(Dependent), IsGenericLambda(IsGeneric),
+        CaptureDefault(CaptureDefault), NumCaptures(0), NumExplicitCaptures(0),
+        MethodTyInfo(Info) {}
+  };
+
+  struct DefinitionData final {
     /// True if this class has any user-declared constructors.
     unsigned UserDeclaredConstructor : 1;
 
     /// The user-declared special members which this class has.
     unsigned UserDeclaredSpecialMembers : 6;
 
     /// True when this class is an aggregate.
     unsigned Aggregate : 1;
struct DefinitionData final {
     /// Whether any declared copy constructor has a const-qualified
     /// parameter.
     unsigned HasDeclaredCopyConstructorWithConstParam : 1;
 
     /// Whether any declared copy assignment operator has either a
     /// const-qualified reference parameter or a non-reference parameter.
     unsigned HasDeclaredCopyAssignmentWithConstParam : 1;
 
-    /// Whether this class describes a C++ lambda.
-    unsigned IsLambda : 1;
-
     /// Whether we are currently parsing base specifiers.
     unsigned IsParsingBaseSpecifiers : 1;
 
     unsigned HasODRHash : 1;
 
     /// A hash of parts of the class to help in ODR checking.
     unsigned ODRHash = 0;
 
struct DefinitionData final {
     CXXRecordDecl *Definition;
 
     /// The first friend declaration in this class, or null if there
     /// aren't any.
     ///
     /// This is actually currently stored in reverse order.
     LazyDeclPtr FirstFriend;
 
+    /// Present if and only if this is for a lambda.
+    std::unique_ptr<LambdaDefinitionData> LambdaData;
+
     DefinitionData(CXXRecordDecl *D);
 
     /// Retrieve the set of direct base classes.
     CXXBaseSpecifier *getBases() const {
       if (!Bases.isOffset())
         return Bases.get(nullptr);
       return getBasesSlowCase();
     }
 
     /// Retrieve the set of virtual base classes.
     CXXBaseSpecifier *getVBases() const {
       if (!VBases.isOffset())
         return VBases.get(nullptr);
       return getVBasesSlowCase();
     }
 
     ArrayRef<CXXBaseSpecifier> bases() const {
       return llvm::makeArrayRef(getBases(), NumBases);
     }
 
     ArrayRef<CXXBaseSpecifier> vbases() const {
       return llvm::makeArrayRef(getVBases(), NumVBases);
     }
 
-  private:
-    CXXBaseSpecifier *getBasesSlowCase() const;
-    CXXBaseSpecifier *getVBasesSlowCase() const;
-  };
-
-  struct DefinitionData *DefinitionData;
-
-  /// Describes a C++ closure type (generated by a lambda expression).
-  struct LambdaDefinitionData : public DefinitionData {
-    using Capture = LambdaCapture;
-
-    /// Whether this lambda is known to be dependent, even if its
-    /// context isn't dependent.
-    ///
-    /// A lambda with a non-dependent context can be dependent if it occurs
-    /// within the default argument of a function template, because the
-    /// 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;
-
-    /// Whether this lambda is a generic lambda.
-    unsigned IsGenericLambda : 1;
-
-    /// The Default Capture.
-    unsigned CaptureDefault : 2;
-
-    /// The number of captures in this lambda is limited 2^NumCaptures.
-    unsigned NumCaptures : 15;
-
-    /// The number of explicit captures in this lambda.
-    unsigned NumExplicitCaptures : 13;
-
-    /// The number used to indicate this lambda expression for name
-    /// mangling in the Itanium C++ ABI.
-    unsigned ManglingNumber = 0;
-
-    /// The declaration that provides context for this lambda, if the
-    /// actual DeclContext does not suffice. This is used for lambdas that
-    /// occur within default arguments of function parameters within the class
-    /// or within a data member initializer.
-    LazyDeclPtr ContextDecl;
-
-    /// The list of captures, both explicit and implicit, for this
-    /// lambda.
-    Capture *Captures = nullptr;
-
-    /// The type of the call method.
-    TypeSourceInfo *MethodTyInfo;
-
-    LambdaDefinitionData(CXXRecordDecl *D, TypeSourceInfo *Info,
+    void SetLambdaDefinitionData(TypeSourceInfo *Info,
                          bool Dependent, bool IsGeneric,
-                         LambdaCaptureDefault CaptureDefault)
-      : DefinitionData(D), Dependent(Dependent), IsGenericLambda(IsGeneric),
-        CaptureDefault(CaptureDefault), NumCaptures(0), NumExplicitCaptures(0),
-        MethodTyInfo(Info) {
-      IsLambda = true;
+                         LambdaCaptureDefault CaptureDefault) {
+      LambdaData.reset(new LambdaDefinitionData(
+          Info, Dependent, IsGeneric, CaptureDefault));
+
+      // Ensure the following members are set correctly, since we know now
+      // this is for a lambda.  These members are set to true by default.
 
       // C++1z [expr.prim.lambda]p4:
       //   This class type is not an aggregate type.
       Aggregate = false;
       PlainOldData = false;
+
+      // Not C-like.
+      HasOnlyCMembers = false;
+    }
+
+    bool hasLambdaData() const {
+      return bool(LambdaData);
     }
+
+  private:
+    CXXBaseSpecifier *getBasesSlowCase() const;
+    CXXBaseSpecifier *getVBasesSlowCase() const;
   };
 
+  struct DefinitionData *DefinitionData;
+
   struct DefinitionData *dataPtr() const {
     // Complete the redecl chain (if necessary).
     getMostRecentDecl();
     return DefinitionData;
   }
 
   struct DefinitionData &data() const {
     auto *DD = dataPtr();
     assert(DD && "queried property of class with no definition");
     return *DD;
   }
 
   struct LambdaDefinitionData &getLambdaData() const {
     // No update required: a merged definition cannot change any lambda
     // properties.
     auto *DD = DefinitionData;
-    assert(DD && DD->IsLambda && "queried lambda property of non-lambda class");
-    return static_cast<LambdaDefinitionData&>(*DD);
+    assert(DD && DD->LambdaData.hasValue() &&
+           "queried lambda property of non-lambda class");
+    return *DD->LambdaData;
   }
 
   /// The template or declaration that this declaration
   /// describes or was instantiated from, respectively.
   ///
   /// For non-templates, this value will be null. For record
   /// declarations that describe a class template, this will be a
   /// pointer to a ClassTemplateDecl. For member
public:
   bool needsOverloadResolutionForDestructor() const {
     return data().NeedOverloadResolutionForDestructor;
   }
 
   /// Determine whether this class describes a lambda function object.
   bool isLambda() const {
     // An update record can't turn a non-lambda into a lambda.
     auto *DD = DefinitionData;
-    return DD && DD->IsLambda;
+    return DD && DD->hasLambdaData();
   }
 
   /// Determine whether this class describes a generic
   /// lambda function object (i.e. function call operator is
   /// a template).
   bool isGenericLambda() const;
 
   /// Retrieve the lambda call operator of the closure type

lib/AST/DeclCXX.cpp

: UserDeclaredConstructor(false), UserDeclaredSpecialMembers(0),
       DefaultedDefaultConstructorIsConstexpr(true),
       HasConstexprDefaultConstructor(false),
       HasNonLiteralTypeFieldsOrBases(false), ComputedVisibleConversions(false),
       UserProvidedDefaultConstructor(false), DeclaredSpecialMembers(0),
       ImplicitCopyConstructorCanHaveConstParamForVBase(true),
       ImplicitCopyConstructorCanHaveConstParamForNonVBase(true),
       ImplicitCopyAssignmentHasConstParam(true),
       HasDeclaredCopyConstructorWithConstParam(false),
-      HasDeclaredCopyAssignmentWithConstParam(false), IsLambda(false),
+      HasDeclaredCopyAssignmentWithConstParam(false),
       IsParsingBaseSpecifiers(false), HasODRHash(false), Definition(D) {}
 
 CXXBaseSpecifier *CXXRecordDecl::DefinitionData::getBasesSlowCase() const {
   return Bases.get(Definition->getASTContext().getExternalSource());
 }
 
 CXXBaseSpecifier *CXXRecordDecl::DefinitionData::getVBasesSlowCase() const {
   return VBases.get(Definition->getASTContext().getExternalSource());

 CXXRecordDecl *
 CXXRecordDecl::CreateLambda(const ASTContext &C, DeclContext *DC,
                             TypeSourceInfo *Info, SourceLocation Loc,
                             bool Dependent, 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,
+  R->DefinitionData = new (C) struct DefinitionData(R);
+  R->DefinitionData->SetLambdaDefinitionData(Info, Dependent, IsGeneric,
                                              CaptureDefault);
   R->setMayHaveOutOfDateDef(false);
   R->setImplicit(true);
   C.getTypeDeclType(R, /*PrevDecl=*/nullptr);
   return R;
 }
 
 CXXRecordDecl *
 CXXRecordDecl::CreateDeserialized(const ASTContext &C, unsigned ID) {

lib/Serialization/ASTReaderDecl.cpp

void ASTDeclReader::VisitUnresolvedUsingTypenameDecl(
   D->TypenameLocation = ReadSourceLocation();
   D->QualifierLoc = Record.readNestedNameSpecifierLoc();
   D->EllipsisLoc = ReadSourceLocation();
   mergeMergeable(D);
 }
 
 void ASTDeclReader::ReadCXXDefinitionData(
     struct CXXRecordDecl::DefinitionData &Data, const CXXRecordDecl *D) {
-  // Note: the caller has deserialized the IsLambda bit already.
+  // Note: the caller has deserialized the bit for `isLambda()` already.
   Data.UserDeclaredConstructor = Record.readInt();
   Data.UserDeclaredSpecialMembers = Record.readInt();
   Data.Aggregate = Record.readInt();
   Data.PlainOldData = Record.readInt();
   Data.Empty = Record.readInt();
   Data.Polymorphic = Record.readInt();
   Data.Abstract = Record.readInt();
   Data.IsStandardLayout = Record.readInt();
void ASTDeclReader::ReadCXXDefinitionData(
   if (Data.NumVBases)
     Data.VBases = ReadGlobalOffset();
 
   Record.readUnresolvedSet(Data.Conversions);
   Record.readUnresolvedSet(Data.VisibleConversions);
   assert(Data.Definition && "Data.Definition should be already set!");
   Data.FirstFriend = ReadDeclID();
 
-  if (Data.IsLambda) {
+  if (Data.hasLambdaData()) {
     using Capture = LambdaCapture;
 
-    auto &Lambda = static_cast<CXXRecordDecl::LambdaDefinitionData &>(Data);
+    auto &Lambda = *Data.LambdaData;
     Lambda.Dependent = Record.readInt();
     Lambda.IsGenericLambda = Record.readInt();
     Lambda.CaptureDefault = Record.readInt();
     Lambda.NumCaptures = Record.readInt();
     Lambda.NumExplicitCaptures = Record.readInt();
     Lambda.ManglingNumber = Record.readInt();
     Lambda.ContextDecl = ReadDeclID();
     Lambda.Captures = (Capture *)Reader.getContext().Allocate(
assert(Reader.Lookups.find(MergeDD.Definition) == Reader.Lookups.end() &&
            "already loaded pending lookups for merged definition");
   }
 
   auto PFDI = Reader.PendingFakeDefinitionData.find(&DD);
   if (PFDI != Reader.PendingFakeDefinitionData.end() &&
       PFDI->second == ASTReader::PendingFakeDefinitionKind::Fake) {
     // We faked up this definition data because we found a class for which we'd
     // not yet loaded the definition. Replace it with the real thing now.
-    assert(!DD.IsLambda && !MergeDD.IsLambda && "faked up lambda definition?");
+    assert(!DD.hasLambdaData() && !MergeDD.hasLambdaData() &&
+           "faked up lambda definition?");
     PFDI->second = ASTReader::PendingFakeDefinitionKind::FakeLoaded;
 
     // Don't change which declaration is the definition; that is required
     // to be invariant once we select it.
     auto *Def = DD.Definition;
     DD = std::move(MergeDD);
     DD.Definition = Def;
     return;
   }
 
   // FIXME: Move this out into a .def file?
   bool DetectedOdrViolation = false;
 #define OR_FIELD(Field) DD.Field |= MergeDD.Field;
 #define MATCH_FIELD(Field) \
#define MATCH_FIELD(Field) \
   // ComputedVisibleConversions is handled below.
   MATCH_FIELD(UserProvidedDefaultConstructor)
   OR_FIELD(DeclaredSpecialMembers)
   MATCH_FIELD(ImplicitCopyConstructorCanHaveConstParamForVBase)
   MATCH_FIELD(ImplicitCopyConstructorCanHaveConstParamForNonVBase)
   MATCH_FIELD(ImplicitCopyAssignmentHasConstParam)
   OR_FIELD(HasDeclaredCopyConstructorWithConstParam)
   OR_FIELD(HasDeclaredCopyAssignmentWithConstParam)
-  MATCH_FIELD(IsLambda)
 #undef OR_FIELD
 #undef MATCH_FIELD
 
   if (DD.NumBases != MergeDD.NumBases || DD.NumVBases != MergeDD.NumVBases)
     DetectedOdrViolation = true;
   // FIXME: Issue a diagnostic if the base classes don't match when we come
   // to lazily load them.
 
   // FIXME: Issue a diagnostic if the list of conversion functions doesn't
   // match when we come to lazily load them.
   if (MergeDD.ComputedVisibleConversions && !DD.ComputedVisibleConversions) {
     DD.VisibleConversions = std::move(MergeDD.VisibleConversions);
     DD.ComputedVisibleConversions = true;
   }
 
   // FIXME: Issue a diagnostic if FirstFriend doesn't match when we come to
   // lazily load it.
 
-  if (DD.IsLambda) {
+  if (DD.hasLambdaData()) {
     // FIXME: ODR-checking for merging lambdas (this happens, for instance,
     // when they occur within the body of a function template specialization).
   }
 
   if (D->getODRHash() != MergeDD.ODRHash) {
     DetectedOdrViolation = true;
   }
 
   if (DetectedOdrViolation)
     Reader.PendingOdrMergeFailures[DD.Definition].push_back(
         {MergeDD.Definition, &MergeDD});
 }
 
 void ASTDeclReader::ReadCXXRecordDefinition(CXXRecordDecl *D, bool Update) {
-  struct CXXRecordDecl::DefinitionData *DD;
   ASTContext &C = Reader.getContext();
 
+  struct CXXRecordDecl::DefinitionData *DD =
+      new (C) struct CXXRecordDecl::DefinitionData(D);
+
   // Determine whether this is a lambda closure type, so that we can
-  // allocate the appropriate DefinitionData structure.
-  bool IsLambda = Record.readInt();
-  if (IsLambda)
-    DD = new (C) CXXRecordDecl::LambdaDefinitionData(D, nullptr, false, false,
-                                                     LCD_None);
-  else
-    DD = new (C) struct CXXRecordDecl::DefinitionData(D);
+  // set the LambdaDefinitionData (into optional \c LambdaData field) as well.
+  bool isLambda = Record.readInt();
+  if (isLambda) {
+    DD->SetLambdaDefinitionData(nullptr, false, false, LCD_None);
+  }
 
   CXXRecordDecl *Canon = D->getCanonicalDecl();
   // Set decl definition data before reading it, so that during deserialization
   // when we read CXXRecordDecl, it already has definition data and we don't
   // set fake one.
   if (!Canon->DefinitionData)
     Canon->DefinitionData = DD;
   D->DefinitionData = Canon->DefinitionData;

lib/Serialization/ASTWriter.cpp

 // FIXME: Move this out of the main ASTRecordWriter interface.
 void ASTRecordWriter::AddCXXCtorInitializers(
     ArrayRef<CXXCtorInitializer *> CtorInits) {
   AddOffset(EmitCXXCtorInitializers(*Writer, CtorInits));
 }
 
 void ASTRecordWriter::AddCXXDefinitionData(const CXXRecordDecl *D) {
   auto &Data = D->data();
-  Record->push_back(Data.IsLambda);
+  Record->push_back(Data.hasLambdaData());  // lambda-specific data added below
   Record->push_back(Data.UserDeclaredConstructor);
   Record->push_back(Data.UserDeclaredSpecialMembers);
   Record->push_back(Data.Aggregate);
   Record->push_back(Data.PlainOldData);
   Record->push_back(Data.Empty);
   Record->push_back(Data.Polymorphic);
   Record->push_back(Data.Abstract);
   Record->push_back(Data.IsStandardLayout);
void ASTRecordWriter::AddCXXDefinitionData(const CXXRecordDecl *D) {
   // getODRHash will compute the ODRHash if it has not been previously computed.
   Record->push_back(D->getODRHash());
   bool ModulesDebugInfo = Writer->Context->getLangOpts().ModulesDebugInfo &&
                           Writer->WritingModule && !D->isDependentType();
   Record->push_back(ModulesDebugInfo);
   if (ModulesDebugInfo)
     Writer->ModularCodegenDecls.push_back(Writer->GetDeclRef(D));
 
-  // IsLambda bit is already saved.
-
   Record->push_back(Data.NumBases);
   if (Data.NumBases > 0)
     AddCXXBaseSpecifiers(Data.bases());
 
   // FIXME: Make VBases lazily computed when needed to avoid storing them.
   Record->push_back(Data.NumVBases);
   if (Data.NumVBases > 0)
     AddCXXBaseSpecifiers(Data.vbases());
 
   AddUnresolvedSet(Data.Conversions.get(*Writer->Context));
   AddUnresolvedSet(Data.VisibleConversions.get(*Writer->Context));
   // Data.Definition is the owning decl, no need to write it.
   AddDeclRef(D->getFirstFriend());
 
   // Add lambda-specific data.
-  if (Data.IsLambda) {
-    auto &Lambda = D->getLambdaData();
+  if (Data.hasLambdaData()) {
+    auto &Lambda = *Data.LambdaData;
     Record->push_back(Lambda.Dependent);
     Record->push_back(Lambda.IsGenericLambda);
     Record->push_back(Lambda.CaptureDefault);
     Record->push_back(Lambda.NumCaptures);
     Record->push_back(Lambda.NumExplicitCaptures);
     Record->push_back(Lambda.ManglingNumber);
     AddDeclRef(D->getLambdaContextDecl());
     AddTypeSourceInfo(Lambda.MethodTyInfo);