Lazily deserialize the "first' friend declaration when deserializing a class

declaration. This PCH a little lazier, and breaks a deserialization cycle that
causes crashes with modules enabled.

llvm-svn: 184904

8 files changed, 47 insertions, 7 deletions

diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index 9722ad8427a..2de99f0925c 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -482,7 +482,7 @@ class CXXRecordDecl : public RecordDecl {
     /// FirstFriend - The first friend declaration in this class, or
     /// null if there aren't any.  This is actually currently stored
     /// in reverse order.
-    FriendDecl *FirstFriend;
+    LazyDeclPtr FirstFriend;
 
     /// \brief Retrieve the set of direct base classes.
     CXXBaseSpecifier *getBases() const {
@@ -597,6 +597,10 @@ class CXXRecordDecl : public RecordDecl {
 
   friend class ASTNodeImporter;
 
+  /// \brief Get the head of our list of friend declarations, possibly
+  /// deserializing the friends from an external AST source.
+  FriendDecl *getFirstFriend() const;
+
 protected:
   CXXRecordDecl(Kind K, TagKind TK, DeclContext *DC,
                 SourceLocation StartLoc, SourceLocation IdLoc,
@@ -749,7 +753,7 @@ public:
 
   /// Determines whether this record has any friends.
   bool hasFriends() const {
-    return data().FirstFriend != 0;
+    return data().FirstFriend.isValid();
   }
 
   /// \brief \c true if we know for sure that this class has a single,
diff --git a/clang/include/clang/AST/DeclFriend.h b/clang/include/clang/AST/DeclFriend.h
index 589178ec6ec..be6f2eb3e3a 100644
--- a/clang/include/clang/AST/DeclFriend.h
+++ b/clang/include/clang/AST/DeclFriend.h
@@ -220,7 +220,7 @@ public:
 };
 
 inline CXXRecordDecl::friend_iterator CXXRecordDecl::friend_begin() const {
-  return friend_iterator(data().FirstFriend);
+  return friend_iterator(getFirstFriend());
 }
 
 inline CXXRecordDecl::friend_iterator CXXRecordDecl::friend_end() const {
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index df8a52675be..910809575b4 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -62,7 +62,7 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D)
     HasDeclaredCopyAssignmentWithConstParam(false),
     FailedImplicitMoveConstructor(false), FailedImplicitMoveAssignment(false),
     IsLambda(false), NumBases(0), NumVBases(0), Bases(), VBases(),
-    Definition(D), FirstFriend(0) {
+    Definition(D), FirstFriend() {
 }
 
 CXXBaseSpecifier *CXXRecordDecl::DefinitionData::getBasesSlowCase() const {
diff --git a/clang/lib/AST/DeclFriend.cpp b/clang/lib/AST/DeclFriend.cpp
index 37a812e71aa..1c639d676dc 100644
--- a/clang/lib/AST/DeclFriend.cpp
+++ b/clang/lib/AST/DeclFriend.cpp
@@ -63,3 +63,8 @@ FriendDecl *FriendDecl::CreateDeserialized(ASTContext &C, unsigned ID,
   return new (Mem) FriendDecl(EmptyShell(), FriendTypeNumTPLists);
 }
 
+FriendDecl *CXXRecordDecl::getFirstFriend() const {
+  ExternalASTSource *Source = getParentASTContext().getExternalSource();
+  Decl *First = data().FirstFriend.get(Source);
+  return First ? cast<FriendDecl>(First) : 0;
+}
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index df6abc934f9..e93eae81127 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1156,7 +1156,7 @@ void ASTDeclReader::ReadCXXDefinitionData(
   Reader.ReadUnresolvedSet(F, Data.Conversions, Record, Idx);
   Reader.ReadUnresolvedSet(F, Data.VisibleConversions, Record, Idx);
   assert(Data.Definition && "Data.Definition should be already set!");
-  Data.FirstFriend = ReadDeclAs<FriendDecl>(Record, Idx);
+  Data.FirstFriend = Record[Idx++];
   
   if (Data.IsLambda) {
     typedef LambdaExpr::Capture Capture;
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 4a4128b4cfd..bf01e9c2f27 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -5054,7 +5054,7 @@ void ASTWriter::AddCXXDefinitionData(const CXXRecordDecl *D, RecordDataImpl &Rec
   AddUnresolvedSet(Data.Conversions, Record);
   AddUnresolvedSet(Data.VisibleConversions, Record);
   // Data.Definition is the owning decl, no need to write it. 
-  AddDeclRef(Data.FirstFriend, Record);
+  AddDeclRef(D->getFirstFriend(), Record);
   
   // Add lambda-specific data.
   if (Data.IsLambda) {
diff --git a/clang/test/PCH/cxx-friends.cpp b/clang/test/PCH/cxx-friends.cpp
index f7d45cea8df..9c75f92f34d 100644
--- a/clang/test/PCH/cxx-friends.cpp
+++ b/clang/test/PCH/cxx-friends.cpp
@@ -3,7 +3,11 @@
 
 // Test with pch.
 // RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/cxx-friends.h
-// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s 
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s -error-on-deserialized-decl doNotDeserialize
+
+// Test with modules.
+// RUN: %clang_cc1 -x c++-header -emit-pch -o %t %S/cxx-friends.h -fmodules
+// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s -error-on-deserialized-decl doNotDeserialize -fmodules
 
 // expected-no-diagnostics
 
@@ -21,3 +25,5 @@ public:
   }
 };
 int k = PR12585::future_base::setter<int>().f();
+
+Lazy::S *p;
diff --git a/clang/test/PCH/cxx-friends.h b/clang/test/PCH/cxx-friends.h
index 05dcc960663..2d20a4d2697 100644
--- a/clang/test/PCH/cxx-friends.h
+++ b/clang/test/PCH/cxx-friends.h
@@ -16,3 +16,28 @@ namespace PR12585 {
     int k;
   };
 }
+
+namespace Lazy {
+  struct S {
+    friend void doNotDeserialize();
+  };
+}
+
+// Reduced testcase from libc++'s <valarray>. Used to crash with modules
+// enabled.
+namespace std {
+
+template <class T> struct valarray;
+
+template <class T> struct valarray {
+  valarray();
+  template <class U> friend struct valarray;
+  template <class U> friend U *begin(valarray<U> &v);
+};
+
+struct gslice {
+  valarray<int> size;
+  gslice() {}
+};
+
+}