When merging class definitions across modules in C++, merge together fields.

This change doesn't go all the way to making fields redeclarable; instead, it
makes them 'mergeable', which means we can find the canonical declaration, but
not much else (and for a declaration that's not from a module, the canonical
declaration is always that declaration).

llvm-svn: 192092

clang/include/clang/AST/ASTContext.h

@@ -271,6 +271,11 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// wasting space in the Decl class.
   llvm::DenseMap<const Decl*, AttrVec*> DeclAttrs;
 
+  /// \brief A mapping from non-redeclarable declarations in modules that were
+  /// merged with other declarations to the canonical declaration that they were
+  /// merged into.
+  llvm::DenseMap<Decl*, Decl*> MergedDecls;
+
 public:
   /// \brief A type synonym for the TemplateOrInstantiation mapping.
   typedef llvm::PointerUnion<VarTemplateDecl *, MemberSpecializationInfo *>
@@ -747,6 +752,14 @@ public:
   }
   import_iterator local_import_end() const { return import_iterator(); }
 
+  Decl *getPrimaryMergedDecl(Decl *D) {
+    Decl *Result = MergedDecls.lookup(D);
+    return Result ? Result : D;
+  }
+  void setPrimaryMergedDecl(Decl *D, Decl *Primary) {
+    MergedDecls[D] = Primary;
+  }
+
   TranslationUnitDecl *getTranslationUnitDecl() const { return TUDecl; }
 
 

clang/include/clang/AST/Decl.h

@@ -2108,7 +2108,7 @@ public:
 
 /// FieldDecl - An instance of this class is created by Sema::ActOnField to
 /// represent a member of a struct/union/class.
-class FieldDecl : public DeclaratorDecl {
+class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
   // FIXME: This can be packed into the bitfields in Decl.
   bool Mutable : 1;
   mutable unsigned CachedFieldIndex : 31;
@@ -2222,6 +2222,14 @@ public:
 
   SourceRange getSourceRange() const LLVM_READONLY;
 
+  /// Retrieves the canonical declaration of this field.
+  FieldDecl *getCanonicalDecl() {
+    return getFirstDeclaration();
+  }
+  const FieldDecl *getCanonicalDecl() const {
+    return getFirstDeclaration();
+  }
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K >= firstField && K <= lastField; }

clang/include/clang/AST/Redeclarable.h

@@ -175,6 +175,42 @@ public:
   friend class ASTDeclWriter;
 };
 
+/// \brief Get the primary declaration for a declaration from an AST file. That
+/// will be the first-loaded declaration.
+Decl *getPrimaryMergedDecl(Decl *D);
+
+/// \brief Provides common interface for the Decls that cannot be redeclared,
+/// but can be merged if the same declaration is brought in from multiple
+/// modules.
+template<typename decl_type>
+class Mergeable {
+public:
+  Mergeable() {}
+
+  /// \brief Return the first declaration of this declaration or itself if this
+  /// is the only declaration.
+  decl_type *getFirstDeclaration() {
+    decl_type *D = static_cast<decl_type*>(this);
+    if (!D->isFromASTFile())
+      return D;
+    return cast<decl_type>(getPrimaryMergedDecl(const_cast<decl_type*>(D)));
+  }
+
+  /// \brief Return the first declaration of this declaration or itself if this
+  /// is the only declaration.
+  const decl_type *getFirstDeclaration() const {
+    const decl_type *D = static_cast<const decl_type*>(this);
+    if (!D->isFromASTFile())
+      return D;
+    return cast<decl_type>(getPrimaryMergedDecl(const_cast<decl_type*>(D)));
+  }
+
+  /// \brief Returns true if this is the first declaration.
+  bool isFirstDeclaration() const {
+    return getFirstDeclaration() == this;
+  }
+};
+
 }
 
 #endif

clang/lib/AST/Decl.cpp

@@ -34,6 +34,10 @@
 
 using namespace clang;
 
+Decl *clang::getPrimaryMergedDecl(Decl *D) {
+  return D->getASTContext().getPrimaryMergedDecl(D);
+}
+
 //===----------------------------------------------------------------------===//
 // NamedDecl Implementation
 //===----------------------------------------------------------------------===//
@@ -3094,6 +3098,10 @@ unsigned FieldDecl::getBitWidthValue(const ASTContext &Ctx) const {
 }
 
 unsigned FieldDecl::getFieldIndex() const {
+  const FieldDecl *Canonical = getCanonicalDecl();
+  if (Canonical != this)
+    return Canonical->getFieldIndex();
+
   if (CachedFieldIndex) return CachedFieldIndex - 1;
 
   unsigned Index = 0;
@@ -3101,7 +3109,7 @@ unsigned FieldDecl::getFieldIndex() const {
 
   for (RecordDecl::field_iterator I = RD->field_begin(), E = RD->field_end();
        I != E; ++I, ++Index)
-    I->CachedFieldIndex = Index + 1;
+    I->getCanonicalDecl()->CachedFieldIndex = Index + 1;
 
   assert(CachedFieldIndex && "failed to find field in parent");
   return CachedFieldIndex - 1;

clang/lib/Serialization/ASTReaderDecl.cpp

@@ -296,6 +296,9 @@ namespace clang {
     void mergeRedeclarable(Redeclarable<T> *D, T *Existing,
                            RedeclarableResult &Redecl);
 
+    template<typename T>
+    void mergeMergeable(Mergeable<T> *D);
+
     // FIXME: Reorder according to DeclNodes.td?
     void VisitObjCMethodDecl(ObjCMethodDecl *D);
     void VisitObjCContainerDecl(ObjCContainerDecl *D);
@@ -914,6 +917,7 @@ void ASTDeclReader::VisitFieldDecl(FieldDecl *FD) {
     if (FieldDecl *Tmpl = ReadDeclAs<FieldDecl>(Record, Idx))
       Reader.getContext().setInstantiatedFromUnnamedFieldDecl(FD, Tmpl);
   }
+  mergeMergeable(FD);
 }
 
 void ASTDeclReader::VisitMSPropertyDecl(MSPropertyDecl *PD) {
@@ -1874,6 +1878,22 @@ void ASTDeclReader::mergeRedeclarable(Redeclarable<T> *D, T *Existing,
   }
 }
 
+/// \brief Attempts to merge the given declaration (D) with another declaration
+/// of the same entity, for the case where the entity is not actually
+/// redeclarable. This happens, for instance, when merging the fields of
+/// identical class definitions from two different modules.
+template<typename T>
+void ASTDeclReader::mergeMergeable(Mergeable<T> *D) {
+  // If modules are not available, there is no reason to perform this merge.
+  if (!Reader.getContext().getLangOpts().Modules)
+    return;
+
+  if (FindExistingResult ExistingRes = findExisting(static_cast<T*>(D)))
+    if (T *Existing = ExistingRes)
+      Reader.Context.setPrimaryMergedDecl(static_cast<T*>(D),
+                                          Existing->getCanonicalDecl());
+}
+
 void ASTDeclReader::VisitOMPThreadPrivateDecl(OMPThreadPrivateDecl *D) {
   VisitDecl(D);
   unsigned NumVars = D->varlist_size();
@@ -2091,6 +2111,15 @@ static bool isSameEntity(NamedDecl *X, NamedDecl *Y) {
                                        TemplateY->getTemplateParameters());
   }
 
+  // Fields with the same name and the same type match.
+  if (FieldDecl *FDX = dyn_cast<FieldDecl>(X)) {
+    FieldDecl *FDY = cast<FieldDecl>(Y);
+    // FIXME: Diagnose if the types don't match. More generally, diagnose if we
+    // get a declaration in a class definition that isn't in the canonical class
+    // definition.
+    return X->getASTContext().hasSameType(FDX->getType(), FDY->getType());
+  }
+
   // FIXME: Many other cases to implement.
   return false;
 }

clang/test/Modules/Inputs/templates-left.h

@@ -19,6 +19,10 @@ namespace N {
   };
 }
 
+constexpr unsigned List<int>::*size_left = &List<int>::size;
+List<int> list_left = { 0, 8 };
+typedef List<int> ListInt_left;
+
 template <typename T>
 void pendingInstantiationEmit(T) {}
 void triggerPendingInstantiation() {

clang/test/Modules/Inputs/templates-right.h

@@ -18,6 +18,10 @@ namespace N {
   };
 }
 
+constexpr unsigned List<int>::*size_right = &List<int>::size;
+List<int> list_right = { 0, 12 };
+typedef List<int> ListInt_right;
+
 template <typename T>
 void pendingInstantiationEmit(T) {}
 void triggerPendingInstantiationToo() {

clang/test/Modules/Inputs/templates-top.h

@@ -3,6 +3,10 @@ template<typename T> class Vector;
 template<typename T> class List {
 public:
   void push_back(T);
+
+  struct node {};
+  node *head;
+  unsigned size;
 };
 
 namespace A {

clang/test/Modules/templates.mm

@@ -1,11 +1,15 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -x objective-c++ -fmodules -fmodules-cache-path=%t -I %S/Inputs -verify %s -Wno-objc-root-class
-// RUN: %clang_cc1 -x objective-c++ -fmodules -fmodules-cache-path=%t -I %S/Inputs -emit-llvm %s -o - -Wno-objc-root-class | grep Emit | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++11 -x objective-c++ -fmodules -fmodules-cache-path=%t -I %S/Inputs -verify %s -Wno-objc-root-class
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++11 -x objective-c++ -fmodules -fmodules-cache-path=%t -I %S/Inputs -emit-llvm %s -o - -Wno-objc-root-class | FileCheck %s
 // expected-no-diagnostics
 
 @import templates_left;
 @import templates_right;
 
+// CHECK: @list_left = global { %{{.*}}*, i32, [4 x i8] } { %{{.*}}* null, i32 8,
+// CHECK: @list_right = global { %{{.*}}*, i32, [4 x i8] } { %{{.*}}* null, i32 12,
+// CHECK: @_ZZ15testMixedStructvE1l = {{.*}} constant { %{{.*}}*, i32, [4 x i8] } { %{{.*}}* null, i32 1,
+// CHECK: @_ZZ15testMixedStructvE1r = {{.*}} constant { %{{.*}}*, i32, [4 x i8] } { %{{.*}}* null, i32 2,
 
 void testTemplateClasses() {
   Vector<int> vec_int;
@@ -39,3 +43,27 @@ typedef Outer<int>::Inner OuterIntInner;
 
 // CHECK: call {{.*pendingInstantiation}}
 // CHECK: call {{.*redeclDefinitionEmit}}
+
+static_assert(size_left == size_right, "same field both ways");
+void useListInt(List<int> &);
+
+// CHECK-LABEL: define i32 @_Z15testMixedStructv(
+unsigned testMixedStruct() {
+  // CHECK: %[[l:.*]] = alloca %[[ListInt:[^ ]*]], align 8
+  // CHECK: %[[r:.*]] = alloca %[[ListInt]], align 8
+
+  // CHECK: call {{.*}}memcpy{{.*}}(i8* %{{.*}}, i8* bitcast ({{.*}}* @_ZZ15testMixedStructvE1l to i8*), i64 16,
+  ListInt_left l{0, 1};
+
+  // CHECK: call {{.*}}memcpy{{.*}}(i8* %{{.*}}, i8* bitcast ({{.*}}* @_ZZ15testMixedStructvE1r to i8*), i64 16,
+  ListInt_right r{0, 2};
+
+  // CHECK: call void @_Z10useListIntR4ListIiE(%[[ListInt]]* %[[l]])
+  useListInt(l);
+  // CHECK: call void @_Z10useListIntR4ListIiE(%[[ListInt]]* %[[r]])
+  useListInt(r);
+
+  // CHECK: load i32* bitcast (i8* getelementptr inbounds (i8* bitcast ({{.*}}* @list_left to i8*), i64 8) to i32*)
+  // CHECK: load i32* bitcast (i8* getelementptr inbounds (i8* bitcast ({{.*}}* @list_right to i8*), i64 8) to i32*)
+  return list_left.*size_right + list_right.*size_left;
+}