[PATCH] D70190: [clang][modules] Add support for merging lifetime-extended temporaries

2019-11-30 Thread Tyker via Phabricator via cfe-commits
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

2019-11-30 Thread Tyker via Phabricator via cfe-commits
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

2019-11-24 Thread Dávid Bolvanský via Phabricator via cfe-commits
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

2019-11-24 Thread Tyker via Phabricator via cfe-commits
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

2019-11-21 Thread Tyker via Phabricator via cfe-commits
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

2019-11-13 Thread Tyker via Phabricator via cfe-commits
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

2019-11-13 Thread Tyker via Phabricator via cfe-commits
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

2019-11-13 Thread Tyker via Phabricator via cfe-commits
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

2019-11-13 Thread Tyker via Phabricator via cfe-commits
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

2019-11-13 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
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

2019-11-13 Thread Tyker via Phabricator via cfe-commits
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