[PATCH] D64241: [ASTImporter] Fix inequivalence of ClassTemplateInstantiations

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366818: [ASTImporter] Fix inequivalence of 
ClassTemplateInstantiations (authored by martong, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64241?vs=210581=211311#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64241/new/

https://reviews.llvm.org/D64241

Files:
  cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
  cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp

Index: cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
===
--- cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
+++ cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp
@@ -235,12 +235,21 @@
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext ,
  const TemplateName ,
  const TemplateName ) {
-  if (N1.getKind() != N2.getKind())
+  TemplateDecl *TemplateDeclN1 = N1.getAsTemplateDecl();
+  TemplateDecl *TemplateDeclN2 = N2.getAsTemplateDecl();
+  if (TemplateDeclN1 && TemplateDeclN2) {
+if (!IsStructurallyEquivalent(Context, TemplateDeclN1, TemplateDeclN2))
+  return false;
+// If the kind is different we compare only the template decl.
+if (N1.getKind() != N2.getKind())
+  return true;
+  } else if (TemplateDeclN1 || TemplateDeclN2)
 return false;
+  else if (N1.getKind() != N2.getKind())
+return false;
+
+  // Check for special case incompatibilities.
   switch (N1.getKind()) {
-  case TemplateName::Template:
-return IsStructurallyEquivalent(Context, N1.getAsTemplateDecl(),
-N2.getAsTemplateDecl());
 
   case TemplateName::OverloadedTemplate: {
 OverloadedTemplateStorage *OS1 = N1.getAsOverloadedTemplate(),
@@ -259,14 +268,6 @@
 return TN1->getDeclName() == TN2->getDeclName();
   }
 
-  case TemplateName::QualifiedTemplate: {
-QualifiedTemplateName *QN1 = N1.getAsQualifiedTemplateName(),
-  *QN2 = N2.getAsQualifiedTemplateName();
-return IsStructurallyEquivalent(Context, QN1->getDecl(), QN2->getDecl()) &&
-   IsStructurallyEquivalent(Context, QN1->getQualifier(),
-QN2->getQualifier());
-  }
-
   case TemplateName::DependentTemplate: {
 DependentTemplateName *DN1 = N1.getAsDependentTemplateName(),
   *DN2 = N2.getAsDependentTemplateName();
@@ -281,15 +282,6 @@
 return false;
   }
 
-  case TemplateName::SubstTemplateTemplateParm: {
-SubstTemplateTemplateParmStorage *TS1 = N1.getAsSubstTemplateTemplateParm(),
- *TS2 = N2.getAsSubstTemplateTemplateParm();
-return IsStructurallyEquivalent(Context, TS1->getParameter(),
-TS2->getParameter()) &&
-   IsStructurallyEquivalent(Context, TS1->getReplacement(),
-TS2->getReplacement());
-  }
-
   case TemplateName::SubstTemplateTemplateParmPack: {
 SubstTemplateTemplateParmPackStorage
 *P1 = N1.getAsSubstTemplateTemplateParmPack(),
@@ -299,8 +291,16 @@
IsStructurallyEquivalent(Context, P1->getParameterPack(),
 P2->getParameterPack());
   }
+
+   case TemplateName::Template:
+   case TemplateName::QualifiedTemplate:
+   case TemplateName::SubstTemplateTemplateParm:
+ // It is sufficient to check value of getAsTemplateDecl.
+ break;
+
   }
-  return false;
+
+  return true;
 }
 
 /// Determine whether two template arguments are equivalent.
Index: cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp
===
--- cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp
+++ cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp
@@ -944,6 +944,67 @@
   EXPECT_FALSE(testStructuralMatch(First, Second));
 }
 
+TEST_F(StructuralEquivalenceTemplateTest,
+   TemplateVsSubstTemplateTemplateParmInArgEq) {
+  auto t = makeDecls(
+  R"(
+template  class Arg { };
+template  class P1> class Primary { };
+
+void f() {
+  // Make specialization with simple template.
+  Primary  A;
+}
+  )",
+  R"(
+template  class Arg { };
+template  class P1> class Primary { };
+
+template  class P1> class Templ {
+  void f() {
+// Make specialization with substituted template template param.
+Primary  A;
+  };
+};
+
+// Instantiate with substitution Arg into P1.
+template class Templ ;
+  )",
+  Lang_CXX, classTemplateSpecializationDecl(hasName("Primary")));
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceTemplateTest,
+   TemplateVsSubstTemplateTemplateParmInArgNotEq) {
+  auto t = makeDecls(
+  R"(
+template  class Arg { };
+template  class P1> class Primary { };
+
+void f() {
+  // Make 

[PATCH] D64241: [ASTImporter] Fix inequivalence of ClassTemplateInstantiations

[Aleksei Sidorin via Phabricator via cfe-commits]

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

Looks good! Sorry for the delay :(


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64241/new/

https://reviews.llvm.org/D64241



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

[PATCH] D64241: [ASTImporter] Fix inequivalence of ClassTemplateInstantiations

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

@a_sidorin This is a polite Ping.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64241/new/

https://reviews.llvm.org/D64241



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

[PATCH] D64241: [ASTImporter] Fix inequivalence of ClassTemplateInstantiations

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D64241#1584972 , @a_sidorin wrote:

> Hi Gabor,
>  This looks fine, but could you please add a test showing how decl shadowing 
> is handled? I.e. if we have Arg in one TU and both Arg and N::Arg in another 
> TU.


Alexei,

Thanks for the review! I've added a new test  case for that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64241/new/

https://reviews.llvm.org/D64241



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

[PATCH] D64241: [ASTImporter] Fix inequivalence of ClassTemplateInstantiations

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 210581.
martong added a comment.

- Add test case ClassTemplSpecWithInequivalentShadowedTemplArg


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64241/new/

https://reviews.llvm.org/D64241

Files:
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/unittests/AST/StructuralEquivalenceTest.cpp

Index: clang/unittests/AST/StructuralEquivalenceTest.cpp
===
--- clang/unittests/AST/StructuralEquivalenceTest.cpp
+++ clang/unittests/AST/StructuralEquivalenceTest.cpp
@@ -892,6 +892,67 @@
   EXPECT_FALSE(testStructuralMatch(First, Second));
 }
 
+TEST_F(StructuralEquivalenceTemplateTest,
+   TemplateVsSubstTemplateTemplateParmInArgEq) {
+  auto t = makeDecls(
+  R"(
+template  class Arg { };
+template  class P1> class Primary { };
+
+void f() {
+  // Make specialization with simple template.
+  Primary  A;
+}
+  )",
+  R"(
+template  class Arg { };
+template  class P1> class Primary { };
+
+template  class P1> class Templ {
+  void f() {
+// Make specialization with substituted template template param.
+Primary  A;
+  };
+};
+
+// Instantiate with substitution Arg into P1.
+template class Templ ;
+  )",
+  Lang_CXX, classTemplateSpecializationDecl(hasName("Primary")));
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceTemplateTest,
+   TemplateVsSubstTemplateTemplateParmInArgNotEq) {
+  auto t = makeDecls(
+  R"(
+template  class Arg { };
+template  class P1> class Primary { };
+
+void f() {
+  // Make specialization with simple template.
+  Primary  A;
+}
+  )",
+  R"(
+// Arg is different from the other, this should cause non-equivalence.
+template  class Arg { int X; };
+template  class P1> class Primary { };
+
+template  class P1> class Templ {
+  void f() {
+// Make specialization with substituted template template param.
+Primary  A;
+  };
+};
+
+// Instantiate with substitution Arg into P1.
+template class Templ ;
+  )",
+  Lang_CXX, classTemplateSpecializationDecl(hasName("Primary")));
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
 struct StructuralEquivalenceDependentTemplateArgsTest
 : StructuralEquivalenceTemplateTest {};
 
@@ -1030,5 +1091,136 @@
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(
+StructuralEquivalenceTemplateTest,
+ClassTemplSpecWithQualifiedAndNonQualifiedTypeArgsShouldBeEqual) {
+  auto t = makeDecls(
+  R"(
+  template  struct Primary {};
+  namespace N {
+struct Arg;
+  }
+  // Explicit instantiation with qualified name.
+  template struct Primary;
+  )",
+  R"(
+  template  struct Primary {};
+  namespace N {
+struct Arg;
+  }
+  using namespace N;
+  // Explicit instantiation with UNqualified name.
+  template struct Primary;
+  )",
+  Lang_CXX,
+  classTemplateSpecializationDecl(hasName("Primary")));
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(
+StructuralEquivalenceTemplateTest,
+ClassTemplSpecWithInequivalentQualifiedAndNonQualifiedTypeArgs) {
+  auto t = makeDecls(
+  R"(
+  template  struct Primary {};
+  namespace N {
+struct Arg { int a; };
+  }
+  // Explicit instantiation with qualified name.
+  template struct Primary;
+  )",
+  R"(
+  template  struct Primary {};
+  namespace N {
+// This struct is not equivalent with the other in the prev TU.
+struct Arg { double b; }; // -- Field mismatch.
+  }
+  using namespace N;
+  // Explicit instantiation with UNqualified name.
+  template struct Primary;
+  )",
+  Lang_CXX,
+  classTemplateSpecializationDecl(hasName("Primary")));
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(
+StructuralEquivalenceTemplateTest,
+ClassTemplSpecWithQualifiedAndNonQualifiedTemplArgsShouldBeEqual) {
+  auto t = makeDecls(
+  R"(
+  template  class T> struct Primary {};
+  namespace N {
+template  struct Arg;
+  }
+  // Explicit instantiation with qualified name.
+  template struct Primary;
+  )",
+  R"(
+  template  class T> struct Primary {};
+  namespace N {
+template  struct Arg;
+  }
+  using namespace N;
+  // Explicit instantiation with UNqualified name.
+  template struct Primary;
+  )",
+  Lang_CXX,
+  classTemplateSpecializationDecl(hasName("Primary")));
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(
+StructuralEquivalenceTemplateTest,
+ClassTemplSpecWithInequivalentQualifiedAndNonQualifiedTemplArgs) {
+  auto t = makeDecls(
+  R"(
+  template  class T> struct Primary {};
+  namespace N {
+template  struct Arg { int a; };
+  }
+  // Explicit instantiation with qualified name.
+  template struct Primary;
+  )",
+  R"(
+  template  class T> 

[PATCH] D64241: [ASTImporter] Fix inequivalence of ClassTemplateInstantiations

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hi Gabor,
This looks fine, but could you please add a test showing how decl shadowing is 
handled? I.e. if we have Arg in one TU and both Arg and N::Arg in another TU.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64241/new/

https://reviews.llvm.org/D64241



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

[PATCH] D64241: [ASTImporter] Fix inequivalence of ClassTemplateInstantiations

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added a reviewer: a_sidorin.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.

We falsely state inequivalence if the template parameter is a
qualified/nonquialified template in the first/second instantiation.
Also, different kinds of TemplateName should be equal if the template
decl (if available) is equal (even if the name kind is different).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64241

Files:
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/unittests/AST/StructuralEquivalenceTest.cpp

Index: clang/unittests/AST/StructuralEquivalenceTest.cpp
===
--- clang/unittests/AST/StructuralEquivalenceTest.cpp
+++ clang/unittests/AST/StructuralEquivalenceTest.cpp
@@ -892,6 +892,67 @@
   EXPECT_FALSE(testStructuralMatch(First, Second));
 }
 
+TEST_F(StructuralEquivalenceTemplateTest,
+   TemplateVsSubstTemplateTemplateParmInArgEq) {
+  auto t = makeDecls(
+  R"(
+template  class Arg { };
+template  class P1> class Primary { };
+
+void f() {
+  // Make specialization with simple template.
+  Primary  A;
+}
+  )",
+  R"(
+template  class Arg { };
+template  class P1> class Primary { };
+
+template  class P1> class Templ {
+  void f() {
+// Make specialization with substituted template template param.
+Primary  A;
+  };
+};
+
+// Instantiate with substitution Arg into P1.
+template class Templ ;
+  )",
+  Lang_CXX, classTemplateSpecializationDecl(hasName("Primary")));
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceTemplateTest,
+   TemplateVsSubstTemplateTemplateParmInArgNotEq) {
+  auto t = makeDecls(
+  R"(
+template  class Arg { };
+template  class P1> class Primary { };
+
+void f() {
+  // Make specialization with simple template.
+  Primary  A;
+}
+  )",
+  R"(
+// Arg is different from the other, this should cause non-equivalence.
+template  class Arg { int X; };
+template  class P1> class Primary { };
+
+template  class P1> class Templ {
+  void f() {
+// Make specialization with substituted template template param.
+Primary  A;
+  };
+};
+
+// Instantiate with substitution Arg into P1.
+template class Templ ;
+  )",
+  Lang_CXX, classTemplateSpecializationDecl(hasName("Primary")));
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
 struct StructuralEquivalenceDependentTemplateArgsTest
 : StructuralEquivalenceTemplateTest {};
 
@@ -1030,5 +1091,111 @@
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(
+StructuralEquivalenceTemplateTest,
+ClassTemplSpecWithQualifiedAndNonQualifiedTypeArgsShouldBeEqual) {
+  auto t = makeDecls(
+  R"(
+  template  struct Primary {};
+  namespace N {
+struct Arg;
+  }
+  // Explicit instantiation with qualified name.
+  template struct Primary;
+  )",
+  R"(
+  template  struct Primary {};
+  namespace N {
+struct Arg;
+  }
+  using namespace N;
+  // Explicit instantiation with UNqualified name.
+  template struct Primary;
+  )",
+  Lang_CXX,
+  classTemplateSpecializationDecl(hasName("Primary")));
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(
+StructuralEquivalenceTemplateTest,
+ClassTemplSpecWithInequivalentQualifiedAndNonQualifiedTypeArgs) {
+  auto t = makeDecls(
+  R"(
+  template  struct Primary {};
+  namespace N {
+struct Arg { int a; };
+  }
+  // Explicit instantiation with qualified name.
+  template struct Primary;
+  )",
+  R"(
+  template  struct Primary {};
+  namespace N {
+// This struct is not equivalent with the other in the prev TU.
+struct Arg { double b; }; // -- Field mismatch.
+  }
+  using namespace N;
+  // Explicit instantiation with UNqualified name.
+  template struct Primary;
+  )",
+  Lang_CXX,
+  classTemplateSpecializationDecl(hasName("Primary")));
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(
+StructuralEquivalenceTemplateTest,
+ClassTemplSpecWithQualifiedAndNonQualifiedTemplArgsShouldBeEqual) {
+  auto t = makeDecls(
+  R"(
+  template  class T> struct Primary {};
+  namespace N {
+template  struct Arg;
+  }
+  // Explicit instantiation with qualified name.
+  template struct Primary;
+  )",
+  R"(
+  template  class T> struct Primary {};
+  namespace N {
+template  struct Arg;
+  }
+  using namespace N;
+  // Explicit instantiation with UNqualified name.
+  template struct Primary;
+  )",
+  Lang_CXX,
+  classTemplateSpecializationDecl(hasName("Primary")));
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(
+StructuralEquivalenceTemplateTest,
+ClassTemplSpecWithInequivalentQualifiedAndNonQualifiedTemplArgs) {
+  auto t