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
