r336021 - PR33924: merge local declarations that have linkage of some kind within
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 3 00:57:12 PDT 2018
I've reverted in r336175 in the meantime to green the tests.
On Mon, Jul 2, 2018 at 5:21 PM, Hans Wennborg <hans at 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 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