Skip to content

Commit 9e2c71c

Browse files
avpfacebook-github-bot
avp
authored and
facebook-github-bot
committed
Return PseudoHandle from SegmentedArray::create.
Summary: Better types result in simpler code. `IsGCObject` needs to be moved in order for `CallResult<PseudoHandle<SegmentedArray>>` to be used. Reviewed By: dulinriley Differential Revision: D19623994 fbshipit-source-id: 934e96d3df32fbbd6beaf5f56be1ed8167743360
1 parent 5cae526 commit 9e2c71c

7 files changed

Lines changed: 30 additions & 28 deletions

File tree

‎include/hermes/VM/HermesValueTraits.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,10 @@ struct IsGCObject<BufferedStringPrimitive<T>> : public std::true_type {};
109109
template <typename T, bool isGCObject = IsGCObject<T>::value>
110110
struct HermesValueTraits;
111111

112+
class SegmentedArray;
113+
template <>
114+
struct IsGCObject<SegmentedArray> : public std::true_type {};
115+
112116
template <>
113117
struct HermesValueTraits<HermesValue> {
114118
/// The type to be returned by Handle<T>::get().

‎include/hermes/VM/SegmentedArray.h‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -243,13 +243,15 @@ class SegmentedArray final
243243

244244
/// Creates a new SegmentedArray that has space for at least the requested \p
245245
/// capacity number of elements, and has size 0.
246-
static CallResult<HermesValue> create(Runtime *runtime, size_type capacity);
247-
static CallResult<HermesValue> createLongLived(
246+
static CallResult<PseudoHandle<SegmentedArray>> create(
247+
Runtime *runtime,
248+
size_type capacity);
249+
static CallResult<PseudoHandle<SegmentedArray>> createLongLived(
248250
Runtime *runtime,
249251
size_type capacity);
250252
/// Same as \c create(runtime, capacity) except fills in the first \p size
251253
/// elements with \p fill, and sets the size to \p size.
252-
static CallResult<HermesValue>
254+
static CallResult<PseudoHandle<SegmentedArray>>
253255
create(Runtime *runtime, size_type capacity, size_type size);
254256

255257
/// Gets the element located at \p index.
@@ -613,8 +615,6 @@ SegmentedArray::maxNumSegmentsWithoutOverflow() {
613615
Segment::kMaxLength);
614616
}
615617

616-
template <>
617-
struct IsGCObject<SegmentedArray> : public std::true_type {};
618618
template <>
619619
struct IsGCObject<SegmentedArray::Segment> : public std::true_type {};
620620

‎lib/VM/JSArray.cpp‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ ExecutionStatus ArrayImpl::setStorageEndIndex(
148148
if (LLVM_UNLIKELY(arrRes == ExecutionStatus::EXCEPTION)) {
149149
return ExecutionStatus::EXCEPTION;
150150
}
151-
auto newStorage = runtime->makeHandle<StorageType>(*arrRes);
151+
auto newStorage = runtime->makeHandle<StorageType>(std::move(*arrRes));
152152
selfHandle->indexedStorage_.set(
153153
runtime, newStorage.get(), &runtime->getHeap());
154154
selfHandle->beginIndex_ = 0;
@@ -214,7 +214,7 @@ CallResult<bool> ArrayImpl::_setOwnIndexedImpl(
214214
if (LLVM_UNLIKELY(arrRes == ExecutionStatus::EXCEPTION)) {
215215
return ExecutionStatus::EXCEPTION;
216216
}
217-
auto newStorage = runtime->makeHandle<StorageType>(*arrRes);
217+
auto newStorage = runtime->makeHandle<StorageType>(std::move(*arrRes));
218218

219219
self = vmcast<ArrayImpl>(selfHandle.get());
220220

@@ -403,13 +403,11 @@ CallResult<Handle<Arguments>> Arguments::create(
403403
size_type length,
404404
Handle<Callable> curFunction,
405405
bool strictMode) {
406-
CallResult<HermesValue> arrRes{ExecutionStatus::EXCEPTION};
407-
if (LLVM_UNLIKELY(
408-
(arrRes = StorageType::create(runtime, length)) ==
409-
ExecutionStatus::EXCEPTION)) {
406+
auto arrRes = StorageType::create(runtime, length);
407+
if (LLVM_UNLIKELY(arrRes == ExecutionStatus::EXCEPTION)) {
410408
return ExecutionStatus::EXCEPTION;
411409
}
412-
auto indexedStorage = runtime->makeHandle<StorageType>(*arrRes);
410+
auto indexedStorage = runtime->makeHandle<StorageType>(std::move(*arrRes));
413411

414412
JSObjectAlloc<Arguments> mem{runtime};
415413
auto selfHandle = mem.initToHandle(new (mem) Arguments(
@@ -590,7 +588,7 @@ CallResult<PseudoHandle<JSArray>> JSArray::create(
590588
if (arrRes == ExecutionStatus::EXCEPTION) {
591589
return ExecutionStatus::EXCEPTION;
592590
}
593-
indexedStorage = vmcast<StorageType>(*arrRes);
591+
indexedStorage = std::move(*arrRes);
594592
}
595593

596594
JSObjectAlloc<JSArray> mem{runtime};

‎lib/VM/JSObject.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3137,7 +3137,7 @@ CallResult<Handle<BigStorage>> getForInPropertyNames(
31373137
if (LLVM_UNLIKELY(arrRes == ExecutionStatus::EXCEPTION)) {
31383138
return ExecutionStatus::EXCEPTION;
31393139
}
3140-
arr = vmcast<BigStorage>(*arrRes);
3140+
arr = std::move(*arrRes);
31413141
if (setProtoClasses(runtime, obj, arr) == ExecutionStatus::EXCEPTION) {
31423142
return ExecutionStatus::EXCEPTION;
31433143
}

‎lib/VM/JSWeakMapImpl.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ CallResult<PseudoHandle<JSWeakMapImpl<C>>> JSWeakMapImpl<C>::create(
376376
if (LLVM_UNLIKELY(valueRes == ExecutionStatus::EXCEPTION)) {
377377
return ExecutionStatus::EXCEPTION;
378378
}
379-
auto valueStorage = runtime->makeHandle<BigStorage>(*valueRes);
379+
auto valueStorage = runtime->makeHandle<BigStorage>(std::move(*valueRes));
380380

381381
JSObjectAlloc<JSWeakMapImpl<C>, HasFinalizer::Yes> mem{runtime};
382382
return mem.initToPseudoHandle(new (mem) JSWeakMapImpl<C>(

‎lib/VM/SegmentedArray.cpp‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ void SegmentedArrayDeserialize(Deserializer &d, CellKind kind) {
136136
}
137137
#endif
138138

139-
CallResult<HermesValue> SegmentedArray::create(
139+
CallResult<PseudoHandle<SegmentedArray>> SegmentedArray::create(
140140
Runtime *runtime,
141141
size_type capacity) {
142142
if (LLVM_UNLIKELY(capacity > maxElements())) {
@@ -148,34 +148,33 @@ CallResult<HermesValue> SegmentedArray::create(
148148
// if it is larger than the inline storage space. That is in order to avoid
149149
// having an extra field to track, and the upper bound of "size" can be used
150150
// instead.
151-
return HermesValue::encodeObjectValue(new (
152-
runtime->alloc<false /*fixedSize*/>(allocationSizeForCapacity(capacity)))
153-
SegmentedArray(runtime, capacity));
151+
return createPseudoHandle(new (runtime->alloc<false /*fixedSize*/>(
152+
allocationSizeForCapacity(capacity))) SegmentedArray(runtime, capacity));
154153
}
155154

156-
CallResult<HermesValue> SegmentedArray::createLongLived(
155+
CallResult<PseudoHandle<SegmentedArray>> SegmentedArray::createLongLived(
157156
Runtime *runtime,
158157
size_type capacity) {
159158
if (LLVM_UNLIKELY(capacity > maxElements())) {
160159
return throwExcessiveCapacityError(runtime, capacity);
161160
}
162161
// Leave the segments as null. Whenever the size is changed, the segments will
163162
// be allocated.
164-
return HermesValue::encodeObjectValue(new (runtime->allocLongLived(
163+
return createPseudoHandle(new (runtime->allocLongLived(
165164
allocationSizeForCapacity(capacity))) SegmentedArray(runtime, capacity));
166165
}
167166

168-
CallResult<HermesValue>
167+
CallResult<PseudoHandle<SegmentedArray>>
169168
SegmentedArray::create(Runtime *runtime, size_type capacity, size_type size) {
170169
auto arrRes = create(runtime, capacity);
171170
if (LLVM_UNLIKELY(arrRes == ExecutionStatus::EXCEPTION)) {
172171
return ExecutionStatus::EXCEPTION;
173172
}
174-
auto self = createPseudoHandle(vmcast<SegmentedArray>(*arrRes));
173+
PseudoHandle<SegmentedArray> self = std::move(*arrRes);
175174
// TODO T25663446: This is potentially optimizable to iterate over the inline
176175
// storage and the segments separately.
177176
self = increaseSize</*Fill*/ true>(runtime, std::move(self), size);
178-
return self.getHermesValue();
177+
return self;
179178
}
180179

181180
SegmentedArray::size_type SegmentedArray::capacity() const {
@@ -291,7 +290,7 @@ ExecutionStatus SegmentedArray::growRight(
291290
if (arrRes == ExecutionStatus::EXCEPTION) {
292291
return ExecutionStatus::EXCEPTION;
293292
}
294-
auto newSegmentedArray = createPseudoHandle(vmcast<SegmentedArray>(*arrRes));
293+
PseudoHandle<SegmentedArray> newSegmentedArray = std::move(*arrRes);
295294
// Copy inline storage and segments over.
296295
// Do this with raw pointers so that the range write barrier occurs.
297296
GCHermesValue::copy(
@@ -321,7 +320,7 @@ ExecutionStatus SegmentedArray::growLeft(
321320
if (arrRes == ExecutionStatus::EXCEPTION) {
322321
return ExecutionStatus::EXCEPTION;
323322
}
324-
auto newSegmentedArray = createPseudoHandle(vmcast<SegmentedArray>(*arrRes));
323+
PseudoHandle<SegmentedArray> newSegmentedArray = std::move(*arrRes);
325324
// Don't fill with empty values, most will be copied in.
326325
newSegmentedArray = increaseSize</*Fill*/ false>(
327326
runtime, std::move(newSegmentedArray), newSize);

‎unittests/VMRuntime/SegmentedArrayTest.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
*/
77

88
#include "hermes/VM/SegmentedArray.h"
9+
#include "hermes/VM/Casting.h"
10+
#include "hermes/VM/HermesValueTraits.h"
911

1012
#include "TestHelpers.h"
1113

@@ -62,8 +64,7 @@ TEST_F(SegmentedArrayTest, AllowTrimming) {
6264
constexpr SegmentedArray::size_type originalCapacity = 4;
6365
// Create an array and put in an element so its size is 1 and its capacity
6466
// is 4.
65-
array = vmcast<SegmentedArray>(
66-
*SegmentedArray::create(runtime, originalCapacity));
67+
array = std::move(*SegmentedArray::create(runtime, originalCapacity));
6768
// The capacity is not guaranteed to match the input parameter, it is taken
6869
// as a hint, so check for <=.
6970
EXPECT_LE(array->capacity(), originalCapacity);

0 commit comments

Comments
 (0)