Bug 1633145 - Replace NON_INTEGER_ATOM_TO_JSID. r=jandem

This also allows us to remove JSID_FROM_BITS and gives us stronger assertions everywhere for the whack integer-string case.

Differential Revision: https://phabricator.services.mozilla.com/D72564

dom/bindings/WebIDLGlobalNameHash.cpp

@@ -190,7 +190,7 @@ bool WebIDLGlobalNameHash::GetNames(JSContext* aCx, JS::Handle<JSObject*> aObj,
         (!entry.mEnabled || entry.mEnabled(aCx, aObj))) {
       JSString* str =
           JS_AtomizeStringN(aCx, sNames + entry.mNameOffset, entry.mNameLength);
-      if (!str || !aNames.append(NON_INTEGER_ATOM_TO_JSID(str))) {
+      if (!str || !aNames.append(PropertyKey::fromNonIntAtom(str))) {
         return false;
       }
     }
@@ -261,7 +261,7 @@ bool WebIDLGlobalNameHash::NewEnumerateSystemGlobal(
     if (!entry.mEnabled || entry.mEnabled(aCx, aObj)) {
       JSString* str =
           JS_AtomizeStringN(aCx, sNames + entry.mNameOffset, entry.mNameLength);
-      if (!str || !aProperties.append(NON_INTEGER_ATOM_TO_JSID(str))) {
+      if (!str || !aProperties.append(PropertyKey::fromNonIntAtom(str))) {
         return false;
       }
     }

js/public/Id.h

@@ -110,6 +110,43 @@ struct PropertyKey {
   // Only use this API when absolutely necessary, otherwise use JS_StringToId.
   static PropertyKey fromPinnedString(JSString* str);
 
+  // Must not be used on atoms that are representable as integer PropertyKey.
+  // Prefer NameToId or AtomToId over this function:
+  //
+  // A PropertyName is an atom that does not contain an integer in the range
+  // [0, UINT32_MAX]. However, PropertyKey can only hold an integer in the range
+  // [0, JSID_INT_MAX] (where JSID_INT_MAX == 2^31-1).  Thus, for the range of
+  // integers (JSID_INT_MAX, UINT32_MAX], to represent as a 'id', it must be
+  // the case id.isString() and id.toString()->isIndex(). In most
+  // cases when creating a PropertyKey, code does not have to care about
+  // this corner case because:
+  //
+  // - When given an arbitrary JSAtom*, AtomToId must be used, which checks for
+  //   integer atoms representable as integer PropertyKey, and does this
+  //   conversion.
+  //
+  // - When given a PropertyName*, NameToId can be used which does not need
+  //   to do any dynamic checks.
+  //
+  // Thus, it is only the rare third case which needs this function, which
+  // handles any JSAtom* that is known not to be representable with an int
+  // PropertyKey.
+  static PropertyKey fromNonIntAtom(JSAtom* atom) {
+    MOZ_ASSERT((size_t(atom) & JSID_TYPE_MASK) == 0);
+    MOZ_ASSERT(PropertyKey::isNonIntAtom(atom));
+    return PropertyKey::fromRawBits(size_t(atom) | JSID_TYPE_STRING);
+  }
+
+  // The JSAtom/JSString type exposed to embedders is opaque.
+  static PropertyKey fromNonIntAtom(JSString* str) {
+    MOZ_ASSERT((size_t(str) & JSID_TYPE_MASK) == 0);
+    MOZ_ASSERT(PropertyKey::isNonIntAtom(str));
+    return PropertyKey::fromRawBits(size_t(str) | JSID_TYPE_STRING);
+  }
+
+ private:
+  static bool isNonIntAtom(JSAtom* atom);
+  static bool isNonIntAtom(JSString* atom);
 } JS_HAZ_GC_POINTER;
 
 }  // namespace JS

js/src/gc/Marking-inl.h

@@ -45,7 +45,7 @@ struct TaggedPtr<JS::Value> {
 template <>
 struct TaggedPtr<jsid> {
   static jsid wrap(JSString* str) {
-    return NON_INTEGER_ATOM_TO_JSID(&str->asAtom());
+    return JS::PropertyKey::fromNonIntAtom(str);
   }
   static jsid wrap(JS::Symbol* sym) { return SYMBOL_TO_JSID(sym); }
   static jsid empty() { return JSID_VOID; }

js/src/jsfriendapi.cpp

@@ -1327,14 +1327,6 @@ JS_FRIEND_API void js::SetXrayJitInfo(XrayJitInfo* info) {
 
 XrayJitInfo* js::GetXrayJitInfo() { return gXrayJitInfo; }
 
-bool js::detail::IdMatchesAtom(jsid id, JSAtom* atom) {
-  return id == AtomToId(atom);
-}
-
-bool js::detail::IdMatchesAtom(jsid id, JSString* atom) {
-  return id == AtomToId(&atom->asAtom());
-}
-
 JS_FRIEND_API void js::PrepareScriptEnvironmentAndInvoke(
     JSContext* cx, HandleObject global,
     ScriptEnvironmentPreparer::Closure& closure) {

js/src/jsfriendapi.h

@@ -2198,59 +2198,6 @@ static MOZ_ALWAYS_INLINE void SET_JITINFO(JSFunction* func,
   fun->jitinfo = info;
 }
 
-/*
- * Engine-internal extensions of jsid.  This code is here only until we
- * eliminate Gecko's dependencies on it!
- */
-
-static MOZ_ALWAYS_INLINE jsid JSID_FROM_BITS(size_t bits) {
-  jsid id;
-  JSID_BITS(id) = bits;
-  return id;
-}
-
-namespace js {
-namespace detail {
-bool IdMatchesAtom(jsid id, JSAtom* atom);
-bool IdMatchesAtom(jsid id, JSString* atom);
-}  // namespace detail
-}  // namespace js
-
-/**
- * Must not be used on atoms that are representable as integer jsids.
- * Prefer NameToId or AtomToId over this function:
- *
- * A PropertyName is an atom that does not contain an integer in the range
- * [0, UINT32_MAX]. However, jsid can only hold an integer in the range
- * [0, JSID_INT_MAX] (where JSID_INT_MAX == 2^31-1).  Thus, for the range of
- * integers (JSID_INT_MAX, UINT32_MAX], to represent as a jsid 'id', it must be
- * the case JSID_IS_ATOM(id) and !JSID_TO_ATOM(id)->isPropertyName().  In most
- * cases when creating a jsid, code does not have to care about this corner
- * case because:
- *
- * - When given an arbitrary JSAtom*, AtomToId must be used, which checks for
- *   integer atoms representable as integer jsids, and does this conversion.
- *
- * - When given a PropertyName*, NameToId can be used which which does not need
- *   to do any dynamic checks.
- *
- * Thus, it is only the rare third case which needs this function, which
- * handles any JSAtom* that is known not to be representable with an int jsid.
- */
-static MOZ_ALWAYS_INLINE jsid NON_INTEGER_ATOM_TO_JSID(JSAtom* atom) {
-  MOZ_ASSERT(((size_t)atom & JSID_TYPE_MASK) == 0);
-  jsid id = JSID_FROM_BITS((size_t)atom | JSID_TYPE_STRING);
-  MOZ_ASSERT(js::detail::IdMatchesAtom(id, atom));
-  return id;
-}
-
-static MOZ_ALWAYS_INLINE jsid NON_INTEGER_ATOM_TO_JSID(JSString* atom) {
-  MOZ_ASSERT(((size_t)atom & JSID_TYPE_MASK) == 0);
-  jsid id = JSID_FROM_BITS((size_t)atom | JSID_TYPE_STRING);
-  MOZ_ASSERT(js::detail::IdMatchesAtom(id, atom));
-  return id;
-}
-
 // All strings stored in jsids are atomized, but are not necessarily property
 // names.
 static MOZ_ALWAYS_INLINE bool JSID_IS_ATOM(jsid id) {
@@ -2258,7 +2205,7 @@ static MOZ_ALWAYS_INLINE bool JSID_IS_ATOM(jsid id) {
 }
 
 static MOZ_ALWAYS_INLINE bool JSID_IS_ATOM(jsid id, JSAtom* atom) {
-  return id == NON_INTEGER_ATOM_TO_JSID(atom);
+  return id == JS::PropertyKey::fromNonIntAtom(atom);
 }
 
 static MOZ_ALWAYS_INLINE JSAtom* JSID_TO_ATOM(jsid id) {

js/src/vm/Id.cpp

@@ -30,3 +30,16 @@ bool JS::PropertyKey::isWellKnownSymbol(JS::SymbolCode code) const {
   MOZ_ASSERT(str->asAtom().isPinned());
   return js::AtomToId(&str->asAtom());
 }
+
+/* static */ bool JS::PropertyKey::isNonIntAtom(JSAtom* atom) {
+  uint32_t index;
+  if (!atom->isIndex(&index)) {
+    return true;
+  }
+  static_assert(JSID_INT_MIN == 0);
+  return index > JSID_INT_MAX;
+}
+
+/* static */ bool JS::PropertyKey::isNonIntAtom(JSString* str) {
+  return JS::PropertyKey::isNonIntAtom(&str->asAtom());
+}

js/src/vm/JSAtom-inl.h

@@ -28,7 +28,7 @@ inline jsid AtomToId(JSAtom* atom) {
     return INT_TO_JSID(int32_t(index));
   }
 
-  return JSID_FROM_BITS(size_t(atom) | JSID_TYPE_STRING);
+  return JS::PropertyKey::fromNonIntAtom(atom);
 }
 
 // Use the NameToId method instead!

js/src/vm/JSAtom.cpp

@@ -1111,7 +1111,7 @@ bool js::IndexToIdSlow(JSContext* cx, uint32_t index, MutableHandleId idp) {
     return false;
   }
 
-  idp.set(JSID_FROM_BITS((size_t)atom | JSID_TYPE_STRING));
+  idp.set(JS::PropertyKey::fromNonIntAtom(atom));
   return true;
 }
 

js/src/vm/StringType.h

@@ -1433,7 +1433,7 @@ static_assert(sizeof(PropertyName) == sizeof(JSString),
               "string subclasses must be binary-compatible with JSString");
 
 static MOZ_ALWAYS_INLINE jsid NameToId(PropertyName* name) {
-  return NON_INTEGER_ATOM_TO_JSID(name);
+  return JS::PropertyKey::fromNonIntAtom(name);
 }
 
 using PropertyNameVector = JS::GCVector<PropertyName*>;