Skip to content

Commit 2af96b3

Browse files
fbmal7meta-codesync[bot]
authored andcommitted
Return ASCII strings from String.prototype.concat when possible
Summary: `String.prototype.concat` is returning a UTF16 string always. this is because the call to `StringBuilder::createStringBuilder` defaults to UTF16. Instead, we can make the choice more accurately: if all arguments (including `this`) to `concat` are ASCII, then make an ASCII string. Otherwise, make a UTF16 string. Reviewed By: avp Differential Revision: D83851016 fbshipit-source-id: f46de6154052f50c55c18a9832a9260b7800ac28
1 parent de05551 commit 2af96b3

File tree

1 file changed

+4
-1
lines changed

1 file changed

+4
-1
lines changed

lib/VM/JSLib/String.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -843,6 +843,8 @@ CallResult<HermesValue> stringPrototypeConcat(void *, Runtime &runtime) {
843843
}
844844
lv.strings = vmcast<ArrayStorageSmall>(*arrRes);
845845

846+
// Track if all strings are ASCII.
847+
bool allASCII = lv.S->isASCII();
846848
// Run toString on the arguments to figure out the final size.
847849
auto marker = gcScope.createMarker();
848850
for (uint32_t i = 0; i < argCount; ++i) {
@@ -859,6 +861,7 @@ CallResult<HermesValue> stringPrototypeConcat(void *, Runtime &runtime) {
859861
runtime.getHeap());
860862
uint32_t strLength = strRes->get()->getStringLength();
861863

864+
allASCII &= (*strRes)->isASCII();
862865
size.add(strLength);
863866
if (LLVM_UNLIKELY(size.isOverflowed())) {
864867
return runtime.raiseRangeError("resulting string length exceeds limit");
@@ -868,7 +871,7 @@ CallResult<HermesValue> stringPrototypeConcat(void *, Runtime &runtime) {
868871
}
869872

870873
// Allocate the complete result.
871-
auto builder = StringBuilder::createStringBuilder(runtime, size);
874+
auto builder = StringBuilder::createStringBuilder(runtime, size, allASCII);
872875
if (builder == ExecutionStatus::EXCEPTION) {
873876
return ExecutionStatus::EXCEPTION;
874877
}

0 commit comments

Comments
 (0)