Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "src: migrate String::Value to String::ValueView" #55828

Merged
merged 2 commits into from
Nov 16, 2024
Merged

Revert "src: migrate String::Value to String::ValueView" #55828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5f43e6d
Revert "src: migrate `String::Value` to `String::ValueView`"
targos Nov 12, 2024
df0b75f
fixup: lint
targos Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/inspector_js_api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,9 @@ static void AsyncTaskScheduledWrapper(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);

CHECK(args[0]->IsString());

TwoByteValue task_name_buffer(args.GetIsolate(), args[0]);
StringView task_name_view(*task_name_buffer, task_name_buffer.length());
Local<String> task_name = args[0].As<String>();
String::Value task_name_value(args.GetIsolate(), task_name);
StringView task_name_view(*task_name_value, task_name_value.length());
Comment on lines -248 to +250
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a full revert, I feel like this part can stay the same?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A full revert is easier to work with. You're free to make incremental changes after it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good illustration of why atomic PRs should always be preferred.


CHECK(args[1]->IsNumber());
int64_t task_id = args[1]->IntegerValue(env->context()).FromJust();
Expand Down
28 changes: 16 additions & 12 deletions src/node_buffer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -965,9 +965,12 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
size_t result = haystack_length;

if (enc == UCS2) {
TwoByteValue needle_buffer(isolate, needle);
String::Value needle_value(isolate, needle);
if (*needle_value == nullptr) {
return args.GetReturnValue().Set(-1);
}

if (haystack_length < 2 || needle_buffer.length() < 1) {
if (haystack_length < 2 || needle_value.length() < 1) {
return args.GetReturnValue().Set(-1);
}

Expand All @@ -987,12 +990,13 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
offset / 2,
is_forward);
} else {
result = nbytes::SearchString(reinterpret_cast<const uint16_t*>(haystack),
haystack_length / 2,
needle_buffer.out(),
needle_buffer.length(),
offset / 2,
is_forward);
result =
nbytes::SearchString(reinterpret_cast<const uint16_t*>(haystack),
haystack_length / 2,
reinterpret_cast<const uint16_t*>(*needle_value),
needle_value.length(),
offset / 2,
is_forward);
}
result *= 2;
} else if (enc == UTF8) {
Expand Down Expand Up @@ -1292,10 +1296,10 @@ static void Btoa(const FunctionCallbackInfo<Value>& args) {
input->Length(),
buffer.out());
} else {
String::ValueView value(env->isolate(), input);
String::Value value(env->isolate(), input);
MaybeStackBuffer<char> stack_buf(value.length());
size_t out_len = simdutf::convert_utf16_to_latin1(
reinterpret_cast<const char16_t*>(value.data16()),
reinterpret_cast<const char16_t*>(*value),
value.length(),
stack_buf.out());
if (out_len == 0) { // error
Expand Down Expand Up @@ -1352,8 +1356,8 @@ static void Atob(const FunctionCallbackInfo<Value>& args) {
buffer.SetLength(expected_length);
result = simdutf::base64_to_binary(data, input->Length(), buffer.out());
} else { // 16-bit case
String::ValueView value(env->isolate(), input);
auto data = reinterpret_cast<const char16_t*>(value.data16());
String::Value value(env->isolate(), input);
auto data = reinterpret_cast<const char16_t*>(*value);
size_t expected_length =
simdutf::maximal_binary_length_from_base64(data, value.length());
buffer.AllocateSufficientStorage(expected_length);
Expand Down
20 changes: 10 additions & 10 deletions src/string_bytes.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,10 +305,11 @@ size_t StringBytes::Write(Isolate* isolate,
input_view.length());
}
} else {
String::Value value(isolate, str);
size_t written_len = buflen;
auto result = simdutf::base64_to_binary_safe(
reinterpret_cast<const char16_t*>(input_view.data16()),
input_view.length(),
reinterpret_cast<const char16_t*>(*value),
value.length(),
buf,
written_len,
simdutf::base64_url);
Expand All @@ -318,8 +319,7 @@ size_t StringBytes::Write(Isolate* isolate,
// The input does not follow the WHATWG forgiving-base64 specification
// (adapted for base64url with + and / replaced by - and _).
// https://infra.spec.whatwg.org/#forgiving-base64-decode
nbytes = nbytes::Base64Decode(
buf, buflen, input_view.data16(), input_view.length());
nbytes = nbytes::Base64Decode(buf, buflen, *value, value.length());
}
}
break;
Expand All @@ -344,19 +344,19 @@ size_t StringBytes::Write(Isolate* isolate,
input_view.length());
}
} else {
String::Value value(isolate, str);
size_t written_len = buflen;
auto result = simdutf::base64_to_binary_safe(
reinterpret_cast<const char16_t*>(input_view.data16()),
input_view.length(),
reinterpret_cast<const char16_t*>(*value),
value.length(),
buf,
written_len);
if (result.error == simdutf::error_code::SUCCESS) {
nbytes = written_len;
} else {
// The input does not follow the WHATWG base64 specification
// https://infra.spec.whatwg.org/#forgiving-base64-decode
nbytes = nbytes::Base64Decode(
buf, buflen, input_view.data16(), input_view.length());
nbytes = nbytes::Base64Decode(buf, buflen, *value, value.length());
}
}
break;
Expand All @@ -369,8 +369,8 @@ size_t StringBytes::Write(Isolate* isolate,
reinterpret_cast<const char*>(input_view.data8()),
input_view.length());
} else {
String::ValueView value(isolate, str);
nbytes = nbytes::HexDecode(buf, buflen, value.data8(), value.length());
String::Value value(isolate, str);
nbytes = nbytes::HexDecode(buf, buflen, *value, value.length());
}
break;

Expand Down
Loading