Bug 1528931 - Introduce JS::ValueType and Value::type. r=jandem
authorTom Schuster <evilpies@gmail.com>
Thu, 21 Feb 2019 20:36:44 +0000
changeset 518338 92f35371d6b31ec12b5563034c1110dd794018f3
parent 518337 2e049c2537d4b5cf0615448c8371950a9990f18d
child 518339 2681b6f4e98fd7447098a5467389297ea1ba17d6
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1528931
milestone67.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 1528931 - Introduce JS::ValueType and Value::type. r=jandem I only converted a few low hanging fruits to the new API. Having to check for PrivateGCThing, which often can't even appear is a bit annoying, but I don't think we really need a different type. I think next I can look into some of the extractNonDoubleType uses. Differential Revision: https://phabricator.services.mozilla.com/D20474
js/public/Value.h
js/src/NamespaceImports.h
js/src/gc/GC.cpp
js/src/jit/CacheIR.cpp
js/src/vm/Interpreter.cpp
js/src/vm/JSFunction.cpp
js/src/vm/JSObject.cpp
--- a/js/public/Value.h
+++ b/js/public/Value.h
@@ -65,16 +65,32 @@ enum JSValueType : uint8_t {
   JSVAL_TYPE_OBJECT = 0x0c,
 
   // These never appear in a jsval; they are only provided as an out-of-band
   // value.
   JSVAL_TYPE_UNKNOWN = 0x20,
   JSVAL_TYPE_MISSING = 0x21
 };
 
+namespace JS {
+enum class ValueType : uint8_t {
+  Double = JSVAL_TYPE_DOUBLE,
+  Int32 = JSVAL_TYPE_INT32,
+  Boolean = JSVAL_TYPE_BOOLEAN,
+  Undefined = JSVAL_TYPE_UNDEFINED,
+  Null = JSVAL_TYPE_NULL,
+  Magic = JSVAL_TYPE_MAGIC,
+  String = JSVAL_TYPE_STRING,
+  Symbol = JSVAL_TYPE_SYMBOL,
+  PrivateGCThing = JSVAL_TYPE_PRIVATE_GCTHING,
+  BigInt = JSVAL_TYPE_BIGINT,
+  Object = JSVAL_TYPE_OBJECT,
+};
+}
+
 static_assert(sizeof(JSValueType) == 1,
               "compiler typed enum support is apparently buggy");
 
 #if defined(JS_NUNBOX32)
 
 JS_ENUM_HEADER(JSValueTag, uint32_t){
     JSVAL_TAG_CLEAR = 0xFFFFFF80,
     JSVAL_TAG_INT32 = JSVAL_TAG_CLEAR | JSVAL_TYPE_INT32,
@@ -785,16 +801,26 @@ union alignas(8) Value {
   uint64_t asRawBits() const { return asBits_; }
 
   JSValueType extractNonDoubleType() const {
     uint32_t type = toTag() & 0xF;
     MOZ_ASSERT(type > JSVAL_TYPE_DOUBLE);
     return JSValueType(type);
   }
 
+  JS::ValueType type() const {
+    if (isDouble()) {
+      return JS::ValueType::Double;
+    }
+
+    JSValueType type = extractNonDoubleType();
+    MOZ_ASSERT(type <= JSVAL_TYPE_OBJECT);
+    return JS::ValueType(type);
+  }
+
   /*
    * Private API
    *
    * Private setters/getters allow the caller to read/write arbitrary types
    * that fit in the 64-bit payload. It is the caller's responsibility, after
    * storing to a value with setPrivateX to read only using getPrivateX.
    * Privates values are given a type which ensures they are not marked.
    */
@@ -1179,16 +1205,17 @@ class WrappedPtrOperations<JS::Value, Wr
   JS::TraceKind traceKind() const { return value().traceKind(); }
   void* toPrivate() const { return value().toPrivate(); }
   uint32_t toPrivateUint32() const { return value().toPrivateUint32(); }
 
   uint64_t asRawBits() const { return value().asRawBits(); }
   JSValueType extractNonDoubleType() const {
     return value().extractNonDoubleType();
   }
+  JS::ValueType type() const { return value().type(); }
 
   JSWhyMagic whyMagic() const { return value().whyMagic(); }
   uint32_t magicUint32() const { return value().magicUint32(); }
 };
 
 /**
  * A class designed for CRTP use in implementing all the mutating parts of the
  * Value interface in Value-like classes.  Wrapper must be a class inheriting
@@ -1278,43 +1305,54 @@ class HeapBase<JS::Value, Wrapper>
     }
   }
 };
 
 // If the Value is a GC pointer type, call |f| with the pointer cast to that
 // type and return the result wrapped in a Maybe, otherwise return None().
 template <typename F>
 auto MapGCThingTyped(const JS::Value& val, F&& f) {
-  if (val.isString()) {
-    JSString* str = val.toString();
-    MOZ_ASSERT(gc::IsCellPointerValid(str));
-    return mozilla::Some(f(str));
-  }
-  if (val.isObject()) {
-    JSObject* obj = &val.toObject();
-    MOZ_ASSERT(gc::IsCellPointerValid(obj));
-    return mozilla::Some(f(obj));
+  switch (val.type()) {
+    case JS::ValueType::String: {
+      JSString* str = val.toString();
+      MOZ_ASSERT(gc::IsCellPointerValid(str));
+      return mozilla::Some(f(str));
+    }
+    case JS::ValueType::Object: {
+      JSObject* obj = &val.toObject();
+      MOZ_ASSERT(gc::IsCellPointerValid(obj));
+      return mozilla::Some(f(obj));
+    }
+    case JS::ValueType::Symbol: {
+      JS::Symbol* sym = val.toSymbol();
+      MOZ_ASSERT(gc::IsCellPointerValid(sym));
+      return mozilla::Some(f(sym));
+    }
+    case JS::ValueType::BigInt: {
+      JS::BigInt* bi = val.toBigInt();
+      MOZ_ASSERT(gc::IsCellPointerValid(bi));
+      return mozilla::Some(f(bi));
+    }
+    case JS::ValueType::PrivateGCThing: {
+      MOZ_ASSERT(gc::IsCellPointerValid(val.toGCThing()));
+      return mozilla::Some(MapGCThingTyped(val.toGCCellPtr(), std::move(f)));
+    }
+    case JS::ValueType::Double:
+    case JS::ValueType::Int32:
+    case JS::ValueType::Boolean:
+    case JS::ValueType::Undefined:
+    case JS::ValueType::Null:
+    case JS::ValueType::Magic: {
+      MOZ_ASSERT(!val.isGCThing());
+      using ReturnType = decltype(f(static_cast<JSObject*>(nullptr)));
+      return mozilla::Maybe<ReturnType>();
+    }
   }
-  if (val.isSymbol()) {
-    JS::Symbol* sym = val.toSymbol();
-    MOZ_ASSERT(gc::IsCellPointerValid(sym));
-    return mozilla::Some(f(sym));
-  }
-  if (val.isBigInt()) {
-    JS::BigInt* bi = val.toBigInt();
-    MOZ_ASSERT(gc::IsCellPointerValid(bi));
-    return mozilla::Some(f(bi));
-  }
-  if (MOZ_UNLIKELY(val.isPrivateGCThing())) {
-    MOZ_ASSERT(gc::IsCellPointerValid(val.toGCThing()));
-    return mozilla::Some(MapGCThingTyped(val.toGCCellPtr(), std::move(f)));
-  }
-  MOZ_ASSERT(!val.isGCThing());
-  using ReturnType = decltype(f(static_cast<JSObject*>(nullptr)));
-  return mozilla::Maybe<ReturnType>();
+
+  MOZ_CRASH("no missing return");
 }
 
 // If the Value is a GC pointer type, call |f| with the pointer cast to that
 // type. Return whether this happened.
 template <typename F>
 bool ApplyGCThingTyped(const JS::Value& val, F&& f) {
   return MapGCThingTyped(val,
                          [&f](auto t) {
--- a/js/src/NamespaceImports.h
+++ b/js/src/NamespaceImports.h
@@ -64,16 +64,17 @@ using JS::NumberValue;
 using JS::ObjectOrNullValue;
 using JS::ObjectValue;
 using JS::PrivateGCThingValue;
 using JS::PrivateUint32Value;
 using JS::PrivateValue;
 using JS::StringValue;
 using JS::UndefinedValue;
 using JS::Value;
+using JS::ValueType;
 
 using JS::ConstTwoByteChars;
 using JS::Latin1Char;
 using JS::Latin1Chars;
 using JS::Latin1CharsZ;
 using JS::TwoByteChars;
 using JS::TwoByteCharsZ;
 using JS::UniqueChars;
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -8365,28 +8365,42 @@ JS_FRIEND_API const char* JS::GCTraceKin
     JS_FOR_EACH_TRACEKIND(MAP_NAME);
 #undef MAP_NAME
     default:
       return "Invalid";
   }
 }
 
 JS::GCCellPtr::GCCellPtr(const Value& v) : ptr(0) {
-  if (v.isString()) {
-    ptr = checkedCast(v.toString(), JS::TraceKind::String);
-  } else if (v.isObject()) {
-    ptr = checkedCast(&v.toObject(), JS::TraceKind::Object);
-  } else if (v.isSymbol()) {
-    ptr = checkedCast(v.toSymbol(), JS::TraceKind::Symbol);
-  } else if (v.isBigInt()) {
-    ptr = checkedCast(v.toBigInt(), JS::TraceKind::BigInt);
-  } else if (v.isPrivateGCThing()) {
-    ptr = checkedCast(v.toGCThing(), v.toGCThing()->getTraceKind());
-  } else {
-    ptr = checkedCast(nullptr, JS::TraceKind::Null);
+  switch (v.type()) {
+    case ValueType::String:
+      ptr = checkedCast(v.toString(), JS::TraceKind::String);
+      break;
+    case ValueType::Object:
+      ptr = checkedCast(&v.toObject(), JS::TraceKind::Object);
+      break;
+    case ValueType::Symbol:
+      ptr = checkedCast(v.toSymbol(), JS::TraceKind::Symbol);
+      break;
+    case ValueType::BigInt:
+      ptr = checkedCast(v.toBigInt(), JS::TraceKind::BigInt);
+      break;
+    case ValueType::PrivateGCThing:
+      ptr = checkedCast(v.toGCThing(), v.toGCThing()->getTraceKind());
+      break;
+    case ValueType::Double:
+    case ValueType::Int32:
+    case ValueType::Boolean:
+    case ValueType::Undefined:
+    case ValueType::Null:
+    case ValueType::Magic: {
+      MOZ_ASSERT(!v.isGCThing());
+      ptr = checkedCast(nullptr, JS::TraceKind::Null);
+      break;
+    }
   }
 }
 
 JS::TraceKind JS::GCCellPtr::outOfLineKind() const {
   MOZ_ASSERT((ptr & OutOfLineTraceKindMask) == OutOfLineTraceKindMask);
   MOZ_ASSERT(asCell()->isTenured());
   return MapAllocToTraceKind(asCell()->asTenured().getAllocKind());
 }
--- a/js/src/jit/CacheIR.cpp
+++ b/js/src/jit/CacheIR.cpp
@@ -1974,33 +1974,44 @@ bool GetPropIRGenerator::tryAttachModule
   writer.typeMonitorResult();
 
   trackAttached("ModuleNamespace");
   return true;
 }
 
 bool GetPropIRGenerator::tryAttachPrimitive(ValOperandId valId, HandleId id) {
   JSProtoKey protoKey;
-  if (val_.isString()) {
-    if (JSID_IS_ATOM(id, cx_->names().length)) {
-      // String length is special-cased, see js::GetProperty.
+  switch (val_.type()) {
+    case ValueType::String:
+      if (JSID_IS_ATOM(id, cx_->names().length)) {
+        // String length is special-cased, see js::GetProperty.
+        return false;
+      }
+      protoKey = JSProto_String;
+      break;
+    case ValueType::Int32:
+    case ValueType::Double:
+      protoKey = JSProto_Number;
+      break;
+    case ValueType::Boolean:
+      protoKey = JSProto_Boolean;
+      break;
+    case ValueType::Symbol:
+      protoKey = JSProto_Symbol;
+      break;
+    case ValueType::BigInt:
+      protoKey = JSProto_BigInt;
+      break;
+    case ValueType::Null:
+    case ValueType::Undefined:
+    case ValueType::Magic:
       return false;
-    }
-    protoKey = JSProto_String;
-  } else if (val_.isNumber()) {
-    protoKey = JSProto_Number;
-  } else if (val_.isBoolean()) {
-    protoKey = JSProto_Boolean;
-  } else if (val_.isSymbol()) {
-    protoKey = JSProto_Symbol;
-  } else if (val_.isBigInt()) {
-    protoKey = JSProto_BigInt;
-  } else {
-    MOZ_ASSERT(val_.isNullOrUndefined() || val_.isMagic());
-    return false;
+    case ValueType::Object:
+    case ValueType::PrivateGCThing:
+      MOZ_CRASH("unexpected type");
   }
 
   RootedObject proto(cx_, cx_->global()->maybeGetPrototype(protoKey));
   if (!proto) {
     return false;
   }
 
   RootedShape shape(cx_);
@@ -5891,17 +5902,17 @@ bool UnaryArithIRGenerator::tryAttachInt
       writer.int32IncResult(intId);
       trackAttached("UnaryArith.Int32Inc");
       break;
     case JSOP_DEC:
       writer.int32DecResult(intId);
       trackAttached("UnaryArith.Int32Dec");
       break;
     default:
-      MOZ_CRASH("Unexected OP");
+      MOZ_CRASH("unexpected OP");
   }
 
   writer.returnFromIC();
   return true;
 }
 
 bool UnaryArithIRGenerator::tryAttachNumber() {
   if (!val_.isNumber() || !res_.isNumber()) {
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -866,39 +866,40 @@ JSType js::TypeOfObject(JSObject* obj) {
   }
   if (obj->isCallable()) {
     return JSTYPE_FUNCTION;
   }
   return JSTYPE_OBJECT;
 }
 
 JSType js::TypeOfValue(const Value& v) {
-  if (v.isNumber()) {
-    return JSTYPE_NUMBER;
-  }
-  if (v.isString()) {
-    return JSTYPE_STRING;
-  }
-  if (v.isNull()) {
-    return JSTYPE_OBJECT;
-  }
-  if (v.isUndefined()) {
-    return JSTYPE_UNDEFINED;
+  switch (v.type()) {
+    case ValueType::Double:
+    case ValueType::Int32:
+      return JSTYPE_NUMBER;
+    case ValueType::String:
+      return JSTYPE_STRING;
+    case ValueType::Null:
+      return JSTYPE_OBJECT;
+    case ValueType::Undefined:
+      return JSTYPE_UNDEFINED;
+    case ValueType::Object:
+      return TypeOfObject(&v.toObject());
+    case ValueType::Boolean:
+      return JSTYPE_BOOLEAN;
+    case ValueType::BigInt:
+      return JSTYPE_BIGINT;
+    case ValueType::Symbol:
+      return JSTYPE_SYMBOL;
+    case ValueType::Magic:
+    case ValueType::PrivateGCThing:
+      break;
   }
-  if (v.isObject()) {
-    return TypeOfObject(&v.toObject());
-  }
-  if (v.isBoolean()) {
-    return JSTYPE_BOOLEAN;
-  }
-  if (v.isBigInt()) {
-    return JSTYPE_BIGINT;
-  }
-  MOZ_ASSERT(v.isSymbol());
-  return JSTYPE_SYMBOL;
+
+  MOZ_CRASH("unexpected type");
 }
 
 bool js::CheckClassHeritageOperation(JSContext* cx, HandleValue heritage) {
   if (IsConstructor(heritage)) {
     return true;
   }
 
   if (heritage.isNull()) {
--- a/js/src/vm/JSFunction.cpp
+++ b/js/src/vm/JSFunction.cpp
@@ -2536,33 +2536,45 @@ JSFunction* js::DefineFunction(
   return fun;
 }
 
 void js::ReportIncompatibleMethod(JSContext* cx, const CallArgs& args,
                                   const Class* clasp) {
   RootedValue thisv(cx, args.thisv());
 
 #ifdef DEBUG
-  if (thisv.isObject()) {
-    MOZ_ASSERT(thisv.toObject().getClass() != clasp ||
-               !thisv.toObject().isNative() ||
-               !thisv.toObject().staticPrototype() ||
-               thisv.toObject().staticPrototype()->getClass() != clasp);
-  } else if (thisv.isString()) {
-    MOZ_ASSERT(clasp != &StringObject::class_);
-  } else if (thisv.isNumber()) {
-    MOZ_ASSERT(clasp != &NumberObject::class_);
-  } else if (thisv.isBoolean()) {
-    MOZ_ASSERT(clasp != &BooleanObject::class_);
-  } else if (thisv.isSymbol()) {
-    MOZ_ASSERT(clasp != &SymbolObject::class_);
-  } else if (thisv.isBigInt()) {
-    MOZ_ASSERT(clasp != &BigIntObject::class_);
-  } else {
-    MOZ_ASSERT(thisv.isUndefined() || thisv.isNull());
+  switch (thisv.type()) {
+    case ValueType::Object:
+      MOZ_ASSERT(thisv.toObject().getClass() != clasp ||
+                 !thisv.toObject().isNative() ||
+                 !thisv.toObject().staticPrototype() ||
+                 thisv.toObject().staticPrototype()->getClass() != clasp);
+      break;
+    case ValueType::String:
+      MOZ_ASSERT(clasp != &StringObject::class_);
+      break;
+    case ValueType::Double:
+    case ValueType::Int32:
+      MOZ_ASSERT(clasp != &NumberObject::class_);
+      break;
+    case ValueType::Boolean:
+      MOZ_ASSERT(clasp != &BooleanObject::class_);
+      break;
+    case ValueType::Symbol:
+      MOZ_ASSERT(clasp != &SymbolObject::class_);
+      break;
+    case ValueType::BigInt:
+      MOZ_ASSERT(clasp != &BigIntObject::class_);
+      break;
+    case ValueType::Undefined:
+    case ValueType::Null:
+      break;
+    case ValueType::Magic:
+    case ValueType::PrivateGCThing:
+      MOZ_CRASH("unexpected type");
   }
 #endif
 
   if (JSFunction* fun = ReportIfNotFunction(cx, args.calleev())) {
     UniqueChars funNameBytes;
     if (const char* funName = GetFunctionNameBytes(cx, fun, &funNameBytes)) {
       JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr,
                                JSMSG_INCOMPATIBLE_PROTO, clasp->name, funName,
--- a/js/src/vm/JSObject.cpp
+++ b/js/src/vm/JSObject.cpp
@@ -108,41 +108,40 @@ void js::ReportNotObjectWithName(JSConte
   UniqueChars bytes;
   if (const char* chars = ValueToSourceForError(cx, v, bytes)) {
     JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr,
                              JSMSG_NOT_NONNULL_OBJECT_NAME, name, chars);
   }
 }
 
 JS_PUBLIC_API const char* JS::InformalValueTypeName(const Value& v) {
-  if (v.isObject()) {
-    return v.toObject().getClass()->name;
-  }
-  if (v.isString()) {
-    return "string";
-  }
-  if (v.isSymbol()) {
-    return "symbol";
-  }
-  if (v.isBigInt()) {
-    return "bigint";
-  }
-  if (v.isNumber()) {
-    return "number";
-  }
-  if (v.isBoolean()) {
-    return "boolean";
-  }
-  if (v.isNull()) {
-    return "null";
-  }
-  if (v.isUndefined()) {
-    return "undefined";
-  }
-  return "value";
+  switch (v.type()) {
+    case ValueType::Double:
+    case ValueType::Int32:
+      return "number";
+    case ValueType::Boolean:
+      return "boolean";
+    case ValueType::Undefined:
+      return "undefined";
+    case ValueType::Null:
+      return "null";
+    case ValueType::String:
+      return "string";
+    case ValueType::Symbol:
+      return "symbol";
+    case ValueType::BigInt:
+      return "bigint";
+    case ValueType::Object:
+      return v.toObject().getClass()->name;
+    case ValueType::Magic:
+    case ValueType::PrivateGCThing:
+      break;
+  }
+
+  MOZ_CRASH("unexpected type");
 }
 
 // ES6 draft rev37 6.2.4.4 FromPropertyDescriptor
 JS_PUBLIC_API bool JS::FromPropertyDescriptor(JSContext* cx,
                                               Handle<PropertyDescriptor> desc,
                                               MutableHandleValue vp) {
   AssertHeapIsIdle();
   CHECK_THREAD(cx);