[gjs: 6/18] jsapi-util: Use constexpr based checks on GjsAutoPointer functions

[Philip Chimento <pchimento src gnome org>]

commit c443c6592ed2f3c3cbec5de929d5276e2de9a6ae
Author: Marco Trevisan (Treviño) <mail 3v1n0 net>
Date:   Thu Oct 1 16:56:35 2020 +0200

    jsapi-util: Use constexpr based checks on GjsAutoPointer functions
    
    We had FIXME's about this for some time now, but we had a quite nice way
    to check whether we're handling nullptr functions at compile time, so
    that we can produce safe code with less dynamic checks:
    
     - Previously we were trying to use `if constexpr (ref_func != nullptr)`
     - The error we were getting was because we were trying to compare
       a template value (that will be evaluated at link time) with
       a null pointer (that yes, is a constant value).
     - However, C++ provides us since the long C++11 times a nice tool that
       allows to generate a type out from any constant value and that we can
       do use it to do constexpr checks.
     - Thus for each template function parameter we can just compute its
       integral constant type, and ensure that it's not matching null.
    
    Under the hood this will generate such types for examples:
    
      for g_object_ref:
        std::integral_constant<GObject* (*)(GObject*), g_object_ref>;
    
      for nullptr:
        std::integral_constant<GObject* (*)(GObject*), 0>
    
    And those types are indeed different.
    
    Added few tests, although to test invalid types we should add some
    no-compilation tests.

 gjs/jsapi-util.h              | 62 ++++++++++++++++++++++++++++---------------
 test/gjs-test-jsapi-utils.cpp | 17 ++++++++++++
 2 files changed, 57 insertions(+), 22 deletions(-)
---
diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h
index 61bce271..c71e8cfc 100644
--- a/gjs/jsapi-util.h
+++ b/gjs/jsapi-util.h
@@ -36,14 +36,39 @@ class CallArgs;
 
 struct GjsAutoTakeOwnership {};
 
-template <typename T, typename F = void, void (*free_func)(F*) = free,
-          F* (*ref_func)(F*) = nullptr>
+template <typename F = void>
+using GjsAutoPointerRefFunction = F* (*)(F*);
+
+template <typename F = void>
+using GjsAutoPointerFreeFunction = void (*)(F*);
+
+template <typename T, typename F = void,
+          GjsAutoPointerFreeFunction<F> free_func = free,
+          GjsAutoPointerRefFunction<F> ref_func = nullptr>
 struct GjsAutoPointer {
     using Tp =
         std::conditional_t<std::is_array_v<T>, std::remove_extent_t<T>, T>;
     using Ptr = std::add_pointer_t<Tp>;
     using ConstPtr = std::add_pointer_t<std::add_const_t<Tp>>;
 
+ private:
+    template <typename FunctionType, FunctionType function>
+    static constexpr bool has_function() {
+        using NullType = std::integral_constant<FunctionType, nullptr>;
+        using ActualType = std::integral_constant<FunctionType, function>;
+
+        return !std::is_same_v<ActualType, NullType>;
+    }
+
+ public:
+    static constexpr bool has_free_function() {
+        return has_function<GjsAutoPointerFreeFunction<F>, free_func>();
+    }
+
+    static constexpr bool has_ref_function() {
+        return has_function<GjsAutoPointerRefFunction<F>, ref_func>();
+    }
+
     constexpr GjsAutoPointer(Ptr ptr = nullptr)  // NOLINT(runtime/explicit)
         : m_ptr(ptr) {}
     template <typename U = T,
@@ -51,10 +76,6 @@ struct GjsAutoPointer {
     explicit constexpr GjsAutoPointer(Tp ptr[]) : m_ptr(ptr) {}
     constexpr GjsAutoPointer(Ptr ptr, const GjsAutoTakeOwnership&)
         : GjsAutoPointer(ptr) {
-        // FIXME: should use if constexpr (...), but that doesn't work with
-        // ubsan, which generates a null pointer check making it not a constexpr
-        // anymore: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71962 - Also a
-        // bogus warning, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94554
         m_ptr = copy();
     }
     constexpr GjsAutoPointer(ConstPtr ptr, const GjsAutoTakeOwnership& o)
@@ -109,15 +130,16 @@ struct GjsAutoPointer {
     }
 
     constexpr void reset(Ptr ptr = nullptr) {
-        // FIXME: Should use if constexpr (...) as above
-        auto ffunc = free_func;
         Ptr old_ptr = m_ptr;
         m_ptr = ptr;
-        if (old_ptr && ffunc) {
-            if constexpr (std::is_array_v<T>)
-                ffunc(reinterpret_cast<T*>(old_ptr));
-            else
-                ffunc(old_ptr);
+
+        if constexpr (has_free_function()) {
+            if (old_ptr) {
+                if constexpr (std::is_array_v<T>)
+                    free_func(reinterpret_cast<T*>(old_ptr));
+                else
+                    free_func(old_ptr);
+            }
         }
     }
 
@@ -132,13 +154,8 @@ struct GjsAutoPointer {
     template <typename U = T>
     [[nodiscard]] constexpr std::enable_if_t<!std::is_array_v<U>, Ptr> copy()
         const {
-        // FIXME: Should use std::enable_if_t<ref_func != nullptr, Ptr>
-        if (!m_ptr)
-            return nullptr;
-
-        auto rf = ref_func;
-        g_assert(rf);
-        return reinterpret_cast<Ptr>(ref_func(m_ptr));
+        static_assert(has_ref_function(), "No ref function provided");
+        return m_ptr ? reinterpret_cast<Ptr>(ref_func(m_ptr)) : nullptr;
     }
 
     template <typename C>
@@ -150,8 +167,9 @@ struct GjsAutoPointer {
     Ptr m_ptr;
 };
 
-template <typename T, typename F = void, void (*free_func)(F*) = free,
-          F* (*ref_func)(F*) = nullptr>
+template <typename T, typename F = void,
+          GjsAutoPointerFreeFunction<F> free_func,
+          GjsAutoPointerRefFunction<F> ref_func>
 constexpr bool operator==(
     GjsAutoPointer<T, F, free_func, ref_func> const& lhs,
     GjsAutoPointer<T, F, free_func, ref_func> const& rhs) {
diff --git a/test/gjs-test-jsapi-utils.cpp b/test/gjs-test-jsapi-utils.cpp
index 0a2ab493..1974a51a 100644
--- a/test/gjs-test-jsapi-utils.cpp
+++ b/test/gjs-test-jsapi-utils.cpp
@@ -8,6 +8,7 @@
 #include <glib-object.h>
 #include <glib.h>
 #include <stddef.h>  // for NULL
+#include <type_traits>  // for remove_reference<>::type
 #include <utility>   // for move, swap
 
 #include "gjs/jsapi-util.h"
@@ -174,6 +175,17 @@ static void test_gjs_autopointer_dtor_take_ownership() {
     g_object_unref(ptr);
 }
 
+static void test_gjs_autopointer_dtor_default_free() {
+    GjsAutoPointer<char, void> autoptr(g_strdup("Please, FREE ME!"));
+    g_assert_cmpstr(autoptr, ==, "Please, FREE ME!");
+}
+
+static void test_gjs_autopointer_dtor_no_free_pointer() {
+    const char* str = "DO NOT FREE ME";
+    GjsAutoPointer<char, void, nullptr> autoptr(const_cast<char*>(str));
+    g_assert_cmpstr(autoptr, ==, "DO NOT FREE ME");
+}
+
 static void test_gjs_autopointer_assign_operator() {
     GjsAutoTestObject autoptr;
     auto* ptr = gjs_test_object_new();
@@ -555,6 +567,11 @@ void gjs_test_add_tests_for_jsapi_utils(void) {
     g_test_add_func(
         "/gjs/jsapi-utils/gjs-autopointer/destructor/take_ownership",
         test_gjs_autopointer_dtor_take_ownership);
+    g_test_add_func("/gjs/jsapi-utils/gjs-autopointer/destructor/default_free",
+                    test_gjs_autopointer_dtor_default_free);
+    g_test_add_func(
+        "/gjs/jsapi-utils/gjs-autopointer/destructor/no_free_pointer",
+        test_gjs_autopointer_dtor_no_free_pointer);
     g_test_add_func("/gjs/jsapi-utils/gjs-autopointer/destructor/c++",
                     test_gjs_autopointer_dtor_cpp);
     g_test_add_func("/gjs/jsapi-utils/gjs-autopointer/destructor/c++-array",