Bug 1038316: Add support for ES6 Symbols to ubi::Node. r=sfink
authorJim Blandy <jimb@mozilla.com>
Wed, 30 Jul 2014 17:14:19 -0700
changeset 196959 7f3a69331538c7159cbe67ea93fddb2fcf0a5d78
parent 196958 1a67ee710c0de0a1878c62fc5c1ef805e9490bd8
child 196960 84e722501e7b352cb6521eec85c1de747f50dddb
push id47010
push userjblandy@mozilla.com
push dateThu, 31 Jul 2014 00:14:40 +0000
treeherdermozilla-inbound@7f3a69331538 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1038316
milestone34.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1038316: Add support for ES6 Symbols to ubi::Node. r=sfink
js/public/UbiNode.h
js/src/builtin/TestingFunctions.cpp
js/src/jit-test/tests/heap-analysis/findPath.js
js/src/vm/UbiNode.cpp
--- a/js/public/UbiNode.h
+++ b/js/public/UbiNode.h
@@ -18,17 +18,17 @@
 #include "js/HashTable.h"
 #include "js/TypeDecls.h"
 
 // JS::ubi::Node
 //
 // JS::ubi::Node is a pointer-like type designed for internal use by heap
 // analysis tools. A ubi::Node can refer to:
 //
-// - a JS value, like a string or object;
+// - a JS value, like a string, object, or symbol;
 // - an internal SpiderMonkey structure, like a shape or a scope chain object
 // - an instance of some embedding-provided type: in Firefox, an XPCOM
 //   object, or an internal DOM node class instance
 //
 // A ubi::Node instance provides metadata about its referent, and can
 // enumerate its referent's outgoing edges, so you can implement heap analysis
 // algorithms that walk the graph - finding paths between objects, or
 // computing heap dominator trees, say - using ubi::Node, while remaining
@@ -310,20 +310,20 @@ class Node {
         return static_cast<T *>(base()->ptr);
     }
 
     template<typename T>
     T *asOrNull() const {
         return is<T>() ? static_cast<T *>(base()->ptr) : nullptr;
     }
 
-    // If this node refers to something that can be represented as a
-    // JavaScript value that is safe to expose to JavaScript code, return that
-    // value. Otherwise return UndefinedValue(). JSStrings and some (but not
-    // all!) JSObjects can be exposed.
+    // If this node refers to something that can be represented as a JavaScript
+    // value that is safe to expose to JavaScript code, return that value.
+    // Otherwise return UndefinedValue(). JSStrings, JS::Symbols, and some (but
+    // not all!) JSObjects can be exposed.
     JS::Value exposeToJS() const;
 
     const jschar *typeName()        const { return base()->typeName(); }
     size_t size()                   const { return base()->size(); }
     EdgeRange *edges(JSContext *cx) const { return base()->edges(cx); }
 
     // A hash policy for ubi::Nodes.
     // This simply uses the stock PointerHasher on the ubi::Node's pointer.
@@ -422,16 +422,17 @@ class TracerConcrete : public Base {
 
   public:
     static const jschar concreteTypeName[];
     static void construct(void *storage, Referent *ptr) { new (storage) TracerConcrete(ptr); };
 };
 
 template<> struct Concrete<JSObject> : TracerConcrete<JSObject> { };
 template<> struct Concrete<JSString> : TracerConcrete<JSString> { };
+template<> struct Concrete<JS::Symbol> : TracerConcrete<JS::Symbol> { };
 template<> struct Concrete<JSScript> : TracerConcrete<JSScript> { };
 template<> struct Concrete<js::LazyScript> : TracerConcrete<js::LazyScript> { };
 template<> struct Concrete<js::jit::JitCode> : TracerConcrete<js::jit::JitCode> { };
 template<> struct Concrete<js::Shape> : TracerConcrete<js::Shape> { };
 template<> struct Concrete<js::BaseShape> : TracerConcrete<js::BaseShape> { };
 template<> struct Concrete<js::types::TypeObject> : TracerConcrete<js::types::TypeObject> { };
 
 // The ubi::Node null pointer. Any attempt to operate on a null ubi::Node asserts.
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -1785,31 +1785,30 @@ FindPath(JSContext *cx, unsigned argc, j
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     if (argc < 2) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_MORE_ARGS_NEEDED,
                              "findPath", "1", "");
         return false;
     }
 
-    // We don't ToString non-objects given as 'start' or 'target'. We can't
-    // see edges to non-string primitive values, and it doesn't make much
-    // sense to ask for paths to or from a freshly allocated string, so
-    // if a non-string primitive appears here it's probably a mistake.
-    if (!args[0].isObject() && !args[0].isString()) {
+    // We don't ToString non-objects given as 'start' or 'target', because this
+    // test is all about object identity, and ToString doesn't preserve that.
+    // Non-GCThing endpoints don't make much sense.
+    if (!args[0].isObject() && !args[0].isString() && !args[0].isSymbol()) {
         js_ReportValueErrorFlags(cx, JSREPORT_ERROR, JSMSG_UNEXPECTED_TYPE,
                                  JSDVG_SEARCH_STACK, args[0], JS::NullPtr(),
-                                 "neither an object nor a string", NULL);
+                                 "not an object, string, or symbol", NULL);
         return false;
     }
 
-    if (!args[1].isObject() && !args[1].isString()) {
+    if (!args[1].isObject() && !args[1].isString() && !args[1].isSymbol()) {
         js_ReportValueErrorFlags(cx, JSREPORT_ERROR, JSMSG_UNEXPECTED_TYPE,
                                  JSDVG_SEARCH_STACK, args[0], JS::NullPtr(),
-                                 "neither an object nor a string", NULL);
+                                 "not an object, string, or symbol", NULL);
         return false;
     }
 
     AutoValueVector nodes(cx);
     Vector<heaptools::EdgeName> edges(cx);
 
     {
         // We can't tolerate the GC moving things around while we're searching
@@ -1832,20 +1831,23 @@ FindPath(JSContext *cx, unsigned argc, j
             return true;
         }
     }
 
     // |nodes| and |edges| contain the path from |start| to |target|, reversed.
     // Construct a JavaScript array describing the path from the start to the
     // target. Each element has the form:
     //
-    //   { node: <object or string>, edge: <string describing outgoing edge from node> }
+    //   {
+    //     node: <object or string or symbol>,
+    //     edge: <string describing outgoing edge from node>
+    //   }
     //
-    // or, if the node is some internal thing, that isn't a proper
-    // JavaScript value:
+    // or, if the node is some internal thing that isn't a proper JavaScript
+    // value:
     //
     //   { node: undefined, edge: <string> }
     size_t length = nodes.length();
     RootedObject result(cx, NewDenseAllocatedArray(cx, length));
     if (!result)
         return false;
     result->ensureDenseInitializedLength(cx, 0, length);
 
@@ -1863,18 +1865,17 @@ FindPath(JSContext *cx, unsigned argc, j
 
         heaptools::EdgeName edgeName = Move(edges[i]);
 
         RootedString edgeStr(cx, NewString<CanGC>(cx, edgeName.get(), js_strlen(edgeName.get())));
         if (!edgeStr)
             return false;
         edgeName.release(); // edgeStr acquired ownership
 
-        if (!JS_DefineProperty(cx, obj, "edge", edgeStr,
-                               JSPROP_ENUMERATE, nullptr, nullptr))
+        if (!JS_DefineProperty(cx, obj, "edge", edgeStr, JSPROP_ENUMERATE, nullptr, nullptr))
             return false;
 
         result->setDenseElement(length - i - 1, ObjectValue(*obj));
     }
 
     args.rval().setObject(*result);
     return true;
 }
--- a/js/src/jit-test/tests/heap-analysis/findPath.js
+++ b/js/src/jit-test/tests/heap-analysis/findPath.js
@@ -37,8 +37,13 @@ print(uneval(findPath(gc, o)));
 
 Match.Pattern([{node: {}, edge: "shape"},
                {node: Match.Pattern.ANY, edge: "base"},
                {node: Match.Pattern.ANY, edge: "parent"},
                {node: {}, edge: "o"}])
   .assert(findPath(o, o));
 print(findPath(o, o).map((e) => e.edge).toString());
 
+// Check that we can generate ubi::Nodes for Symbols.
+var so = { sym: Symbol() };
+Match.Pattern([{node: {}, edge: "sym" }])
+  .assert(findPath(so, so.sym));
+print(findPath(so, so.sym).map((e) => e.edge).toString());
--- a/js/src/vm/UbiNode.cpp
+++ b/js/src/vm/UbiNode.cpp
@@ -32,16 +32,17 @@ const jschar *Concrete<void>::typeName()
 size_t Concrete<void>::size() const                 { MOZ_CRASH("null ubi::Node"); }
 EdgeRange *Concrete<void>::edges(JSContext *) const { MOZ_CRASH("null ubi::Node"); }
 
 Node::Node(JSGCTraceKind kind, void *ptr)
 {
     switch (kind) {
       case JSTRACE_OBJECT:      construct(static_cast<JSObject *>(ptr));              break;
       case JSTRACE_STRING:      construct(static_cast<JSString *>(ptr));              break;
+      case JSTRACE_SYMBOL:      construct(static_cast<JS::Symbol *>(ptr));            break;
       case JSTRACE_SCRIPT:      construct(static_cast<JSScript *>(ptr));              break;
       case JSTRACE_LAZY_SCRIPT: construct(static_cast<js::LazyScript *>(ptr));        break;
       case JSTRACE_JITCODE:     construct(static_cast<js::jit::JitCode *>(ptr));      break;
       case JSTRACE_SHAPE:       construct(static_cast<js::Shape *>(ptr));             break;
       case JSTRACE_BASE_SHAPE:  construct(static_cast<js::BaseShape *>(ptr));         break;
       case JSTRACE_TYPE_OBJECT: construct(static_cast<js::types::TypeObject *>(ptr)); break;
 
       default:
@@ -50,16 +51,18 @@ Node::Node(JSGCTraceKind kind, void *ptr
 }
 
 Node::Node(Value value)
 {
     if (value.isObject())
         construct(&value.toObject());
     else if (value.isString())
         construct(value.toString());
+    else if (value.isSymbol())
+        construct(value.toSymbol());
     else
         construct<void>(nullptr);
 }
 
 Value
 Node::exposeToJS() const
 {
     Value v;
@@ -70,16 +73,18 @@ Node::exposeToJS() const
             v.setUndefined();
         } else if (obj.is<JSFunction>() && IsInternalFunctionObject(&obj)) {
             v.setUndefined();
         } else {
             v.setObject(obj);
         }
     } else if (is<JSString>()) {
         v.setString(as<JSString>());
+    } else if (is<JS::Symbol>()) {
+        v.setSymbol(as<JS::Symbol>());
     } else {
         v.setUndefined();
     }
 
     return v;
 }
 
 // A dumb Edge concrete class. All but the most essential members have the
@@ -205,16 +210,18 @@ TracerConcrete<Referent>::edges(JSContex
 
     return r.forget();
 }
 
 template<> const jschar TracerConcrete<JSObject>::concreteTypeName[] =
     MOZ_UTF16("JSObject");
 template<> const jschar TracerConcrete<JSString>::concreteTypeName[] =
     MOZ_UTF16("JSString");
+template<> const jschar TracerConcrete<JS::Symbol>::concreteTypeName[] =
+    MOZ_UTF16("JS::Symbol");
 template<> const jschar TracerConcrete<JSScript>::concreteTypeName[] =
     MOZ_UTF16("JSScript");
 template<> const jschar TracerConcrete<js::LazyScript>::concreteTypeName[] =
     MOZ_UTF16("js::LazyScript");
 template<> const jschar TracerConcrete<js::jit::JitCode>::concreteTypeName[] =
     MOZ_UTF16("js::jit::JitCode");
 template<> const jschar TracerConcrete<js::Shape>::concreteTypeName[] =
     MOZ_UTF16("js::Shape");