r336021 - PR33924: merge local declarations that have linkage of some kind within
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 2 08:21:18 PDT 2018
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 at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list