Skip to content

Commit

Permalink
Return PseudoHandle from GCCell creation functions.
Browse files Browse the repository at this point in the history
Summary:
Remove many uses of `CallResult<HermesValue>` as the return type
for `GCCell` creation functions.

This requires work because `NativeConstructor` relies on all
constructible `GCCell`s having a default creator function with
the same signature. We now call that function `creatorFunction`
and have it wrap more useful `create` functions.

This results in better typing, better ownership tracking,
and less `CallResult`s that can only be `RETURNED`.

Reviewed By: tmikov

Differential Revision: D19455872

fbshipit-source-id: 8c7ffc6c2e4b1d9d3ff9896fdb033ba6cfba7d9a
  • Loading branch information
avp authored and facebook-github-bot committed Feb 26, 2020
1 parent 0c811ee commit 8f71bce
Show file tree
Hide file tree
Showing 59 changed files with 311 additions and 341 deletions.
38 changes: 28 additions & 10 deletions include/hermes/VM/Callable.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "hermes/VM/ArrayStorage.h"
#include "hermes/VM/Domain.h"
#include "hermes/VM/Handle.h"
#include "hermes/VM/JSObject.h"
#include "hermes/VM/NativeArgs.h"

Expand Down Expand Up @@ -706,7 +707,8 @@ class NativeFunction : public Callable {
/// Object.
class NativeConstructor final : public NativeFunction {
public:
using CreatorFunction = CallResult<HermesValue>(Runtime *, Handle<JSObject>);
using CreatorFunction =
CallResult<PseudoHandle<JSObject>>(Runtime *, Handle<JSObject>);

/// Unifies signatures of various GCCells so that they may be stored
/// in the NativeConstructor.
Expand Down Expand Up @@ -850,7 +852,11 @@ class NativeConstructor final : public NativeFunction {
Runtime *runtime,
Handle<JSObject> parentHandle) {
auto nativeConsHandle = Handle<NativeConstructor>::vmcast(selfHandle);
return nativeConsHandle->creator_(runtime, parentHandle);
auto res = nativeConsHandle->creator_(runtime, parentHandle);
if (LLVM_UNLIKELY(res == ExecutionStatus::EXCEPTION)) {
return ExecutionStatus::EXCEPTION;
}
return res->getHermesValue();
}

#ifndef NDEBUG
Expand Down Expand Up @@ -956,7 +962,7 @@ class JSFunction : public Callable {
}

/// Create a Function with the prototype property set to new Object().
static CallResult<HermesValue> create(
static PseudoHandle<JSFunction> create(
Runtime *runtime,
Handle<Domain> domain,
Handle<JSObject> parentHandle,
Expand All @@ -965,7 +971,7 @@ class JSFunction : public Callable {

/// Create a Function with no environment and a CodeBlock simply returning
/// undefined, with the prototype property auto-initialized to new Object().
static CallResult<HermesValue> create(
static PseudoHandle<JSFunction> create(
Runtime *runtime,
Handle<Domain> domain,
Handle<JSObject> parentHandle) {
Expand All @@ -979,13 +985,11 @@ class JSFunction : public Callable {

/// Create a Function with no environment and a CodeBlock simply returning
/// undefined, with the prototype property auto-initialized to new Object().
/// This creates a new Domain for the new Function to exist in, because it was
/// compiled separately from any currently executing JS code.
static CallResult<HermesValue> createWithNewDomain(
static PseudoHandle<JSFunction> create(
Runtime *runtime,
Handle<JSObject> protoHandle) {
Handle<JSObject> parentHandle) {
return create(
runtime, runtime->makeHandle(Domain::create(runtime)), protoHandle);
runtime, runtime->makeHandle(Domain::create(runtime)), parentHandle);
}

/// \return the code block containing the function code.
Expand Down Expand Up @@ -1025,13 +1029,27 @@ class JSGeneratorFunction final : public JSFunction {
static CallableVTable vt;

/// Create a GeneratorFunction.
static CallResult<HermesValue> create(
static PseudoHandle<JSGeneratorFunction> create(
Runtime *runtime,
Handle<Domain> domain,
Handle<JSObject> parentHandle,
Handle<Environment> envHandle,
CodeBlock *codeBlock);

/// Create a GeneratorFunction with no environment and a CodeBlock simply
/// returning undefined, with the prototype property auto-initialized to new
/// Object().
static PseudoHandle<JSGeneratorFunction> create(
Runtime *runtime,
Handle<JSObject> parentHandle) {
return create(
runtime,
runtime->makeHandle(Domain::create(runtime)),
parentHandle,
runtime->makeNullHandle<Environment>(),
runtime->getEmptyCodeBlock());
}

static bool classof(const GCCell *cell) {
return cell->getKind() == CellKind::GeneratorFunctionKind;
}
Expand Down
4 changes: 1 addition & 3 deletions include/hermes/VM/Domain.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,7 @@ class Domain final : public GCCell {

/// \return the throwing require function with require.context bound to a
/// context for this domain.
PseudoHandle<NativeFunction> getThrowingRequire(Runtime *runtime) const {
return createPseudoHandle(throwingRequire_.get(runtime));
}
PseudoHandle<NativeFunction> getThrowingRequire(Runtime *runtime) const;

private:
/// Create a domain with no associated RuntimeModules.
Expand Down
10 changes: 5 additions & 5 deletions include/hermes/VM/JSArray.h
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ class Arguments final : public ArrayImpl {

/// Create an instance of Arguments, with size and capacity equal to \p length
/// and a property "length" initialized to that value.
static CallResult<HermesValue> create(
static CallResult<Handle<Arguments>> create(
Runtime *runtime,
size_type length,
Handle<Callable> curFunction,
Expand Down Expand Up @@ -311,14 +311,14 @@ class JSArray final : public ArrayImpl {
/// Create an instance of Array, with [[Prototype]] initialized with
/// \p prototypeHandle, with capacity for \p capacity elements and actual size
/// \p length.
static CallResult<HermesValue> create(
static CallResult<PseudoHandle<JSArray>> create(
Runtime *runtime,
Handle<JSObject> prototypeHandle,
Handle<HiddenClass> classHandle,
size_type capacity = 0,
size_type length = 0);

static CallResult<HermesValue> create(
static CallResult<PseudoHandle<JSArray>> create(
Runtime *runtime,
Handle<JSObject> prototypeHandle,
size_type capacity,
Expand All @@ -332,7 +332,7 @@ class JSArray final : public ArrayImpl {
capacity,
length);
}
static CallResult<HermesValue> create(
static CallResult<PseudoHandle<JSArray>> create(
Runtime *runtime,
Handle<JSObject> prototypeHandle) {
return create(runtime, prototypeHandle, 0, 0);
Expand Down Expand Up @@ -435,7 +435,7 @@ class JSArrayIterator : public JSObject {
return cell->getKind() == CellKind::ArrayIteratorKind;
}

static CallResult<HermesValue>
static PseudoHandle<JSArrayIterator>
create(Runtime *runtime, Handle<JSObject> array, IterationKind iterationKind);

/// Iterate to the next element and return.
Expand Down
2 changes: 1 addition & 1 deletion include/hermes/VM/JSArrayBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class JSArrayBuffer final : public JSObject {
return cell->getKind() == CellKind::ArrayBufferKind;
}

static CallResult<HermesValue> create(
static PseudoHandle<JSArrayBuffer> create(
Runtime *runtime,
Handle<JSObject> prototype);

Expand Down
2 changes: 1 addition & 1 deletion include/hermes/VM/JSDataView.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class JSDataView final : public JSObject {
return cell->getKind() == CellKind::DataViewKind;
}

static CallResult<HermesValue> create(
static PseudoHandle<JSDataView> create(
Runtime *runtime,
Handle<JSObject> prototype);

Expand Down
4 changes: 2 additions & 2 deletions include/hermes/VM/JSDate.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ class JSDate final : public JSObject {
return cell->getKind() == CellKind::DateKind;
}

static CallResult<HermesValue>
static PseudoHandle<JSDate>
create(Runtime *runtime, double value, Handle<JSObject> prototype);

static CallResult<HermesValue> create(
static PseudoHandle<JSDate> create(
Runtime *runtime,
Handle<JSObject> prototype) {
return create(runtime, std::numeric_limits<double>::quiet_NaN(), prototype);
Expand Down
6 changes: 3 additions & 3 deletions include/hermes/VM/JSError.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ class JSError final : public JSObject {
}

/// Create an Error Object.
static CallResult<HermesValue> create(
static PseudoHandle<JSError> create(
Runtime *runtime,
Handle<JSObject> prototype);
/// Create an uncatchable Error Object. If this object is thrown, no catch
/// handlers or finally handlers are called.
/// NOTE: This should be used only in very specific circumstances where it is
/// impossible or undesirable to continue running the VM. The error should be
/// considered a fatal abort, but one that cleans up internal VM resources.
static CallResult<HermesValue> createUncatchable(
static PseudoHandle<JSError> createUncatchable(
Runtime *runtime,
Handle<JSObject> prototype);

Expand Down Expand Up @@ -128,7 +128,7 @@ class JSError final : public JSObject {
static void _finalizeImpl(GCCell *cell, GC *gc);
static size_t _mallocSizeImpl(GCCell *cell);

static CallResult<HermesValue>
static PseudoHandle<JSError>
create(Runtime *runtime, Handle<JSObject> prototype, bool catchable);

/// A pointer to the stack trace, or nullptr if it has not been set.
Expand Down
4 changes: 2 additions & 2 deletions include/hermes/VM/JSMapImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class JSMapImpl final : public JSObject {
return cell->getKind() == C;
}

static CallResult<HermesValue> create(
static PseudoHandle<JSMapImpl<C>> create(
Runtime *runtime,
Handle<JSObject> parentHandle);

Expand Down Expand Up @@ -183,7 +183,7 @@ class JSMapIteratorImpl final : public JSObject {
}

/// Create a handle of JSMapIterator.
static CallResult<HermesValue> create(
static PseudoHandle<JSMapIteratorImpl<C>> create(
Runtime *runtime,
Handle<JSObject> prototype);

Expand Down
3 changes: 1 addition & 2 deletions include/hermes/VM/JSNativeFunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@ namespace vm {
CallResult<HermesValue> func(void *, Runtime *, NativeArgs);
#include "hermes/VM/NativeFunctions.def"

using CreatorFunction = CallResult<HermesValue>(Runtime *, Handle<JSObject>);
/// Get a human-readable name of a native function.
const char *getFunctionName(NativeFunctionPtr);
const char *getFunctionName(CreatorFunction *);
const char *getFunctionName(NativeConstructor::CreatorFunction *);

} // namespace vm
} // namespace hermes
Expand Down
11 changes: 2 additions & 9 deletions include/hermes/VM/JSObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -407,13 +407,6 @@ class JSObject : public GCCell {
Runtime *runtime,
Handle<HiddenClass> clazz);

/// Attempts to allocate a JSObject and returns whether it succeeded or not.
/// NOTE: This function always returns \c ExecutionStatus::RETURNED, it is
/// only used in interfaces where other creators may throw a JS exception.
static CallResult<HermesValue> createWithException(
Runtime *runtime,
Handle<JSObject> parentHandle);

~JSObject() = default;

/// Must be called immediately after the construction of any JSObject.
Expand Down Expand Up @@ -1579,14 +1572,14 @@ inline CallResult<PseudoHandle<JSObject>> JSObject::allocatePropStorage(
if (LLVM_LIKELY(size <= DIRECT_PROPERTY_SLOTS))
return self;

auto selfHandle = runtime->makeHandle(std::move(self));
Handle<JSObject> selfHandle = runtime->makeHandle(std::move(self));
if (LLVM_UNLIKELY(
allocatePropStorage(selfHandle, runtime, size) ==
ExecutionStatus::EXCEPTION)) {
return ExecutionStatus::EXCEPTION;
}

return PseudoHandle<JSObject>(selfHandle);
return PseudoHandle<JSObject>{selfHandle};
}

template <typename T>
Expand Down
2 changes: 1 addition & 1 deletion include/hermes/VM/JSProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class JSProxy : public JSObject {

static PseudoHandle<JSProxy> create(Runtime *runtime);

static CallResult<HermesValue> create(
static PseudoHandle<JSProxy> create(
Runtime *runtime,
Handle<JSObject> prototype);

Expand Down
4 changes: 1 addition & 3 deletions include/hermes/VM/JSRegExp.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@ class JSRegExp final : public JSObject {
};

/// Create a JSRegExp, with the empty string for pattern and flags
static CallResult<HermesValue> create(
Runtime *runtime,
Handle<JSObject> prototype);
static Handle<JSRegExp> create(Runtime *runtime, Handle<JSObject> prototype);

/// Perform validation of the pattern and flags and throw \c SyntaxError on
/// error. If valid, set the source and flags to the given strings, and set
Expand Down
2 changes: 1 addition & 1 deletion include/hermes/VM/JSTypedArray.h
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ class JSTypedArray final : public JSTypedArrayBase {
return cell->getKind() == C;
}

static CallResult<HermesValue> create(
static PseudoHandle<JSTypedArray<T, C>> create(
Runtime *runtime,
Handle<JSObject> prototype);

Expand Down
2 changes: 1 addition & 1 deletion include/hermes/VM/JSWeakMapImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ class JSWeakMapImpl final : public JSWeakMapImplBase {
}

/// Create a new WeakMap with prototype property \p parentHandle.
static CallResult<HermesValue> create(
static CallResult<PseudoHandle<JSWeakMapImpl<C>>> create(
Runtime *runtime,
Handle<JSObject> parentHandle);

Expand Down
52 changes: 27 additions & 25 deletions include/hermes/VM/NativeFunctions.def
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
* LICENSE file in the root directory of this source tree.
*/

// Updated Dec 13, 2019
#define NATIVE_FUNCTION_VERSION_VALUE 7
// Updated Jan 28, 2019
#define NATIVE_FUNCTION_VERSION_VALUE 8

#ifndef NATIVE_FUNCTION
#define NATIVE_FUNCTION(func)
Expand Down Expand Up @@ -392,30 +392,32 @@ NATIVE_FUNCTION(instrumentReturnNth)

// NativeConstructor

// creator funcitons
NATIVE_CONSTRUCTOR(JSObject::createWithException)
NATIVE_CONSTRUCTOR(JSFunction::createWithNewDomain)
// Creator functions
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSObject>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSFunction>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSArray>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSArrayBuffer>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSBoolean>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSDataView>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSDate>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSError>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSMap>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSNumber>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSProxy>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSRegExp>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSSet>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSString>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSSymbol>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSWeakMap>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSWeakSet>)
NATIVE_CONSTRUCTOR(NativeConstructor::creatorFunction<JSGeneratorFunction>)

// NativeClass::create
NATIVE_CONSTRUCTOR(JSArray::create)
NATIVE_CONSTRUCTOR(JSArrayBuffer::create)
NATIVE_CONSTRUCTOR(JSBoolean::create)
NATIVE_CONSTRUCTOR(JSDataView::create)
NATIVE_CONSTRUCTOR(JSDate::create)
NATIVE_CONSTRUCTOR(JSError::create)
NATIVE_CONSTRUCTOR(JSMap::create)
NATIVE_CONSTRUCTOR(JSNumber::create)
NATIVE_CONSTRUCTOR(JSProxy::create)
NATIVE_CONSTRUCTOR(JSRegExp::create)
NATIVE_CONSTRUCTOR(JSSet::create)
NATIVE_CONSTRUCTOR(JSString::create)
NATIVE_CONSTRUCTOR(JSSymbol::create)
NATIVE_CONSTRUCTOR(JSWeakMap::create)
NATIVE_CONSTRUCTOR(JSWeakSet::create)

#define TYPED_ARRAY(name, type) \
NATIVE_CONSTRUCTOR_TYPED( \
JSTypedArray, type, CellKind::name##ArrayKind, create)
#define TYPED_ARRAY(name, type) \
NATIVE_CONSTRUCTOR_TYPED( \
JSTypedArray, \
type, \
CellKind::name##ArrayKind, \
NativeConstructor::creatorFunction)
#include "hermes/VM/TypedArrays.def"
#undef TYPED_ARRAY

Expand Down
1 change: 1 addition & 0 deletions include/hermes/VM/Operations.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "hermes/VM/CallResult.h"
#include "hermes/VM/InternalProperty.h"
#include "hermes/VM/Runtime.h"
#include "hermes/VM/StringPrimitive.h"
#include "hermes/VM/SymbolID.h"

#include "llvm/ADT/SmallVector.h"
Expand Down
Loading

0 comments on commit 8f71bce

Please sign in to comment.