From 0b87e0739e02b839ab80bf06ce89aab5cbf57bc5 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Mon, 7 Oct 2013 08:02:11 +0000 Subject: [PATCH] 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 | 15 ++++++++- clang/include/clang/AST/Decl.h | 10 +++++- clang/include/clang/AST/Redeclarable.h | 36 +++++++++++++++++++++ clang/lib/AST/Decl.cpp | 10 +++++- clang/lib/Serialization/ASTReaderDecl.cpp | 29 +++++++++++++++++ clang/test/Modules/Inputs/templates-left.h | 4 +++ clang/test/Modules/Inputs/templates-right.h | 4 +++ clang/test/Modules/Inputs/templates-top.h | 4 +++ clang/test/Modules/templates.mm | 32 ++++++++++++++++-- 9 files changed, 139 insertions(+), 5 deletions(-) diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index b0205426750e..06ae604821e8 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -271,6 +271,11 @@ class ASTContext : public RefCountedBase { /// wasting space in the Decl class. llvm::DenseMap 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 MergedDecls; + public: /// \brief A type synonym for the TemplateOrInstantiation mapping. typedef llvm::PointerUnion @@ -746,7 +751,15 @@ public: return import_iterator(FirstLocalImport); } 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; } diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index ec43cb572716..6d0a7f1730a2 100644 --- a/clang/include/clang/AST/Decl.h +++ b/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 { // 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; } diff --git a/clang/include/clang/AST/Redeclarable.h b/clang/include/clang/AST/Redeclarable.h index e3b340a5a1fe..fbf89963ef44 100644 --- a/clang/include/clang/AST/Redeclarable.h +++ b/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 +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(this); + if (!D->isFromASTFile()) + return D; + return cast(getPrimaryMergedDecl(const_cast(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(this); + if (!D->isFromASTFile()) + return D; + return cast(getPrimaryMergedDecl(const_cast(D))); + } + + /// \brief Returns true if this is the first declaration. + bool isFirstDeclaration() const { + return getFirstDeclaration() == this; + } +}; + } #endif diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index a0b3894a0868..6ec14720007a 100644 --- a/clang/lib/AST/Decl.cpp +++ b/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; diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index a5fb21cb66c1..341f8eb068de 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -296,6 +296,9 @@ namespace clang { void mergeRedeclarable(Redeclarable *D, T *Existing, RedeclarableResult &Redecl); + template + void mergeMergeable(Mergeable *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(Record, Idx)) Reader.getContext().setInstantiatedFromUnnamedFieldDecl(FD, Tmpl); } + mergeMergeable(FD); } void ASTDeclReader::VisitMSPropertyDecl(MSPropertyDecl *PD) { @@ -1874,6 +1878,22 @@ void ASTDeclReader::mergeRedeclarable(Redeclarable *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 +void ASTDeclReader::mergeMergeable(Mergeable *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(D))) + if (T *Existing = ExistingRes) + Reader.Context.setPrimaryMergedDecl(static_cast(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(X)) { + FieldDecl *FDY = cast(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; } diff --git a/clang/test/Modules/Inputs/templates-left.h b/clang/test/Modules/Inputs/templates-left.h index 7c9be8c6514b..e76598d6bd5c 100644 --- a/clang/test/Modules/Inputs/templates-left.h +++ b/clang/test/Modules/Inputs/templates-left.h @@ -19,6 +19,10 @@ namespace N { }; } +constexpr unsigned List::*size_left = &List::size; +List list_left = { 0, 8 }; +typedef List ListInt_left; + template void pendingInstantiationEmit(T) {} void triggerPendingInstantiation() { diff --git a/clang/test/Modules/Inputs/templates-right.h b/clang/test/Modules/Inputs/templates-right.h index bacaa49b60ca..16d0a714d90e 100644 --- a/clang/test/Modules/Inputs/templates-right.h +++ b/clang/test/Modules/Inputs/templates-right.h @@ -18,6 +18,10 @@ namespace N { }; } +constexpr unsigned List::*size_right = &List::size; +List list_right = { 0, 12 }; +typedef List ListInt_right; + template void pendingInstantiationEmit(T) {} void triggerPendingInstantiationToo() { diff --git a/clang/test/Modules/Inputs/templates-top.h b/clang/test/Modules/Inputs/templates-top.h index b5d0b6bda135..87dcd8b7f465 100644 --- a/clang/test/Modules/Inputs/templates-top.h +++ b/clang/test/Modules/Inputs/templates-top.h @@ -3,6 +3,10 @@ template class Vector; template class List { public: void push_back(T); + + struct node {}; + node *head; + unsigned size; }; namespace A { diff --git a/clang/test/Modules/templates.mm b/clang/test/Modules/templates.mm index 08b216646c32..080f9e7c665e 100644 --- a/clang/test/Modules/templates.mm +++ b/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 vec_int; @@ -39,3 +43,27 @@ typedef Outer::Inner OuterIntInner; // CHECK: call {{.*pendingInstantiation}} // CHECK: call {{.*redeclDefinitionEmit}} + +static_assert(size_left == size_right, "same field both ways"); +void useListInt(List &); + +// 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; +}