From 1420a79d5fd73dbf009acd75e7224fcf8115ef09 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Tue, 17 May 2016 22:44:15 +0000 Subject: [PATCH] PR27754: CXXRecordDecl::data() needs to perform an update even if it's called on a declaration that already knows the location of the DefinitionData object. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@269858 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/DeclCXX.h | 48 ++++++------------- lib/AST/DeclCXX.cpp | 2 +- lib/Serialization/ASTReaderDecl.cpp | 20 ++++---- .../Inputs/PR27754/RConversionRuleParser.h | 4 ++ test/Modules/Inputs/PR27754/TMetaUtils.h | 2 + test/Modules/Inputs/PR27754/TSchemaType.h | 2 + test/Modules/Inputs/PR27754/algobase.h | 4 ++ test/Modules/Inputs/PR27754/module.modulemap | 3 ++ test/Modules/pr27754.cpp | 7 +++ 9 files changed, 47 insertions(+), 45 deletions(-) create mode 100644 test/Modules/Inputs/PR27754/RConversionRuleParser.h create mode 100644 test/Modules/Inputs/PR27754/TMetaUtils.h create mode 100644 test/Modules/Inputs/PR27754/TSchemaType.h create mode 100644 test/Modules/Inputs/PR27754/algobase.h create mode 100644 test/Modules/Inputs/PR27754/module.modulemap create mode 100644 test/Modules/pr27754.cpp diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h index 20aa7d1ab4..cd1e1d6b8d 100644 --- a/include/clang/AST/DeclCXX.h +++ b/include/clang/AST/DeclCXX.h @@ -257,30 +257,6 @@ public: TypeSourceInfo *getTypeSourceInfo() const { return BaseTypeInfo; } }; -/// \brief A lazy pointer to the definition data for a declaration. -/// FIXME: This is a little CXXRecordDecl-specific that the moment. -template class LazyDefinitionDataPtr { - llvm::PointerUnion DataOrCanonicalDecl; - - LazyDefinitionDataPtr update() { - if (Decl *Canon = DataOrCanonicalDecl.template dyn_cast()) { - if (Canon->isCanonicalDecl()) - Canon->getMostRecentDecl(); - else - // Declaration isn't canonical any more; - // update it and perform path compression. - *this = Canon->getPreviousDecl()->DefinitionData.update(); - } - return *this; - } - -public: - LazyDefinitionDataPtr(Decl *Canon) : DataOrCanonicalDecl(Canon) {} - LazyDefinitionDataPtr(T *Data) : DataOrCanonicalDecl(Data) {} - T *getNotUpdated() { return DataOrCanonicalDecl.template dyn_cast(); } - T *get() { return update().getNotUpdated(); } -}; - /// \brief Represents a C++ struct/union/class. class CXXRecordDecl : public RecordDecl { @@ -543,11 +519,7 @@ class CXXRecordDecl : public RecordDecl { CXXBaseSpecifier *getVBasesSlowCase() const; }; - typedef LazyDefinitionDataPtr - DefinitionDataPtr; - friend class LazyDefinitionDataPtr; - - mutable DefinitionDataPtr DefinitionData; + struct DefinitionData *DefinitionData; /// \brief Describes a C++ closure type (generated by a lambda expression). struct LambdaDefinitionData : public DefinitionData { @@ -610,8 +582,14 @@ class CXXRecordDecl : public RecordDecl { }; + struct DefinitionData *dataPtr() const { + // Complete the redecl chain (if necessary). + getMostRecentDecl(); + return DefinitionData; + } + struct DefinitionData &data() const { - auto *DD = DefinitionData.get(); + auto *DD = dataPtr(); assert(DD && "queried property of class with no definition"); return *DD; } @@ -619,7 +597,7 @@ class CXXRecordDecl : public RecordDecl { struct LambdaDefinitionData &getLambdaData() const { // No update required: a merged definition cannot change any lambda // properties. - auto *DD = DefinitionData.getNotUpdated(); + auto *DD = DefinitionData; assert(DD && DD->IsLambda && "queried lambda property of non-lambda class"); return static_cast(*DD); } @@ -696,11 +674,13 @@ public: } CXXRecordDecl *getDefinition() const { - auto *DD = DefinitionData.get(); + // We only need an update if we don't already know which + // declaration is the definition. + auto *DD = DefinitionData ? DefinitionData : dataPtr(); return DD ? DD->Definition : nullptr; } - bool hasDefinition() const { return DefinitionData.get(); } + bool hasDefinition() const { return DefinitionData || dataPtr(); } static CXXRecordDecl *Create(const ASTContext &C, TagKind TK, DeclContext *DC, SourceLocation StartLoc, SourceLocation IdLoc, @@ -1044,7 +1024,7 @@ public: /// \brief Determine whether this class describes a lambda function object. bool isLambda() const { // An update record can't turn a non-lambda into a lambda. - auto *DD = DefinitionData.getNotUpdated(); + auto *DD = DefinitionData; return DD && DD->IsLambda; } diff --git a/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp index f22f2ee0af..9dc0771ead 100644 --- a/lib/AST/DeclCXX.cpp +++ b/lib/AST/DeclCXX.cpp @@ -88,7 +88,7 @@ CXXRecordDecl::CXXRecordDecl(Kind K, TagKind TK, const ASTContext &C, CXXRecordDecl *PrevDecl) : RecordDecl(K, TK, C, DC, StartLoc, IdLoc, Id, PrevDecl), DefinitionData(PrevDecl ? PrevDecl->DefinitionData - : DefinitionDataPtr(this)), + : nullptr), TemplateOrInstantiation() {} CXXRecordDecl *CXXRecordDecl::Create(const ASTContext &C, TagKind TK, diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp index 1b8044e123..325acd3181 100644 --- a/lib/Serialization/ASTReaderDecl.cpp +++ b/lib/Serialization/ASTReaderDecl.cpp @@ -1540,9 +1540,9 @@ void ASTDeclReader::ReadCXXDefinitionData( void ASTDeclReader::MergeDefinitionData( CXXRecordDecl *D, struct CXXRecordDecl::DefinitionData &&MergeDD) { - assert(D->DefinitionData.getNotUpdated() && + assert(D->DefinitionData && "merging class definition into non-definition"); - auto &DD = *D->DefinitionData.getNotUpdated(); + auto &DD = *D->DefinitionData; if (DD.Definition != MergeDD.Definition) { // Track that we merged the definitions. @@ -1665,7 +1665,7 @@ void ASTDeclReader::ReadCXXRecordDefinition(CXXRecordDecl *D, bool Update) { // because we're reading an update record, or because we've already done some // merging. Either way, just merge into it. CXXRecordDecl *Canon = D->getCanonicalDecl(); - if (Canon->DefinitionData.getNotUpdated()) { + if (Canon->DefinitionData) { MergeDefinitionData(Canon, std::move(*DD)); D->DefinitionData = Canon->DefinitionData; return; @@ -2001,8 +2001,8 @@ ASTDeclReader::VisitClassTemplateSpecializationDeclImpl( // This declaration might be a definition. Merge with any existing // definition. - if (auto *DDD = D->DefinitionData.getNotUpdated()) { - if (CanonSpec->DefinitionData.getNotUpdated()) + if (auto *DDD = D->DefinitionData) { + if (CanonSpec->DefinitionData) MergeDefinitionData(CanonSpec, std::move(*DDD)); else CanonSpec->DefinitionData = D->DefinitionData; @@ -2326,8 +2326,8 @@ void ASTDeclReader::mergeTemplatePattern(RedeclarableTemplateDecl *D, // FIXME: This is duplicated in several places. Refactor. auto *ExistingClass = cast(ExistingPattern)->getCanonicalDecl(); - if (auto *DDD = DClass->DefinitionData.getNotUpdated()) { - if (ExistingClass->DefinitionData.getNotUpdated()) { + if (auto *DDD = DClass->DefinitionData) { + if (ExistingClass->DefinitionData) { MergeDefinitionData(ExistingClass, std::move(*DDD)); } else { ExistingClass->DefinitionData = DClass->DefinitionData; @@ -2765,9 +2765,9 @@ DeclContext *ASTDeclReader::getPrimaryContextForMerging(ASTReader &Reader, if (CXXRecordDecl *RD = dyn_cast(DC)) { // Try to dig out the definition. - auto *DD = RD->DefinitionData.getNotUpdated(); + auto *DD = RD->DefinitionData; if (!DD) - DD = RD->getCanonicalDecl()->DefinitionData.getNotUpdated(); + DD = RD->getCanonicalDecl()->DefinitionData; // If there's no definition yet, then DC's definition is added by an update // record, but we've not yet loaded that update record. In this case, we @@ -3772,7 +3772,7 @@ void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile, case UPD_CXX_INSTANTIATED_CLASS_DEFINITION: { auto *RD = cast(D); - auto *OldDD = RD->getCanonicalDecl()->DefinitionData.getNotUpdated(); + auto *OldDD = RD->getCanonicalDecl()->DefinitionData; bool HadRealDefinition = OldDD && (OldDD->Definition != RD || !Reader.PendingFakeDefinitionData.count(OldDD)); diff --git a/test/Modules/Inputs/PR27754/RConversionRuleParser.h b/test/Modules/Inputs/PR27754/RConversionRuleParser.h new file mode 100644 index 0000000000..057dd14698 --- /dev/null +++ b/test/Modules/Inputs/PR27754/RConversionRuleParser.h @@ -0,0 +1,4 @@ +#include "algobase.h" +typedef integral_constant true_type; +class _Rb_tree { _Rb_tree() { true_type(); } }; +#include "TSchemaType.h" diff --git a/test/Modules/Inputs/PR27754/TMetaUtils.h b/test/Modules/Inputs/PR27754/TMetaUtils.h new file mode 100644 index 0000000000..835b7c6dc6 --- /dev/null +++ b/test/Modules/Inputs/PR27754/TMetaUtils.h @@ -0,0 +1,2 @@ +#include "RConversionRuleParser.h" +void fn1() { true_type(); } diff --git a/test/Modules/Inputs/PR27754/TSchemaType.h b/test/Modules/Inputs/PR27754/TSchemaType.h new file mode 100644 index 0000000000..2c47793170 --- /dev/null +++ b/test/Modules/Inputs/PR27754/TSchemaType.h @@ -0,0 +1,2 @@ +#include "algobase.h" +struct A : integral_constant {}; diff --git a/test/Modules/Inputs/PR27754/algobase.h b/test/Modules/Inputs/PR27754/algobase.h new file mode 100644 index 0000000000..f5e47d8dc7 --- /dev/null +++ b/test/Modules/Inputs/PR27754/algobase.h @@ -0,0 +1,4 @@ +#ifndef _STL_ALGOBASE_H +#define _STL_ALGOBASE_H +template struct integral_constant {}; +#endif diff --git a/test/Modules/Inputs/PR27754/module.modulemap b/test/Modules/Inputs/PR27754/module.modulemap new file mode 100644 index 0000000000..90dcdbb92b --- /dev/null +++ b/test/Modules/Inputs/PR27754/module.modulemap @@ -0,0 +1,3 @@ +module "RConversionRuleParser.h" { header "RConversionRuleParser.h" } +module "TMetaUtils.h" { header "TMetaUtils.h" } +module "TSchemaType.h" { header "TSchemaType.h" } diff --git a/test/Modules/pr27754.cpp b/test/Modules/pr27754.cpp new file mode 100644 index 0000000000..0482595429 --- /dev/null +++ b/test/Modules/pr27754.cpp @@ -0,0 +1,7 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -std=c++11 -I%S/Inputs/PR27754 -verify %s +// RUN: %clang_cc1 -std=c++11 -fmodules -fmodule-map-file=%S/Inputs/PR27754/module.modulemap -fmodules-cache-path=%t -I%S/Inputs/PR27754/ -verify %s + +#include "TMetaUtils.h" + +// expected-no-diagnostics -- 2.40.0