[PATCH] D70190: [clang][modules] Add support for merging lifetime-extended temporaries
This revision was automatically updated to reflect the committed changes. Closed by commit rG3c7f6b439699: [clang][modules] Add support for merging lifetime-extended temporaries (authored by Tyker). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70190/new/ https://reviews.llvm.org/D70190 Files: clang/include/clang/AST/DeclCXX.h clang/include/clang/AST/TextNodeDumper.h clang/include/clang/Serialization/ASTReader.h clang/lib/AST/TextNodeDumper.cpp clang/lib/Serialization/ASTReaderDecl.cpp clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap clang/test/Modules/merge-lifetime-extended-temporary.cpp Index: clang/test/Modules/merge-lifetime-extended-temporary.cpp === --- /dev/null +++ clang/test/Modules/merge-lifetime-extended-temporary.cpp @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I%S/Inputs/merge-lifetime-extended-temporary -verify -std=c++11 %s -DORDER=1 +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I%S/Inputs/merge-lifetime-extended-temporary -verify -std=c++11 %s -DORDER=2 + +// expected-no-diagnostics +#if ORDER == 1 +#include "c.h" +#include "b.h" +#else +#include "b.h" +#include "c.h" +#endif + +static_assert(PtrTemp1 == , ""); +static_assert(PtrTemp1 == PtrTemp2, ""); Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap @@ -0,0 +1,14 @@ +module "a" { + export * + header "a.h" +} + +module "b" { + export * + header "b.h" +} + +module "c" { + export * + header "c.h" +} Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h @@ -0,0 +1,4 @@ + +#include "a.h" + +constexpr const int* PtrTemp2 = Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h @@ -0,0 +1,4 @@ + +#include "a.h" + +constexpr const int* PtrTemp1 = Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h @@ -0,0 +1,2 @@ + +constexpr const int& LETemp = 0; Index: clang/lib/Serialization/ASTReaderDecl.cpp === --- clang/lib/Serialization/ASTReaderDecl.cpp +++ clang/lib/Serialization/ASTReaderDecl.cpp @@ -424,6 +424,9 @@ template void mergeMergeable(Mergeable *D); +template <> +void mergeMergeable(Mergeable *D); + void mergeTemplatePattern(RedeclarableTemplateDecl *D, RedeclarableTemplateDecl *Existing, DeclID DsID, bool IsKeyDecl); @@ -2358,6 +2361,7 @@ if (Record.readInt()) D->Value = new (D->getASTContext()) APValue(Record.readAPValue()); D->ManglingNumber = Record.readInt(); + mergeMergeable(D); } std::pair @@ -2555,6 +2559,28 @@ return false; } +/// Attempts to merge LifetimeExtendedTemporaryDecl with +/// identical class definitions from two different modules. +template<> +void ASTDeclReader::mergeMergeable( +Mergeable *D) { + // If modules are not available, there is no reason to perform this merge. + if (!Reader.getContext().getLangOpts().Modules) +return; + + LifetimeExtendedTemporaryDecl *LETDecl = + static_cast(D); + + LifetimeExtendedTemporaryDecl * = + Reader.LETemporaryForMerging[std::make_pair( + LETDecl->getExtendingDecl(), LETDecl->getManglingNumber())]; + if (LookupResult) +Reader.getContext().setPrimaryMergedDecl(LETDecl, + LookupResult->getCanonicalDecl()); + else +LookupResult = LETDecl; +} + /// Attempts to merge the given declaration (D) with another declaration /// of the same entity, for the case where the entity is not actually /// redeclarable. This happens, for instance, when merging the fields of Index: clang/lib/AST/TextNodeDumper.cpp === --- clang/lib/AST/TextNodeDumper.cpp +++ clang/lib/AST/TextNodeDumper.cpp @@ -1338,6 +1338,17 @@ OS << " <>>"; } +void TextNodeDumper::VisitLifetimeExtendedTemporaryDecl( +const
[PATCH] D70190: [clang][modules] Add support for merging lifetime-extended temporaries
Tyker updated this revision to Diff 231589. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70190/new/ https://reviews.llvm.org/D70190 Files: clang/include/clang/AST/DeclCXX.h clang/include/clang/AST/TextNodeDumper.h clang/include/clang/Serialization/ASTReader.h clang/lib/AST/TextNodeDumper.cpp clang/lib/Serialization/ASTReaderDecl.cpp clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap clang/test/Modules/merge-lifetime-extended-temporary.cpp Index: clang/test/Modules/merge-lifetime-extended-temporary.cpp === --- /dev/null +++ clang/test/Modules/merge-lifetime-extended-temporary.cpp @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I%S/Inputs/merge-lifetime-extended-temporary -verify -std=c++11 %s -DORDER=1 +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I%S/Inputs/merge-lifetime-extended-temporary -verify -std=c++11 %s -DORDER=2 + +// expected-no-diagnostics +#if ORDER == 1 +#include "c.h" +#include "b.h" +#else +#include "b.h" +#include "c.h" +#endif + +static_assert(PtrTemp1 == , ""); +static_assert(PtrTemp1 == PtrTemp2, ""); Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap @@ -0,0 +1,14 @@ +module "a" { + export * + header "a.h" +} + +module "b" { + export * + header "b.h" +} + +module "c" { + export * + header "c.h" +} Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h @@ -0,0 +1,4 @@ + +#include "a.h" + +constexpr const int* PtrTemp2 = Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h @@ -0,0 +1,4 @@ + +#include "a.h" + +constexpr const int* PtrTemp1 = Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h @@ -0,0 +1,2 @@ + +constexpr const int& LETemp = 0; Index: clang/lib/Serialization/ASTReaderDecl.cpp === --- clang/lib/Serialization/ASTReaderDecl.cpp +++ clang/lib/Serialization/ASTReaderDecl.cpp @@ -424,6 +424,9 @@ template void mergeMergeable(Mergeable *D); +template <> +void mergeMergeable(Mergeable *D); + void mergeTemplatePattern(RedeclarableTemplateDecl *D, RedeclarableTemplateDecl *Existing, DeclID DsID, bool IsKeyDecl); @@ -2358,6 +2361,7 @@ if (Record.readInt()) D->Value = new (D->getASTContext()) APValue(Record.readAPValue()); D->ManglingNumber = Record.readInt(); + mergeMergeable(D); } std::pair @@ -2555,6 +2559,28 @@ return false; } +/// Attempts to merge LifetimeExtendedTemporaryDecl with +/// identical class definitions from two different modules. +template<> +void ASTDeclReader::mergeMergeable( +Mergeable *D) { + // If modules are not available, there is no reason to perform this merge. + if (!Reader.getContext().getLangOpts().Modules) +return; + + LifetimeExtendedTemporaryDecl *LETDecl = + static_cast(D); + + LifetimeExtendedTemporaryDecl * = + Reader.LETemporaryForMerging[std::make_pair( + LETDecl->getExtendingDecl(), LETDecl->getManglingNumber())]; + if (LookupResult) +Reader.getContext().setPrimaryMergedDecl(LETDecl, + LookupResult->getCanonicalDecl()); + else +LookupResult = LETDecl; +} + /// Attempts to merge the given declaration (D) with another declaration /// of the same entity, for the case where the entity is not actually /// redeclarable. This happens, for instance, when merging the fields of Index: clang/lib/AST/TextNodeDumper.cpp === --- clang/lib/AST/TextNodeDumper.cpp +++ clang/lib/AST/TextNodeDumper.cpp @@ -1338,6 +1338,17 @@ OS << " <>>"; } +void TextNodeDumper::VisitLifetimeExtendedTemporaryDecl( +const LifetimeExtendedTemporaryDecl *D) { + OS << " extended by "; + dumpBareDeclRef(D->getExtendingDecl()); + OS << " mangling "; + { +ColorScope Color(OS, ShowColors, ValueColor); +OS << D->getManglingNumber(); +
[PATCH] D70190: [clang][modules] Add support for merging lifetime-extended temporaries
xbolva00 added inline comments. Comment at: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap:15 +} \ No newline at end of file Add newline here. Dtto in other tests above. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70190/new/ https://reviews.llvm.org/D70190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70190: [clang][modules] Add support for merging lifetime-extended temporaries
Tyker updated this revision to Diff 230790. Tyker added a comment. rebased added new lines. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70190/new/ https://reviews.llvm.org/D70190 Files: clang/include/clang/AST/DeclCXX.h clang/include/clang/AST/TextNodeDumper.h clang/include/clang/Serialization/ASTReader.h clang/lib/AST/TextNodeDumper.cpp clang/lib/Serialization/ASTReaderDecl.cpp clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap clang/test/Modules/merge-lifetime-extended-temporary.cpp Index: clang/test/Modules/merge-lifetime-extended-temporary.cpp === --- /dev/null +++ clang/test/Modules/merge-lifetime-extended-temporary.cpp @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I%S/Inputs/merge-lifetime-extended-temporary -verify -std=c++11 %s -DORDER=1 +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I%S/Inputs/merge-lifetime-extended-temporary -verify -std=c++11 %s -DORDER=2 + +// expected-no-diagnostics +#if ORDER == 1 +#include "c.h" +#include "b.h" +#else +#include "b.h" +#include "c.h" +#endif + +static_assert(PtrTemp1 == , ""); +static_assert(PtrTemp1 == PtrTemp2, ""); Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap @@ -0,0 +1,14 @@ +module "a" { + export * + header "a.h" +} + +module "b" { + export * + header "b.h" +} + +module "c" { + export * + header "c.h" +} Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h @@ -0,0 +1,4 @@ + +#include "a.h" + +constexpr const int* PtrTemp2 = Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h @@ -0,0 +1,4 @@ + +#include "a.h" + +constexpr const int* PtrTemp1 = Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h @@ -0,0 +1,2 @@ + +constexpr const int& LETemp = 0; Index: clang/lib/Serialization/ASTReaderDecl.cpp === --- clang/lib/Serialization/ASTReaderDecl.cpp +++ clang/lib/Serialization/ASTReaderDecl.cpp @@ -424,6 +424,9 @@ template void mergeMergeable(Mergeable *D); +template <> +void mergeMergeable(Mergeable *D); + void mergeTemplatePattern(RedeclarableTemplateDecl *D, RedeclarableTemplateDecl *Existing, DeclID DsID, bool IsKeyDecl); @@ -2358,6 +2361,7 @@ if (Record.readInt()) D->Value = new (D->getASTContext()) APValue(Record.readAPValue()); D->ManglingNumber = Record.readInt(); + mergeMergeable(D); } std::pair @@ -2555,6 +2559,28 @@ return false; } +/// Attempts to merge LifetimeExtendedTemporaryDecl with +/// identical class definitions from two different modules. +template<> +void ASTDeclReader::mergeMergeable( +Mergeable *D) { + // If modules are not available, there is no reason to perform this merge. + if (!Reader.getContext().getLangOpts().Modules) +return; + + LifetimeExtendedTemporaryDecl *LETDecl = + static_cast(D); + + LifetimeExtendedTemporaryDecl * = + Reader.LETemporaryForMerging[std::make_pair( + LETDecl->getExtendingDecl(), LETDecl->getManglingNumber())]; + if (LookupResult) +Reader.getContext().setPrimaryMergedDecl(LETDecl, + LookupResult->getCanonicalDecl()); + else +LookupResult = LETDecl; +} + /// Attempts to merge the given declaration (D) with another declaration /// of the same entity, for the case where the entity is not actually /// redeclarable. This happens, for instance, when merging the fields of Index: clang/lib/AST/TextNodeDumper.cpp === --- clang/lib/AST/TextNodeDumper.cpp +++ clang/lib/AST/TextNodeDumper.cpp @@ -1338,6 +1338,17 @@ OS << " <>>"; } +void TextNodeDumper::VisitLifetimeExtendedTemporaryDecl( +const LifetimeExtendedTemporaryDecl *D) { + OS << " extended by "; + dumpBareDeclRef(D->getExtendingDecl()); + OS << " mangling "; + { +ColorScope Color(OS, ShowColors,
[PATCH] D70190: [clang][modules] Add support for merging lifetime-extended temporaries
Tyker added a comment. @rsmith ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70190/new/ https://reviews.llvm.org/D70190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70190: [clang][modules] Add support for merging lifetime-extended temporaries
Tyker updated this revision to Diff 229176. Tyker added a comment. fixed comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70190/new/ https://reviews.llvm.org/D70190 Files: clang/include/clang/AST/DeclCXX.h clang/include/clang/AST/TextNodeDumper.h clang/include/clang/Serialization/ASTReader.h clang/lib/AST/TextNodeDumper.cpp clang/lib/Serialization/ASTReaderDecl.cpp clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap clang/test/Modules/merge-lifetime-extended-temporary.cpp Index: clang/test/Modules/merge-lifetime-extended-temporary.cpp === --- /dev/null +++ clang/test/Modules/merge-lifetime-extended-temporary.cpp @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I%S/Inputs/merge-lifetime-extended-temporary -verify -std=c++11 %s -DORDER=1 +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I%S/Inputs/merge-lifetime-extended-temporary -verify -std=c++11 %s -DORDER=2 + +// expected-no-diagnostics +#if ORDER == 1 +#include "c.h" +#include "b.h" +#else +#include "b.h" +#include "c.h" +#endif + +static_assert(PtrTemp1 == , ""); +static_assert(PtrTemp1 == PtrTemp2, ""); Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap @@ -0,0 +1,14 @@ +module "a" { + export * + header "a.h" +} + +module "b" { + export * + header "b.h" +} + +module "c" { + export * + header "c.h" +} \ No newline at end of file Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h @@ -0,0 +1,4 @@ + +#include "a.h" + +constexpr const int* PtrTemp2 = \ No newline at end of file Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h @@ -0,0 +1,4 @@ + +#include "a.h" + +constexpr const int* PtrTemp1 = \ No newline at end of file Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h @@ -0,0 +1,2 @@ + +constexpr const int& LETemp = 0; Index: clang/lib/Serialization/ASTReaderDecl.cpp === --- clang/lib/Serialization/ASTReaderDecl.cpp +++ clang/lib/Serialization/ASTReaderDecl.cpp @@ -424,6 +424,9 @@ template void mergeMergeable(Mergeable *D); +template <> +void mergeMergeable(Mergeable *D); + void mergeTemplatePattern(RedeclarableTemplateDecl *D, RedeclarableTemplateDecl *Existing, DeclID DsID, bool IsKeyDecl); @@ -2355,6 +2358,7 @@ if (Record.readInt()) D->Value = new (D->getASTContext()) APValue(Record.readAPValue()); D->ManglingNumber = Record.readInt(); + mergeMergeable(D); } std::pair @@ -2552,6 +2556,28 @@ return false; } +/// Attempts to merge LifetimeExtendedTemporaryDecl with +/// identical class definitions from two different modules. +template<> +void ASTDeclReader::mergeMergeable( +Mergeable *D) { + // If modules are not available, there is no reason to perform this merge. + if (!Reader.getContext().getLangOpts().Modules) +return; + + LifetimeExtendedTemporaryDecl *LETDecl = + static_cast(D); + + LifetimeExtendedTemporaryDecl * = + Reader.LETemporaryForMerging[std::make_pair( + LETDecl->getExtendingDecl(), LETDecl->getManglingNumber())]; + if (LookupResult) +Reader.getContext().setPrimaryMergedDecl(LETDecl, + LookupResult->getCanonicalDecl()); + else +LookupResult = LETDecl; +} + /// Attempts to merge the given declaration (D) with another declaration /// of the same entity, for the case where the entity is not actually /// redeclarable. This happens, for instance, when merging the fields of Index: clang/lib/AST/TextNodeDumper.cpp === --- clang/lib/AST/TextNodeDumper.cpp +++ clang/lib/AST/TextNodeDumper.cpp @@ -1338,6 +1338,17 @@ OS << " <>>"; } +void TextNodeDumper::VisitLifetimeExtendedTemporaryDecl( +const LifetimeExtendedTemporaryDecl *D) { + OS << " extended by "; +
[PATCH] D70190: [clang][modules] Add support for merging lifetime-extended temporaries
Tyker marked an inline comment as done. Tyker added inline comments. Comment at: clang/lib/AST/TextNodeDumper.cpp:1349-1350 + } + OS << " subexpr"; + dumpPointer(D); +} Tyker wrote: > rsmith wrote: > > We shouldn't need this: the address of the declaration is dumped anyway by > > the infrastructure. (If you meant to dump the subexpression, I don't think > > that's what this does.) > > > > Traversing from the `LifetimeExtendedTemporaryDecl` to its subexpression > > for dumping purposes should be done by `ASTNodeTraverser` (in > > `include/clang/AST/ASTNodeTraverser.h`). > I needed it during debugging and I thought i could be useful to others. but > yes it is unreachable from -ast-dump > > > If you meant to dump the subexpression, I don't think that's what this does. > it dumps the value of the pointer which can be used to know which > MaterializedTemporaryExpr it is associated with. ill add the traversal in the previous patch and remove subexpr here because it is better CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70190/new/ https://reviews.llvm.org/D70190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70190: [clang][modules] Add support for merging lifetime-extended temporaries
Tyker added inline comments. Comment at: clang/lib/AST/TextNodeDumper.cpp:1349-1350 + } + OS << " subexpr"; + dumpPointer(D); +} rsmith wrote: > We shouldn't need this: the address of the declaration is dumped anyway by > the infrastructure. (If you meant to dump the subexpression, I don't think > that's what this does.) > > Traversing from the `LifetimeExtendedTemporaryDecl` to its subexpression for > dumping purposes should be done by `ASTNodeTraverser` (in > `include/clang/AST/ASTNodeTraverser.h`). I needed it during debugging and I thought i could be useful to others. but yes it is unreachable from -ast-dump > If you meant to dump the subexpression, I don't think that's what this does. it dumps the value of the pointer which can be used to know which MaterializedTemporaryExpr it is associated with. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70190/new/ https://reviews.llvm.org/D70190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70190: [clang][modules] Add support for merging lifetime-extended temporaries
Tyker updated this revision to Diff 229156. Tyker marked 6 inline comments as done. Tyker added a comment. fixed most comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70190/new/ https://reviews.llvm.org/D70190 Files: clang/include/clang/AST/DeclCXX.h clang/include/clang/AST/TextNodeDumper.h clang/include/clang/Serialization/ASTReader.h clang/lib/AST/TextNodeDumper.cpp clang/lib/Serialization/ASTReaderDecl.cpp clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap clang/test/Modules/merge-lifetime-extended-temporary.cpp Index: clang/test/Modules/merge-lifetime-extended-temporary.cpp === --- /dev/null +++ clang/test/Modules/merge-lifetime-extended-temporary.cpp @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I%S/Inputs/merge-lifetime-extended-temporary -verify -std=c++11 %s -DORDER=1 +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I%S/Inputs/merge-lifetime-extended-temporary -verify -std=c++11 %s -DORDER=2 + +// expected-no-diagnostics +#if ORDER == 1 +#include "c.h" +#include "b.h" +#else +#include "b.h" +#include "c.h" +#endif + +static_assert(PtrTemp1 == , ""); +static_assert(PtrTemp1 == PtrTemp2, ""); Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap @@ -0,0 +1,14 @@ +module "a" { + export * + header "a.h" +} + +module "b" { + export * + header "b.h" +} + +module "c" { + export * + header "c.h" +} \ No newline at end of file Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h @@ -0,0 +1,4 @@ + +#include "a.h" + +constexpr const int* PtrTemp2 = \ No newline at end of file Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h @@ -0,0 +1,4 @@ + +#include "a.h" + +constexpr const int* PtrTemp1 = \ No newline at end of file Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h @@ -0,0 +1,2 @@ + +constexpr const int& LETemp = 0; Index: clang/lib/Serialization/ASTReaderDecl.cpp === --- clang/lib/Serialization/ASTReaderDecl.cpp +++ clang/lib/Serialization/ASTReaderDecl.cpp @@ -424,6 +424,9 @@ template void mergeMergeable(Mergeable *D); +template <> +void mergeMergeable(Mergeable *D); + void mergeTemplatePattern(RedeclarableTemplateDecl *D, RedeclarableTemplateDecl *Existing, DeclID DsID, bool IsKeyDecl); @@ -2355,6 +2358,7 @@ if (Record.readInt()) D->Value = new (D->getASTContext()) APValue(Record.readAPValue()); D->ManglingNumber = Record.readInt(); + mergeMergeable(D); } std::pair @@ -2552,6 +2556,28 @@ return false; } +/// Attempts to merge LifetimeExtendedTemporaryDecl with +/// identical class definitions from two different modules. +template<> +void ASTDeclReader::mergeMergeable( +Mergeable *D) { + // If modules are not available, there is no reason to perform this merge. + if (!Reader.getContext().getLangOpts().Modules) +return; + + LifetimeExtendedTemporaryDecl *LETDecl = + static_cast(D); + + LifetimeExtendedTemporaryDecl * = + Reader.LETemporaryForMerging[std::make_pair( + LETDecl->getExtendingDecl(), LETDecl->getManglingNumber())]; + if (LookupResult) +Reader.getContext().setPrimaryMergedDecl(LETDecl, + LookupResult->getCanonicalDecl()); + else +LookupResult = LETDecl; +} + /// Attempts to merge the given declaration (D) with another declaration /// of the same entity, for the case where the entity is not actually /// redeclarable. This happens, for instance, when merging the fields of Index: clang/lib/AST/TextNodeDumper.cpp === --- clang/lib/AST/TextNodeDumper.cpp +++ clang/lib/AST/TextNodeDumper.cpp @@ -1338,6 +1338,19 @@ OS << " <>>"; } +void TextNodeDumper::VisitLifetimeExtendedTemporaryDecl( +const LifetimeExtendedTemporaryDecl *D) { + OS << "
[PATCH] D70190: [clang][modules] Add support for merging lifetime-extended temporaries
rsmith added a comment. Functionally, this looks good, thanks. Comment at: clang/include/clang/Serialization/ASTReader.h:555 + /// Key used to identify LifetimeExtendedTemporaryDecl for merging. + /// containg the lifetime-extending declaration and the mangling number. + using LETemporaryKey = std::pair; Typos: "[...] merging. containg the [...]" -> "[...] merging, containing the [...]" Comment at: clang/lib/AST/TextNodeDumper.cpp:1349-1350 + } + OS << " subexpr"; + dumpPointer(D); +} We shouldn't need this: the address of the declaration is dumped anyway by the infrastructure. (If you meant to dump the subexpression, I don't think that's what this does.) Traversing from the `LifetimeExtendedTemporaryDecl` to its subexpression for dumping purposes should be done by `ASTNodeTraverser` (in `include/clang/AST/ASTNodeTraverser.h`). Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:2566 + Decl* RealDecl = static_cast(D); + `*` on the right, please. Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:2576 + + if (auto *LETDecl = dyn_cast(RealDecl)) { +LETDecl->dump(); This case is essentially entirely different from the primary implementation of `mergeMergeable`. Consider adding an overload or explicit specialization for the `LifetimeExtendedTemporaryDecl` case instead; we don't need the branch on the `dyn_cast` result below, and we don't need the `allowODRLikeMergeInC` check above (because lifetime-extended temporaries only exist in C++). Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:2577 + if (auto *LETDecl = dyn_cast(RealDecl)) { +LETDecl->dump(); +auto LookupResult = We shouldn't be dumping from here :) Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:2579 +auto LookupResult = +Reader.LETemporaryForMerging.FindAndConstruct(std::make_pair( +LETDecl->getExtendingDecl(), LETDecl->getManglingNumber())); Prefer the normal container interface here -- `insert` or `operator[]` -- rather than the nonstandard extension `FindAndConstruct`. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70190/new/ https://reviews.llvm.org/D70190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70190: [clang][modules] Add support for merging lifetime-extended temporaries
Tyker created this revision. Tyker added a reviewer: rsmith. Tyker added a project: clang. Herald added a subscriber: cfe-commits. Tyker added a parent revision: D69360: [NFC] Refactor representation of materialized temporaries. Add support for merging lifetime-extended temporaries Repository: rC Clang https://reviews.llvm.org/D70190 Files: clang/include/clang/AST/DeclCXX.h clang/include/clang/AST/TextNodeDumper.h clang/include/clang/Serialization/ASTReader.h clang/lib/AST/TextNodeDumper.cpp clang/lib/Serialization/ASTReaderDecl.cpp clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap clang/test/Modules/merge-lifetime-extended-temporary.cpp Index: clang/test/Modules/merge-lifetime-extended-temporary.cpp === --- /dev/null +++ clang/test/Modules/merge-lifetime-extended-temporary.cpp @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I%S/Inputs/merge-lifetime-extended-temporary -verify -std=c++11 %s -DORDER=1 +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I%S/Inputs/merge-lifetime-extended-temporary -verify -std=c++11 %s -DORDER=2 + +// expected-no-diagnostics +#if ORDER == 1 +#include "c.h" +#include "b.h" +#else +#include "b.h" +#include "c.h" +#endif + +static_assert(PtrTemp1 == , ""); +static_assert(PtrTemp1 == PtrTemp2, ""); Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/module.modulemap @@ -0,0 +1,14 @@ +module "a" { + export * + header "a.h" +} + +module "b" { + export * + header "b.h" +} + +module "c" { + export * + header "c.h" +} \ No newline at end of file Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/c.h @@ -0,0 +1,4 @@ + +#include "a.h" + +constexpr const int* PtrTemp2 = \ No newline at end of file Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/b.h @@ -0,0 +1,4 @@ + +#include "a.h" + +constexpr const int* PtrTemp1 = \ No newline at end of file Index: clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h === --- /dev/null +++ clang/test/Modules/Inputs/merge-lifetime-extended-temporary/a.h @@ -0,0 +1,2 @@ + +constexpr const int& LETemp = 0; Index: clang/lib/Serialization/ASTReaderDecl.cpp === --- clang/lib/Serialization/ASTReaderDecl.cpp +++ clang/lib/Serialization/ASTReaderDecl.cpp @@ -2355,6 +2355,7 @@ if (Record.readInt()) D->Value = new (D->getASTContext()) APValue(Record.readAPValue()); D->ManglingNumber = Record.readInt(); + mergeMergeable(D); } std::pair @@ -2562,17 +2563,32 @@ if (!Reader.getContext().getLangOpts().Modules) return; + Decl* RealDecl = static_cast(D); + // ODR-based merging is performed in C++ and in some cases (tag types) in C. // Note that C identically-named things in different translation units are // not redeclarations, but may still have compatible types, where ODR-like // semantics may apply. if (!Reader.getContext().getLangOpts().CPlusPlus && - !allowODRLikeMergeInC(dyn_cast(static_cast(D + !allowODRLikeMergeInC(dyn_cast(RealDecl))) +return; + + if (auto *LETDecl = dyn_cast(RealDecl)) { +LETDecl->dump(); +auto LookupResult = +Reader.LETemporaryForMerging.FindAndConstruct(std::make_pair( +LETDecl->getExtendingDecl(), LETDecl->getManglingNumber())); +if (LookupResult.getSecond()) + Reader.getContext().setPrimaryMergedDecl( + LETDecl, LookupResult.getSecond()->getCanonicalDecl()); +else + LookupResult.getSecond() = LETDecl; return; + } - if (FindExistingResult ExistingRes = findExisting(static_cast(D))) + if (FindExistingResult ExistingRes = findExisting(cast(RealDecl))) if (T *Existing = ExistingRes) - Reader.getContext().setPrimaryMergedDecl(static_cast(D), + Reader.getContext().setPrimaryMergedDecl(RealDecl, Existing->getCanonicalDecl()); } Index: clang/lib/AST/TextNodeDumper.cpp === --- clang/lib/AST/TextNodeDumper.cpp +++ clang/lib/AST/TextNodeDumper.cpp @@ -1338,6