[Clang][Sema] Fix explicit specializations of member function templates with a deduced return type

[sdkrystian]

Currently, clang erroneously rejects the following:

template<typename T>
struct A
{
    template<typename U>
    auto f();
};

template<>
template<typename U>
auto A<int>::f(); // error: conflicting types for 'f'

This happens because the explicit specialization of f has its return type replaced with a dependent AutoType in ActOnFunctionDeclarator, but no such replacement occurs for the implicitly instantiated function template A<int>::f. Since the return types don't match, the explicit specialization is diagnosed as an invalid redeclaration.

This patch moves the replacement of the return type to CheckFunctionDeclaration so it also happens during instantiation. setObjectOfFriendDecl will have been called by then, so the isFriend && CurContext->isDependentContext() condition is made redundant & removed (as it already happens in DeclContext::isDependentContext). Sema::IsOverload only checks the declared return type (which isn't changed by the adjustment), so adjusting the return type afterwards should be safe.

[llvmbot]

@llvm/pr-subscribers-clang-tidy
@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang

Author: Krystian Stasiowski (sdkrystian)

Changes

Currently, clang erroneously rejects the following:

template<typename T>
struct A
{
    template<typename U>
    auto f();
};

template<>
template<typename U>
auto A<int>::f(); // error: conflicting types for 'f'

This happens because the explicit specialization of f has its return type replaced with a dependent AutoType in ActOnFunctionDeclarator, but no such replacement occurs for the implicitly instantiated function template A<int>::f. Since the return types don't match, the explicit specialization is diagnosed as an invalid redeclaration.

This patch moves the replacement of the return type to CheckFunctionDeclaration so it also happens during instantiation. setObjectOfFriendDecl will have been called by then, so the isFriend && CurContext->isDependentContext() condition is made redundant & removed (as it already happens in DeclContext::isDependentContext). Sema::IsOverload only checks the declared return type (which isn't changed by the adjustment), so adjusting the return type afterwards should be safe.

---

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

3 Files Affected:

• (modified) clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp (-2)
• (modified) clang/lib/Sema/SemaDecl.cpp (+29-17)
• (modified) clang/test/SemaCXX/deduced-return-type-cxx14.cpp (+18)

diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
index d0efc5ca763753..e333c83b895ee7 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp
@@ -68,6 +68,4 @@ auto S<>::foo(auto)
 {
     return 1;
 }
-// CHECK8: error: conflicting types for 'foo' [clang-diagnostic-error]
-// CHECK8: note: previous declaration is here
 #endif
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 9a286e0b26a4c6..be9a05dd2ce570 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -10119,23 +10119,6 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
         Diag(D.getDeclSpec().getVirtualSpecLoc(), diag::err_auto_fn_virtual);
     }
 
-    if (getLangOpts().CPlusPlus14 &&
-        (NewFD->isDependentContext() ||
-         (isFriend && CurContext->isDependentContext())) &&
-        NewFD->getReturnType()->isUndeducedType()) {
-      // If the function template is referenced directly (for instance, as a
-      // member of the current instantiation), pretend it has a dependent type.
-      // This is not really justified by the standard, but is the only sane
-      // thing to do.
-      // FIXME: For a friend function, we have not marked the function as being
-      // a friend yet, so 'isDependentContext' on the FD doesn't work.
-      const FunctionProtoType *FPT =
-          NewFD->getType()->castAs<FunctionProtoType>();
-      QualType Result = SubstAutoTypeDependent(FPT->getReturnType());
-      NewFD->setType(Context.getFunctionType(Result, FPT->getParamTypes(),
-                                             FPT->getExtProtoInfo()));
-    }
-
     // C++ [dcl.fct.spec]p3:
     //  The inline specifier shall not appear on a block scope function
     //  declaration.
@@ -12107,6 +12090,35 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
 
   CheckConstPureAttributesUsage(*this, NewFD);
 
+  // C++23 [dcl.spec.auto.general]p12:
+  //   Return type deduction for a templated function with a placeholder in its
+  //   declared type occurs when the definition is instantiated even if the
+  //   function body contains a return statement with a non-type-dependent
+  //   operand.
+  //
+  // C++23 [temp.dep.expr]p3:
+  //   An id-expression is type-dependent if it is a template-id that is not a
+  //   concept-id and is dependent; or if its terminal name is:
+  //   - [...]
+  //   - associated by name lookup with one or more declarations of member
+  //     functions of a class that is the current instantiation declared with a
+  //     return type that contains a placeholder type,
+  //   - [...]
+  //
+  // If this is a templated function with a placeholder in its return type,
+  // make the placeholder type dependent since it won't be deduced until the
+  // definition is instantiated. We do this here because it needs to happen
+  // for implicitly instantiated member functions/member function templates.
+  if (getLangOpts().CPlusPlus14 &&
+      (NewFD->isDependentContext() &&
+       NewFD->getReturnType()->isUndeducedType())) {
+    const FunctionProtoType *FPT =
+        NewFD->getType()->castAs<FunctionProtoType>();
+    QualType NewReturnType = SubstAutoTypeDependent(FPT->getReturnType());
+    NewFD->setType(Context.getFunctionType(NewReturnType, FPT->getParamTypes(),
+                                           FPT->getExtProtoInfo()));
+  }
+
   // C++11 [dcl.constexpr]p8:
   //   A constexpr specifier for a non-static member function that is not
   //   a constructor declares that member function to be const.
diff --git a/clang/test/SemaCXX/deduced-return-type-cxx14.cpp b/clang/test/SemaCXX/deduced-return-type-cxx14.cpp
index 431d77ca785b8e..c33e07088ba32f 100644
--- a/clang/test/SemaCXX/deduced-return-type-cxx14.cpp
+++ b/clang/test/SemaCXX/deduced-return-type-cxx14.cpp
@@ -237,6 +237,24 @@ namespace Templates {
     int (S::*(*p)())(double) = f;
     int (S::*(*q)())(double) = f<S, double>;
   }
+
+  template<typename T>
+  struct MemberSpecialization {
+    auto f();
+    template<typename U> auto f(U);
+    template<typename U> auto *f(U);
+  };
+
+  template<>
+  auto MemberSpecialization<int>::f();
+
+  template<>
+  template<typename U>
+  auto MemberSpecialization<int>::f(U);
+
+  template<>
+  template<typename U>
+  auto *MemberSpecialization<int>::f(U);
 }
 
 auto fwd_decl_using();

[sdkrystian]

Still needs a release note but should otherwise be good to go

[Sirraide]

LGTM after looking at this for a while, except for some minor comments.

[Sirraide]

LGTM

[sdkrystian]

Ping @erichkeane