From d3ea49f094bb01646e8e122af4babb2840d108b7 Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Tue, 3 Jul 2018 07:51:41 +0000 Subject: [PATCH] Revert r336021 "PR33924: merge local declarations that have linkage of some kind within" This caused test failures in 32-bit builds (PR38015). > merged function definitions; also merge functions with deduced return > types. > > This seems like two independent fixes, but unfortunately they are hard > to separate because it's challenging to reliably test either one of them > without also testing the other. > > A complication arises with deduced return type support: we need the type > of the function in order to know how to merge it, but we can't load the > actual type of the function because it might reference an entity > declared within the function (and we need to have already merged the > function to correctly merge that entity, which we would need to do to > determine if the function types match). So we instead compare the > declared function type when merging functions, and defer loading the > actual type of a function with a deduced type until we've finished > loading and merging the function. llvm-svn: 336175 --- clang/include/clang/Serialization/ASTReader.h | 2 +- clang/lib/Serialization/ASTCommon.cpp | 16 +-- clang/lib/Serialization/ASTReaderDecl.cpp | 103 ++++-------------- clang/test/Modules/merge-deduced-return.cpp | 33 ------ clang/test/Modules/merge-lambdas.cpp | 48 -------- clang/test/Modules/merge-static-locals.cpp | 27 ----- 6 files changed, 26 insertions(+), 203 deletions(-) delete mode 100644 clang/test/Modules/merge-deduced-return.cpp delete mode 100644 clang/test/Modules/merge-lambdas.cpp delete mode 100644 clang/test/Modules/merge-static-locals.cpp diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index b33f31776530..bf9ed38a15c7 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -546,7 +546,7 @@ private: /// Mergeable declaration contexts that have anonymous declarations /// within them, and those anonymous declarations. - llvm::DenseMap> + llvm::DenseMap> AnonymousDeclarationsForMerging; struct FileDeclsInfo { diff --git a/clang/lib/Serialization/ASTCommon.cpp b/clang/lib/Serialization/ASTCommon.cpp index da482717f450..fcebe5982ffb 100644 --- a/clang/lib/Serialization/ASTCommon.cpp +++ b/clang/lib/Serialization/ASTCommon.cpp @@ -419,21 +419,9 @@ bool serialization::needsAnonymousDeclarationNumber(const NamedDecl *D) { return true; } - // At block scope, we number everything that we need to deduplicate, since we - // can't just use name matching to keep things lined up. - // FIXME: This is only necessary for an inline function or a template or - // similar. - if (D->getLexicalDeclContext()->isFunctionOrMethod()) { - if (auto *VD = dyn_cast(D)) - return VD->isStaticLocal(); - // FIXME: What about CapturedDecls (and declarations nested within them)? - return isa(D) || isa(D); - } - - // Otherwise, we only care about anonymous class members / block-scope decls. - // FIXME: We need to handle lambdas and blocks within inline / templated - // variables too. + // Otherwise, we only care about anonymous class members. if (D->getDeclName() || !isa(D->getLexicalDeclContext())) return false; return isa(D) || isa(D); } + diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 8701d80653b0..b0c7bbec208f 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -87,7 +87,7 @@ namespace clang { using RecordData = ASTReader::RecordData; - TypeID DeferredTypeID = 0; + TypeID TypeIDForTypeDecl = 0; unsigned AnonymousDeclNumber; GlobalDeclID NamedDeclForTagDecl = 0; IdentifierInfo *TypedefNameForLinkage = nullptr; @@ -177,8 +177,6 @@ namespace clang { void MergeDefinitionData(ObjCProtocolDecl *D, struct ObjCProtocolDecl::DefinitionData &&NewDD); - static DeclContext *getPrimaryDCForAnonymousDecl(DeclContext *LexicalDC); - static NamedDecl *getAnonymousDeclForMerging(ASTReader &Reader, DeclContext *DC, unsigned Index); @@ -530,7 +528,7 @@ void ASTDeclReader::Visit(Decl *D) { if (auto *TD = dyn_cast(D)) { // We have a fully initialized TypeDecl. Read its type now. - TD->setTypeForDecl(Reader.GetType(DeferredTypeID).getTypePtrOrNull()); + TD->setTypeForDecl(Reader.GetType(TypeIDForTypeDecl).getTypePtrOrNull()); // If this is a tag declaration with a typedef name for linkage, it's safe // to load that typedef now. @@ -539,11 +537,8 @@ void ASTDeclReader::Visit(Decl *D) { cast(Reader.GetDecl(NamedDeclForTagDecl)); } else if (auto *ID = dyn_cast(D)) { // if we have a fully initialized TypeDecl, we can safely read its type now. - ID->TypeForDecl = Reader.GetType(DeferredTypeID).getTypePtrOrNull(); + ID->TypeForDecl = Reader.GetType(TypeIDForTypeDecl).getTypePtrOrNull(); } else if (auto *FD = dyn_cast(D)) { - if (DeferredTypeID) - FD->setType(Reader.GetType(DeferredTypeID)); - // FunctionDecl's body was written last after all other Stmts/Exprs. // We only read it if FD doesn't already have a body (e.g., from another // module). @@ -663,7 +658,7 @@ void ASTDeclReader::VisitTypeDecl(TypeDecl *TD) { VisitNamedDecl(TD); TD->setLocStart(ReadSourceLocation()); // Delay type reading until after we have fully initialized the decl. - DeferredTypeID = Record.getGlobalTypeID(Record.readInt()); + TypeIDForTypeDecl = Record.getGlobalTypeID(Record.readInt()); } ASTDeclReader::RedeclarableResult @@ -796,13 +791,7 @@ ASTDeclReader::VisitRecordDeclImpl(RecordDecl *RD) { void ASTDeclReader::VisitValueDecl(ValueDecl *VD) { VisitNamedDecl(VD); - // For function declarations, defer reading the type in case the function has - // a deduced return type that references an entity declared within the - // function. - if (isa(VD)) - DeferredTypeID = Record.getGlobalTypeID(Record.readInt()); - else - VD->setType(Record.readType()); + VD->setType(Record.readType()); } void ASTDeclReader::VisitEnumConstantDecl(EnumConstantDecl *ECD) { @@ -831,19 +820,6 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) { RedeclarableResult Redecl = VisitRedeclarable(FD); VisitDeclaratorDecl(FD); - // Attach a type to this function. Use the real type if possible, but fall - // back to the type as written if it involves a deduced return type. - if (FD->getTypeSourceInfo() && - FD->getTypeSourceInfo()->getType()->castAs() - ->getReturnType()->getContainedAutoType()) { - // We'll set up the real type in Visit, once we've finished loading the - // function. - FD->setType(FD->getTypeSourceInfo()->getType()); - } else { - FD->setType(Reader.GetType(DeferredTypeID)); - DeferredTypeID = 0; - } - ReadDeclarationNameLoc(FD->DNLoc, FD->getDeclName()); FD->IdentifierNamespace = Record.readInt(); @@ -1108,7 +1084,7 @@ void ASTDeclReader::MergeDefinitionData(ObjCInterfaceDecl *D, void ASTDeclReader::VisitObjCInterfaceDecl(ObjCInterfaceDecl *ID) { RedeclarableResult Redecl = VisitRedeclarable(ID); VisitObjCContainerDecl(ID); - DeferredTypeID = Record.getGlobalTypeID(Record.readInt()); + TypeIDForTypeDecl = Record.getGlobalTypeID(Record.readInt()); mergeRedeclarable(ID, Redecl); ID->TypeParamList = ReadObjCTypeParamList(); @@ -1914,7 +1890,7 @@ ASTDeclReader::VisitCXXRecordDeclImpl(CXXRecordDecl *D) { // // Beware: we do not yet know our canonical declaration, and may still // get merged once the surrounding class template has got off the ground. - DeferredTypeID = 0; + TypeIDForTypeDecl = 0; } break; } @@ -2867,12 +2843,8 @@ static bool isSameEntity(NamedDecl *X, NamedDecl *Y) { return true; // Must be in the same context. - // - // Note that we can't use DeclContext::Equals here, because the DeclContexts - // could be two different declarations of the same function. (We will fix the - // semantic DC to refer to the primary definition after merging.) - if (!declaresSameEntity(cast(X->getDeclContext()->getRedeclContext()), - cast(Y->getDeclContext()->getRedeclContext()))) + if (!X->getDeclContext()->getRedeclContext()->Equals( + Y->getDeclContext()->getRedeclContext())) return false; // Two typedefs refer to the same entity if they have the same underlying @@ -2934,21 +2906,18 @@ static bool isSameEntity(NamedDecl *X, NamedDecl *Y) { } ASTContext &C = FuncX->getASTContext(); - auto GetTypeAsWritten = [](const FunctionDecl *FD) { - return FD->getTypeSourceInfo() ? FD->getTypeSourceInfo()->getType() - : FD->getType(); - }; - QualType XT = GetTypeAsWritten(FuncX), YT = GetTypeAsWritten(FuncY); - if (!C.hasSameType(XT, YT)) { + if (!C.hasSameType(FuncX->getType(), FuncY->getType())) { // We can get functions with different types on the redecl chain in C++17 // if they have differing exception specifications and at least one of // the excpetion specs is unresolved. - auto *XFPT = XT->getAs(); - auto *YFPT = YT->getAs(); + // FIXME: Do we need to check for C++14 deduced return types here too? + auto *XFPT = FuncX->getType()->getAs(); + auto *YFPT = FuncY->getType()->getAs(); if (C.getLangOpts().CPlusPlus17 && XFPT && YFPT && (isUnresolvedExceptionSpec(XFPT->getExceptionSpecType()) || isUnresolvedExceptionSpec(YFPT->getExceptionSpecType())) && - C.hasSameFunctionTypeIgnoringExceptionSpec(XT, YT)) + C.hasSameFunctionTypeIgnoringExceptionSpec(FuncX->getType(), + FuncY->getType())) return true; return false; } @@ -3140,50 +3109,23 @@ static NamedDecl *getDeclForMerging(NamedDecl *Found, return nullptr; } -/// Find the declaration to use to populate the anonymous declaration table -/// for the given lexical DeclContext. We only care about finding local -/// definitions of the context; we'll merge imported ones as we go. -DeclContext * -ASTDeclReader::getPrimaryDCForAnonymousDecl(DeclContext *LexicalDC) { - // For classes, we track the definition as we merge. - if (auto *RD = dyn_cast(LexicalDC)) { - auto *DD = RD->getCanonicalDecl()->DefinitionData; - return DD ? DD->Definition : nullptr; - } - - // For anything else, walk its merged redeclarations looking for a definition. - // Note that we can't just call getDefinition here because the redeclaration - // chain isn't wired up. - for (auto *D : merged_redecls(cast(LexicalDC))) { - if (auto *FD = dyn_cast(D)) - if (FD->isThisDeclarationADefinition()) - return FD; - if (auto *MD = dyn_cast(D)) - if (MD->isThisDeclarationADefinition()) - return MD; - } - - // No merged definition yet. - return nullptr; -} - NamedDecl *ASTDeclReader::getAnonymousDeclForMerging(ASTReader &Reader, DeclContext *DC, unsigned Index) { // If the lexical context has been merged, look into the now-canonical // definition. - auto *CanonDC = cast(DC)->getCanonicalDecl(); + if (auto *Merged = Reader.MergedDeclContexts.lookup(DC)) + DC = Merged; // If we've seen this before, return the canonical declaration. - auto &Previous = Reader.AnonymousDeclarationsForMerging[CanonDC]; + auto &Previous = Reader.AnonymousDeclarationsForMerging[DC]; if (Index < Previous.size() && Previous[Index]) return Previous[Index]; // If this is the first time, but we have parsed a declaration of the context, // build the anonymous declaration list from the parsed declaration. - auto *PrimaryDC = getPrimaryDCForAnonymousDecl(DC); - if (PrimaryDC && !cast(PrimaryDC)->isFromASTFile()) { - numberAnonymousDeclsWithin(PrimaryDC, [&](NamedDecl *ND, unsigned Number) { + if (!cast(DC)->isFromASTFile()) { + numberAnonymousDeclsWithin(DC, [&](NamedDecl *ND, unsigned Number) { if (Previous.size() == Number) Previous.push_back(cast(ND->getCanonicalDecl())); else @@ -3197,9 +3139,10 @@ NamedDecl *ASTDeclReader::getAnonymousDeclForMerging(ASTReader &Reader, void ASTDeclReader::setAnonymousDeclForMerging(ASTReader &Reader, DeclContext *DC, unsigned Index, NamedDecl *D) { - auto *CanonDC = cast(DC)->getCanonicalDecl(); + if (auto *Merged = Reader.MergedDeclContexts.lookup(DC)) + DC = Merged; - auto &Previous = Reader.AnonymousDeclarationsForMerging[CanonDC]; + auto &Previous = Reader.AnonymousDeclarationsForMerging[DC]; if (Index >= Previous.size()) Previous.resize(Index + 1); if (!Previous[Index]) diff --git a/clang/test/Modules/merge-deduced-return.cpp b/clang/test/Modules/merge-deduced-return.cpp deleted file mode 100644 index 0a4de7b97554..000000000000 --- a/clang/test/Modules/merge-deduced-return.cpp +++ /dev/null @@ -1,33 +0,0 @@ -// RUN: %clang_cc1 -fmodules -std=c++17 -verify %s -// RUN: %clang_cc1 -fmodules -std=c++17 -verify %s -DLOCAL -// expected-no-diagnostics - -#pragma clang module build A -module A {} -#pragma clang module contents -#pragma clang module begin A -inline auto f() { struct X {}; return X(); } -inline auto a = f(); -#pragma clang module end -#pragma clang module endbuild - -#pragma clang module build B -module B {} -#pragma clang module contents -#pragma clang module begin B -inline auto f() { struct X {}; return X(); } -inline auto b = f(); -#pragma clang module end -#pragma clang module endbuild - -#ifdef LOCAL -inline auto f() { struct X {}; return X(); } -inline auto b = f(); -#else -#pragma clang module import B -#endif - -#pragma clang module import A - -using T = decltype(a); -using T = decltype(b); diff --git a/clang/test/Modules/merge-lambdas.cpp b/clang/test/Modules/merge-lambdas.cpp deleted file mode 100644 index d14483aa3aa7..000000000000 --- a/clang/test/Modules/merge-lambdas.cpp +++ /dev/null @@ -1,48 +0,0 @@ -// RUN: %clang_cc1 -fmodules -verify %s -// expected-no-diagnostics - -#pragma clang module build A -module A {} -#pragma clang module contents -#pragma clang module begin A -template auto f() { return []{}; } -#pragma clang module end -#pragma clang module endbuild - -#pragma clang module build B -module B {} -#pragma clang module contents -#pragma clang module begin B -#pragma clang module import A -inline auto x1() { return f(); } -inline auto z() { return []{}; } -inline auto x2() { return z(); } -#pragma clang module end -#pragma clang module endbuild - -#pragma clang module build C -module C {} -#pragma clang module contents -#pragma clang module begin C -#pragma clang module import A -inline auto y1() { return f(); } -inline auto z() { return []{}; } -inline auto y2() { return z(); } -inline auto q() { return []{}; } -inline auto y3() { return q(); } -#pragma clang module end -#pragma clang module endbuild - -inline auto q() { return []{}; } -inline auto x3() { return q(); } - -#pragma clang module import B -#pragma clang module import C -using T = decltype(x1); -using T = decltype(y1); - -using U = decltype(x2); -using U = decltype(y2); - -using V = decltype(x3); -using V = decltype(y3); diff --git a/clang/test/Modules/merge-static-locals.cpp b/clang/test/Modules/merge-static-locals.cpp deleted file mode 100644 index 37ae22ee38a3..000000000000 --- a/clang/test/Modules/merge-static-locals.cpp +++ /dev/null @@ -1,27 +0,0 @@ -// RUN: %clang_cc1 -std=c++17 -fmodules -verify %s -// expected-no-diagnostics - -#pragma clang module build A -module A {} -#pragma clang module contents -#pragma clang module begin A -template struct X {}; -auto get() { static int n; return X<&n>(); } -using A = decltype(get()); -#pragma clang module end -#pragma clang module endbuild - -#pragma clang module build B -module B {} -#pragma clang module contents -#pragma clang module begin B -template struct X {}; -auto get() { static int n; return X<&n>(); } -using B = decltype(get()); -#pragma clang module end -#pragma clang module endbuild - -#pragma clang module import A -#pragma clang module import B -using T = A; -using T = B;