[libc++] Allow assigning std::rethrow_if_nested() to a function pointer

[xingxue]

Summary

This patch fixes the error with assigning std::rethrow_if_nested() to a function pointer. The fix is similar to what is done for std::throw_with_nested().

The following test case demonstrates the issue.

#include <exception>

int main()
{
  struct A {};
  void (*p)(const A&) = &std::rethrow_if_nested<A>;
}

[Mordante]

I'm not too familiar with this code so I won't approve it. I wonder whether we want to allow taking the address of this function.

libcxx/test/std/language.support/support.exception/except.nested/call_thru_funcptr.pass.cpp

7 ↗

(On Diff #446510)

please don't use thru in the filename, use the complete word.

35 ↗

(On Diff #446510)

Some of our supported platforms require this.

39 ↗

(On Diff #446510)

Is there wording in the Standard allowing this?

[xingxue]

Addressed comments.

• Renamed test case to call_through_funcptr.pass.cpp;
• Define main as 'main(int, char**)`;
• Formatting changes.

[xingxue]

In D130283#3669617, @Mordante wrote:

I'm not too familiar with this code so I won't approve it. I wonder whether we want to allow taking the address of this function.

From this document namespace.std, it seems it is allowed to take the address of std::rethrow_if_nested. @ldionne, @hubert.reinterpretcast, please correct me if my interpretation is wrong. Thanks!

Let F denote a standard library function ([global.functions]), a standard library static member function, or an instantiation of a standard library function template.
Unless F is designated an addressable function, the behavior of a C++ program is unspecified (possibly ill-formed) if it explicitly or implicitly attempts to form a pointer to F.
[Note 1: Possible means of forming such pointers include application of the unary & operator ([expr.unary.op]), addressof ([specialized.addressof]), or a function-to-pointer standard conversion ([conv.func]).
— end note]
Moreover, the behavior of a C++ program is unspecified (possibly ill-formed) if it attempts to form a reference to F or if it attempts to form a pointer-to-member designating either a standard library non-static member function ([member.functions]) or an instantiation of a standard library member function template.

[Mordante]

In D130283#3672185, @xingxue wrote:

In D130283#3669617, @Mordante wrote:

I'm not too familiar with this code so I won't approve it. I wonder whether we want to allow taking the address of this function.

From this document namespace.std, it seems it is allowed to take the address of std::rethrow_if_nested. @ldionne, @hubert.reinterpretcast, please correct me if my interpretation is wrong. Thanks!

Let F denote a standard library function ([global.functions]), a standard library static member function, or an instantiation of a standard library function template.
Unless F is designated an addressable function, the behavior of a C++ program is unspecified (possibly ill-formed) if it explicitly or implicitly attempts to form a pointer to F.
[Note 1: Possible means of forming such pointers include application of the unary & operator ([expr.unary.op]), addressof ([specialized.addressof]), or a function-to-pointer standard conversion ([conv.func]).
— end note]
Moreover, the behavior of a C++ program is unspecified (possibly ill-formed) if it attempts to form a reference to F or if it attempts to form a pointer-to-member designating either a standard library non-static member function ([member.functions]) or an instantiation of a standard library member function template.

Did I overlook something and is std::rethrow_if_nested marked as addressable?

[xingxue]

Did I overlook something and is std::rethrow_if_nested marked as addressable?

Hi @Mordante, Thanks for your comments. I searched the standards and could not find any thing that is marked as addressable. I can be wrong but my reasoning is that rethrow_if_nested() is a global function that has a physical implementation and a physical address. Therefore, it is addressable. The following code snippet and 'nm' output show it has a physical address, noting that the signature of rethrow_if_nested() has __enable_if etc. as an argument and that is the reason why it does not match the signature defined by the standard, i.e., void rethrow_if_nested(const E&), when assigned to a function pointer. throw_with_nested() used to have the same problem but was fixed. This patch takes the same approach as used for throw_with_nested().

$ cat t.cpp:
#include <exception>
int main(int, char**)
{
    int i=5;
    std::rethrow_if_nested(i);
    return 0;
}
$ nm t.o
0000000000000000 T main
                 U .TOC.
0000000000000000 W _ZSt17rethrow_if_nestedB6v15000IiEvRKT_PNSt3__19enable_ifIXntsr18__can_dynamic_castIS0_St16nested_exceptionEE5valueEvE4typeE

[Mordante]

In D130283#3676514, @xingxue wrote:

Did I overlook something and is std::rethrow_if_nested marked as addressable?

Hi @Mordante, Thanks for your comments. I searched the standards and could not find any thing that is marked as addressable.

These should be some things marked, IIRC mainly in <iostream>.

I can be wrong but my reasoning is that rethrow_if_nested() is a global function that has a physical implementation and a physical address. Therefore, it is addressable. The following code snippet and 'nm' output show it has a physical address, noting that the signature of rethrow_if_nested() has __enable_if etc. as an argument and that is the reason why it does not match the signature defined by the standard, i.e., void rethrow_if_nested(const E&), when assigned to a function pointer. throw_with_nested() used to have the same problem but was fixed. This patch takes the same approach as used for throw_with_nested().

True, it's addressable now. But this isn't guaranteed by the Standard, hence the question do we want to guarantee this works with libc++. If we do we probably should document that as an extension. Since this isn't Standard behaviour the test should be moved from std to libcxx since this might fail on other implementations.

But let's wait and see what @ldionne thinks about adding this guarantee.

[xingxue]

Hi @ldionne, What do you think about this? The affected test case works with GCC.

[hubert.reinterpretcast]

I agree that this function is not defined as addressable in the Standard. My interpretation is that "addressable" is being used by the standard as a specification tool and whether a function is "addressable" comes only from the Standard itself.

[philnik]

e.g. http://eel.is/c++draft/fmtflags.manip#1 specifies that functions are addressable. rethrow_if_nested isn't addressable AFAIK, so I don't think we want to provide that guarantee.

[xingxue]

Thanks very much, @hubert.reinterpretcast @philnik @Mordante for your insights! Abandoning this patch since rethrow_if_nested() is not specified as addressable in the Standard.

libcxx/include/exception

 }
 
 template <class _From, class _To>
 struct __can_dynamic_cast : _BoolConstant<
               is_polymorphic<_From>::value &&
                  (!is_base_of<_To, _From>::value ||
                    is_convertible<const _From*, const _To*>::value)> {};
 
+template <class _Ep, bool>
+struct __rethrow_if_nested;
+
 template <class _Ep>
-inline _LIBCPP_INLINE_VISIBILITY
-void
-rethrow_if_nested(const _Ep& __e,
-                  __enable_if_t< __can_dynamic_cast<_Ep, nested_exception>::value>* = 0)
-{
+struct __rethrow_if_nested<_Ep, true> {
+  static inline _LIBCPP_INLINE_VISIBILITY void __do_rethrow(const _Ep& __e) {
     const nested_exception* __nep = dynamic_cast<const nested_exception*>(_VSTD::addressof(__e));
     if (__nep)
-        __nep->rethrow_nested();
   }
+};
 
 template <class _Ep>
-inline _LIBCPP_INLINE_VISIBILITY
-void
-rethrow_if_nested(const _Ep&,
-                  __enable_if_t<!__can_dynamic_cast<_Ep, nested_exception>::value>* = 0)
-{
+struct __rethrow_if_nested<_Ep, false> {
+  static inline _LIBCPP_INLINE_VISIBILITY void __do_rethrow(const _Ep&) {}
+};
+
+template <class _Ep>
+inline _LIBCPP_INLINE_VISIBILITY void rethrow_if_nested(const _Ep& __e) {
+  __rethrow_if_nested<_Ep, __can_dynamic_cast<_Ep, nested_exception>::value>::__do_rethrow(__e);
 }
 
 } // namespace std
 
 #endif // _LIBCPP_EXCEPTION

libcxx/test/std/language.support/support.exception/except.nested/call_through_funcptr.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// Check to call std::throw_with_nested or std::rethrow_if_nested
+// througn a function pointer.
+
+// UNSUPPORTED: no-exceptions
+
+// FIXME: This test fails in MSVC mode due to a stack overflow
+// XFAIL: msvc
+
+#include <exception>
+#include <cassert>
+
+#include "test_macros.h"
+
+class A {
+  int data_;
+
+public:
+  explicit A(int data) : data_(data) {}
+  virtual ~A() TEST_NOEXCEPT {}
+
+  friend bool operator==(const A& x, const A& y) { return x.data_ == y.data_; }
+};
+
+class B : public std::nested_exception, public A {
+public:
+  explicit B(int data) : A(data) {}
+  B(const B& b) : A(b) {}
+};
+
+int main(int, char**) {
+  {
+    try {
+      B b(6);
+      void (*p)(B&) = &std::throw_with_nested;
+      p(b);
+      assert(false);
+    } catch (const std::nested_exception& e) {
+      assert(e.nested_ptr() == nullptr);
+      const B& b = dynamic_cast<const B&>(e);
+      assert(b == B(6));
+    }
+  }
+
+  {
+    try {
+      throw B(5);
+    } catch (const B& b) {
+      try {
+        throw b;
+      } catch (const A& a) {
+        try {
+          void (*p)(const A&) = &std::rethrow_if_nested<A>;
+          p(a);
+          assert(false);
+        } catch (const B& b2) {
+          assert(b2 == B(5));
+        }
+      }
+    }
+  }
+  return 0;
+}