[Sema] Improve handling of function pointer conversions

[Mordante]

Summary

Starting with C++17 the noexcept is part of the function signature but a noexcept function can be converted to a noexcept(false) function. The overload resolution code handles it properly but expression builder doesn't. It causes the following code to trigger and assertion failure:

struct S {
    int f(void) noexcept { return 110; }
} s;

template <int(S::*a)(void)> int f10(void) { return (s.*a)(); }

int foo(void)
{
  return f10<&S::f >();
}

The fix adds an extra implicit cast when needed. I picked the CK_NoOp as CastKind since it seems to be the best fit. However I wonder whether it would be better to add a new value CK_FunctionPointerConversion.

This fixes bug 40024.

[Mordante]

Ping

[rsmith]

clang/lib/Sema/SemaTemplate.cpp

6996

Please capitalize local variable names to match the local convention.

clang/test/CXX/conv/conv.fctptr/template-noexcept-invalid.cpp

8 ↗

(On Diff #210369)

You can just say PR40024 instead of spelling out the complete URL. That'll also work better if we migrate our bug tracker to a different system one day. (Incidentally, llvm.org/PR40024 works too.)

22 ↗

(On Diff #210369)

Please use a -verify test for this instead of FileCheck, and combine the -valid and -invalid testcases into the same test file.

[Mordante]

Addresses the review remarks:

• resultTy -> ResultTy
• Move all unit tests in one file
• Change the unit test to use -verify instead of FileCheck
• Use PR40024 instead of link to Bugzilla

[rsmith]

LGTM, though I'd like to see this generalized to handle noreturn too.

clang/lib/Sema/SemaTemplate.cpp

6997

Can we remove the check for C++17 here? I would expect we want to consider the other kind of function conversion (from noreturn function to potentially-returning function) here too, and that applies in all language modes.

Testcase for that bug:

template<int (*)()> struct X {};
int f() __attribute__((noreturn));
X<&f> x;

[Mordante]

Removed the language version test.

I also added clang/test/CXX/conv/conv.fctptr/template-noreturn.cpp to show this is not enough to allow noreturn to all language versions, so this file should not be committed.

Do you want me to look at a way to allow the noreturn for C++98/C++11? If yes I prefer to make that a separate commit.

AFAIK the [[noreturn]] conversion is a clang extension, should I file a DR for the standard? AFAIK there isn't one yet.

If you still like the patch could you commit it (except clang/test/CXX/conv/conv.fctptr/template-noreturn.cpp) since I don't have commit access?

[Mordante]

@rsmith I now have commit access so I can commit the patch. Any suggestions regarding the previous remark?

clang/lib/Sema/SemaTemplate.cpp

if (ParamType->isMemberPointerType()) {
       // the element type on the parameter could be more qualified than the
       // element type in the expression we constructed.
       bool ObjCLifetimeConversion;
       if (IsQualificationConversion(((Expr*) RefExpr.get())->getType(),
                                     ParamType.getUnqualifiedType(), false,
                                     ObjCLifetimeConversion))
         RefExpr = ImpCastExprToType(RefExpr.get(), ParamType.getUnqualifiedType(), CK_NoOp);
 
+      // Starting with C++17 the noexcept is part of the function signature but
+      // a noexcept function can be converted to a noexcept(false) function.
+      QualType ResultTy;
+      if (IsFunctionConversion(((Expr *)RefExpr.get())->getType(),
+                               ParamType.getUnqualifiedType(), ResultTy))
+        RefExpr = ImpCastExprToType(RefExpr.get(), ResultTy, CK_NoOp);
+
       assert(!RefExpr.isInvalid() &&
              Context.hasSameType(((Expr*) RefExpr.get())->getType(),
                                  ParamType.getUnqualifiedType()));
       return RefExpr;
     }
   }
 
   QualType T = VD->getType().getNonReferenceType();

clang/test/CXX/conv/conv.fctptr/template-noexcept.cpp

+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify %s
+
+// Starting with C++17 the noexcept is part of the function signature but
+// a noexcept function can be converted to a noexcept(false) function.
+// The tests were added for PR40024.
+
+#if __cplusplus < 201703L
+// expected-no-diagnostics
+#endif
+
+namespace valid_conversion {
+struct S {
+  int f(void) noexcept { return 110; }
+} s;
+
+template <int (S::*a)(void)>
+int f10(void) { return (s.*a)(); }
+
+int foo(void) {
+  return f10<&S::f>();
+}
+} // namespace valid_conversion
+
+namespace invalid_conversion {
+struct S {
+  int f(void) { return 110; }
+} s;
+
+#if __cplusplus >= 201703L
+// expected-note@+3 {{candidate template ignored: invalid explicitly-specified argument for template parameter 'a'}}
+#endif
+template <int (S::*a)(void) noexcept>
+int f10(void) { return (s.*a)(); }
+
+#if __cplusplus >= 201703L
+// expected-error@+3 {{no matching function for call to 'f10'}}
+#endif
+int foo(void) {
+  return f10<&S::f>();
+}
+} // namespace invalid_conversion

clang/test/CXX/conv/conv.fctptr/template-noreturn.cpp

+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify %s
+
+typedef int (*fp)();
+typedef int (*fpnr)() __attribute__((noreturn));
+#if __cplusplus < 201703L
+// expected-note@+2 {{template parameter is declared here}}
+#endif
+template <fp>
+struct FP {};
+
+#if __cplusplus < 201703L
+// expected-note@+2 {{template parameter is declared here}}
+#endif
+template <fpnr>
+struct FPNR {};
+
+int f();
+int fnr() __attribute__((noreturn));
+
+FP<&f> fp_valid;
+FPNR<&fnr> fpnr_valid;
+
+#if __cplusplus < 201703L
+// expected-error@+2 {{non-type template argument of type 'int (*)() __attribute__((noreturn))' cannot be converted to a value of type 'fp' (aka 'int (*)()')}}
+#endif
+FP<&fnr> fp_invalid_before_cxx17;
+
+#if __cplusplus < 201703L
+// expected-error@+4 {{non-type template argument of type 'int (*)()' cannot be converted to a value of type 'fpnr' (aka 'int (*)() __attribute__((noreturn))')}}
+#else
+// expected-error@+2 {{value of type 'int (*)()' is not implicitly convertible to 'fpnr' (aka 'int (*)() __attribute__((noreturn))')}}
+#endif
+FPNR<&f> fpnr_invalid;