I've reverted in r336175 in the meantime to green the tests.
On Mon, Jul 2, 2018 at 5:21 PM, Hans Wennborg <h...@chromium.org> wrote: > Hi Richard, > > This introduced test failures on Windows, but for some reason only in > 32-bit builds: https://bugs.llvm.org/show_bug.cgi?id=38015 > > I don't know what's going on yet, just finished the bisection. Maybe > it reproduces in 32-bit linux builds too? > > - Hans > > On Fri, Jun 29, 2018 at 11:58 PM, Richard Smith via cfe-commits > <cfe-commits@lists.llvm.org> wrote: >> Author: rsmith >> Date: Fri Jun 29 14:58:50 2018 >> New Revision: 336021 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=336021&view=rev >> Log: >> PR33924: merge local declarations that have linkage of some kind within >> 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. >> >> Added: >> cfe/trunk/test/Modules/merge-deduced-return.cpp >> cfe/trunk/test/Modules/merge-lambdas.cpp >> cfe/trunk/test/Modules/merge-static-locals.cpp >> Modified: >> cfe/trunk/include/clang/Serialization/ASTReader.h >> cfe/trunk/lib/Serialization/ASTCommon.cpp >> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp >> >> Modified: cfe/trunk/include/clang/Serialization/ASTReader.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=336021&r1=336020&r2=336021&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Serialization/ASTReader.h (original) >> +++ cfe/trunk/include/clang/Serialization/ASTReader.h Fri Jun 29 14:58:50 >> 2018 >> @@ -546,7 +546,7 @@ private: >> >> /// Mergeable declaration contexts that have anonymous declarations >> /// within them, and those anonymous declarations. >> - llvm::DenseMap<DeclContext*, llvm::SmallVector<NamedDecl*, 2>> >> + llvm::DenseMap<Decl*, llvm::SmallVector<NamedDecl*, 2>> >> AnonymousDeclarationsForMerging; >> >> struct FileDeclsInfo { >> >> Modified: cfe/trunk/lib/Serialization/ASTCommon.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTCommon.cpp?rev=336021&r1=336020&r2=336021&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Serialization/ASTCommon.cpp (original) >> +++ cfe/trunk/lib/Serialization/ASTCommon.cpp Fri Jun 29 14:58:50 2018 >> @@ -419,9 +419,21 @@ bool serialization::needsAnonymousDeclar >> return true; >> } >> >> - // Otherwise, we only care about anonymous class members. >> + // 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<VarDecl>(D)) >> + return VD->isStaticLocal(); >> + // FIXME: What about CapturedDecls (and declarations nested within >> them)? >> + return isa<TagDecl>(D) || isa<BlockDecl>(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. >> if (D->getDeclName() || !isa<CXXRecordDecl>(D->getLexicalDeclContext())) >> return false; >> return isa<TagDecl>(D) || isa<FieldDecl>(D); >> } >> - >> >> Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=336021&r1=336020&r2=336021&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original) >> +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Jun 29 14:58:50 2018 >> @@ -87,7 +87,7 @@ namespace clang { >> >> using RecordData = ASTReader::RecordData; >> >> - TypeID TypeIDForTypeDecl = 0; >> + TypeID DeferredTypeID = 0; >> unsigned AnonymousDeclNumber; >> GlobalDeclID NamedDeclForTagDecl = 0; >> IdentifierInfo *TypedefNameForLinkage = nullptr; >> @@ -177,6 +177,8 @@ namespace clang { >> void MergeDefinitionData(ObjCProtocolDecl *D, >> struct ObjCProtocolDecl::DefinitionData >> &&NewDD); >> >> + static DeclContext *getPrimaryDCForAnonymousDecl(DeclContext >> *LexicalDC); >> + >> static NamedDecl *getAnonymousDeclForMerging(ASTReader &Reader, >> DeclContext *DC, >> unsigned Index); >> @@ -528,7 +530,7 @@ void ASTDeclReader::Visit(Decl *D) { >> >> if (auto *TD = dyn_cast<TypeDecl>(D)) { >> // We have a fully initialized TypeDecl. Read its type now. >> - >> TD->setTypeForDecl(Reader.GetType(TypeIDForTypeDecl).getTypePtrOrNull()); >> + TD->setTypeForDecl(Reader.GetType(DeferredTypeID).getTypePtrOrNull()); >> >> // If this is a tag declaration with a typedef name for linkage, it's >> safe >> // to load that typedef now. >> @@ -537,8 +539,11 @@ void ASTDeclReader::Visit(Decl *D) { >> cast<TypedefNameDecl>(Reader.GetDecl(NamedDeclForTagDecl)); >> } else if (auto *ID = dyn_cast<ObjCInterfaceDecl>(D)) { >> // if we have a fully initialized TypeDecl, we can safely read its type >> now. >> - ID->TypeForDecl = Reader.GetType(TypeIDForTypeDecl).getTypePtrOrNull(); >> + ID->TypeForDecl = Reader.GetType(DeferredTypeID).getTypePtrOrNull(); >> } else if (auto *FD = dyn_cast<FunctionDecl>(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). >> @@ -658,7 +663,7 @@ void ASTDeclReader::VisitTypeDecl(TypeDe >> VisitNamedDecl(TD); >> TD->setLocStart(ReadSourceLocation()); >> // Delay type reading until after we have fully initialized the decl. >> - TypeIDForTypeDecl = Record.getGlobalTypeID(Record.readInt()); >> + DeferredTypeID = Record.getGlobalTypeID(Record.readInt()); >> } >> >> ASTDeclReader::RedeclarableResult >> @@ -791,7 +796,13 @@ ASTDeclReader::VisitRecordDeclImpl(Recor >> >> void ASTDeclReader::VisitValueDecl(ValueDecl *VD) { >> VisitNamedDecl(VD); >> - VD->setType(Record.readType()); >> + // 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<FunctionDecl>(VD)) >> + DeferredTypeID = Record.getGlobalTypeID(Record.readInt()); >> + else >> + VD->setType(Record.readType()); >> } >> >> void ASTDeclReader::VisitEnumConstantDecl(EnumConstantDecl *ECD) { >> @@ -820,6 +831,19 @@ void ASTDeclReader::VisitFunctionDecl(Fu >> 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<FunctionType>() >> + ->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(); >> >> @@ -1084,7 +1108,7 @@ void ASTDeclReader::MergeDefinitionData( >> void ASTDeclReader::VisitObjCInterfaceDecl(ObjCInterfaceDecl *ID) { >> RedeclarableResult Redecl = VisitRedeclarable(ID); >> VisitObjCContainerDecl(ID); >> - TypeIDForTypeDecl = Record.getGlobalTypeID(Record.readInt()); >> + DeferredTypeID = Record.getGlobalTypeID(Record.readInt()); >> mergeRedeclarable(ID, Redecl); >> >> ID->TypeParamList = ReadObjCTypeParamList(); >> @@ -1890,7 +1914,7 @@ ASTDeclReader::VisitCXXRecordDeclImpl(CX >> // >> // Beware: we do not yet know our canonical declaration, and may still >> // get merged once the surrounding class template has got off the >> ground. >> - TypeIDForTypeDecl = 0; >> + DeferredTypeID = 0; >> } >> break; >> } >> @@ -2843,8 +2867,12 @@ static bool isSameEntity(NamedDecl *X, N >> return true; >> >> // Must be in the same context. >> - if (!X->getDeclContext()->getRedeclContext()->Equals( >> - Y->getDeclContext()->getRedeclContext())) >> + // >> + // 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<Decl>(X->getDeclContext()->getRedeclContext()), >> + >> cast<Decl>(Y->getDeclContext()->getRedeclContext()))) >> return false; >> >> // Two typedefs refer to the same entity if they have the same underlying >> @@ -2906,18 +2934,21 @@ static bool isSameEntity(NamedDecl *X, N >> } >> >> ASTContext &C = FuncX->getASTContext(); >> - if (!C.hasSameType(FuncX->getType(), FuncY->getType())) { >> + 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)) { >> // 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. >> - // FIXME: Do we need to check for C++14 deduced return types here too? >> - auto *XFPT = FuncX->getType()->getAs<FunctionProtoType>(); >> - auto *YFPT = FuncY->getType()->getAs<FunctionProtoType>(); >> + auto *XFPT = XT->getAs<FunctionProtoType>(); >> + auto *YFPT = YT->getAs<FunctionProtoType>(); >> if (C.getLangOpts().CPlusPlus17 && XFPT && YFPT && >> (isUnresolvedExceptionSpec(XFPT->getExceptionSpecType()) || >> isUnresolvedExceptionSpec(YFPT->getExceptionSpecType())) && >> - C.hasSameFunctionTypeIgnoringExceptionSpec(FuncX->getType(), >> - FuncY->getType())) >> + C.hasSameFunctionTypeIgnoringExceptionSpec(XT, YT)) >> return true; >> return false; >> } >> @@ -3109,23 +3140,50 @@ static NamedDecl *getDeclForMerging(Name >> 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<CXXRecordDecl>(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<Decl>(LexicalDC))) { >> + if (auto *FD = dyn_cast<FunctionDecl>(D)) >> + if (FD->isThisDeclarationADefinition()) >> + return FD; >> + if (auto *MD = dyn_cast<ObjCMethodDecl>(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. >> - if (auto *Merged = Reader.MergedDeclContexts.lookup(DC)) >> - DC = Merged; >> + auto *CanonDC = cast<Decl>(DC)->getCanonicalDecl(); >> >> // If we've seen this before, return the canonical declaration. >> - auto &Previous = Reader.AnonymousDeclarationsForMerging[DC]; >> + auto &Previous = Reader.AnonymousDeclarationsForMerging[CanonDC]; >> 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. >> - if (!cast<Decl>(DC)->isFromASTFile()) { >> - numberAnonymousDeclsWithin(DC, [&](NamedDecl *ND, unsigned Number) { >> + auto *PrimaryDC = getPrimaryDCForAnonymousDecl(DC); >> + if (PrimaryDC && !cast<Decl>(PrimaryDC)->isFromASTFile()) { >> + numberAnonymousDeclsWithin(PrimaryDC, [&](NamedDecl *ND, unsigned >> Number) { >> if (Previous.size() == Number) >> Previous.push_back(cast<NamedDecl>(ND->getCanonicalDecl())); >> else >> @@ -3139,10 +3197,9 @@ NamedDecl *ASTDeclReader::getAnonymousDe >> void ASTDeclReader::setAnonymousDeclForMerging(ASTReader &Reader, >> DeclContext *DC, unsigned >> Index, >> NamedDecl *D) { >> - if (auto *Merged = Reader.MergedDeclContexts.lookup(DC)) >> - DC = Merged; >> + auto *CanonDC = cast<Decl>(DC)->getCanonicalDecl(); >> >> - auto &Previous = Reader.AnonymousDeclarationsForMerging[DC]; >> + auto &Previous = Reader.AnonymousDeclarationsForMerging[CanonDC]; >> if (Index >= Previous.size()) >> Previous.resize(Index + 1); >> if (!Previous[Index]) >> >> Added: cfe/trunk/test/Modules/merge-deduced-return.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-deduced-return.cpp?rev=336021&view=auto >> ============================================================================== >> --- cfe/trunk/test/Modules/merge-deduced-return.cpp (added) >> +++ cfe/trunk/test/Modules/merge-deduced-return.cpp Fri Jun 29 14:58:50 2018 >> @@ -0,0 +1,33 @@ >> +// 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); >> >> Added: cfe/trunk/test/Modules/merge-lambdas.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-lambdas.cpp?rev=336021&view=auto >> ============================================================================== >> --- cfe/trunk/test/Modules/merge-lambdas.cpp (added) >> +++ cfe/trunk/test/Modules/merge-lambdas.cpp Fri Jun 29 14:58:50 2018 >> @@ -0,0 +1,48 @@ >> +// 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<typename T> 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<int>(); } >> +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<int>(); } >> +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); >> >> Added: cfe/trunk/test/Modules/merge-static-locals.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-static-locals.cpp?rev=336021&view=auto >> ============================================================================== >> --- cfe/trunk/test/Modules/merge-static-locals.cpp (added) >> +++ cfe/trunk/test/Modules/merge-static-locals.cpp Fri Jun 29 14:58:50 2018 >> @@ -0,0 +1,27 @@ >> +// 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<int*> 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<int*> 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; >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
