[MERGE #5685 @jackhorton] Remove questionable uses of LiteralString class

Merge pull request #5685 from jackhorton:literal-strings

Found this as a driveby while making a different change. To me, `LiteralString::NewCopy*` is an anti-pattern -- LiteralString doesn't override NewCopy*, so it calls the JavascriptString version regardless, and it gives the impression that some optimization was in mind but not achieved. Additionally, by searching for more cases, I found a number of cases where we were wastefully creating JavascriptStrings for various string constants (most were especially brutal since they were going in rarely used ConcatStrings).

Not sure if there is value in actually hardening LiteralString against incorrect uses, but I can implement that if others see value.

Author: jackhorton

lib/Runtime/Library/BoundFunction.cpp

@@ -267,7 +267,7 @@ namespace Js
                 displayName = JavascriptString::FromVar(value);
             }
         }
-        return LiteralString::Concat(LiteralString::NewCopySz(_u("bound "), this->GetScriptContext()), displayName);
+        return JavascriptString::Concat(GetLibrary()->GetBoundFunctionPrefixString(), displayName);
     }
 
     RecyclableObject* BoundFunction::GetBoundThis()

lib/Runtime/Library/CompoundString.h

@@ -1243,7 +1243,7 @@ namespace Js
             return;
         }
 
-        JavascriptString *const js = LiteralString::NewCopyBuffer(s, appendCharLength, toString->GetScriptContext());
+        JavascriptString *const js = JavascriptString::NewCopyBuffer(s, appendCharLength, toString->GetScriptContext());
         if(TryAppendGeneric(js, appendCharLength, toString))
             return;
         toString->AppendSlow(js);

lib/Runtime/Library/JavascriptFunction.cpp

@@ -3185,19 +3185,20 @@ void __cdecl _alloca_probe_16()
         ParseableFunctionInfo * func = this->GetFunctionProxy()->EnsureDeserialized();
         if (func->GetDisplayName() == Js::Constants::FunctionCode)
         {
-            return LiteralString::NewCopyBuffer(Js::Constants::Anonymous, Js::Constants::AnonymousLength, scriptContext);
+            // TODO(jahorto): multiple places use pointer equality on the Constants:: string buffers. Consider moving these to StringCacheList.h and use backing buffer pointer equality if need be.
+            return JavascriptString::NewWithBuffer(Constants::Anonymous, Constants::AnonymousLength, scriptContext);
         }
         else if (func->GetIsAccessor())
         {
             const char16* accessorName = func->GetDisplayName();
             if (accessorName[0] == _u('g'))
             {
-                return LiteralString::Concat(LiteralString::NewCopySz(_u("get "), scriptContext), LiteralString::NewCopyBuffer(name, length, scriptContext));
+                return JavascriptString::Concat(scriptContext->GetLibrary()->GetGetterFunctionPrefixString(), JavascriptString::NewCopyBuffer(name, length, scriptContext));
             }
             AssertMsg(accessorName[0] == _u('s'), "should be a set");
-            return LiteralString::Concat(LiteralString::NewCopySz(_u("set "), scriptContext), LiteralString::NewCopyBuffer(name, length, scriptContext));
+            return JavascriptString::Concat(scriptContext->GetLibrary()->GetSetterFunctionPrefixString(), JavascriptString::NewCopyBuffer(name, length, scriptContext));
         }
-        return LiteralString::NewCopyBuffer(name, length, scriptContext);
+        return JavascriptString::NewCopyBuffer(name, length, scriptContext);
     }
 
     bool JavascriptFunction::GetFunctionName(JavascriptString** name) const
@@ -3246,7 +3247,14 @@ void __cdecl _alloca_probe_16()
         if (proxy)
         {
             ParseableFunctionInfo * func = proxy->EnsureDeserialized();
-            return LiteralString::NewCopySz(func->GetDisplayName(), scriptContext);
+            if (func->GetDisplayNameIsRecyclerAllocated())
+            {
+                return JavascriptString::NewWithSz(func->GetDisplayName(), scriptContext);
+            }
+            else
+            {
+                return JavascriptString::NewCopySz(func->GetDisplayName(), scriptContext);
+            }
         }
         JavascriptString* sourceStringName = nullptr;
         if (GetSourceStringName(&sourceStringName))

lib/Runtime/Library/JavascriptLibrary.cpp

@@ -2757,7 +2757,7 @@ namespace Js
 
         library->AddMember(functionPrototype, PropertyIds::constructor, library->functionConstructor);
         library->AddMember(functionPrototype, PropertyIds::length, TaggedInt::ToVarUnchecked(0), PropertyConfigurable);
-        library->AddMember(functionPrototype, PropertyIds::name, LiteralString::CreateEmptyString(scriptContext->GetLibrary()->GetStringTypeStatic()), PropertyConfigurable);
+        library->AddMember(functionPrototype, PropertyIds::name, library->GetEmptyString(), PropertyConfigurable);
 
         JavascriptFunction *func = library->AddFunctionToLibraryObject(functionPrototype, PropertyIds::apply, &JavascriptFunction::EntryInfo::Apply, 2);
         builtinFuncs[BuiltinFunction::JavascriptFunction_Apply] = func;
@@ -4500,15 +4500,15 @@ namespace Js
 
     RuntimeFunction* JavascriptLibrary::CreateGetterFunction(PropertyId nameId, FunctionInfo* functionInfo)
     {
-        Var name_withGetPrefix = LiteralString::Concat(LiteralString::NewCopySz(_u("get "), scriptContext), scriptContext->GetPropertyString(nameId));
+        Var name_withGetPrefix = JavascriptString::Concat(GetGetterFunctionPrefixString(), scriptContext->GetPropertyString(nameId));
         RuntimeFunction* getterFunction = DefaultCreateFunction(functionInfo, 0, nullptr, nullptr, name_withGetPrefix);
         getterFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(0), PropertyConfigurable, nullptr);
         return getterFunction;
     }
 
     RuntimeFunction* JavascriptLibrary::CreateSetterFunction(PropertyId nameId, FunctionInfo* functionInfo)
     {
-        Var name_withSetPrefix = LiteralString::Concat(LiteralString::NewCopySz(_u("set "), scriptContext), scriptContext->GetPropertyString(nameId));
+        Var name_withSetPrefix = JavascriptString::Concat(GetSetterFunctionPrefixString(), scriptContext->GetPropertyString(nameId));
         RuntimeFunction* setterFunction = DefaultCreateFunction(functionInfo, 0, nullptr, nullptr, name_withSetPrefix);
         setterFunction->SetPropertyWithAttributes(PropertyIds::length, TaggedInt::ToVarUnchecked(1), PropertyConfigurable, nullptr);
         return setterFunction;

lib/Runtime/Library/ScriptFunction.cpp

@@ -391,7 +391,7 @@ using namespace Js;
                 wmemcpy_s(buffer, decodedCount, builder.DangerousGetWritableBuffer(), decodedCount);
                 buffer[decodedCount] = 0;
 
-                cachedSourceString = LiteralString::New(scriptContext->GetLibrary()->GetStringTypeStatic(), buffer, static_cast<charcount_t>(decodedCount), recycler);
+                cachedSourceString = JavascriptString::NewWithBuffer(buffer, static_cast<charcount_t>(decodedCount), scriptContext);
             }
             else
             {

lib/Runtime/Library/StringCacheList.h

@@ -37,6 +37,9 @@ STRING(ObjectRegExpDisplay, _u("[object RegExp]"))
 STRING(ObjectStringDisplay, _u("[object String]"))
 STRING(ObjectNullDisplay, _u("[object Null]"))
 STRING(ObjectUndefinedDisplay, _u("[object Undefined]"))
+STRING(GetterFunctionPrefix, _u("get "))
+STRING(SetterFunctionPrefix, _u("set "))
+STRING(BoundFunctionPrefix, _u("bound "))
 PROPERTY_STRING(UndefinedDisplay, _u("undefined"))
 PROPERTY_STRING(NaNDisplay, _u("NaN"))
 PROPERTY_STRING(NullDisplay, _u("null"))