Avoid compiler warnings from function pointer conversions

gcc8 -Wextra prints a warning when a single reinterpret_cast is used for
conversion between different types of function pointers. The previous fix
with a union in Glib::bitwise_equivalent_cast<>() is not standard C++.
Rename the function to Glib::function_pointer_cast<>(), and use two
reinterpret_casts as recommended in gcc's documentation.

* glib/src/optiongroup.ccg: Use a reinterpret_cast to convert from
a function pointer to void*. That's possible now. It's "conditionally
supported", starting with C++11.

See https://github.com/libsigcplusplus/libsigcplusplus/issues/8

5 files changed, 32 insertions, 31 deletions

diff --git a/gio/giomm/socketsource.cc b/gio/giomm/socketsource.cc
index 934ca004..497a27c8 100644
--- a/gio/giomm/socketsource.cc
+++ b/gio/giomm/socketsource.cc
@@ -74,7 +74,7 @@ SignalSocket::connect(const sigc::slot<bool, Glib::IOCondition>& slot,
     g_socket_create_source(socket->gobj(), (GIOCondition)condition, Glib::unwrap(cancellable));
   return Glib::Source::attach_signal_source(
     slot, priority, source, context_,
-    Glib::bitwise_equivalent_cast<GSourceFunc>(&giomm_signalsocket_callback));
+    Glib::function_pointer_cast<GSourceFunc>(&giomm_signalsocket_callback));
 }
 
 SignalSocket
@@ -97,7 +97,7 @@ SocketSource::SocketSource(const Glib::RefPtr<Socket>& socket, Glib::IOCondition
   const Glib::RefPtr<Cancellable>& cancellable)
 : IOSource(
     g_socket_create_source(socket->gobj(), (GIOCondition)condition, Glib::unwrap(cancellable)),
-    Glib::bitwise_equivalent_cast<GSourceFunc>(&giomm_socketsource_callback))
+    Glib::function_pointer_cast<GSourceFunc>(&giomm_socketsource_callback))
 {
 }
 
diff --git a/glib/glibmm/main.cc b/glib/glibmm/main.cc
index ff609a43..cffa0e0a 100644
--- a/glib/glibmm/main.cc
+++ b/glib/glibmm/main.cc
@@ -541,7 +541,7 @@ SignalChildWatch::connect(const sigc::slot<void, GPid, int>& slot, GPid pid, int
   if (priority != G_PRIORITY_DEFAULT)
     g_source_set_priority(source, priority);
 
-  g_source_set_callback(source, Glib::bitwise_equivalent_cast<GSourceFunc>(&glibmm_child_watch_callback),
+  g_source_set_callback(source, Glib::function_pointer_cast<GSourceFunc>(&glibmm_child_watch_callback),
     conn_node, &SourceConnectionNode::destroy_notify_callback);
 
   conn_node->install(source);
@@ -1310,7 +1310,7 @@ IOSource::IOSource(PollFD::fd_t fd, IOCondition condition) : poll_fd_(fd, condit
 
 IOSource::IOSource(const Glib::RefPtr<IOChannel>& channel, IOCondition condition)
 : Source(g_io_create_watch(channel->gobj(), (GIOCondition)condition),
-    Glib::bitwise_equivalent_cast<GSourceFunc>(&glibmm_iosource_callback))
+    Glib::function_pointer_cast<GSourceFunc>(&glibmm_iosource_callback))
 {
 }
 
diff --git a/glib/glibmm/utility.h b/glib/glibmm/utility.h
index 39c74a9f..0f1e9ef6 100644
--- a/glib/glibmm/utility.h
+++ b/glib/glibmm/utility.h
@@ -161,20 +161,23 @@ destroy_notify_delete(void* data)
 // Conversion between different types of function pointers with
 // reinterpret_cast can make gcc8 print a warning.
 // https://github.com/libsigcplusplus/libsigcplusplus/issues/1
-/** Returns the supplied bit pattern, interpreted as another type.
+// https://github.com/libsigcplusplus/libsigcplusplus/issues/8
+/** Returns the supplied function pointer, cast to a pointer to another function type.
  *
- * When reinterpret_cast causes a compiler warning or error, this function
- * may work. Intended mainly for conversion between different types of pointers.
+ * When a single reinterpret_cast between function pointer types causes a
+ * compiler warning or error, this function may work.
+ *
+ * Qualify calls with namespace names: sigc::internal::function_pointer_cast<>().
+ * If you don't, indirect calls from another library that also contains a
+ * function_pointer_cast<>() (perhaps glibmm), can be ambiguous due to ADL
+ * (argument-dependent lookup).
  */
 template <typename T_out, typename T_in>
-inline T_out bitwise_equivalent_cast(T_in in)
+inline T_out function_pointer_cast(T_in in)
 {
-  union {
-    T_in in;
-    T_out out;
-  } u;
-  u.in = in;
-  return u.out;
+  // The double reinterpret_cast suppresses a warning from gcc8 with the
+  // -Wcast-function-type option.
+  return reinterpret_cast<T_out>(reinterpret_cast<void (*)()>(in));
 }
 
 } // namespace Glib
diff --git a/glib/src/optiongroup.ccg b/glib/src/optiongroup.ccg
index 483e8354..f45c5a96 100644
--- a/glib/src/optiongroup.ccg
+++ b/glib/src/optiongroup.ccg
@@ -571,22 +571,20 @@ OptionGroup::CppOptionEntry::allocate_c_arg()
   case G_OPTION_ARG_CALLBACK:
   {
     // The C arg pointer is a function pointer, cast to void*.
-    carg_ = Glib::bitwise_equivalent_cast<void*>(&OptionGroup::option_arg_callback);
-
-    // With all compiler warnings turned on and a high optimization level
-    // it's difficult to cast a function pointer to a void*. See bug 589197.
-    // A few results with g++ 4.4.5 with the flags -pedantic -O2 -Werror:
-    //
-    // carg_ = reinterpret_cast<void*>(&OptionGroup::option_arg_callback);
-    // error: ISO C++ forbids casting between pointer-to-function and pointer-to-object
-    //
-    // *reinterpret_cast<GOptionArgFunc*>(&carg_) = &OptionGroup::option_arg_callback;
-    // *(OptionArgFunc*)&carg_ = &OptionGroup::option_arg_callback;
-    // error: dereferencing type-punned pointer will break strict-aliasing rules
     //
-    // If any compiler dislikes the union, the following code is worth testing:
-    // carg_ = reinterpret_cast<void*>(
-    //         reinterpret_cast<unsigned long>(&OptionGroup::option_arg_callback));
+    //   carg_ = reinterpret_cast<void*>(&OptionGroup::option_arg_callback);
+    // or
+    //   union {
+    //     void* dp;
+    //     GOptionArgFunc fp;
+    //   } u;
+    //   u.fp = &OptionGroup::option_arg_callback;
+    //   carg_ = u.dp;
+    // ? See
+    // https://bugzilla.gnome.org/show_bug.cgi?id=589197
+    // https://github.com/libsigcplusplus/libsigcplusplus/issues/1
+    // https://github.com/libsigcplusplus/libsigcplusplus/issues/8
+    carg_ = reinterpret_cast<void*>(&OptionGroup::option_arg_callback);
 
     break;
   }
diff --git a/tests/glibmm_vector/main.cc b/tests/glibmm_vector/main.cc
index dd83553e..a3719532 100644
--- a/tests/glibmm_vector/main.cc
+++ b/tests/glibmm_vector/main.cc
@@ -198,12 +198,12 @@ public:
   {
     if (glist_)
     {
-      g_list_foreach(glist_, Glib::bitwise_equivalent_cast<GFunc>(g_object_unref), nullptr);
+      g_list_foreach(glist_, Glib::function_pointer_cast<GFunc>(g_object_unref), nullptr);
       g_list_free(glist_);
     }
     if (gslist_)
     {
-      g_slist_foreach(gslist_, Glib::bitwise_equivalent_cast<GFunc>(g_object_unref), nullptr);
+      g_slist_foreach(gslist_, Glib::function_pointer_cast<GFunc>(g_object_unref), nullptr);
       g_slist_free(gslist_);
     }
     if (garray_)