[clang] fix deduction of member pointers with dependent named classes

[mizvekov]

This fixes a regression when interpreting a nested name specifier for deduction purposes in member pointers.

This introduces a helper for fully translating a nested name specifier into a type.

This regression was introduced here: #130537
and was reported here: #132401 (comment)

No release notes, since this regression was never released.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

This fixes a regression when interpreting a nested name specifier for deduction purposes in member pointers.

This introduces a helper for fully translating a nested name specifier into a type.

Other existing potential users will be deduplicated by using this helper in subsequent patches.

This regression was introduced here: #130537
and was reported here: #132401 (comment)

No release notes, since this regression was never released.

---

Full diff: https://github.com/llvm/llvm-project/pull/133113.diff

5 Files Affected:

• (modified) clang/include/clang/AST/NestedNameSpecifier.h (+5)
• (modified) clang/lib/AST/NestedNameSpecifier.cpp (+46)
• (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+22-12)
• (modified) clang/test/SemaCXX/member-pointer.cpp (+43)
• (modified) clang/test/SemaTemplate/instantiation-backtrace.cpp (+3-3)

diff --git a/clang/include/clang/AST/NestedNameSpecifier.h b/clang/include/clang/AST/NestedNameSpecifier.h
index 051d632f1cdf9..273e73e7c1e95 100644
--- a/clang/include/clang/AST/NestedNameSpecifier.h
+++ b/clang/include/clang/AST/NestedNameSpecifier.h
@@ -201,6 +201,11 @@ class NestedNameSpecifier : public llvm::FoldingSetNode {
     return nullptr;
   }
 
+  /// Fully translate this nested name specifier to a type.
+  /// Unlike getAsType, this will convert this entire nested
+  /// name specifier chain into its equivalent type.
+  const Type *translateToType(const ASTContext &Context) const;
+
   NestedNameSpecifierDependence getDependence() const;
 
   /// Whether this nested name specifier refers to a dependent
diff --git a/clang/lib/AST/NestedNameSpecifier.cpp b/clang/lib/AST/NestedNameSpecifier.cpp
index a256a87695afc..206e462a58a79 100644
--- a/clang/lib/AST/NestedNameSpecifier.cpp
+++ b/clang/lib/AST/NestedNameSpecifier.cpp
@@ -245,6 +245,52 @@ bool NestedNameSpecifier::containsErrors() const {
   return getDependence() & NestedNameSpecifierDependence::Error;
 }
 
+const Type *
+NestedNameSpecifier::translateToType(const ASTContext &Context) const {
+  NestedNameSpecifier *Prefix = getPrefix();
+  switch (getKind()) {
+  case SpecifierKind::Identifier:
+    return Context
+        .getDependentNameType(ElaboratedTypeKeyword::None, Prefix,
+                              getAsIdentifier())
+        .getTypePtr();
+  case SpecifierKind::TypeSpec:
+  case SpecifierKind::TypeSpecWithTemplate: {
+    const Type *T = getAsType();
+    switch (T->getTypeClass()) {
+    case Type::DependentTemplateSpecialization: {
+      const auto *DT = cast<DependentTemplateSpecializationType>(T);
+      // FIXME: The type node can't represent the template keyword.
+      return Context
+          .getDependentTemplateSpecializationType(ElaboratedTypeKeyword::None,
+                                                  Prefix, DT->getIdentifier(),
+                                                  DT->template_arguments())
+          .getTypePtr();
+    }
+    case Type::Record:
+    case Type::TemplateSpecialization:
+    case Type::Using:
+    case Type::Enum:
+    case Type::Typedef:
+    case Type::UnresolvedUsing:
+      return Context
+          .getElaboratedType(ElaboratedTypeKeyword::None, Prefix,
+                             QualType(T, 0))
+          .getTypePtr();
+    default:
+      assert(Prefix == nullptr && "unexpected type with elaboration");
+      return T;
+    }
+  }
+  case SpecifierKind::Global:
+  case SpecifierKind::Namespace:
+  case SpecifierKind::NamespaceAlias:
+  case SpecifierKind::Super:
+    // These are not representable as types.
+    return nullptr;
+  }
+}
+
 /// Print this nested name specifier to the given output
 /// stream.
 void NestedNameSpecifier::print(raw_ostream &OS, const PrintingPolicy &Policy,
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 410b5a2c83e8d..e4ef5e7f72737 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -2127,19 +2127,29 @@ static TemplateDeductionResult DeduceTemplateArgumentsByTypeMatch(
               /*DeducedFromArrayBound=*/false, HasDeducedAnyParam);
           Result != TemplateDeductionResult::Success)
         return Result;
-      const Type *QP = MPP->getQualifier()->getAsType(),
-                 *QA = MPA->getQualifier()->getAsType();
-      CXXRecordDecl *ClsP = MPP->getMostRecentCXXRecordDecl(),
-                    *ClsA = MPA->getMostRecentCXXRecordDecl();
-      // FIXME: Don't drop the rest of the prefixes here.
-      QualType P = !ClsP || declaresSameEntity(QP->getAsCXXRecordDecl(), ClsP)
-                       ? QualType(QP, 0)
-                       : S.Context.getTypeDeclType(ClsP);
-      QualType A = !ClsA || declaresSameEntity(QA->getAsCXXRecordDecl(), ClsA)
-                       ? QualType(QA, 0)
-                       : S.Context.getTypeDeclType(ClsA);
+
+      QualType TP;
+      if (MPP->isSugared()) {
+        TP = S.Context.getTypeDeclType(MPP->getMostRecentCXXRecordDecl());
+      } else {
+        NestedNameSpecifier *QP = MPP->getQualifier();
+        if (QP->getKind() == clang::NestedNameSpecifier::Identifier)
+          // Skip translation if it's a non-deduced context anyway.
+          return TemplateDeductionResult::Success;
+        TP = QualType(QP->translateToType(S.Context), 0);
+      }
+      assert(!TP.isNull() && "member pointer with non-type class");
+
+      QualType TA;
+      if (MPA->isSugared()) {
+        TA = S.Context.getTypeDeclType(MPA->getMostRecentCXXRecordDecl());
+      } else {
+        NestedNameSpecifier *QA = MPA->getQualifier();
+        TA = QualType(QA->translateToType(S.Context), 0);
+        assert(!TA->isNullPtrType() && "member pointer with non-type class");
+      }
       return DeduceTemplateArgumentsByTypeMatch(
-          S, TemplateParams, P, A, Info, Deduced, SubTDF,
+          S, TemplateParams, TP, TA, Info, Deduced, SubTDF,
           degradeCallPartialOrderingKind(POK),
           /*DeducedFromArrayBound=*/false, HasDeducedAnyParam);
     }
diff --git a/clang/test/SemaCXX/member-pointer.cpp b/clang/test/SemaCXX/member-pointer.cpp
index 3d9dd05755b8c..490e290e88058 100644
--- a/clang/test/SemaCXX/member-pointer.cpp
+++ b/clang/test/SemaCXX/member-pointer.cpp
@@ -355,3 +355,46 @@ namespace GH132401 {
   };
   template struct CallableHelper<void (QIODevice::*)()>;
 } // namespace GH132401
+
+namespace deduction1 {
+  template <typename> struct RunCallImpl;
+
+  template <typename Derived>
+  struct RunCallImpl<int (Derived::Info::*)(Derived *)> {};
+
+  template <typename d>
+  void RunCall(d) {
+    RunCallImpl<d>();
+  }
+
+  struct Filter {
+    virtual void MakeCall();
+    virtual ~Filter() = default;
+  };
+
+  template <typename Derived>
+  struct ImplementFilter : Filter {
+    void MakeCall() { RunCall(&Derived::Info::OnStuffHandler); }
+  };
+
+  struct FoobarFilter : ImplementFilter<FoobarFilter> {
+    struct Info {
+      int OnStuffHandler(FoobarFilter *);
+    };
+  };
+} // namespace deduction1
+
+namespace deduction2 {
+  template <typename> struct A;
+  template <typename T>
+  struct A<void (T::C::*)(int &, T *)> {};
+  template <typename T> void e(T) {
+    A<T> f;
+  }
+  struct S {
+    struct C {
+      void h(int &, S *);
+    };
+    void i() { e(&C::h); }
+  };
+} // namespace deduction2
diff --git a/clang/test/SemaTemplate/instantiation-backtrace.cpp b/clang/test/SemaTemplate/instantiation-backtrace.cpp
index 39dfb0b32a2fb..6b51d0eba7979 100644
--- a/clang/test/SemaTemplate/instantiation-backtrace.cpp
+++ b/clang/test/SemaTemplate/instantiation-backtrace.cpp
@@ -22,7 +22,7 @@ void g() {
   (void)sizeof(B<X>); // expected-note{{in instantiation of template class 'B<X>' requested here}}
 }
 
-template<typename T> 
+template<typename T>
 struct G : A<T>, // expected-error{{implicit instantiation of undefined template 'A<int>'}}
   A<T*> // expected-error{{implicit instantiation of undefined template 'A<int *>'}}
   { };
@@ -39,13 +39,13 @@ namespace PR13365 {
   template <class T1, class T2>
     typename ResultTy<T2>::error Deduce( void (T1::*member)(T2) ) {} // \
     // expected-note {{instantiation of template class 'PR13365::ResultTy<int &>'}} \
-    // expected-note {{substitution failure [with T1 = PR13365::Cls, T2 = int &]}}
+    // expected-note {{substitution failure [with T1 = Cls, T2 = int &]}}
 
   struct Cls {
     void method(int&);
   };
   void test() {
     Deduce(&Cls::method); // expected-error {{no matching function}} \
-                          // expected-note {{substituting deduced template arguments into function template 'Deduce' [with T1 = PR13365::Cls, T2 = int &]}}
+                          // expected-note {{substituting deduced template arguments into function template 'Deduce' [with T1 = Cls, T2 = int &]}}
   }
 }

[rupprecht]

I can't comment on the contents of this fix, but I patched it in and the crash no longer reproduces on the original target I saw this in. I think this also takes care of the targets that @alexfh reported.

[mizvekov]

Note this was unrelated to another regression, as reported here: #132317 (comment)

Since this fixes a regression which recently landed, I am going to go ahead and merge, relying on post-commit review.

[erichkeane]

Just a minor suggestion on this one.