[PATCH] D64874: [Sema] Improve handling of function pointer conversions
Mordante added a comment. @rsmith I now have commit access so I can commit the patch. Any suggestions regarding the previous remark? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64874/new/ https://reviews.llvm.org/D64874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D64874: [Sema] Improve handling of function pointer conversions
Mordante updated this revision to Diff 224753.
Mordante added a comment.
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?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64874/new/
https://reviews.llvm.org/D64874
Files:
clang/lib/Sema/SemaTemplate.cpp
clang/test/CXX/conv/conv.fctptr/template-noexcept.cpp
clang/test/CXX/conv/conv.fctptr/template-noreturn.cpp
Index: clang/test/CXX/conv/conv.fctptr/template-noreturn.cpp
===
--- /dev/null
+++ clang/test/CXX/conv/conv.fctptr/template-noreturn.cpp
@@ -0,0 +1,36 @@
+// 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
+struct FP {};
+
+#if __cplusplus < 201703L
+// expected-note@+2 {{template parameter is declared here}}
+#endif
+template
+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;
Index: clang/test/CXX/conv/conv.fctptr/template-noexcept.cpp
===
--- /dev/null
+++ clang/test/CXX/conv/conv.fctptr/template-noexcept.cpp
@@ -0,0 +1,44 @@
+// 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 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 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
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -6991,6 +6991,13 @@
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()));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D64874: [Sema] Improve handling of function pointer conversions
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM, though I'd like to see this generalized to handle noreturn too.
Comment at: clang/lib/Sema/SemaTemplate.cpp:6997
+ QualType ResultTy;
+ if (getLangOpts().CPlusPlus17 &&
+ IsFunctionConversion(((Expr *)RefExpr.get())->getType(),
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 struct X {};
int f() __attribute__((noreturn));
X<&f> x;
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64874/new/
https://reviews.llvm.org/D64874
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D64874: [Sema] Improve handling of function pointer conversions
Mordante updated this revision to Diff 218215.
Mordante added a comment.
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
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64874/new/
https://reviews.llvm.org/D64874
Files:
clang/lib/Sema/SemaTemplate.cpp
clang/test/CXX/conv/conv.fctptr/template-noexcept.cpp
Index: clang/test/CXX/conv/conv.fctptr/template-noexcept.cpp
===
--- /dev/null
+++ clang/test/CXX/conv/conv.fctptr/template-noexcept.cpp
@@ -0,0 +1,44 @@
+// 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 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 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
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -6991,6 +6991,14 @@
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 (getLangOpts().CPlusPlus17 &&
+ 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()));
Index: clang/test/CXX/conv/conv.fctptr/template-noexcept.cpp
===
--- /dev/null
+++ clang/test/CXX/conv/conv.fctptr/template-noexcept.cpp
@@ -0,0 +1,44 @@
+// 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 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 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
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -6991,6 +6991,14 @@
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 (getLangOpts().CPlusPlus17 &&
+ IsFunctionConversion(((Expr *)RefExpr.get())->getType(),
+ ParamType.getUnqualifiedType(), ResultTy))
+RefExpr = ImpCastExprToType(RefExpr.get(), ResultTy,
[PATCH] D64874: [Sema] Improve handling of function pointer conversions
rsmith added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:6996 + // a noexcept function can be converted to a noexcept(false) function. + QualType resultTy; + if (getLangOpts().CPlusPlus17 && Please capitalize local variable names to match the local convention. Comment at: clang/test/CXX/conv/conv.fctptr/template-noexcept-invalid.cpp:8 +// a noexcept function can be converted to a noexcept(false) function. +// The tests were added for https://bugs.llvm.org/show_bug.cgi?id=40024 +// Tests the no longer valid function pointer conversions 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.) Comment at: clang/test/CXX/conv/conv.fctptr/template-noexcept-invalid.cpp:22 + +// CHECK: error: no matching function for call to 'f10' Please use a `-verify` test for this instead of `FileCheck`, and combine the `-valid` and `-invalid` testcases into the same test file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64874/new/ https://reviews.llvm.org/D64874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
