Revise Symbol description getter methods

Signed-off-by: HyukWoo Park <[email protected]>

src/api/EscargotPublic.cpp

@@ -784,12 +784,14 @@ SymbolRef* SymbolRef::fromGlobalSymbolRegistry(VMInstanceRef* vm, StringRef* des
     return toRef(Symbol::fromGlobalSymbolRegistry(toImpl(vm), toImpl(desc)));
 }
 
-OptionalRef<StringRef> 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)

src/api/EscargotPublic.h

@@ -1162,7 +1162,8 @@ class ESCARGOT_EXPORT SymbolRef : public PointerValueRef {
 public:
     static SymbolRef* create(OptionalRef<StringRef> desc);
     static SymbolRef* fromGlobalSymbolRegistry(VMInstanceRef* context, StringRef* desc); // this is same with Symbol.for
-    OptionalRef<StringRef> description();
+    StringRef* descriptionString();
+    ValueRef* descriptionValue();
     StringRef* symbolDescriptiveString();
 };
 

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<String*> 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<Object*> 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");
     }

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) {

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 {

src/runtime/Symbol.cpp

@@ -24,8 +24,19 @@
 
 namespace Escargot {
 
+String* Symbol::descriptionString() const
+{
+    return m_description ? const_cast<String*>(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();
 }

src/runtime/Symbol.h

@@ -47,12 +47,11 @@ class Symbol : public PointerValue {
         , m_description(desc)
         , m_finalizerData(nullptr)
     {
+        // nullptr of desc represents `undefined`
     }
 
-    Optional<String*> description() const
-    {
-        return m_description;
-    }
+    String* descriptionString() const;
+    Value descriptionValue() const;
 
     SymbolFinalizerData* finalizerData() const
     {
@@ -63,6 +62,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();

src/runtime/VMInstance.h

@@ -65,7 +65,7 @@ struct GlobalSymbols {
 };
 
 struct GlobalSymbolRegistryItem {
-    Optional<String*> key;
+    String* key;
     Symbol* symbol;
 };
 

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

src/runtime/serialization/Serializer.cpp

@@ -46,8 +46,8 @@ std::unique_ptr<SerializedValue> Serializer::serialize(const Value& value)
     } else if (value.isBigInt()) {
         return std::unique_ptr<SerializedValue>(new SerializedBigIntValue(value.asBigInt()->toString()->toNonGCUTF8StringData()));
     } else if (value.isSymbol()) {
-        if (value.asSymbol()->description()) {
-            return std::unique_ptr<SerializedValue>(new SerializedSymbolValue(value.asSymbol()->description()->toNonGCUTF8StringData()));
+        if (value.asSymbol()->descriptionString()->length()) {
+            return std::unique_ptr<SerializedValue>(new SerializedSymbolValue(value.asSymbol()->descriptionString()->toNonGCUTF8StringData()));
         } else {
             return std::unique_ptr<SerializedValue>(new SerializedSymbolValue());
         }

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)