[ASTImporter] Structural eq: handle DependentScopeDeclRefExpr

[martong]

Summary

Structural equivalence did not handle dependent template args properly
when the arg contained a DependentScopeDeclRefExpr.

[a_sidorin]

Hello Gabor,
I have a few questions inline.

clang/lib/AST/ASTStructuralEquivalence.cpp

124 ↗

(On Diff #200996)

Should we check the equivalence of getCXXNameType() in such cases?

127 ↗

(On Diff #200996)

Should we check the equivalence of the whole Name1.getCXXDeductionGuideTemplate() (with the template arguments)?

147 ↗

(On Diff #200996)

llvm_unreachable()?

163 ↗

(On Diff #200996)

Should we compare TemplateArgs (getTemplateArgs) somehow?

[martong]

clang/lib/AST/ASTStructuralEquivalence.cpp

124 ↗

(On Diff #200996)

Good catch, thanks!

127 ↗

(On Diff #200996)

Good catch, thanks!

147 ↗

(On Diff #200996)

Good catch, thanks!

163 ↗

(On Diff #200996)

No, that is not needed. Because getQualifier() returns with a NestedNameSpecifier and then in the appropriare overload of IsStructurallyEquivalent we will investigate further the type together with the template params:

213   case NestedNameSpecifier::TypeSpec:
214   case NestedNameSpecifier::TypeSpecWithTemplate:
215     return IsStructurallyEquivalent(Context, QualType(NNS1->getAsType(), 0),
216                                     QualType(NNS2->getAsType(), 0));

[martong]

• Add further checks for the DeclarationName overload

[martong]

Alexei, thank you very much for the review, you caught quite a few missing things!

[a_sidorin]

LGTM, thanks for the fixes!

[martong]

• Handle ImplicitCastExpr. In Clang7 in the last two test cases we did not have any ImplicitCastExpr in the AST. With never Clang we have, so we must handle the cast expr too.

[a_sidorin]

cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp

173

Hi Gabor,
Is there any test for this branch?

[martong]

cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp

173

Yes, there are implicit tests.
I had to add this branch because after rebasing to master a new AST node is created during the parsing of the test code.
This test code

template <typename T, typename enable_if<S1<T>::value>::type>

had this AST before the rebase:

TemplateSpecializationType 0xbd2b80 'enable_if<S1<T>::value>' dependent enable_if
`-TemplateArgument expr
  `-DependentScopeDeclRefExpr 0xbd2b28 '<dependent type>' lvalue

and this after rebase:

TemplateSpecializationType 0xc0f0c0 'enable_if<S1<T>::value>' dependent enable_if
`-TemplateArgument expr
  `-ImplicitCastExpr 0xc0f090 '_Bool' <Dependent>
    `-DependentScopeDeclRefExpr 0xc0f058 '<dependent type>' lvalue

So, SameStructsInDependentScopeDeclRefExpr, DifferentStructsInDependentScopeDeclRefExpr and DifferentValueInDependentScopeDeclRefExpr do implicitly test this branch. These tests would not work without the branch for ImplicitCastExpr.

cfe/trunk/lib/AST/ASTStructuralEquivalence.cpp

 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDiagnostic.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/ExceptionSpecificationType.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"

 
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
                                      QualType T1, QualType T2);
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
                                      Decl *D1, Decl *D2);
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
                                      const TemplateArgument &Arg1,
                                      const TemplateArgument &Arg2);
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+                                     NestedNameSpecifier *NNS1,
+                                     NestedNameSpecifier *NNS2);
+static bool IsStructurallyEquivalent(const IdentifierInfo *Name1,
+                                     const IdentifierInfo *Name2);
+
+static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
+                                     const DeclarationName Name1,
+                                     const DeclarationName Name2) {
+  if (Name1.getNameKind() != Name2.getNameKind())
+    return false;
+
+  switch (Name1.getNameKind()) {
+
+  case DeclarationName::Identifier:
+    return IsStructurallyEquivalent(Name1.getAsIdentifierInfo(),
+                                    Name2.getAsIdentifierInfo());
+
+  case DeclarationName::CXXConstructorName:
+  case DeclarationName::CXXDestructorName:
+  case DeclarationName::CXXConversionFunctionName:
+    return IsStructurallyEquivalent(Context, Name1.getCXXNameType(),
+                                    Name2.getCXXNameType());
+
+  case DeclarationName::CXXDeductionGuideName: {
+    if (!IsStructurallyEquivalent(
+            Context, Name1.getCXXDeductionGuideTemplate()->getDeclName(),
+            Name2.getCXXDeductionGuideTemplate()->getDeclName()))
+      return false;
+    return IsStructurallyEquivalent(Context,
+                                    Name1.getCXXDeductionGuideTemplate(),
+                                    Name2.getCXXDeductionGuideTemplate());
+  }
+
+  case DeclarationName::CXXOperatorName:
+    return Name1.getCXXOverloadedOperator() == Name2.getCXXOverloadedOperator();
+
+  case DeclarationName::CXXLiteralOperatorName:
+    return IsStructurallyEquivalent(Name1.getCXXLiteralIdentifier(),
+                                    Name2.getCXXLiteralIdentifier());
+
+  case DeclarationName::CXXUsingDirective:
+    return true; // FIXME When do we consider two using directives equal?
+
+  case DeclarationName::ObjCZeroArgSelector:
+  case DeclarationName::ObjCOneArgSelector:
+  case DeclarationName::ObjCMultiArgSelector:
+    return true; // FIXME
+  }
+
+  llvm_unreachable("Unhandled kind of DeclarationName");
+  return true;
+}
 
 /// Determine structural equivalence of two expressions.
 static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
                                      const Expr *E1, const Expr *E2) {
   if (!E1 || !E2)
     return E1 == E2;
 
-  // FIXME: Actually perform a structural comparison!
+  if (auto *DE1 = dyn_cast<DependentScopeDeclRefExpr>(E1)) {
+    auto *DE2 = dyn_cast<DependentScopeDeclRefExpr>(E2);
+    if (!DE2)
+      return false;
+    if (!IsStructurallyEquivalent(Context, DE1->getDeclName(),
+                                  DE2->getDeclName()))
+      return false;
+    return IsStructurallyEquivalent(Context, DE1->getQualifier(),
+                                    DE2->getQualifier());
+  } else if (auto CastE1 = dyn_cast<ImplicitCastExpr>(E1)) {
+    auto *CastE2 = dyn_cast<ImplicitCastExpr>(E2);
+    if (!CastE2)
+      return false;
+    if (!IsStructurallyEquivalent(Context, CastE1->getType(),
+                                  CastE2->getType()))
+      return false;
+    return IsStructurallyEquivalent(Context, CastE1->getSubExpr(),
+                                    CastE2->getSubExpr());
+  }
+  // FIXME: Handle other kind of expressions!
   return true;
 }
 
 /// Determine whether two identifiers are equivalent.
 static bool IsStructurallyEquivalent(const IdentifierInfo *Name1,
                                      const IdentifierInfo *Name2) {
   if (!Name1 || !Name2)
     return Name1 == Name2;

cfe/trunk/unittests/AST/StructuralEquivalenceTest.cpp

auto Decls = makeNamedDecls("template <bool b> struct foo {explicit(b) foo(int);};",
                               "template <bool b> struct foo {explicit(!b) foo(int);};", Lang_CXX2a);
   CXXConstructorDecl *First = FirstDeclMatcher<CXXConstructorDecl>().match(
       get<0>(Decls), cxxConstructorDecl(hasName("foo<b>")));
   CXXConstructorDecl *Second = FirstDeclMatcher<CXXConstructorDecl>().match(
       get<1>(Decls), cxxConstructorDecl(hasName("foo<b>")));
   EXPECT_FALSE(testStructuralMatch(First, Second));
 }
 
+struct StructuralEquivalenceDependentTemplateArgsTest
+    : StructuralEquivalenceTemplateTest {};
+
+TEST_F(StructuralEquivalenceDependentTemplateArgsTest,
+       SameStructsInDependentArgs) {
+  std::string Code =
+      R"(
+      template <typename>
+      struct S1;
+
+      template <typename>
+      struct enable_if;
+
+      struct S
+      {
+        template <typename T, typename enable_if<S1<T>>::type>
+        void f();
+      };
+      )";
+  auto t = makeDecls<FunctionTemplateDecl>(Code, Code, Lang_CXX11,
+                                           functionTemplateDecl(hasName("f")));
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceDependentTemplateArgsTest,
+       DifferentStructsInDependentArgs) {
+  std::string Code =
+      R"(
+      template <typename>
+      struct S1;
+
+      template <typename>
+      struct S2;
+
+      template <typename>
+      struct enable_if;
+      )";
+  auto t = makeDecls<FunctionTemplateDecl>(Code + R"(
+      struct S
+      {
+        template <typename T, typename enable_if<S1<T>>::type>
+        void f();
+      };
+      )",
+                                           Code + R"(
+      struct S
+      {
+        template <typename T, typename enable_if<S2<T>>::type>
+        void f();
+      };
+      )",
+                                           Lang_CXX11,
+                                           functionTemplateDecl(hasName("f")));
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceDependentTemplateArgsTest,
+       SameStructsInDependentScopeDeclRefExpr) {
+  std::string Code =
+      R"(
+      template <typename>
+      struct S1;
+
+      template <bool>
+      struct enable_if;
+
+      struct S
+      {
+        template <typename T, typename enable_if<S1<T>::value>::type>
+        void f();   // DependentScopeDeclRefExpr:^^^^^^^^^^^^
+      };
+      )";
+  auto t = makeDecls<FunctionTemplateDecl>(Code, Code, Lang_CXX11,
+                                           functionTemplateDecl(hasName("f")));
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceDependentTemplateArgsTest,
+       DifferentStructsInDependentScopeDeclRefExpr) {
+  std::string Code =
+      R"(
+      template <typename>
+      struct S1;
+
+      template <typename>
+      struct S2;
+
+      template <bool>
+      struct enable_if;
+      )";
+  auto t = makeDecls<FunctionTemplateDecl>(Code + R"(
+      struct S
+      {
+        template <typename T, typename enable_if<S1<T>::value>::type>
+        void f();   // DependentScopeDeclRefExpr:^^^^^^^^^^^^
+      };
+      )",
+                                           Code + R"(
+      struct S
+      {
+        template <typename T, typename enable_if<S2<T>::value>::type>
+        void f();
+      };
+      )",
+                                           Lang_CXX,
+                                           functionTemplateDecl(hasName("f")));
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceDependentTemplateArgsTest,
+       DifferentValueInDependentScopeDeclRefExpr) {
+  std::string Code =
+      R"(
+      template <typename>
+      struct S1;
+
+      template <bool>
+      struct enable_if;
+      )";
+  auto t = makeDecls<FunctionTemplateDecl>(Code + R"(
+      struct S
+      {
+        template <typename T, typename enable_if<S1<T>::value1>::type>
+        void f();   // DependentScopeDeclRefExpr:^^^^^^^^^^^^
+      };
+      )",
+                                           Code + R"(
+      struct S
+      {
+        template <typename T, typename enable_if<S1<T>::value2>::type>
+        void f();
+      };
+      )",
+                                           Lang_CXX,
+                                           functionTemplateDecl(hasName("f")));
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
 } // end namespace ast_matchers
 } // end namespace clang