[PATCH] D26753: ASTImporter: improve support for C++ templates

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Main revisions: https://reviews.llvm.org/rL292776, 
https://reviews.llvm.org/rL292778. Sorry for not mentioning them in 
Differential Revision.


Repository:
  rL LLVM

https://reviews.llvm.org/D26753



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26753: ASTImporter: improve support for C++ templates

[Aleksei Sidorin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL292779: ASTImporter: quick test fix (authored by a.sidorin).

Changed prior to commit:
  https://reviews.llvm.org/D26753?vs=79054=85332#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D26753

Files:
  cfe/trunk/test/ASTMerge/class-template-partial-spec/test.cpp


Index: cfe/trunk/test/ASTMerge/class-template-partial-spec/test.cpp
===
--- cfe/trunk/test/ASTMerge/class-template-partial-spec/test.cpp
+++ cfe/trunk/test/ASTMerge/class-template-partial-spec/test.cpp
@@ -9,17 +9,17 @@
 static_assert(sizeof(Dst::Z0Dst.member) == sizeof(double));
 static_assert(sizeof(One::Child1::member) == sizeof(double));
 
-// CHECK: 
llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:21:32:
 error: external variable 'X1' declared with incompatible types in different 
translation units ('TwoOptionTemplate' vs. 'TwoOptionTemplate')
-// CHECK: 
llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:21:31:
 note: declared here with type 'TwoOptionTemplate'
+// CHECK: Inputs/class-template-partial-spec2.cpp:21:32: error: external 
variable 'X1' declared with incompatible types in different translation units 
('TwoOptionTemplate' vs. 'TwoOptionTemplate')
+// CHECK: Inputs/class-template-partial-spec1.cpp:21:31: note: declared here 
with type 'TwoOptionTemplate'
 
-// CHECK: 
llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:24:29:
 error: external variable 'X4' declared with incompatible types in different 
translation units ('TwoOptionTemplate' vs. 'TwoOptionTemplate')
-// CHECK: 
llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:24:33:
 note: declared here with type 'TwoOptionTemplate'
+// CHECK: Inputs/class-template-partial-spec2.cpp:24:29: error: external 
variable 'X4' declared with incompatible types in different translation units 
('TwoOptionTemplate' vs. 'TwoOptionTemplate')
+// CHECK: Inputs/class-template-partial-spec1.cpp:24:33: note: declared here 
with type 'TwoOptionTemplate'
 
-// CHECK: 
llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:38:8:
 warning: type 'IntTemplateSpec<5, void *>' has incompatible definitions in 
different translation units
-// CHECK: 
llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:39:7:
 note: field 'member' has type 'int' here
-// CHECK: 
llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:39:10:
 note: field 'member' has type 'double' here
+// CHECK: Inputs/class-template-partial-spec1.cpp:38:8: warning: type 
'IntTemplateSpec<5, void *>' has incompatible definitions in different 
translation units
+// CHECK: Inputs/class-template-partial-spec1.cpp:39:7: note: field 'member' 
has type 'int' here
+// CHECK: Inputs/class-template-partial-spec2.cpp:39:10: note: field 'member' 
has type 'double' here
 
-// CHECK: 
llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:52:25:
 error: external variable 'Y3' declared with incompatible types in different 
translation units ('IntTemplateSpec<2, int>' vs. 'IntTemplateSpec<3, int>')
-// CHECK: 
llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:52:25:
 note: declared here with type 'IntTemplateSpec<3, int>'
+// CHECK: Inputs/class-template-partial-spec2.cpp:52:25: error: external 
variable 'Y3' declared with incompatible types in different translation units 
('IntTemplateSpec<2, int>' vs. 'IntTemplateSpec<3, int>')
+// CHECK: Inputs/class-template-partial-spec1.cpp:52:25: note: declared here 
with type 'IntTemplateSpec<3, int>'
 
 // CHECK-NOT: static_assert


Index: cfe/trunk/test/ASTMerge/class-template-partial-spec/test.cpp
===
--- cfe/trunk/test/ASTMerge/class-template-partial-spec/test.cpp
+++ cfe/trunk/test/ASTMerge/class-template-partial-spec/test.cpp
@@ -9,17 +9,17 @@
 static_assert(sizeof(Dst::Z0Dst.member) == sizeof(double));
 static_assert(sizeof(One::Child1::member) == sizeof(double));
 
-// CHECK: llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:21:32: error: external variable 'X1' declared with incompatible types in different translation units ('TwoOptionTemplate' vs. 'TwoOptionTemplate')
-// CHECK: llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:21:31: 

[PATCH] D26753: ASTImporter: improve support for C++ templates

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

I got it. I have hard-coded paths in CHECK-lines so these tests are passed on 
my machine but not on other. Thank you Kareem!


https://reviews.llvm.org/D26753



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26753: ASTImporter: improve support for C++ templates

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Kareem, I have re-checked it and I cannot see the failure. But I'm not going to 
commit it until NY holidays end (and, anyway, I will not commit it if somebody 
has failing tests). Could you describe your configuration?


https://reviews.llvm.org/D26753



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26753: ASTImporter: improve support for C++ templates

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

I didn't notice this failure but I'll recheck this. Thank you Kareem!


https://reviews.llvm.org/D26753



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26753: ASTImporter: improve support for C++ templates

[Kareem Khazem via Phabricator via cfe-commits]

khazem added a comment.

Sorry for the late comment, but one of the tests that this introduces is 
breaking on my end:

  
/usr/local/google/home/khazem/doc/llvm/tools/clang/test/ASTMerge/class-template-partial-spec.cpp:9:11:
 error: expected string not found in input
  // CHECK: 
/media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/Inputs/class-template-partial-spec2.cpp:21:32:
 error: external variable 'X1' declared with incompatible types in different 
translation units ('TwoOptionTemplate' vs. 'TwoOptionTemplate')
^
  :1:1: note: scanning from here
  
/usr/local/google/home/khazem/doc/llvm/tools/clang/test/ASTMerge/Inputs/class-template-partial-spec1.cpp:20:30:
 error: external variable 'X0' defined in multiple translation units
  ^
  :7:15: note: possible intended match here
  
/usr/local/google/home/khazem/doc/llvm/tools/clang/test/ASTMerge/Inputs/class-template-partial-spec2.cpp:21:32:
 error: external variable 'X1' declared with incompatible types in different 
translation units ('TwoOptionTemplate' vs. 'TwoOptionTemplate')
^

Is this expected?


https://reviews.llvm.org/D26753



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26753: ASTImporter: improve support for C++ templates

[Sean Callanan via Phabricator via cfe-commits]

spyffe accepted this revision.
spyffe added a comment.
This revision is now accepted and ready to land.

Yeah, that test looks great!  Thanks!




Comment at: lib/AST/ASTImporter.cpp:496
+return false;
+  if (DN1->isIdentifier())
+return IsStructurallyEquivalent(DN1->getIdentifier(),

spyffe wrote:
> We should probably also check whether `DN1->isIdentifier() == 
> DN2->isIdentifier()`.
Looking at my comment with fresh post Thanksgiving eyes, that would be totally 
wrong.  The `IsStructurallyEquivalent` is fine.



Comment at: 
test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:67
+template
+struct Child1: public Two::Three::Parent {
+  char member;

ooh, nice!


https://reviews.llvm.org/D26753



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26753: ASTImporter: improve support for C++ templates

[Aleksei Sidorin via cfe-commits]

a.sidorin updated this revision to Diff 79054.
a.sidorin added a comment.

Add a simple test for import of complex `NestedNameSpecifierLoc`s.


https://reviews.llvm.org/D26753

Files:
  include/clang/AST/TemplateBase.h
  lib/AST/ASTImporter.cpp
  
test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp
  
test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp
  test/ASTMerge/class-template-partial-spec/test.cpp

Index: test/ASTMerge/class-template-partial-spec/test.cpp
===
--- /dev/null
+++ test/ASTMerge/class-template-partial-spec/test.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -emit-pch -std=c++1z -o %t.1.ast %S/Inputs/class-template-partial-spec1.cpp
+// RUN: %clang_cc1 -emit-pch -std=c++1z -o %t.2.ast %S/Inputs/class-template-partial-spec2.cpp
+// RUN: not %clang_cc1 -std=c++1z -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only %s 2>&1 | FileCheck %s
+
+static_assert(sizeof(**SingleSource.member) == sizeof(**SingleDest.member));
+static_assert(sizeof(SecondDoubleSource.member) == sizeof(SecondDoubleDest.member));
+static_assert(NumberSource.val == 42);
+static_assert(sizeof(Z0Source.member) == sizeof(char));
+static_assert(sizeof(Dst::Z0Dst.member) == sizeof(double));
+static_assert(sizeof(One::Child1::member) == sizeof(double));
+
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:21:32: error: external variable 'X1' declared with incompatible types in different translation units ('TwoOptionTemplate' vs. 'TwoOptionTemplate')
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:21:31: note: declared here with type 'TwoOptionTemplate'
+
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:24:29: error: external variable 'X4' declared with incompatible types in different translation units ('TwoOptionTemplate' vs. 'TwoOptionTemplate')
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:24:33: note: declared here with type 'TwoOptionTemplate'
+
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:38:8: warning: type 'IntTemplateSpec<5, void *>' has incompatible definitions in different translation units
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:39:7: note: field 'member' has type 'int' here
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:39:10: note: field 'member' has type 'double' here
+
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:52:25: error: external variable 'Y3' declared with incompatible types in different translation units ('IntTemplateSpec<2, int>' vs. 'IntTemplateSpec<3, int>')
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:52:25: note: declared here with type 'IntTemplateSpec<3, int>'
+
+// CHECK-NOT: static_assert
Index: test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp
===
--- /dev/null
+++ test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp
@@ -0,0 +1,79 @@
+template
+struct TwoOptionTemplate {};
+
+template
+struct TwoOptionTemplate {
+  int member;
+};
+
+
+template
+struct TwoOptionTemplate {
+  float member;
+};
+
+template
+struct TwoOptionTemplate {
+  T** member;
+};
+
+TwoOptionTemplate X0;
+TwoOptionTemplate X1;
+TwoOptionTemplate X2;
+TwoOptionTemplate X3;
+TwoOptionTemplate X4;
+TwoOptionTemplate SingleDest;
+TwoOptionTemplate SecondDoubleDest;
+
+
+template
+struct IntTemplateSpec {};
+
+template
+struct IntTemplateSpec<4, C> {
+  C member;
+};
+
+template
+struct IntTemplateSpec {
+  double member;
+  static constexpr int val = I;
+};
+
+template
+struct IntTemplateSpec {
+  char member;
+  static constexpr int val = I;
+};
+
+IntTemplateSpec<4, wchar_t>Y0;
+IntTemplateSpec<5, void *> Y1;
+IntTemplateSpec<1, int> Y2;
+IntTemplateSpec<2, int> Y3;
+IntTemplateSpec<43, double> NumberDest;
+
+namespace One {
+namespace Two {
+namespace Three {
+
+template
+class Parent {};
+
+} // namespace Three
+
+} // namespace Two

[PATCH] D26753: ASTImporter: improve support for C++ templates

[Aleksei Sidorin via cfe-commits]

a.sidorin added a comment.

Thank you! I'll update this review again when I have a test for 
NestedNameSpecifierLocs.




Comment at: lib/AST/ASTImporter.cpp:458
+  }
+  return true;
+}

spyffe wrote:
> Is this really an appropriate default result?  I would argue for `false` here 
> so that an error would propagate, as is typical in ASTImporter.
> Note that this does disagree with the original source's `true` but I think 
> that was because we knew we didn't handle anything, whereas now the 
> assumption is we handle everything.
Good point. Default `false` will also fail import if a new non-handled property 
or option will be added in AST and clearly indicate an error so I will change 
it.


https://reviews.llvm.org/D26753



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26753: ASTImporter: improve support for C++ templates

[Aleksei Sidorin via cfe-commits]

a.sidorin updated this revision to Diff 78382.
a.sidorin added a comment.

Address review comments; fix tests.


https://reviews.llvm.org/D26753

Files:
  include/clang/AST/TemplateBase.h
  lib/AST/ASTImporter.cpp
  
test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp
  
test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp
  test/ASTMerge/class-template-partial-spec/test.cpp

Index: test/ASTMerge/class-template-partial-spec/test.cpp
===
--- /dev/null
+++ test/ASTMerge/class-template-partial-spec/test.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -emit-pch -std=c++1z -o %t.1.ast %S/Inputs/class-template-partial-spec1.cpp
+// RUN: %clang_cc1 -emit-pch -std=c++1z -o %t.2.ast %S/Inputs/class-template-partial-spec2.cpp
+// RUN: not %clang_cc1 -std=c++1z -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only %s 2>&1 | FileCheck %s
+
+static_assert(sizeof(**SingleSource.member) == sizeof(**SingleDest.member));
+static_assert(sizeof(SecondDoubleSource.member) == sizeof(SecondDoubleDest.member));
+static_assert(NumberSource.val == 42);
+static_assert(sizeof(Z0Source.member) == sizeof(char));
+static_assert(sizeof(Dst::Z0Dst.member) == sizeof(double));
+static_assert(sizeof(One::Child1::member) == sizeof(double));
+
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:21:32: error: external variable 'X1' declared with incompatible types in different translation units ('TwoOptionTemplate' vs. 'TwoOptionTemplate')
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:21:31: note: declared here with type 'TwoOptionTemplate'
+
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:24:29: error: external variable 'X4' declared with incompatible types in different translation units ('TwoOptionTemplate' vs. 'TwoOptionTemplate')
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:24:33: note: declared here with type 'TwoOptionTemplate'
+
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:38:8: warning: type 'IntTemplateSpec<5, void *>' has incompatible definitions in different translation units
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:39:7: note: field 'member' has type 'int' here
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:39:10: note: field 'member' has type 'double' here
+
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:52:25: error: external variable 'Y3' declared with incompatible types in different translation units ('IntTemplateSpec<2, int>' vs. 'IntTemplateSpec<3, int>')
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec1.cpp:52:25: note: declared here with type 'IntTemplateSpec<3, int>'
+
+// CHECK-NOT: static_assert
Index: test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp
===
--- /dev/null
+++ test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp
@@ -0,0 +1,79 @@
+template
+struct TwoOptionTemplate {};
+
+template
+struct TwoOptionTemplate {
+  int member;
+};
+
+
+template
+struct TwoOptionTemplate {
+  float member;
+};
+
+template
+struct TwoOptionTemplate {
+  T** member;
+};
+
+TwoOptionTemplate X0;
+TwoOptionTemplate X1;
+TwoOptionTemplate X2;
+TwoOptionTemplate X3;
+TwoOptionTemplate X4;
+TwoOptionTemplate SingleDest;
+TwoOptionTemplate SecondDoubleDest;
+
+
+template
+struct IntTemplateSpec {};
+
+template
+struct IntTemplateSpec<4, C> {
+  C member;
+};
+
+template
+struct IntTemplateSpec {
+  double member;
+  static constexpr int val = I;
+};
+
+template
+struct IntTemplateSpec {
+  char member;
+  static constexpr int val = I;
+};
+
+IntTemplateSpec<4, wchar_t>Y0;
+IntTemplateSpec<5, void *> Y1;
+IntTemplateSpec<1, int> Y2;
+IntTemplateSpec<2, int> Y3;
+IntTemplateSpec<43, double> NumberDest;
+
+namespace One {
+namespace Two {
+namespace Three {
+
+template
+class Parent {};
+
+} // namespace Three
+
+} // namespace Two
+
+template
+struct Child1: 

[PATCH] D26753: ASTImporter: improve support for C++ templates

[Sean Callanan via cfe-commits]

spyffe requested changes to this revision.
spyffe added a comment.
This revision now requires changes to proceed.

This looks amazing.  I have a few minor quibbles and a testing concern, but 
overall this looks like a great step forward!  Thank you!




Comment at: lib/AST/ASTImporter.cpp:458
+  }
+  return true;
+}

Is this really an appropriate default result?  I would argue for `false` here 
so that an error would propagate, as is typical in ASTImporter.
Note that this does disagree with the original source's `true` but I think that 
was because we knew we didn't handle anything, whereas now the assumption is we 
handle everything.



Comment at: lib/AST/ASTImporter.cpp:496
+return false;
+  if (DN1->isIdentifier())
+return IsStructurallyEquivalent(DN1->getIdentifier(),

We should probably also check whether `DN1->isIdentifier() == 
DN2->isIdentifier()`.



Comment at: lib/AST/ASTImporter.cpp:520
+  }
   return true;
 }

As above, I'd argue for `false` here now that we're flipping to the assumption 
that this code is complete.



Comment at: lib/AST/ASTImporter.cpp:4911
+
+if (D->getTypeAsWritten()) {
+  TypeSourceInfo *TInfo = Importer.Import(D->getTypeAsWritten());

Could you assign this to a variable here, to avoid the redundant call a line 
below?



Comment at: lib/AST/ASTImporter.cpp:4931
 LexicalDC->addDeclInternal(D2);
+
   }

This blank line is probably not needed.



Comment at: lib/AST/ASTImporter.cpp:7035
 
 NestedNameSpecifierLoc ASTImporter::Import(NestedNameSpecifierLoc FromNNS) {
+  // Copied from NestedNameSpecifier mostly.

Is this function properly covered by the test?  I would like to see some 
deeply-neded name specifiers in the test, with entries for all the cases here.
If I'm missing the part of the test that covers this, please let me know.


https://reviews.llvm.org/D26753



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26753: ASTImporter: improve support for C++ templates

[Kareem Khazem via cfe-commits]

khazem added a comment.

Thanks very much for this patch! It certainly fixes the infinite recursion 
issue on our codebase. It LGTM, but I'd like to add a test case before landing 
it.


https://reviews.llvm.org/D26753



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26753: ASTImporter: improve support for C++ templates

[Aleksei Sidorin via cfe-commits]

a.sidorin created this revision.
a.sidorin added reviewers: spyffe, khazem.
a.sidorin added a subscriber: cfe-commits.

- Support template partial specialization
- Avoid infinite recursion in IsStructurallyEquivalent for TemplateArgument 
with implementing IsStructurallyEquivalent for TemplateName


https://reviews.llvm.org/D26753

Files:
  include/clang/AST/TemplateBase.h
  lib/AST/ASTImporter.cpp
  test/ASTMerge/Inputs/class-template-partial-spec1.cpp
  test/ASTMerge/Inputs/class-template-partial-spec2.cpp
  test/ASTMerge/class-template-partial-spec.cpp

Index: test/ASTMerge/class-template-partial-spec.cpp
===
--- /dev/null
+++ test/ASTMerge/class-template-partial-spec.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -emit-pch -std=c++1z -o %t.1.ast %S/Inputs/class-template-partial-spec1.cpp
+// RUN: %clang_cc1 -emit-pch -std=c++1z -o %t.2.ast %S/Inputs/class-template-partial-spec2.cpp
+// RUN: not %clang_cc1 -std=c++1z -ast-merge %t.1.ast -ast-merge %t.2.ast -fsyntax-only %s 2>&1 | FileCheck %s
+
+static_assert(sizeof(**SingleSource.member) == sizeof(**SingleDest.member));
+static_assert(sizeof(SecondDoubleSource.member) == sizeof(SecondDoubleDest.member));
+static_assert(NumberSource.val == NumberDest.val);
+
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/Inputs/class-template-partial-spec2.cpp:21:32: error: external variable 'X1' declared with incompatible types in different translation units ('TwoOptionTemplate' vs. 'TwoOptionTemplate')
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/Inputs/class-template-partial-spec1.cpp:21:31: note: declared here with type 'TwoOptionTemplate'
+
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/Inputs/class-template-partial-spec2.cpp:24:29: error: external variable 'X4' declared with incompatible types in different translation units ('TwoOptionTemplate' vs. 'TwoOptionTemplate')
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/Inputs/class-template-partial-spec1.cpp:24:33: note: declared here with type 'TwoOptionTemplate'
+
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/Inputs/class-template-partial-spec1.cpp:38:8: warning: type 'IntTemplateSpec<5, void *>' has incompatible definitions in different translation units
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/Inputs/class-template-partial-spec1.cpp:39:7: note: field 'member' has type 'int' here
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/Inputs/class-template-partial-spec2.cpp:39:10: note: field 'member' has type 'double' here
+
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/Inputs/class-template-partial-spec2.cpp:47:25: error: external variable 'Y3' declared with incompatible types in different translation units ('IntTemplateSpec<2, int>' vs. 'IntTemplateSpec<3, int>')
+// CHECK: /media/build/smrc-llvm/master/llvm/tools/clang/test/ASTMerge/Inputs/class-template-partial-spec1.cpp:47:25: note: declared here with type 'IntTemplateSpec<3, int>'
+
Index: test/ASTMerge/Inputs/class-template-partial-spec2.cpp
===
--- /dev/null
+++ test/ASTMerge/Inputs/class-template-partial-spec2.cpp
@@ -0,0 +1,48 @@
+template
+struct TwoOptionTemplate {};
+
+template
+struct TwoOptionTemplate {
+  int member;
+};
+
+
+template
+struct TwoOptionTemplate {
+  float member;
+};
+
+template
+struct TwoOptionTemplate {
+  T** member;
+};
+
+TwoOptionTemplate X0;
+TwoOptionTemplate X1;
+TwoOptionTemplate X2;
+TwoOptionTemplate X3;
+TwoOptionTemplate X4;
+TwoOptionTemplate SingleDest;
+TwoOptionTemplate SecondDoubleDest;
+
+
+template
+struct IntTemplateSpec {};
+
+template
+struct IntTemplateSpec<4, C> {
+  C member;
+};
+
+template
+struct IntTemplateSpec {
+  double member;
+  const int val = I;
+};
+
+
+IntTemplateSpec<4, wchar_t> Y0;
+IntTemplateSpec<5, void *> Y1;
+IntTemplateSpec<1, int> Y2;
+IntTemplateSpec<2, int> Y3;
+IntTemplateSpec<42, void *> NumberDest;
Index: test/ASTMerge/Inputs/class-template-partial-spec1.cpp
===
--- /dev/null
+++ test/ASTMerge/Inputs/class-template-partial-spec1.cpp
@@ -0,0 +1,48 @@
+template
+struct TwoOptionTemplate {};
+
+template
+struct TwoOptionTemplate {
+  int member;
+};
+
+
+template
+struct TwoOptionTemplate {
+  float member;
+};
+
+template
+struct TwoOptionTemplate {
+  T** member;
+};
+
+TwoOptionTemplate X0;
+TwoOptionTemplate X1;
+TwoOptionTemplate X2;
+TwoOptionTemplate X3;
+TwoOptionTemplate X4;
+TwoOptionTemplate SingleSource;