[PATCH] D50949: lambdas in modules: handle lambdas in .pcm [de]serialization

[Steve O'Brien via Phabricator via cfe-commits]

elsteveogrande updated this revision to Diff 166130.
elsteveogrande added a comment.

rebase atop https://reviews.llvm.org/D50948 (correctly this time) to exclude 
those changes


Repository:
  rC Clang

https://reviews.llvm.org/D50949

Files:
  include/clang/AST/LambdaCapture.h
  lib/Serialization/ASTReaderDecl.cpp
  test/Modules/merge-lambdas.cpp

Index: test/Modules/merge-lambdas.cpp
===
--- test/Modules/merge-lambdas.cpp
+++ test/Modules/merge-lambdas.cpp
@@ -49,3 +49,25 @@
 
 #pragma clang module import A
 void (*p)() = f();
+
+#pragma clang module build PR38531
+module PR38531 {}
+#pragma clang module contents
+#pragma clang module begin PR38531
+template 
+struct S {};
+struct Fun {
+  template ())>
+  Fun(F) {}
+};
+template 
+void foo(Fun=[]{}) {}
+struct T {
+  void func() {
+foo();
+  }
+};
+#pragma clang module end
+#pragma clang module endbuild
+#pragma clang module import PR38531
+S s;
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -1761,8 +1761,6 @@
   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.hasLambdaData() && !MergeDD.hasLambdaData() &&
-   "faked up lambda definition?");
 PFDI->second = ASTReader::PendingFakeDefinitionKind::FakeLoaded;
 
 // Don't change which declaration is the definition; that is required
@@ -1845,9 +1843,41 @@
   // FIXME: Issue a diagnostic if FirstFriend doesn't match when we come to
   // lazily load it.
 
-  if (DD.hasLambdaData()) {
-// FIXME: ODR-checking for merging lambdas (this happens, for instance,
-// when they occur within the body of a function template specialization).
+  assert (DD.hasLambdaData() == MergeDD.hasLambdaData() &&
+  "Merging definitions, one of which is a lambda, the other not?");
+  if (DD.hasLambdaData() && MergeDD.hasLambdaData()) {
+auto  = *DD.LambdaData;
+auto  = *MergeDD.LambdaData;
+
+#define MATCH_LAM_FIELD(Field) \
+DetectedOdrViolation |= LDD.Field != MergeLDD.Field;
+MATCH_LAM_FIELD(Dependent)
+MATCH_LAM_FIELD(IsGenericLambda)
+MATCH_LAM_FIELD(CaptureDefault)
+MATCH_LAM_FIELD(NumCaptures)
+MATCH_LAM_FIELD(NumExplicitCaptures)
+MATCH_LAM_FIELD(ManglingNumber)
+#undef MATCH_LAM_FIELD
+
+auto *C1 = LDD.Captures;
+auto *C2 = MergeLDD.Captures;
+for (int I = 0; !DetectedOdrViolation && I < LDD.NumCaptures; ++I) {
+  if (C1[I] != C2[I]) {
+DetectedOdrViolation = true;
+  }
+}
+
+if (LDD.MethodTyInfo || MergeLDD.MethodTyInfo) {
+  if (!LDD.MethodTyInfo ||
+  !MergeLDD.MethodTyInfo ||
+  (LDD.MethodTyInfo->getType().getTypePtrOrNull() !=
+  MergeLDD.MethodTyInfo->getType().getTypePtrOrNull())) {
+DetectedOdrViolation = true;
+  }
+}
+
+// FIXME: Issue a diagnostic if ContextDecl doesn't match when we come to
+// lazily load it.
   }
 
   if (D->getODRHash() != MergeDD.ODRHash) {
Index: include/clang/AST/LambdaCapture.h
===
--- include/clang/AST/LambdaCapture.h
+++ include/clang/AST/LambdaCapture.h
@@ -135,6 +135,16 @@
 assert(isPackExpansion() && "No ellipsis location for a non-expansion");
 return EllipsisLoc;
   }
+
+  // [In]Equality operators -- primarily for ODR-compliance checking.
+
+  bool operator==(LambdaCapture const ) const {
+return DeclAndBits == RHS.DeclAndBits;
+  }
+
+  bool operator!=(LambdaCapture const ) const {
+return !operator==(RHS);
+  }
 };
 
 } // end namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50949: lambdas in modules: handle lambdas in .pcm [de]serialization

[Steve O'Brien via Phabricator via cfe-commits]

elsteveogrande updated this revision to Diff 165948.
elsteveogrande added a comment.

Rebase atop updated dependency https://reviews.llvm.org/D50948.  Small cleanups 
to this diff.


Repository:
  rC Clang

https://reviews.llvm.org/D50949

Files:
  include/clang/AST/DeclCXX.h
  include/clang/AST/LambdaCapture.h
  lib/AST/DeclCXX.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriter.cpp
  test/Modules/merge-lambdas.cpp

Index: test/Modules/merge-lambdas.cpp
===
--- test/Modules/merge-lambdas.cpp
+++ test/Modules/merge-lambdas.cpp
@@ -49,3 +49,25 @@
 
 #pragma clang module import A
 void (*p)() = f();
+
+#pragma clang module build PR38531
+module PR38531 {}
+#pragma clang module contents
+#pragma clang module begin PR38531
+template 
+struct S {};
+struct Fun {
+  template ())>
+  Fun(F) {}
+};
+template 
+void foo(Fun=[]{}) {}
+struct T {
+  void func() {
+foo();
+  }
+};
+#pragma clang module end
+#pragma clang module endbuild
+#pragma clang module import PR38531
+S s;
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -6010,7 +6010,7 @@
 
 void ASTRecordWriter::AddCXXDefinitionData(const CXXRecordDecl *D) {
   auto  = 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);
@@ -6068,8 +6068,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());
@@ -6085,8 +6083,8 @@
   AddDeclRef(D->getFirstFriend());
 
   // Add lambda-specific data.
-  if (Data.IsLambda) {
-auto  = D->getLambdaData();
+  if (Data.hasLambdaData()) {
+auto  = *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 , 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  = static_cast(Data);
+auto  = *Data.LambdaData;
 Lambda.Dependent = Record.readInt();
 Lambda.IsGenericLambda = Record.readInt();
 Lambda.CaptureDefault = Record.readInt();
@@ -1761,7 +1761,6 @@
   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?");
 PFDI->second = ASTReader::PendingFakeDefinitionKind::FakeLoaded;
 
 // Don't change which declaration is the definition; that is required
@@ -1826,7 +1825,6 @@
   MATCH_FIELD(ImplicitCopyAssignmentHasConstParam)
   OR_FIELD(HasDeclaredCopyConstructorWithConstParam)
   OR_FIELD(HasDeclaredCopyAssignmentWithConstParam)
-  MATCH_FIELD(IsLambda)
 #undef OR_FIELD
 #undef MATCH_FIELD
 
@@ -1845,9 +1843,41 @@
   // FIXME: Issue a diagnostic if FirstFriend doesn't match when we come to
   // lazily load it.
 
-  if (DD.IsLambda) {
-// FIXME: ODR-checking for merging lambdas (this happens, for instance,
-// when they occur within the body of a function template specialization).
+  assert (DD.hasLambdaData() == MergeDD.hasLambdaData() &&
+  "Merging definitions, one of which is a lambda, the other not?");
+  if (DD.hasLambdaData() && MergeDD.hasLambdaData()) {
+auto  = *DD.LambdaData;
+auto  = *MergeDD.LambdaData;
+
+#define MATCH_LAM_FIELD(Field) \
+DetectedOdrViolation |= LDD.Field != MergeLDD.Field;
+MATCH_LAM_FIELD(Dependent)
+MATCH_LAM_FIELD(IsGenericLambda)
+MATCH_LAM_FIELD(CaptureDefault)
+MATCH_LAM_FIELD(NumCaptures)
+MATCH_LAM_FIELD(NumExplicitCaptures)
+MATCH_LAM_FIELD(ManglingNumber)
+#undef MATCH_LAM_FIELD
+
+auto *C1 = LDD.Captures;
+auto *C2 = MergeLDD.Captures;
+for (int