elsteveogrande created this revision.
Herald added a subscriber: cfe-commits.

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`


Repository:
  rC Clang

https://reviews.llvm.org/D50948

Files:
  include/clang/AST/DeclCXX.h
  lib/AST/DeclCXX.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriter.cpp

Index: lib/Serialization/ASTWriter.cpp
===================================================================
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -6017,7 +6017,7 @@
 
 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);
@@ -6075,8 +6075,6 @@
   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());
@@ -6092,8 +6090,8 @@
   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);
Index: lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -1636,7 +1636,7 @@
 
 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();
@@ -1703,10 +1703,10 @@
   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();
@@ -1764,7 +1764,10 @@
     Reader.PendingFakeDefinitionData.erase(PFDI);
 
     // FIXME: handle serialized lambdas
-    assert(!DD.IsLambda && !MergeDD.IsLambda && "faked up lambda definition?");
+    assert(!(DD.hasLambdaData() || MergeDD.hasLambdaData()) &&
+           "faked up lambda definition?");
+
+    assert(DD.hasLambdaData() == MergeDD.hasLambdaData());
 
     // Don't change which declaration is the definition; that is required
     // to be invariant once we select it.
@@ -1828,7 +1831,6 @@
   MATCH_FIELD(ImplicitCopyAssignmentHasConstParam)
   OR_FIELD(HasDeclaredCopyConstructorWithConstParam)
   OR_FIELD(HasDeclaredCopyAssignmentWithConstParam)
-  MATCH_FIELD(IsLambda)
 #undef OR_FIELD
 #undef MATCH_FIELD
 
@@ -1847,7 +1849,7 @@
   // 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).
   }
@@ -1862,17 +1864,17 @@
 }
 
 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
Index: lib/AST/DeclCXX.cpp
===================================================================
--- lib/AST/DeclCXX.cpp
+++ lib/AST/DeclCXX.cpp
@@ -102,7 +102,7 @@
       ImplicitCopyConstructorCanHaveConstParamForNonVBase(true),
       ImplicitCopyAssignmentHasConstParam(true),
       HasDeclaredCopyConstructorWithConstParam(false),
-      HasDeclaredCopyAssignmentWithConstParam(false), IsLambda(false),
+      HasDeclaredCopyAssignmentWithConstParam(false),
       IsParsingBaseSpecifiers(false), HasODRHash(false), Definition(D) {}
 
 CXXBaseSpecifier *CXXRecordDecl::DefinitionData::getBasesSlowCase() const {
@@ -144,9 +144,9 @@
   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 DefinitionData(R);
+  R->DefinitionData->SetLambdaDefinitionData(Info, Dependent, IsGeneric,
+                                             CaptureDefault);
   R->setMayHaveOutOfDateDef(false);
   R->setImplicit(true);
   C.getTypeDeclType(R, /*PrevDecl=*/nullptr);
Index: include/clang/AST/DeclCXX.h
===================================================================
--- include/clang/AST/DeclCXX.h
+++ include/clang/AST/DeclCXX.h
@@ -329,6 +329,58 @@
     SMF_All = 0x3f
   };
 
+  /// 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 {
     /// True if this class has any user-declared constructors.
     unsigned UserDeclaredConstructor : 1;
@@ -530,9 +582,6 @@
     /// 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;
 
@@ -577,6 +626,9 @@
     /// This is actually currently stored in reverse order.
     LazyDeclPtr FirstFriend;
 
+    /// Present if and only if this is for a lambda.
+    llvm::Optional<LambdaDefinitionData> LambdaData;
+
     DefinitionData(CXXRecordDecl *D);
 
     /// Retrieve the set of direct base classes.
@@ -601,71 +653,35 @@
       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.emplace(
+          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 LambdaData.hasValue();
     }
+
+  private:
+    CXXBaseSpecifier *getBasesSlowCase() const;
+    CXXBaseSpecifier *getVBasesSlowCase() const;
   };
 
+  struct DefinitionData *DefinitionData;
+
   struct DefinitionData *dataPtr() const {
     // Complete the redecl chain (if necessary).
     getMostRecentDecl();
@@ -682,8 +698,9 @@
     // 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
@@ -1210,7 +1227,7 @@
   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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to