Skip to content

Commit ba3dece

Browse files
mhorowitzfacebook-github-bot
mhorowitz
authored and
facebook-github-bot
committed
Make hasNamed* return CallResult<bool>
Summary: Proxy will add cases which can fail. Reviewed By: avp Differential Revision: D18284935 fbshipit-source-id: 8eede42771b7bdaa29e3ef1077b22379b5c080a1
1 parent 1edbef3 commit ba3dece

5 files changed

Lines changed: 31 additions & 24 deletions

File tree

‎API/hermes/hermes.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1523,7 +1523,9 @@ bool HermesRuntimeImpl::hasProperty(
15231523
vm::GCScope gcScope(&runtime_);
15241524
auto h = handle(obj);
15251525
vm::SymbolID nameID = phv(name).getSymbol();
1526-
return h->hasNamedOrIndexed(h, &runtime_, nameID);
1526+
auto result = h->hasNamedOrIndexed(h, &runtime_, nameID);
1527+
checkStatus(result.getStatus());
1528+
return result.getValue();
15271529
}
15281530

15291531
void HermesRuntimeImpl::setPropertyValue(

‎include/hermes/VM/JSObject.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -734,12 +734,12 @@ class JSObject : public GCCell {
734734
/// hasNamed is an optimized path for checking existence of a property
735735
/// for SymbolID when it is statically known that the SymbolIDs is not
736736
/// index-like.
737-
static bool
737+
static CallResult<bool>
738738
hasNamed(Handle<JSObject> selfHandle, Runtime *runtime, SymbolID name);
739739

740740
/// hasNamedOrIndexed checks existence of a property for a SymbolID which may
741741
/// be index-like.
742-
static bool hasNamedOrIndexed(
742+
static CallResult<bool> hasNamedOrIndexed(
743743
Handle<JSObject> selfHandle,
744744
Runtime *runtime,
745745
SymbolID name);

‎lib/VM/JSLib/HermesInternal.cpp‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -663,9 +663,14 @@ hermesInternalCopyDataProperties(void *, Runtime *runtime, NativeArgs args) {
663663
return true;
664664

665665
// Skip excluded items.
666-
if (excludedItems &&
667-
JSObject::hasNamedOrIndexed(excludedItems, runtime, sym)) {
668-
return true;
666+
if (excludedItems) {
667+
auto cr = JSObject::hasNamedOrIndexed(excludedItems, runtime, sym);
668+
assert(
669+
cr != ExecutionStatus::EXCEPTION &&
670+
"hasNamedOrIndex failed, which can only happen with a proxy, "
671+
"but excludedItems should never be a proxy");
672+
if (*cr)
673+
return true;
669674
}
670675

671676
auto cr =

‎lib/VM/JSObject.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,7 +1027,7 @@ CallResult<HermesValue> JSObject::getComputed_RJS(
10271027
}
10281028
}
10291029

1030-
bool JSObject::hasNamed(
1030+
CallResult<bool> JSObject::hasNamed(
10311031
Handle<JSObject> selfHandle,
10321032
Runtime *runtime,
10331033
SymbolID name) {
@@ -1036,7 +1036,7 @@ bool JSObject::hasNamed(
10361036
return propObj ? true : false;
10371037
}
10381038

1039-
bool JSObject::hasNamedOrIndexed(
1039+
CallResult<bool> JSObject::hasNamedOrIndexed(
10401040
Handle<JSObject> selfHandle,
10411041
Runtime *runtime,
10421042
SymbolID name) {

‎unittests/VMRuntime/ObjectModelTest.cpp‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -774,30 +774,30 @@ TEST_F(ObjectModelTest, HasProperty) {
774774
ASSERT_FALSE(*JSObject::hasComputed(self, runtime, nonIndexIDString));
775775
ASSERT_FALSE(*JSObject::hasComputed(self, runtime, indexIDNum));
776776
ASSERT_FALSE(*JSObject::hasComputed(self, runtime, indexID2Num));
777-
ASSERT_FALSE(JSObject::hasNamedOrIndexed(self, runtime, *nonIndexID));
778-
ASSERT_FALSE(JSObject::hasNamedOrIndexed(self, runtime, *indexID));
779-
ASSERT_FALSE(JSObject::hasNamedOrIndexed(self, runtime, *indexID2));
780-
ASSERT_FALSE(JSObject::hasNamed(self, runtime, *nonIndexID));
777+
ASSERT_FALSE(*JSObject::hasNamedOrIndexed(self, runtime, *nonIndexID));
778+
ASSERT_FALSE(*JSObject::hasNamedOrIndexed(self, runtime, *indexID));
779+
ASSERT_FALSE(*JSObject::hasNamedOrIndexed(self, runtime, *indexID2));
780+
ASSERT_FALSE(*JSObject::hasNamed(self, runtime, *nonIndexID));
781781

782782
ASSERT_TRUE(*JSObject::putNamedOrIndexed(self, runtime, *nonIndexID, self));
783783

784784
ASSERT_TRUE(*JSObject::hasComputed(self, runtime, nonIndexIDString));
785785
ASSERT_FALSE(*JSObject::hasComputed(self, runtime, indexIDNum));
786786
ASSERT_FALSE(*JSObject::hasComputed(self, runtime, indexID2Num));
787-
ASSERT_TRUE(JSObject::hasNamedOrIndexed(self, runtime, *nonIndexID));
788-
ASSERT_FALSE(JSObject::hasNamedOrIndexed(self, runtime, *indexID));
789-
ASSERT_FALSE(JSObject::hasNamedOrIndexed(self, runtime, *indexID2));
790-
ASSERT_TRUE(JSObject::hasNamed(self, runtime, *nonIndexID));
787+
ASSERT_TRUE(*JSObject::hasNamedOrIndexed(self, runtime, *nonIndexID));
788+
ASSERT_FALSE(*JSObject::hasNamedOrIndexed(self, runtime, *indexID));
789+
ASSERT_FALSE(*JSObject::hasNamedOrIndexed(self, runtime, *indexID2));
790+
ASSERT_TRUE(*JSObject::hasNamed(self, runtime, *nonIndexID));
791791

792792
ASSERT_TRUE(*JSObject::putNamedOrIndexed(self, runtime, *indexID, self));
793793

794794
ASSERT_TRUE(*JSObject::hasComputed(self, runtime, nonIndexIDString));
795795
ASSERT_TRUE(*JSObject::hasComputed(self, runtime, indexIDNum));
796796
ASSERT_FALSE(*JSObject::hasComputed(self, runtime, indexID2Num));
797-
ASSERT_TRUE(JSObject::hasNamedOrIndexed(self, runtime, *nonIndexID));
798-
ASSERT_TRUE(JSObject::hasNamedOrIndexed(self, runtime, *indexID));
799-
ASSERT_FALSE(JSObject::hasNamedOrIndexed(self, runtime, *indexID2));
800-
ASSERT_TRUE(JSObject::hasNamed(self, runtime, *nonIndexID));
797+
ASSERT_TRUE(*JSObject::hasNamedOrIndexed(self, runtime, *nonIndexID));
798+
ASSERT_TRUE(*JSObject::hasNamedOrIndexed(self, runtime, *indexID));
799+
ASSERT_FALSE(*JSObject::hasNamedOrIndexed(self, runtime, *indexID2));
800+
ASSERT_TRUE(*JSObject::hasNamed(self, runtime, *nonIndexID));
801801

802802
DefinePropertyFlags dpf{};
803803
ASSERT_TRUE(*JSObject::defineOwnProperty(
@@ -806,10 +806,10 @@ TEST_F(ObjectModelTest, HasProperty) {
806806
ASSERT_TRUE(*JSObject::hasComputed(self, runtime, nonIndexIDString));
807807
ASSERT_TRUE(*JSObject::hasComputed(self, runtime, indexIDNum));
808808
ASSERT_TRUE(*JSObject::hasComputed(self, runtime, indexID2Num));
809-
ASSERT_TRUE(JSObject::hasNamedOrIndexed(self, runtime, *nonIndexID));
810-
ASSERT_TRUE(JSObject::hasNamedOrIndexed(self, runtime, *indexID));
811-
ASSERT_TRUE(JSObject::hasNamedOrIndexed(self, runtime, *indexID2));
812-
ASSERT_TRUE(JSObject::hasNamed(self, runtime, *nonIndexID));
809+
ASSERT_TRUE(*JSObject::hasNamedOrIndexed(self, runtime, *nonIndexID));
810+
ASSERT_TRUE(*JSObject::hasNamedOrIndexed(self, runtime, *indexID));
811+
ASSERT_TRUE(*JSObject::hasNamedOrIndexed(self, runtime, *indexID2));
812+
ASSERT_TRUE(*JSObject::hasNamed(self, runtime, *nonIndexID));
813813
}
814814

815815
TEST_F(ObjectModelTest, UpdatePropertyFlagsWithoutTransitionsTest) {

0 commit comments

Comments
 (0)