From 6de7a247824b4d15e97c551a919410cbce90a24f Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 31 Jul 2014 23:46:44 +0000 Subject: [modules] Maintain an AST invariant across module load/save: if any declaration of a function has a resolved exception specification, then all declarations of the function do. We should probably improve the AST representation to make this implicit (perhaps only store the exception specification on the canonical declaration), but this fixes things for now. The testcase for this (which used to assert) also exposes the actual bug I was trying to reduce here: we sometimes fail to emit the body of an imported special member function definition. Fix for that to follow. llvm-svn: 214458 --- clang/lib/Serialization/ASTReaderDecl.cpp | 155 ++++++++++++++++++++++-------- 1 file changed, 113 insertions(+), 42 deletions(-) (limited to 'clang/lib/Serialization') diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index f68284b9dde..d1906e0d005 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -199,9 +199,10 @@ namespace clang { TypeIDForTypeDecl(0), HasPendingBody(false) { } template - static void attachPreviousDeclImpl(Redeclarable *D, Decl *Previous); - static void attachPreviousDeclImpl(...); - static void attachPreviousDecl(Decl *D, Decl *previous); + static void attachPreviousDeclImpl(ASTReader &Reader, + Redeclarable *D, Decl *Previous); + static void attachPreviousDeclImpl(ASTReader &Reader, ...); + static void attachPreviousDecl(ASTReader &Reader, Decl *D, Decl *Previous); template static void attachLatestDeclImpl(Redeclarable *D, Decl *Latest); @@ -2484,22 +2485,68 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { } template -void ASTDeclReader::attachPreviousDeclImpl(Redeclarable *D, +void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader, + Redeclarable *D, Decl *Previous) { D->RedeclLink.setPrevious(cast(Previous)); } -void ASTDeclReader::attachPreviousDeclImpl(...) { +template<> +void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader, + Redeclarable *D, + Decl *Previous) { + FunctionDecl *FD = static_cast(D); + FunctionDecl *PrevFD = cast(Previous); + + FD->RedeclLink.setPrevious(PrevFD); + + // If the previous declaration is an inline function declaration, then this + // declaration is too. + if (PrevFD->IsInline != FD->IsInline) { + // FIXME: [dcl.fct.spec]p4: + // If a function with external linkage is declared inline in one + // translation unit, it shall be declared inline in all translation + // units in which it appears. + // + // Be careful of this case: + // + // module A: + // template struct X { void f(); }; + // template inline void X::f() {} + // + // module B instantiates the declaration of X::f + // module C instantiates the definition of X::f + // + // If module B and C are merged, we do not have a violation of this rule. + FD->IsInline = true; + } + + // If this declaration has an unresolved exception specification but the + // previous declaration had a resolved one, resolve the exception + // specification now. + auto *FPT = FD->getType()->getAs(); + auto *PrevFPT = PrevFD->getType()->getAs(); + if (FPT && PrevFPT && + isUnresolvedExceptionSpec(FPT->getExceptionSpecType()) && + !isUnresolvedExceptionSpec(PrevFPT->getExceptionSpecType())) { + FunctionProtoType::ExtProtoInfo EPI = PrevFPT->getExtProtoInfo(); + FD->setType(Reader.Context.getFunctionType( + FPT->getReturnType(), FPT->getParamTypes(), + FPT->getExtProtoInfo().withExceptionSpec(EPI.ExceptionSpec))); + } +} +void ASTDeclReader::attachPreviousDeclImpl(ASTReader &Reader, ...) { llvm_unreachable("attachPreviousDecl on non-redeclarable declaration"); } -void ASTDeclReader::attachPreviousDecl(Decl *D, Decl *Previous) { +void ASTDeclReader::attachPreviousDecl(ASTReader &Reader, Decl *D, + Decl *Previous) { assert(D && Previous); switch (D->getKind()) { #define ABSTRACT_DECL(TYPE) -#define DECL(TYPE, BASE) \ - case Decl::TYPE: \ - attachPreviousDeclImpl(cast(D), Previous); \ +#define DECL(TYPE, BASE) \ + case Decl::TYPE: \ + attachPreviousDeclImpl(Reader, cast(D), Previous); \ break; #include "clang/AST/DeclNodes.inc" } @@ -2517,32 +2564,6 @@ void ASTDeclReader::attachPreviousDecl(Decl *D, Decl *Previous) { // be too. if (Previous->Used) D->Used = true; - - // If the previous declaration is an inline function declaration, then this - // declaration is too. - if (auto *FD = dyn_cast(D)) { - if (cast(Previous)->IsInline != FD->IsInline) { - // FIXME: [dcl.fct.spec]p4: - // If a function with external linkage is declared inline in one - // translation unit, it shall be declared inline in all translation - // units in which it appears. - // - // Be careful of this case: - // - // module A: - // template struct X { void f(); }; - // template inline void X::f() {} - // - // module B instantiates the declaration of X::f - // module C instantiates the definition of X::f - // - // If module B and C are merged, we do not have a violation of this rule. - // - //if (!FD->IsInline || Previous->getOwningModule()) - // Diag(FD->getLocation(), diag::err_odr_differing_inline); - FD->IsInline = true; - } - } } template @@ -3041,7 +3062,7 @@ void ASTReader::loadPendingDeclChain(serialization::GlobalDeclID ID) { if (Chain[I] == CanonDecl) continue; - ASTDeclReader::attachPreviousDecl(Chain[I], MostRecent); + ASTDeclReader::attachPreviousDecl(*this, Chain[I], MostRecent); MostRecent = Chain[I]; } @@ -3171,6 +3192,46 @@ void ASTReader::loadObjCCategories(serialization::GlobalDeclID ID, ModuleMgr.visit(ObjCCategoriesVisitor::visit, &Visitor); } +namespace { +/// Iterator over the redeclarations of a declaration that have already +/// been merged into the same redeclaration chain. +template +class MergedRedeclIterator { + DeclT *Start, *Canonical, *Current; +public: + MergedRedeclIterator() : Current(nullptr) {} + MergedRedeclIterator(DeclT *Start) + : Start(Start), Canonical(nullptr), Current(Start) {} + + DeclT *operator*() { return Current; } + + MergedRedeclIterator &operator++() { + if (Current->isFirstDecl()) + Canonical = Current; + Current = Current->getPreviousDecl(); + + // If we started in the merged portion, we'll reach our start position + // eventually. Otherwise, we'll never reach it, but the second declaration + // we reached was the canonical declaration, so stop when we see that one + // again. + if (Current == Start || Current == Canonical) + Current = nullptr; + return *this; + } + + friend bool operator!=(const MergedRedeclIterator &A, + const MergedRedeclIterator &B) { + return A.Current != B.Current; + } +}; +} +template +llvm::iterator_range> merged_redecls(DeclT *D) { + return llvm::iterator_range>( + MergedRedeclIterator(D), + MergedRedeclIterator()); +} + void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile, const RecordData &Record) { while (Idx < Record.size()) { @@ -3288,14 +3349,24 @@ void ASTDeclReader::UpdateDecl(Decl *D, ModuleFile &ModuleFile, } case UPD_CXX_RESOLVED_EXCEPTION_SPEC: { - auto *FD = cast(D); - auto *FPT = FD->getType()->castAs(); - SmallVector ExceptionStorage; + // FIXME: This doesn't send the right notifications if there are + // ASTMutationListeners other than an ASTWriter. FunctionProtoType::ExceptionSpecInfo ESI; + SmallVector ExceptionStorage; Reader.readExceptionSpec(ModuleFile, ExceptionStorage, ESI, Record, Idx); - FD->setType(Reader.Context.getFunctionType( - FPT->getReturnType(), FPT->getParamTypes(), - FPT->getExtProtoInfo().withExceptionSpec(ESI))); + for (auto *Redecl : merged_redecls(D)) { + auto *FD = cast(Redecl); + auto *FPT = FD->getType()->castAs(); + if (!isUnresolvedExceptionSpec(FPT->getExceptionSpecType())) { + // AST invariant: if any exception spec in the redecl chain is + // resolved, all are resolved. We don't need to go any further. + // FIXME: If the exception spec is resolved, check that it matches. + break; + } + FD->setType(Reader.Context.getFunctionType( + FPT->getReturnType(), FPT->getParamTypes(), + FPT->getExtProtoInfo().withExceptionSpec(ESI))); + } break; } -- cgit v1.2.3