diff --git a/build/android/escargot/src/androidTest/java/com/samsung/lwe/escargot/EscargotTest.java b/build/android/escargot/src/androidTest/java/com/samsung/lwe/escargot/EscargotTest.java index 486cced66..f0dae09fd 100644 --- a/build/android/escargot/src/androidTest/java/com/samsung/lwe/escargot/EscargotTest.java +++ b/build/android/escargot/src/androidTest/java/com/samsung/lwe/escargot/EscargotTest.java @@ -648,18 +648,18 @@ public void symbolValueTest() { Context context = Context.create(vmInstance); JavaScriptSymbol symbol = JavaScriptSymbol.create(Optional.empty()); - assertFalse(symbol.description().isPresent()); + assertTrue(symbol.descriptionValue().isUndefined()); assertTrue(symbol.symbolDescriptiveString().toJavaString().equals("Symbol()")); printNegativeTC("symbol null test 1"); symbol = JavaScriptSymbol.create((Optional) null); - assertFalse(symbol.description().isPresent()); + assertTrue(symbol.descriptionValue().isUndefined()); assertTrue(symbol.symbolDescriptiveString().toJavaString().equals("Symbol()")); printNegativeTC("symbol null test 2"); symbol = JavaScriptSymbol.create(Optional.of(JavaScriptString.create("foobar"))); - assertTrue(symbol.description().isPresent()); - assertTrue(symbol.description().get().toJavaString().equals("foobar")); + assertFalse(symbol.descriptionValue().isUndefined()); + assertTrue(symbol.descriptionString().toJavaString().equals("foobar")); assertFalse(symbol.equalsTo(context, JavaScriptSymbol.create(Optional.of(JavaScriptString.create("foobar")))).get().booleanValue()); printNegativeTC("symbol null test 4"); @@ -1927,4 +1927,4 @@ public Optional callback(Context context, JavaScriptValue recei vmInstance = null; finalizeEngine(); } -} \ No newline at end of file +} diff --git a/build/android/escargot/src/main/cpp/JNISymbol.cpp b/build/android/escargot/src/main/cpp/JNISymbol.cpp index db8d49663..bd8d1b08e 100644 --- a/build/android/escargot/src/main/cpp/JNISymbol.cpp +++ b/build/android/escargot/src/main/cpp/JNISymbol.cpp @@ -56,10 +56,18 @@ Java_com_samsung_lwe_escargot_JavaScriptSymbol_fromGlobalSymbolRegistry(JNIEnv* extern "C" JNIEXPORT jobject JNICALL -Java_com_samsung_lwe_escargot_JavaScriptSymbol_description(JNIEnv* env, jobject thiz) +Java_com_samsung_lwe_escargot_JavaScriptSymbol_descriptionString(JNIEnv* env, jobject thiz) { - auto desc = unwrapValueRefFromValue(env, env->GetObjectClass(thiz), thiz)->asSymbol()->description(); - return nativeOptionalValueIntoJavaOptionalValue(env, desc); + auto desc = unwrapValueRefFromValue(env, env->GetObjectClass(thiz), thiz)->asSymbol()->descriptionString(); + return createJavaObjectFromValue(env, desc); +} + +extern "C" +JNIEXPORT jobject JNICALL +Java_com_samsung_lwe_escargot_JavaScriptSymbol_descriptionValue(JNIEnv* env, jobject thiz) +{ + auto desc = unwrapValueRefFromValue(env, env->GetObjectClass(thiz), thiz)->asSymbol()->descriptionValue(); + return createJavaObjectFromValue(env, desc); } extern "C" @@ -68,4 +76,4 @@ Java_com_samsung_lwe_escargot_JavaScriptSymbol_symbolDescriptiveString(JNIEnv* e { auto desc = unwrapValueRefFromValue(env, env->GetObjectClass(thiz), thiz)->asSymbol()->symbolDescriptiveString(); return createJavaObjectFromValue(env, desc); -} \ No newline at end of file +} diff --git a/build/android/escargot/src/main/java/com/samsung/lwe/escargot/JavaScriptSymbol.java b/build/android/escargot/src/main/java/com/samsung/lwe/escargot/JavaScriptSymbol.java index 294eac338..4724cc6fe 100644 --- a/build/android/escargot/src/main/java/com/samsung/lwe/escargot/JavaScriptSymbol.java +++ b/build/android/escargot/src/main/java/com/samsung/lwe/escargot/JavaScriptSymbol.java @@ -9,7 +9,8 @@ protected JavaScriptSymbol(long nativePointer) } native static public JavaScriptSymbol create(Optional value); - native public Optional description(); + native public JavaScriptString descriptionString(); + native public JavaScriptValue descriptionValue(); native public JavaScriptString symbolDescriptiveString(); /** diff --git a/src/api/EscargotPublic.cpp b/src/api/EscargotPublic.cpp index 6d66121f9..0483d2b43 100644 --- a/src/api/EscargotPublic.cpp +++ b/src/api/EscargotPublic.cpp @@ -784,12 +784,14 @@ SymbolRef* SymbolRef::fromGlobalSymbolRegistry(VMInstanceRef* vm, StringRef* des return toRef(Symbol::fromGlobalSymbolRegistry(toImpl(vm), toImpl(desc))); } -OptionalRef SymbolRef::description() +StringRef* SymbolRef::descriptionString() { - if (toImpl(this)->description().hasValue()) { - return toRef(toImpl(this)->description().value()); - } - return nullptr; + return toRef(toImpl(this)->descriptionString()); +} + +ValueRef* SymbolRef::descriptionValue() +{ + return toRef(toImpl(this)->descriptionValue()); } StringRef* SymbolRef::symbolDescriptiveString() @@ -3823,7 +3825,7 @@ StringRef* StringObjectRef::primitiveValue() SymbolObjectRef* SymbolObjectRef::create(ExecutionStateRef* state) { - return toRef(new SymbolObject(*toImpl(state), new Symbol(String::emptyString))); + return toRef(new SymbolObject(*toImpl(state), new Symbol())); } void SymbolObjectRef::setPrimitiveValue(ExecutionStateRef* state, SymbolRef* value) diff --git a/src/api/EscargotPublic.h b/src/api/EscargotPublic.h index dafa6ab99..e70925469 100644 --- a/src/api/EscargotPublic.h +++ b/src/api/EscargotPublic.h @@ -1162,7 +1162,8 @@ class ESCARGOT_EXPORT SymbolRef : public PointerValueRef { public: static SymbolRef* create(OptionalRef desc); static SymbolRef* fromGlobalSymbolRegistry(VMInstanceRef* context, StringRef* desc); // this is same with Symbol.for - OptionalRef description(); + StringRef* descriptionString(); + ValueRef* descriptionValue(); StringRef* symbolDescriptiveString(); }; diff --git a/src/builtins/BuiltinSymbol.cpp b/src/builtins/BuiltinSymbol.cpp index 95ca73d4c..640de945f 100644 --- a/src/builtins/BuiltinSymbol.cpp +++ b/src/builtins/BuiltinSymbol.cpp @@ -33,14 +33,16 @@ static Value builtinSymbolConstructor(ExecutionState& state, Value thisValue, si if (newTarget.hasValue()) { ErrorObject::throwBuiltinError(state, ErrorCode::TypeError, "illegal constructor Symbol"); } - Optional descString = nullptr; + // If description is undefined, let descString be undefined. - if (!(argc == 0 || argv[0].isUndefined())) { + // (NOTE) we represent `undefined` as nullptr here + String* descStr = nullptr; + if (argc > 0 && !argv[0].isUndefined()) { // Else, let descString be ? ToString(description). - descString = argv[0].toString(state); + descStr = argv[0].toString(state); } - // Return a new unique Symbol value whose [[Description]] value is descString. - return new Symbol(descString); + // Return a new Symbol whose [[Description]] is descString. + return new Symbol(descStr); } #define RESOLVE_THIS_BINDING_TO_SYMBOL(NAME, OBJ, BUILT_IN_METHOD) \ @@ -88,35 +90,15 @@ static Value builtinSymbolKeyFor(ExecutionState& state, Value thisValue, size_t ErrorObject::throwBuiltinError(state, ErrorCode::TypeError, ErrorObject::Messages::GlobalObject_IllegalFirstArgument); } Symbol* sym = argv[0].asSymbol(); - // For each element e of the GlobalSymbolRegistry List (see 19.4.2.1), - auto& list = state.context()->vmInstance()->globalSymbolRegistry(); - for (size_t i = 0; i < list.size(); i++) { - // If SameValue(e.[[Symbol]], sym) is true, return e.[[Key]]. - if (list[i].symbol == sym) { - return list[i].key.value(); - } - } - // Assert: GlobalSymbolRegistry does not currently contain an entry for sym. - // Return undefined. - return Value(); + return Symbol::keyForSymbol(state.context()->vmInstance(), sym); } static Value builtinSymbolDescriptionGetter(ExecutionState& state, Value thisValue, size_t argc, Value* argv, Optional newTarget) { if (thisValue.isSymbol()) { - if (!thisValue.asSymbol()->description().hasValue()) { - return Value(); - } - - return thisValue.asSymbol()->description().value(); - + return thisValue.asSymbol()->descriptionValue(); } else if (thisValue.isObject() && thisValue.asObject()->isSymbolObject()) { - if (!thisValue.asObject()->asSymbolObject()->primitiveValue()->description().hasValue()) { - return Value(); - } - - return thisValue.asObject()->asSymbolObject()->primitiveValue()->description().value(); - + return thisValue.asObject()->asSymbolObject()->primitiveValue()->descriptionValue(); } else { ErrorObject::throwBuiltinError(state, ErrorCode::TypeError, "getter called on non-Symbol object"); } diff --git a/src/debugger/Debugger.cpp b/src/debugger/Debugger.cpp index 3faab597e..0530f81e9 100644 --- a/src/debugger/Debugger.cpp +++ b/src/debugger/Debugger.cpp @@ -602,11 +602,7 @@ static void sendProperty(DebuggerRemote* debugger, ExecutionState* state, Atomic } else { type = DebuggerRemote::ESCARGOT_VARIABLE_SYMBOL; Symbol* symbol = value.asSymbol(); - - valueStr = String::emptyString; - if (symbol->description().hasValue()) { - valueStr = symbol->description().value(); - } + valueStr = symbol->descriptionString(); } if (valueStr->length() >= ESCARGOT_DEBUGGER_MAX_VARIABLE_LENGTH) { diff --git a/src/interpreter/ByteCodeInterpreter.cpp b/src/interpreter/ByteCodeInterpreter.cpp index c17c27f43..678f31958 100644 --- a/src/interpreter/ByteCodeInterpreter.cpp +++ b/src/interpreter/ByteCodeInterpreter.cpp @@ -4165,10 +4165,10 @@ static Value createObjectPropertyFunctionName(ExecutionState& state, const Value StringBuilder builder; if (name.isSymbol()) { builder.appendString(prefix); - if (name.asSymbol()->description().hasValue()) { + if (name.asSymbol()->descriptionString()->length()) { // add symbol name if it is not an empty symbol builder.appendString("["); - builder.appendString(name.asSymbol()->description().value()); + builder.appendString(name.asSymbol()->descriptionString()); builder.appendString("]"); } } else { diff --git a/src/runtime/Symbol.cpp b/src/runtime/Symbol.cpp index ddda8e859..68843db18 100644 --- a/src/runtime/Symbol.cpp +++ b/src/runtime/Symbol.cpp @@ -24,8 +24,19 @@ namespace Escargot { +String* Symbol::descriptionString() const +{ + return m_description ? const_cast(m_description.value()) : String::emptyString; +} + +Value Symbol::descriptionValue() const +{ + return m_description ? m_description.value() : Value(); +} + Symbol* Symbol::fromGlobalSymbolRegistry(VMInstance* vm, String* stringKey) { + ASSERT(!!stringKey); // Let stringKey be ? ToString(key). // For each element e of the GlobalSymbolRegistry List, auto& list = vm->globalSymbolRegistry(); @@ -47,12 +58,26 @@ Symbol* Symbol::fromGlobalSymbolRegistry(VMInstance* vm, String* stringKey) return newSymbol; } +Value Symbol::keyForSymbol(VMInstance* vm, Symbol* sym) +{ + ASSERT(!!sym); + auto& list = vm->globalSymbolRegistry(); + + for (size_t i = 0; i < list.size(); i++) { + // If SameValue(e.[[Symbol]], sym) is true, return e.[[Key]]. + if (list[i].symbol == sym) { + ASSERT(!!list[i].key); + return list[i].key; + } + } + return Value(); +} + String* Symbol::symbolDescriptiveString() const { StringBuilder sb; sb.appendString("Symbol("); - if (description().hasValue()) - sb.appendString(description().value()); + sb.appendString(descriptionString()); sb.appendString(")"); return sb.finalize(); } diff --git a/src/runtime/Symbol.h b/src/runtime/Symbol.h index 12564642f..75ae35535 100644 --- a/src/runtime/Symbol.h +++ b/src/runtime/Symbol.h @@ -49,10 +49,8 @@ class Symbol : public PointerValue { { } - Optional description() const - { - return m_description; - } + String* descriptionString() const; + Value descriptionValue() const; SymbolFinalizerData* finalizerData() const { @@ -63,6 +61,7 @@ class Symbol : public PointerValue { String* symbolDescriptiveString() const; static Symbol* fromGlobalSymbolRegistry(VMInstance* vm, String* stringKey); + static Value keyForSymbol(VMInstance* vm, Symbol* sym); private: SymbolFinalizerData* ensureFinalizerData(); @@ -71,7 +70,7 @@ class Symbol : public PointerValue { void tryToShrinkFinalizers(); size_t m_typeTag; - Optional m_description; + Optional m_description; // nullptr of desc represents `undefined` SymbolFinalizerData* m_finalizerData; // handle finalizer data of Symbol }; } // namespace Escargot diff --git a/src/runtime/VMInstance.h b/src/runtime/VMInstance.h index e67286ac8..026b1057c 100644 --- a/src/runtime/VMInstance.h +++ b/src/runtime/VMInstance.h @@ -65,7 +65,7 @@ struct GlobalSymbols { }; struct GlobalSymbolRegistryItem { - Optional key; + String* key; Symbol* symbol; }; diff --git a/src/runtime/Value.cpp b/src/runtime/Value.cpp index 1d86f048d..dc232ca92 100644 --- a/src/runtime/Value.cpp +++ b/src/runtime/Value.cpp @@ -114,10 +114,9 @@ String* Value::toStringSlowCase(ExecutionState& ec) const // $7.1.12 ToString ASSERT_NOT_REACHED(); } else if (isBigInt()) { return asBigInt()->toString(); - } else { - return toPrimitive(ec, PreferString).toString(ec); } - return nullptr; + + return toPrimitive(ec, PreferString).toString(ec); } Object* Value::toObjectSlowCase(ExecutionState& state) const // $7.1.13 ToObject diff --git a/src/runtime/serialization/Serializer.cpp b/src/runtime/serialization/Serializer.cpp index 5bbfec768..ec7ecb345 100644 --- a/src/runtime/serialization/Serializer.cpp +++ b/src/runtime/serialization/Serializer.cpp @@ -46,8 +46,8 @@ std::unique_ptr Serializer::serialize(const Value& value) } else if (value.isBigInt()) { return std::unique_ptr(new SerializedBigIntValue(value.asBigInt()->toString()->toNonGCUTF8StringData())); } else if (value.isSymbol()) { - if (value.asSymbol()->description()) { - return std::unique_ptr(new SerializedSymbolValue(value.asSymbol()->description()->toNonGCUTF8StringData())); + if (value.asSymbol()->descriptionString()->length()) { + return std::unique_ptr(new SerializedSymbolValue(value.asSymbol()->descriptionString()->toNonGCUTF8StringData())); } else { return std::unique_ptr(new SerializedSymbolValue()); } diff --git a/test/cctest/testapi.cpp b/test/cctest/testapi.cpp index 84ac1239f..6bb9c1ec2 100644 --- a/test/cctest/testapi.cpp +++ b/test/cctest/testapi.cpp @@ -1874,7 +1874,7 @@ TEST(EnumerateObjectOwnProperties, Basic1) EXPECT_TRUE(indexes[1] == 10); EXPECT_TRUE(strings->at(0)->equalsTo(state, StringRef::createFromASCII("a_enum"))); EXPECT_TRUE(strings->at(1)->equalsTo(state, StringRef::createFromASCII("-20"))); - EXPECT_TRUE(symbols->at(0)->asSymbol()->description()->equalsTo(state, StringRef::createFromASCII("sym_enum"))); + EXPECT_TRUE(symbols->at(0)->asSymbol()->descriptionString()->equalsTo(state, StringRef::createFromASCII("sym_enum"))); return ValueRef::createUndefined(); }); @@ -2318,7 +2318,7 @@ TEST(Serializer, Basic7) SerializerRef::serializeInto(v1, ostream); std::istringstream istream(ostream.str()); ValueRef* v2 = SerializerRef::deserializeFrom(g_context.get(), istream); - EXPECT_FALSE(v2->asSymbol()->description().hasValue()); + EXPECT_TRUE(v2->asSymbol()->descriptionValue()->isUndefined()); } TEST(Serializer, Basic8) @@ -2328,7 +2328,7 @@ TEST(Serializer, Basic8) SerializerRef::serializeInto(v1, ostream); std::istringstream istream(ostream.str()); ValueRef* v2 = SerializerRef::deserializeFrom(g_context.get(), istream); - EXPECT_TRUE(v2->asSymbol()->description().value()->equalsWithASCIIString("asdfasdfasdf", 12)); + EXPECT_TRUE(v2->asSymbol()->descriptionString()->equalsWithASCIIString("asdfasdfasdf", 12)); } TEST(Serializer, Basic9)