GNOME.org
 

[gjs: 12/16] jsapi-util: Make GjsAutoInfo more typesafe

commit 4d47c0ecb388a8d1f0ad0a997e829c5cd9d45603
Author: Philip Chimento <philip chimento gmail com>
Date:   Sun Oct 7 22:34:51 2018 -0700

    jsapi-util: Make GjsAutoInfo more typesafe
    
    The types GIFunctionInfo, GIObjectInfo, etc., are just typedefs for
    GIBaseInfo. So, this template actually wasn't specialized at all,
    everything was just an alias for GjsAutoInfo<GIBaseInfo>. Instead, we
    make the GIInfoType enum value the template parameter, and do some
    validation with the tag whenever an instance takes ownership of a
    pointer.
    
    This requires a bit of extra specialization for GICallableInfo since that
    type may have more than one GIInfoType tag.
    
    This should provide much better type safety than the old code.

 .clang-format      |  1 +
 gi/fundamental.cpp |  2 +-
 gi/object.cpp      | 99 ++++++++++++++++++++++--------------------------------
 gi/object.h        |  2 +-
 gi/param.cpp       |  3 +-
 gjs/jsapi-util.h   | 82 ++++++++++++++++++++++++++++++++++++++------
 6 files changed, 116 insertions(+), 73 deletions(-)
---
diff --git a/.clang-format b/.clang-format
index bc6649f9..44a21ec5 100644
--- a/.clang-format
+++ b/.clang-format
@@ -8,6 +8,7 @@ BasedOnStyle: Google
 AccessModifierOffset: -3  # to match cpplint
 AllowShortIfStatementsOnASingleLine: false
 AllowShortLoopsOnASingleLine: false
+CommentPragmas: '^ NOLINT'
 DerivePointerAlignment: false
 ForEachMacros: []
 IndentPPDirectives: AfterHash
diff --git a/gi/fundamental.cpp b/gi/fundamental.cpp
index 5b0463d8..b7015018 100644
--- a/gi/fundamental.cpp
+++ b/gi/fundamental.cpp
@@ -623,7 +623,7 @@ static JSObject*
 gjs_lookup_fundamental_prototype_from_gtype(JSContext *context,
                                             GType      gtype)
 {
-    GjsAutoInfo<GIObjectInfo> info;
+    GjsAutoObjectInfo info;
 
     /* A given gtype might not have any definition in the introspection
      * data. If that's the case, try to look for a definition of any of the
diff --git a/gi/object.cpp b/gi/object.cpp
index 8ca4bc90..26fbc74f 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -400,13 +400,11 @@ ObjectInstance::prop_getter_impl(JSContext             *cx,
     return true;
 }
 
-static GjsAutoInfo<GIFieldInfo>
-lookup_field_info(GIObjectInfo *info,
-                  const char   *name)
-{
+static GjsAutoFieldInfo lookup_field_info(GIObjectInfo* info,
+                                          const char* name) {
     int n_fields = g_object_info_get_n_fields(info);
     int ix;
-    GjsAutoInfo<GIFieldInfo> retval;
+    GjsAutoFieldInfo retval;
 
     for (ix = 0; ix < n_fields; ix++) {
         retval = g_object_info_get_field(info, ix);
@@ -432,7 +430,7 @@ GIFieldInfo* ObjectPrototype::find_field_info_from_id(JSContext* cx,
     if (!gjs_string_to_utf8(cx, JS::StringValue(key), &js_prop_name))
         return nullptr;
 
-    GjsAutoInfo<GIFieldInfo> field = lookup_field_info(m_info, js_prop_name);
+    GjsAutoFieldInfo field = lookup_field_info(m_info, js_prop_name);
 
     if (!field) {
         _gjs_proxy_throw_nonexistent_field(cx, m_gtype, js_prop_name);
@@ -480,7 +478,7 @@ ObjectInstance::field_getter_impl(JSContext             *cx,
     gjs_debug_jsprop(GJS_DEBUG_GOBJECT, "Overriding %s with GObject field",
                      gjs_debug_string(name).c_str());
 
-    GjsAutoInfo<GITypeInfo> type = g_field_info_get_type(field);
+    GjsAutoTypeInfo type = g_field_info_get_type(field);
     tag = g_type_info_get_tag(type);
     if (tag == GI_TYPE_TAG_ARRAY ||
         tag == GI_TYPE_TAG_INTERFACE ||
@@ -634,21 +632,19 @@ bool ObjectPrototype::is_vfunc_unchanged(GIVFuncInfo* info) {
     return addr1 == addr2;
 }
 
-static GjsAutoInfo<GIVFuncInfo>
-find_vfunc_on_parents(GIObjectInfo *info,
-                      const char   *name,
-                      bool         *out_defined_by_parent)
-{
+static GjsAutoVFuncInfo find_vfunc_on_parents(GIObjectInfo* info,
+                                              const char* name,
+                                              bool* out_defined_by_parent) {
     bool defined_by_parent = false;
 
     /* ref the first info so that we don't destroy
      * it when unrefing parents later */
-    GjsAutoInfo<GIObjectInfo> parent = g_base_info_ref(info);
+    GjsAutoObjectInfo parent = g_base_info_ref(info);
 
     /* Since it isn't possible to override a vfunc on
      * an interface without reimplementing it, we don't need
      * to search the parent types when looking for a vfunc. */
-    GjsAutoInfo<GIVFuncInfo> vfunc =
+    GjsAutoVFuncInfo vfunc =
         g_object_info_find_vfunc_using_interfaces(parent, name, nullptr);
     while (!vfunc && parent) {
         parent = g_object_info_get_parent(parent);
@@ -679,7 +675,7 @@ static void canonicalize_key(const GjsAutoChar& key) {
 static bool is_ginterface_property_name(GIInterfaceInfo* info,
                                         const char* name) {
     int n_props = g_interface_info_get_n_properties(info);
-    GjsAutoInfo<GIPropertyInfo> prop_info;
+    GjsAutoPropertyInfo prop_info;
 
     for (int ix = 0; ix < n_props; ix++) {
         prop_info = g_interface_info_get_property(info, ix);
@@ -740,15 +736,12 @@ bool ObjectPrototype::resolve_no_info(JSContext* cx, JS::HandleObject obj,
 
     GType *interfaces = g_type_interfaces(m_gtype, &n_interfaces);
     for (i = 0; i < n_interfaces; i++) {
-        GjsAutoInfo<GIInterfaceInfo> iface_info =
+        GjsAutoInterfaceInfo iface_info =
             g_irepository_find_by_gtype(nullptr, interfaces[i]);
         if (!iface_info)
             continue;
 
-        /* An interface GType ought to have interface introspection info */
-        g_assert(iface_info.type() == GI_INFO_TYPE_INTERFACE);
-
-        GjsAutoInfo<GIFunctionInfo> method_info =
+        GjsAutoFunctionInfo method_info =
             g_interface_info_find_method(iface_info, name);
         if (method_info != NULL) {
             if (g_function_info_get_flags (method_info) & GI_FUNCTION_IS_METHOD) {
@@ -787,7 +780,7 @@ is_gobject_property_name(GIObjectInfo *info,
     int n_props = g_object_info_get_n_properties(info);
     int n_ifaces = g_object_info_get_n_interfaces(info);
     int ix;
-    GjsAutoInfo<GIPropertyInfo> prop_info;
+    GjsAutoPropertyInfo prop_info;
 
     GjsAutoChar canonical_name = gjs_hyphen_from_camel(name);
     canonicalize_key(canonical_name);
@@ -801,7 +794,7 @@ is_gobject_property_name(GIObjectInfo *info,
 
     if (!prop_info) {
         for (ix = 0; ix < n_ifaces; ix++) {
-            GjsAutoInfo<GIInterfaceInfo> iface_info =
+            GjsAutoInterfaceInfo iface_info =
                 g_object_info_get_interface(info, ix);
             if (is_ginterface_property_name(iface_info, canonical_name))
                 return true;
@@ -874,9 +867,8 @@ bool ObjectPrototype::resolve_impl(JSContext* context, JS::HandleObject obj,
 
         const char *name_without_vfunc_ = &(name[6]);  /* lifetime tied to name */
         bool defined_by_parent;
-        GjsAutoInfo<GIVFuncInfo> vfunc = find_vfunc_on_parents(m_info,
-                                                               name_without_vfunc_,
-                                                               &defined_by_parent);
+        GjsAutoVFuncInfo vfunc = find_vfunc_on_parents(
+            m_info, name_without_vfunc_, &defined_by_parent);
         if (vfunc != NULL) {
 
             /* In the event that the vfunc is unchanged, let regular
@@ -898,7 +890,7 @@ bool ObjectPrototype::resolve_impl(JSContext* context, JS::HandleObject obj,
     if (is_gobject_property_name(m_info, name))
         return lazy_define_gobject_property(context, obj, id, resolved, name);
 
-    GjsAutoInfo<GIFieldInfo> field_info = lookup_field_info(m_info, name);
+    GjsAutoFieldInfo field_info = lookup_field_info(m_info, name);
     if (field_info) {
         bool found = false;
         if (!JS_AlreadyHasOwnPropertyById(context, obj, id, &found))
@@ -936,7 +928,7 @@ bool ObjectPrototype::resolve_impl(JSContext* context, JS::HandleObject obj,
      * introduces the iface)
      */
 
-    GjsAutoInfo<GIFunctionInfo> method_info =
+    GjsAutoFunctionInfo method_info =
         g_object_info_find_method_using_interfaces(m_info, name, nullptr);
 
     /**
@@ -949,7 +941,7 @@ bool ObjectPrototype::resolve_impl(JSContext* context, JS::HandleObject obj,
                                ConsiderOnlyMethods);
 
 #if GJS_VERBOSE_ENABLE_GI_USAGE
-    _gjs_log_info_usage((GIBaseInfo*) method_info);
+    _gjs_log_info_usage(method_info);
 #endif
 
     if (g_function_info_get_flags (method_info) & GI_FUNCTION_IS_METHOD) {
@@ -993,7 +985,7 @@ bool ObjectPrototype::new_enumerate_impl(JSContext* cx, JS::HandleObject obj,
     GType* interfaces = g_type_interfaces(gtype(), &n_interfaces);
 
     for (unsigned k = 0; k < n_interfaces; k++) {
-        GjsAutoInfo<GIInterfaceInfo> iface_info =
+        GjsAutoInterfaceInfo iface_info =
             g_irepository_find_by_gtype(nullptr, interfaces[k]);
 
         if (!iface_info) {
@@ -1003,7 +995,7 @@ bool ObjectPrototype::new_enumerate_impl(JSContext* cx, JS::HandleObject obj,
         // Methods
         int n_methods = g_interface_info_get_n_methods(iface_info);
         for (int i = 0; i < n_methods; i++) {
-            GjsAutoInfo<GIFunctionInfo> meth_info =
+            GjsAutoFunctionInfo meth_info =
                 g_interface_info_get_method(iface_info, i);
             GIFunctionInfoFlags flags = g_function_info_get_flags(meth_info);
 
@@ -1018,7 +1010,7 @@ bool ObjectPrototype::new_enumerate_impl(JSContext* cx, JS::HandleObject obj,
         // Properties
         int n_properties = g_interface_info_get_n_properties(iface_info);
         for (int i = 0; i < n_properties; i++) {
-            GjsAutoInfo<GIPropertyInfo> prop_info =
+            GjsAutoPropertyInfo prop_info =
                 g_interface_info_get_property(iface_info, i);
 
             GjsAutoChar js_name = gjs_hyphen_to_underscore(prop_info.name());
@@ -1035,8 +1027,7 @@ bool ObjectPrototype::new_enumerate_impl(JSContext* cx, JS::HandleObject obj,
         // Methods
         int n_methods = g_object_info_get_n_methods(info());
         for (int i = 0; i < n_methods; i++) {
-            GjsAutoInfo<GIFunctionInfo> meth_info =
-                g_object_info_get_method(info(), i);
+            GjsAutoFunctionInfo meth_info = g_object_info_get_method(info(), i);
             GIFunctionInfoFlags flags = g_function_info_get_flags(meth_info);
 
             if (flags & GI_FUNCTION_IS_METHOD) {
@@ -1050,7 +1041,7 @@ bool ObjectPrototype::new_enumerate_impl(JSContext* cx, JS::HandleObject obj,
         // Properties
         int n_properties = g_object_info_get_n_properties(info());
         for (int i = 0; i < n_properties; i++) {
-            GjsAutoInfo<GIPropertyInfo> prop_info =
+            GjsAutoPropertyInfo prop_info =
                 g_object_info_get_property(info(), i);
 
             GjsAutoChar js_name = gjs_hyphen_to_underscore(prop_info.name());
@@ -1825,8 +1816,7 @@ static JSObject *
 gjs_lookup_object_prototype(JSContext *context,
                             GType      gtype)
 {
-    GjsAutoInfo<GIObjectInfo> info =
-        g_irepository_find_by_gtype(nullptr, gtype);
+    GjsAutoObjectInfo info = g_irepository_find_by_gtype(nullptr, gtype);
     return gjs_lookup_object_prototype_from_info(context, info, gtype);
 }
 
@@ -2102,7 +2092,7 @@ gjs_object_define_static_methods(JSContext       *context,
     for (i = 0; i < n_methods; i++) {
         GIFunctionInfoFlags flags;
 
-        GjsAutoInfo<GICallableInfo> meth_info =
+        GjsAutoCallableInfo meth_info =
             g_object_info_get_method(object_info, i);
         flags = g_function_info_get_flags (meth_info);
 
@@ -2119,7 +2109,7 @@ gjs_object_define_static_methods(JSContext       *context,
         }
     }
 
-    GjsAutoInfo<GIStructInfo> gtype_struct =
+    GjsAutoStructInfo gtype_struct =
         g_object_info_get_class_struct(object_info);
     if (gtype_struct == NULL)
         return true;  /* not an error? */
@@ -2127,7 +2117,7 @@ gjs_object_define_static_methods(JSContext       *context,
     n_methods = g_struct_info_get_n_methods(gtype_struct);
 
     for (i = 0; i < n_methods; i++) {
-        GjsAutoInfo<GICallableInfo> meth_info =
+        GjsAutoCallableInfo meth_info =
             g_struct_info_get_method(gtype_struct, i);
 
         if (!gjs_define_function(context, constructor, gtype, meth_info))
@@ -2377,19 +2367,14 @@ ObjectInstance::typecheck_object(JSContext *context,
     return result;
 }
 
-
-static bool
-find_vfunc_info (JSContext *context,
-                 GType implementor_gtype,
-                 GIBaseInfo *vfunc_info,
-                 const char   *vfunc_name,
-                 gpointer *implementor_vtable_ret,
-                 GjsAutoInfo<GIFieldInfo> *field_info_ret)
-{
+static bool find_vfunc_info(JSContext* context, GType implementor_gtype,
+                            GIBaseInfo* vfunc_info, const char* vfunc_name,
+                            void** implementor_vtable_ret,
+                            GjsAutoFieldInfo* field_info_ret) {
     GType ancestor_gtype;
     int length, i;
     GIBaseInfo *ancestor_info;
-    GjsAutoInfo<GIStructInfo> struct_info;
+    GjsAutoStructInfo struct_info;
     bool is_interface;
 
     field_info_ret->reset();
@@ -2421,12 +2406,11 @@ find_vfunc_info (JSContext *context,
 
     length = g_struct_info_get_n_fields(struct_info);
     for (i = 0; i < length; i++) {
-        GjsAutoInfo<GIFieldInfo> field_info =
-            g_struct_info_get_field(struct_info, i);
+        GjsAutoFieldInfo field_info = g_struct_info_get_field(struct_info, i);
         if (strcmp(field_info.name(), vfunc_name) != 0)
             continue;
 
-        GjsAutoInfo<GITypeInfo> type_info = g_field_info_get_type(field_info);
+        GjsAutoTypeInfo type_info = g_field_info_get_type(field_info);
         if (g_type_info_get_tag(type_info) != GI_TYPE_TAG_INTERFACE) {
             /* We have a field with the same name, but it's not a callback.
              * There's no hope of being another field with a correct name,
@@ -2475,7 +2459,7 @@ bool ObjectPrototype::hook_up_vfunc_impl(JSContext* cx,
      * This is awful, so abort now. */
     g_assert(info != NULL);
 
-    GjsAutoInfo<GIVFuncInfo> vfunc = find_vfunc_on_parents(info, name, nullptr);
+    GjsAutoVFuncInfo vfunc = find_vfunc_on_parents(info, name, nullptr);
 
     if (!vfunc) {
         guint i, n_interfaces;
@@ -2484,7 +2468,7 @@ bool ObjectPrototype::hook_up_vfunc_impl(JSContext* cx,
         interface_list = g_type_interfaces(m_gtype, &n_interfaces);
 
         for (i = 0; i < n_interfaces; i++) {
-            GjsAutoInfo<GIInterfaceInfo> interface =
+            GjsAutoInterfaceInfo interface =
                 g_irepository_find_by_gtype(nullptr, interface_list[i]);
 
             /* The interface doesn't have to exist -- it could be private
@@ -2507,7 +2491,7 @@ bool ObjectPrototype::hook_up_vfunc_impl(JSContext* cx,
     }
 
     void *implementor_vtable;
-    GjsAutoInfo<GIFieldInfo> field_info;
+    GjsAutoFieldInfo field_info;
     if (!find_vfunc_info(cx, m_gtype, vfunc, name, &implementor_vtable,
                          &field_info))
         return false;
@@ -2537,10 +2521,7 @@ gjs_lookup_object_constructor(JSContext             *context,
 {
     JSObject *constructor;
 
-    GjsAutoInfo<GIObjectInfo> object_info =
-        g_irepository_find_by_gtype(nullptr, gtype);
-
-    g_assert(!object_info || object_info.type() == GI_INFO_TYPE_OBJECT);
+    GjsAutoObjectInfo object_info = g_irepository_find_by_gtype(nullptr, gtype);
 
     constructor = gjs_lookup_object_constructor_from_info(context, object_info, gtype);
 
diff --git a/gi/object.h b/gi/object.h
index 1f091eb7..17e88435 100644
--- a/gi/object.h
+++ b/gi/object.h
@@ -250,7 +250,7 @@ class ObjectPrototype : public ObjectBase {
         JS::GCHashMap<JS::Heap<JSString*>, GjsAutoParam,
                       js::DefaultHasher<JSString*>, js::SystemAllocPolicy>;
     using FieldCache =
-        JS::GCHashMap<JS::Heap<JSString*>, GjsAutoInfo<GIFieldInfo>,
+        JS::GCHashMap<JS::Heap<JSString*>, GjsAutoInfo<GI_INFO_TYPE_FIELD>,
                       js::DefaultHasher<JSString*>, js::SystemAllocPolicy>;
 
     GIObjectInfo* m_info;
diff --git a/gi/param.cpp b/gi/param.cpp
index 6a1bfaa5..4c780113 100644
--- a/gi/param.cpp
+++ b/gi/param.cpp
@@ -232,8 +232,7 @@ gjs_define_param_class(JSContext       *context,
                            JSPROP_PERMANENT))
         return false;
 
-    GjsAutoInfo<GIObjectInfo> info =
-        g_irepository_find_by_gtype(g_irepository_get_default(), G_TYPE_PARAM);
+    GjsAutoObjectInfo info = g_irepository_find_by_gtype(nullptr, G_TYPE_PARAM);
     if (!gjs_object_define_static_methods(context, constructor, G_TYPE_PARAM, info))
         return false;
 
diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h
index 2391b2c3..65b08bb8 100644
--- a/gjs/jsapi-util.h
+++ b/gjs/jsapi-util.h
@@ -80,21 +80,83 @@ public:
     C *as() const { return reinterpret_cast<C*>(operator T *()); }
 };
 
-template<typename T = GIBaseInfo>
-class GjsAutoInfo : public std::unique_ptr<T, decltype(&g_base_info_unref)> {
-public:
-    GjsAutoInfo(T *ptr = nullptr) : GjsAutoInfo::unique_ptr(ptr, g_base_info_unref) {}
+// Use this class for owning a GIBaseInfo* of indeterminate type. Any type (e.g.
+// GIFunctionInfo*, GIObjectInfo*) will fit. If you know that the info is of a
+// certain type (e.g. you are storing the return value of a function that
+// returns GIFunctionInfo*,) use one of the derived classes below.
+class GjsAutoBaseInfo
+    : public std::unique_ptr<GIBaseInfo, decltype(&g_base_info_unref)> {
+ public:
+    GjsAutoBaseInfo(GIBaseInfo* ptr = nullptr)
+        : GjsAutoBaseInfo::unique_ptr(ptr, g_base_info_unref) {}
+
+    operator GIBaseInfo*() const { return get(); }
+
+    const char* name(void) const { return g_base_info_get_name(get()); }
+    const char* ns(void) const { return g_base_info_get_namespace(get()); }
+    GIInfoType type(void) const { return g_base_info_get_type(get()); }
+};
+
+// Use GjsAutoInfo, preferably its typedefs below, when you know for sure that
+// the info is either of a certain type or null.
+template <GIInfoType TAG>
+class GjsAutoInfo : public GjsAutoBaseInfo {
+    void validate(void) const {
+        if (*this)
+            g_assert(g_base_info_get_type(get()) == TAG);
+    }
 
-    operator T *() const { return GjsAutoInfo::unique_ptr::get(); }
+ public:
+    // Normally one-argument constructors should be explicit, but we are trying
+    // to conform to the interface of std::unique_ptr here.
+    GjsAutoInfo(GIBaseInfo* ptr = nullptr)  // NOLINT(runtime/explicit)
+        : GjsAutoBaseInfo(ptr) {
+        validate();
+    }
+
+    void reset(GIBaseInfo* other = nullptr) {
+        GjsAutoInfo::unique_ptr::reset(other);
+        validate();
+    }
 
-    const char *name(void) const { return g_base_info_get_name(this->get()); }
-    GIInfoType type(void) const { return g_base_info_get_type(this->get()); }
+    // You should not need this method, because you already know the answer.
+    GIInfoType type(void) = delete;
 };
 
-/* For use of GjsAutoInfo<T> in GC hash maps */
+using GjsAutoEnumInfo = GjsAutoInfo<GI_INFO_TYPE_ENUM>;
+using GjsAutoFieldInfo = GjsAutoInfo<GI_INFO_TYPE_FIELD>;
+using GjsAutoFunctionInfo = GjsAutoInfo<GI_INFO_TYPE_FUNCTION>;
+using GjsAutoInterfaceInfo = GjsAutoInfo<GI_INFO_TYPE_INTERFACE>;
+using GjsAutoObjectInfo = GjsAutoInfo<GI_INFO_TYPE_OBJECT>;
+using GjsAutoPropertyInfo = GjsAutoInfo<GI_INFO_TYPE_PROPERTY>;
+using GjsAutoStructInfo = GjsAutoInfo<GI_INFO_TYPE_STRUCT>;
+using GjsAutoTypeInfo = GjsAutoInfo<GI_INFO_TYPE_TYPE>;
+using GjsAutoVFuncInfo = GjsAutoInfo<GI_INFO_TYPE_VFUNC>;
+
+// GICallableInfo can be one of several tags, so we have to have a separate
+// class, and use GI_IS_CALLABLE_INFO() to validate.
+class GjsAutoCallableInfo : public GjsAutoBaseInfo {
+    void validate(void) const {
+        if (*this)
+            g_assert(GI_IS_CALLABLE_INFO(get()));
+    }
+
+ public:
+    GjsAutoCallableInfo(GIBaseInfo* ptr = nullptr)  // NOLINT(runtime/explicit)
+        : GjsAutoBaseInfo(ptr) {
+        validate();
+    }
+
+    void reset(GIBaseInfo* other = nullptr) {
+        GjsAutoCallableInfo::unique_ptr::reset(other);
+        validate();
+    }
+};
+
+/* For use of GjsAutoInfo<TAG> in GC hash maps */
 namespace JS {
-template<typename T>
-struct GCPolicy<GjsAutoInfo<T>> : public IgnoreGCPolicy<GjsAutoInfo<T>> {};
+template <GIInfoType TAG>
+struct GCPolicy<GjsAutoInfo<TAG>> : public IgnoreGCPolicy<GjsAutoInfo<TAG>> {};
 }
 
 class GjsAutoParam
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]