[gjs/wip/ptomato/mozjs38: 24/26] WIP - Switch to new JSClass.enumerate



commit c7fab33a96a8afb9654140c3a1794a991d38ad2c
Author: Philip Chimento <philip chimento gmail com>
Date:   Wed Jan 11 23:38:42 2017 -0800

    WIP - Switch to new JSClass.enumerate
    
    Enumeration without defining properties has been removed from the public
    API in SpiderMonkey 38. A JSNewEnumerateOp can now only be added through
    the jsfriend API in js::Class, which is really JSClass but with some
    extra fields that are masked in the public version.
    
    In addition, JSNewEnumerateOp has changed somewhat. In practice
    SpiderMonkey was not doing lazy enumeration, so they decided to change
    the enumerate callback from being called incrementally for each property,
    to just being called once the first time an object's properties are
    enumerated.

 gjs/importer.cpp |  293 ++++++++++++++++++++----------------------------------
 1 files changed, 107 insertions(+), 186 deletions(-)
---
diff --git a/gjs/importer.cpp b/gjs/importer.cpp
index 09a7dd5..53314d5 100644
--- a/gjs/importer.cpp
+++ b/gjs/importer.cpp
@@ -48,9 +48,11 @@ typedef struct {
     unsigned int index;
 } ImporterIterator;
 
-extern struct JSClass gjs_importer_class;
+extern js::Class gjs_importer_extended_class;
+// FIXME make JSClass const in macro
+static JSClass *gjs_importer_class = (JSClass *)(&gjs_importer_extended_class);
 
-GJS_DEFINE_PRIV_FROM_JS(Importer, gjs_importer_class)
+GJS_DEFINE_PRIV_FROM_JS(Importer, (*gjs_importer_class))
 
 static bool
 importer_to_string(JSContext *cx,
@@ -91,7 +93,7 @@ define_meta_properties(JSContext       *context,
     /* We define both __moduleName__ and __parentModule__ to null
      * on the root importer
      */
-    parent_is_module = parent && JS_InstanceOf(context, parent, &gjs_importer_class, NULL);
+    parent_is_module = parent && JS_InstanceOf(context, parent, gjs_importer_class, NULL);
 
     gjs_debug(GJS_DEBUG_IMPORTER, "Defining parent %p of %p '%s' is mod %d",
               parent.get(), module_obj.get(),
@@ -390,10 +392,9 @@ load_module_init(JSContext       *context,
 static void
 load_module_elements(JSContext        *cx,
                      JS::HandleObject  in_object,
-                     ImporterIterator *iter,
+                     JS::AutoIdVector& prop_ids,
                      const char       *init_path)
 {
-    size_t ix, length;
     JS::RootedObject module_obj(cx, load_module_init(cx, in_object, init_path));
 
     if (module_obj == NULL)
@@ -403,14 +404,8 @@ load_module_elements(JSContext        *cx,
     if (!ids)
         return;
 
-    for (ix = 0, length = ids.length(); ix < length; ix++) {
-        char *name;
-        if (!gjs_get_string_id(cx, ids[ix], &name))
-            continue;
-
-        /* Pass ownership of name */
-        g_ptr_array_add(iter->elements, name);
-    }
+    for (size_t ix = 0; ix < ids.length(); ix++)
+        prop_ids.append(ids[ix]);
 }
 
 static bool
@@ -651,200 +646,107 @@ do_import(JSContext       *context,
     return result;
 }
 
-static ImporterIterator *
-importer_iterator_new(void)
-{
-    ImporterIterator *iter;
-
-    iter = g_slice_new0(ImporterIterator);
-
-    iter->elements = g_ptr_array_new();
-    iter->index = 0;
-
-    return iter;
-}
-
-static void
-importer_iterator_free(ImporterIterator *iter)
-{
-    g_ptr_array_foreach(iter->elements, (GFunc)g_free, NULL);
-    g_ptr_array_free(iter->elements, true);
-    g_slice_free(ImporterIterator, iter);
-}
-
-/*
- * Like JSEnumerateOp, but enum provides contextual information as follows:
- *
- * JSENUMERATE_INIT: allocate private enum struct in state_p, return number
- * of elements in *id_p
- * JSENUMERATE_NEXT: return next property id in *id_p, and if no new property
- * free state_p and set to JS::NullValue()
- * JSENUMERATE_DESTROY : destroy state_p
- *
- * Note that in a for ... in loop, this will be called first on the object,
+/* Note that in a for ... in loop, this will be called first on the object,
  * then on its prototype.
- *
  */
 static bool
-importer_new_enumerate(JSContext  *context,
-                       JS::HandleObject object,
-                       JSIterateOp enum_op,
-                       JS::MutableHandleValue statep,
-                       JS::MutableHandleId idp)
+importer_enumerate(JSContext        *context,
+                   JS::HandleObject  object,
+                   JS::AutoIdVector& properties)
 {
-    ImporterIterator *iter;
+    Importer *priv;
+    guint32 search_path_len;
+    guint32 i;
 
-    switch (enum_op) {
-    case JSENUMERATE_INIT_ALL:
-    case JSENUMERATE_INIT: {
-        Importer *priv;
-        JS::RootedObject search_path(context);
-        guint32 search_path_len;
-        guint32 i;
+    priv = priv_from_js(context, object);
 
-        statep.setNull();
+    if (!priv)
+        /* we are enumerating the prototype properties */
+        return true;
 
-        idp.set(INT_TO_JSID(0));
+    JS::RootedObject search_path(context);
+    if (!gjs_object_require_property(context, object, "importer",
+                                     GJS_STRING_SEARCH_PATH,
+                                     &search_path))
+        return false;
 
-        priv = priv_from_js(context, object);
+    if (!JS_IsArrayObject(context, search_path)) {
+        gjs_throw(context, "searchPath property on importer is not an array");
+        return false;
+    }
 
-        if (!priv)
-            /* we are enumerating the prototype properties */
-            return true;
+    if (!JS_GetArrayLength(context, search_path, &search_path_len)) {
+        gjs_throw(context, "searchPath array has no length");
+        return false;
+    }
 
-        if (!gjs_object_require_property(context, object, "importer",
-                                         GJS_STRING_SEARCH_PATH,
-                                         &search_path))
-            return false;
+    JS::RootedValue elem(context);
+    for (i = 0; i < search_path_len; ++i) {
+        char *dirname = NULL;
+        char *init_path;
+        const char *filename;
+        GDir *dir = NULL;
 
-        if (!JS_IsArrayObject(context, search_path)) {
-            gjs_throw(context, "searchPath property on importer is not an array");
+        elem.setUndefined();
+        if (!JS_GetElement(context, search_path, i, &elem)) {
+            /* this means there was an exception, while elem.isUndefined()
+             * means no element found
+             */
             return false;
         }
 
-        if (!JS_GetArrayLength(context, search_path, &search_path_len)) {
-            gjs_throw(context, "searchPath array has no length");
+        if (elem.isUndefined())
+            continue;
+
+        if (!elem.isString()) {
+            gjs_throw(context, "importer searchPath contains non-string");
             return false;
         }
 
-        iter = importer_iterator_new();
-
-        JS::RootedValue elem(context);
-        for (i = 0; i < search_path_len; ++i) {
-            char *dirname = NULL;
-            char *init_path;
-            const char *filename;
-            GDir *dir = NULL;
-
-            elem = JS::UndefinedValue();
-            if (!JS_GetElement(context, search_path, i, &elem)) {
-                /* this means there was an exception, while elem.isUndefined()
-                 * means no element found
-                 */
-                importer_iterator_free(iter);
-                return false;
-            }
-
-            if (elem.isUndefined())
-                continue;
-
-            if (!elem.isString()) {
-                gjs_throw(context, "importer searchPath contains non-string");
-                importer_iterator_free(iter);
-                return false;
-            }
-
-            if (!gjs_string_to_utf8(context, elem, &dirname)) {
-                importer_iterator_free(iter);
-                return false; /* Error message already set */
-            }
-
-            init_path = g_build_filename(dirname, MODULE_INIT_FILENAME,
-                                         NULL);
-
-            load_module_elements(context, object, iter, init_path);
-
-            g_free(init_path);
-
-            dir = g_dir_open(dirname, 0, NULL);
-
-            if (!dir) {
-                g_free(dirname);
-                continue;
-            }
-
-            while ((filename = g_dir_read_name(dir))) {
-                char *full_path;
+        if (!gjs_string_to_utf8(context, elem, &dirname)) {
+            return false; /* Error message already set */
+        }
 
-                /* skip hidden files and directories (.svn, .git, ...) */
-                if (filename[0] == '.')
-                    continue;
+        init_path = g_build_filename(dirname, MODULE_INIT_FILENAME, NULL);
 
-                /* skip module init file */
-                if (strcmp(filename, MODULE_INIT_FILENAME) == 0)
-                    continue;
+        load_module_elements(context, object, properties, init_path);
 
-                full_path = g_build_filename(dirname, filename, NULL);
+        g_free(init_path);
 
-                if (g_file_test(full_path, G_FILE_TEST_IS_DIR)) {
-                    g_ptr_array_add(iter->elements, g_strdup(filename));
-                } else {
-                    if (g_str_has_suffix(filename, "." G_MODULE_SUFFIX) ||
-                        g_str_has_suffix(filename, ".js")) {
-                        g_ptr_array_add(iter->elements,
-                                        g_strndup(filename, strlen(filename) - 3));
-                    }
-                }
-
-                g_free(full_path);
-            }
-            g_dir_close(dir);
+        dir = g_dir_open(dirname, 0, NULL);
 
+        if (!dir) {
             g_free(dirname);
+            continue;
         }
 
-        statep.set(JS::PrivateValue(context));
-
-        idp.set(INT_TO_JSID(iter->elements->len));
-
-        break;
-    }
-
-    case JSENUMERATE_NEXT: {
-        if (statep.isNull()) /* Iterating prototype */
-            return true;
-
-        iter = (ImporterIterator*) statep.get().toPrivate();
-
-        if (iter->index < iter->elements->len) {
-            JS::RootedValue element_val(context);
-            if (!gjs_string_from_utf8(context,
-                                         (const char*) g_ptr_array_index(iter->elements,
-                                                           iter->index++),
-                                         -1,
-                                         &element_val))
-                return false;
+        while ((filename = g_dir_read_name(dir))) {
+            char *full_path;
 
-            if (!JS_ValueToId(context, element_val, idp))
-                return false;
+            /* skip hidden files and directories (.svn, .git, ...) */
+            if (filename[0] == '.')
+                continue;
 
-            break;
-        }
-        /* else fall through to destroying the iterator */
-    }
+            /* skip module init file */
+            if (strcmp(filename, MODULE_INIT_FILENAME) == 0)
+                continue;
 
-    case JSENUMERATE_DESTROY: {
-        if (!statep.isNull()) {
-            iter = (ImporterIterator*) statep.get().toPrivate();
+            full_path = g_build_filename(dirname, filename, NULL);
 
-            importer_iterator_free(iter);
+            if (g_file_test(full_path, G_FILE_TEST_IS_DIR)) {
+                properties.append(gjs_intern_string_to_id(context, filename));
+            } else if (g_str_has_suffix(filename, "." G_MODULE_SUFFIX) ||
+                       g_str_has_suffix(filename, ".js")) {
+                g_autofree char *filename_noext =
+                    g_strndup(filename, strlen(filename) - 3);
+                properties.append(gjs_intern_string_to_id(context, filename_noext));
+            }
 
-            statep.setNull();
+            g_free(full_path);
         }
-    }
+        g_dir_close(dir);
 
-    default:
-        ;
+        g_free(dirname);
     }
 
     return true;
@@ -903,8 +805,8 @@ importer_resolve(JSContext        *context,
 GJS_NATIVE_CONSTRUCTOR_DEFINE_ABSTRACT(importer)
 
 static void
-importer_finalize(JSFreeOp *fop,
-                  JSObject *obj)
+importer_finalize(js::FreeOp *fop,
+                  JSObject   *obj)
 {
     Importer *priv;
 
@@ -922,19 +824,38 @@ importer_finalize(JSFreeOp *fop,
  * instances of the object, and to the prototype that instances of the
  * class have.
  */
-struct JSClass gjs_importer_class = {
+js::Class gjs_importer_extended_class = {
     "GjsFileImporter",
     JSCLASS_HAS_PRIVATE |
-    JSCLASS_NEW_ENUMERATE |
     JSCLASS_IMPLEMENTS_BARRIERS,
     NULL,  /* addProperty */
     NULL,  /* deleteProperty */
     NULL,  /* getProperty */
     NULL,  /* setProperty */
-    (JSEnumerateOp) importer_new_enumerate, /* needs cast since it's the new enumerate signature */
+    NULL,  /* enumerate (see below) */
     importer_resolve,
     NULL,  /* convert */
-    importer_finalize
+    importer_finalize,
+    NULL,  /* call */
+    NULL,  /* hasInstance */
+    NULL,  /* construct */
+    NULL,  /* trace */
+    JS_NULL_CLASS_SPEC,
+    JS_NULL_CLASS_EXT,
+    {
+        NULL,  /* lookupProperty */
+        NULL,  /* defineProperty */
+        NULL,  /* hasProperty */
+        NULL,  /* getProperty */
+        NULL,  /* setProperty */
+        NULL,  /* getOwnPropertyDescriptor */
+        NULL,  /* deleteProperty */
+        NULL,  /* watch */
+        NULL,  /* unwatch */
+        NULL,  /* getElements */
+        importer_enumerate,
+        NULL,  /* thisObject */
+    }
 };
 
 JSPropertySpec gjs_importer_proto_props[] = {
@@ -955,7 +876,7 @@ importer_new(JSContext *context,
 
     JS::RootedObject global(context, gjs_get_import_global(context));
 
-    if (!JS_HasProperty(context, global, gjs_importer_class.name, &found))
+    if (!JS_HasProperty(context, global, gjs_importer_class->name, &found))
         g_error("HasProperty call failed creating importer class");
 
     if (!found) {
@@ -966,7 +887,7 @@ importer_new(JSContext *context,
                                   * Object.prototype
                                   */
                                  JS::NullPtr(),
-                                 &gjs_importer_class,
+                                 gjs_importer_class,
                                  /* constructor for instances (NULL for
                                   * none - just name the prototype like
                                   * Math - rarely correct)
@@ -983,14 +904,14 @@ importer_new(JSContext *context,
                                  /* funcs of constructor, MyConstructor.myfunc() */
                                  NULL);
         if (prototype == NULL)
-            g_error("Can't init class %s", gjs_importer_class.name);
+            g_error("Can't init class %s", gjs_importer_class->name);
 
         gjs_debug(GJS_DEBUG_IMPORTER, "Initialized class %s prototype %p",
-                  gjs_importer_class.name, prototype);
+                  gjs_importer_class->name, prototype);
     }
 
     JS::RootedObject importer(context,
-        JS_NewObject(context, &gjs_importer_class, global));
+        JS_NewObject(context, gjs_importer_class, global));
     if (importer == NULL)
         g_error("No memory to create importer importer");
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]