[gjs: 4/9] arg-cache: Create Return and Instance arguments only if we have one

[Philip Chimento <pchimento src gnome org>]

commit 5583be8ab231ef2e2eb52f22126ac1114af9e7b2
Author: Marco Trevisan (Treviño) <mail 3v1n0 net>
Date:   Fri Oct 23 21:47:02 2020 +0200

    arg-cache: Create Return and Instance arguments only if we have one
    
    This is something that also happened in the original cache implementation,
    but only for the instance parameter.
    
    Now, given that we handle differently the "skip-all" (AKA void) return
    value, we can also avoid to allocate memory for it, if there is none.
    
    This saves some more space compared to the previous iteration on the
    cache itself, but also make things safer as the access to the elements
    is protected from the outside and we use a constexpr function to get the
    actual element index, without having to guess much.

 gi/arg-cache.cpp | 59 ++++++++++++++++++++++++++++++++++----------------------
 gi/arg-cache.h   | 37 ++++++++++++++++++++++++++---------
 gi/function.cpp  |  4 +---
 3 files changed, 65 insertions(+), 35 deletions(-)
---
diff --git a/gi/arg-cache.cpp b/gi/arg-cache.cpp
index a838643d..67eb60af 100644
--- a/gi/arg-cache.cpp
+++ b/gi/arg-cache.cpp
@@ -323,6 +323,8 @@ struct GenericOut : GenericInOut {
             JS::HandleValue) override;
     bool release(JSContext*, GjsFunctionCallState*, GIArgument*,
                  GIArgument*) override;
+
+    const ReturnValue* as_return_value() const override { return this; }
 };
 
 struct GenericReturn : ReturnValue {
@@ -439,6 +441,8 @@ struct Instance : NullableIn {
         return skip();
     }
 
+    const Instance* as_instance() const override { return this; }
+
     //  The instance GType can be useful only in few cases such as GObjects and
     //  GInterfaces, so we don't store it by default, unless needed.
     //  See Function's code to see where this is relevant.
@@ -1525,7 +1529,16 @@ bool ArgsCache::initialize(JSContext* cx, GICallableInfo* callable) {
         return false;
     }
 
+    GITypeInfo type_info;
+    g_callable_info_load_return_type(callable, &type_info);
+
+    m_has_return = g_type_info_get_tag(&type_info) != GI_TYPE_TAG_VOID ||
+                   g_type_info_is_pointer(&type_info);
+    m_is_method = !!g_callable_info_is_method(callable);
+
     int size = g_callable_info_get_n_args(callable);
+    size += (m_is_method ? 1 : 0);
+    size += (m_has_return ? 1 : 0);
 
     if (size > Argument::MAX_ARGS) {
         gjs_throw(cx,
@@ -1540,8 +1553,6 @@ bool ArgsCache::initialize(JSContext* cx, GICallableInfo* callable) {
 }
 
 void ArgsCache::clear() {
-    m_return.reset();
-    m_instance.reset();
     m_args.reset();
 }
 
@@ -1551,15 +1562,8 @@ T* ArgsCache::set_argument(uint8_t index, const char* name,
                            GjsArgumentFlags flags, Args&&... args) {
     std::unique_ptr<T> arg = Argument::make<T, ArgKind>(
         index, name, type_info, transfer, flags, args...);
-    T* arg_ptr = arg.get();
-    if constexpr (ArgKind == Arg::Kind::RETURN_VALUE) {
-        m_return.reset(static_cast<Arg::ReturnValue*>(arg.release()));
-    } else if constexpr (ArgKind == Arg::Kind::INSTANCE) {
-        m_instance.reset(static_cast<Arg::Instance*>(arg.release()));
-    } else {
-        m_args[index] = std::move(arg);
-    }
-    return arg_ptr;
+    arg_get<ArgKind>(index) = std::move(arg);
+    return static_cast<T*>(arg_get<ArgKind>(index).get());
 }
 
 template <typename T, Arg::Kind ArgKind, typename... Args>
@@ -1600,25 +1604,32 @@ T* ArgsCache::set_instance(GITransfer transfer, GjsArgumentFlags flags) {
 }
 
 Argument* ArgsCache::instance() const {
-    return static_cast<Argument*>(m_instance.get());
+    if (!m_is_method)
+        return nullptr;
+
+    return arg_get<Arg::Kind::INSTANCE>().get();
 }
 
 GType ArgsCache::instance_type() const {
-    if (!m_instance)
+    if (!m_is_method)
         return G_TYPE_NONE;
 
-    return m_instance->gtype();
+    return instance()->as_instance()->gtype();
 }
 
 Argument* ArgsCache::return_value() const {
-    return static_cast<Argument*>(m_return.get());
+    if (!m_has_return)
+        return nullptr;
+
+    return arg_get<Arg::Kind::RETURN_VALUE>().get();
 }
 
 GITypeInfo* ArgsCache::return_type() const {
-    if (!m_return || m_return->skip_out())
+    Argument* rval = return_value();
+    if (!rval)
         return nullptr;
 
-    return &m_return->m_type_info;
+    return const_cast<GITypeInfo*>(rval->as_return_value()->type_info());
 }
 
 void ArgsCache::set_skip_all(uint8_t index, const char* name) {
@@ -1672,17 +1683,16 @@ void ArgsCache::set_array_argument(GICallableInfo* callable, uint8_t gi_index,
 void ArgsCache::build_return(GICallableInfo* callable, bool* inc_counter_out) {
     g_assert(inc_counter_out && "forgot out parameter");
 
+    if (!m_has_return) {
+        *inc_counter_out = false;
+        return;
+    }
+
     GITypeInfo type_info;
     g_callable_info_load_return_type(callable, &type_info);
     GITransfer transfer = g_callable_info_get_caller_owns(callable);
     GITypeTag tag = g_type_info_get_tag(&type_info);
 
-    if (tag == GI_TYPE_TAG_VOID && !g_type_info_is_pointer(&type_info)) {
-        *inc_counter_out = false;
-        m_return.reset();
-        return;
-    }
-
     *inc_counter_out = true;
     GjsArgumentFlags flags = GjsArgumentFlags::SKIP_IN;
 
@@ -2009,6 +2019,9 @@ void ArgsCache::build_normal_in_arg(uint8_t gi_index, GITypeInfo* type_info,
 }
 
 void ArgsCache::build_instance(GICallableInfo* callable) {
+    if (!m_is_method)
+        return;
+
     GIBaseInfo* interface_info = g_base_info_get_container(callable);  // !owned
 
     GITransfer transfer =
diff --git a/gi/arg-cache.h b/gi/arg-cache.h
index 1560c42a..94410aac 100644
--- a/gi/arg-cache.h
+++ b/gi/arg-cache.h
@@ -11,6 +11,7 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include <limits>
 #include <memory>
 
 #include <girepository.h>
@@ -80,14 +81,11 @@ struct Argument {
         return flags;
     }
 
-    // Introspected functions can have up to 253 arguments. 255 is a placeholder
-    // for the return value and 254 for the instance parameter. The callback
+    // Introspected functions can have up to 253 arguments. The callback
     // closure or destroy notify parameter may have a value of 255 to indicate
     // that it is absent.
-    static constexpr uint8_t MAX_ARGS = 253;
-    static constexpr uint8_t INSTANCE_PARAM = 254;
-    static constexpr uint8_t RETURN_VALUE = 255;
-    static constexpr uint8_t ABSENT = 255;
+    static constexpr uint8_t MAX_ARGS = std::numeric_limits<uint8_t>::max() - 2;
+    static constexpr uint8_t ABSENT = std::numeric_limits<uint8_t>::max();
 
     constexpr const char* arg_name() const { return m_arg_name; }
 
@@ -98,6 +96,9 @@ struct Argument {
  protected:
     Argument() : m_skip_in(false), m_skip_out(false) {}
 
+    virtual const Arg::ReturnValue* as_return_value() const { return nullptr; }
+    virtual const Arg::Instance* as_instance() const { return nullptr; }
+
     void set_instance_parameter() {
         m_arg_name = "instance parameter";
         m_skip_out = true;
@@ -156,7 +157,7 @@ struct ArgsCache {
 
     void build_instance(GICallableInfo* callable);
 
-    Argument* argument(uint8_t index) const { return m_args[index].get(); }
+    Argument* argument(uint8_t index) const { return arg_get(index).get(); }
 
     Argument* instance() const;
     GType instance_type() const;
@@ -204,10 +205,28 @@ struct ArgsCache {
 
     void set_skip_all(uint8_t index, const char* name = nullptr);
 
+    template <Arg::Kind ArgKind = Arg::Kind::NORMAL>
+    constexpr uint8_t arg_index(uint8_t index
+                                [[maybe_unused]] = Argument::MAX_ARGS) const {
+        if constexpr (ArgKind == Arg::Kind::RETURN_VALUE)
+            return 0;
+        else if constexpr (ArgKind == Arg::Kind::INSTANCE)
+            return (m_has_return ? 1 : 0);
+        else if constexpr (ArgKind == Arg::Kind::NORMAL)
+            return (m_has_return ? 1 : 0) + (m_is_method ? 1 : 0) + index;
+    }
+
+    template <Arg::Kind ArgKind = Arg::Kind::NORMAL>
+    inline Argument::UniquePtr& arg_get(
+        uint8_t index = Argument::MAX_ARGS) const {
+        return m_args[arg_index<ArgKind>(index)];
+    }
+
  private:
     std::unique_ptr<Argument::UniquePtr[]> m_args;
-    std::unique_ptr<Arg::ReturnValue> m_return;
-    std::unique_ptr<Arg::Instance> m_instance;
+
+    bool m_is_method : 1;
+    bool m_has_return : 1;
 };
 
 }  // namespace Gjs
diff --git a/gi/function.cpp b/gi/function.cpp
index 59ce4c48..af5e72db 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -1258,14 +1258,12 @@ bool Function::init(JSContext* context, GType gtype /* = G_TYPE_NONE */) {
             return gjs_throw_gerror(context, error);
     }
 
-    bool is_method = g_callable_info_is_method(m_info);
     uint8_t n_args = g_callable_info_get_n_args(m_info);
 
     if (!m_arguments.initialize(context, m_info))
         return false;
 
-    if (is_method)
-        m_arguments.build_instance(m_info);
+    m_arguments.build_instance(m_info);
 
     bool inc_counter;
     m_arguments.build_return(m_info, &inc_counter);