Reland "[Intl] move localeCompare to C++"

This is a reland of 51ad234ffe

With a manual layout rebaseline of js/fast/string-prototype-properties
[1], this CL can be relanded without breaking the layout test.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1154289
also marks the test for skipping until this fix is rolled to Chromium.


Original change's description:
> [Intl] move localeCompare to C++
>
>
> Bug: v8:7958
> Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
> Change-Id: I84a27dda5205c8581a7ffe37213d685cc49974fa
> Reviewed-on: https://chromium-review.googlesource.com/1144644
> Commit-Queue: Frank Tang <ftang@chromium.org>
> Reviewed-by: Sathya Gunasekaran <gsathya@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#54740}

TBR=ftang@chromium.org, gsathya@chromium.org

Bug: v8:7958
Test: layout test: js/fast/string-prototype-properties
Change-Id: Ic546349fcbc935917ded018801f7d942e50565d5
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/1154247
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Reviewed-by: Jungshik Shin <jshin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54772}
This commit is contained in:
Frank Tang 2018-07-26 17:14:24 -07:00 committed by Commit Bot
parent e6bebb3a28
commit 22c7dd2eb5
7 changed files with 104 additions and 45 deletions

View File

@ -2030,8 +2030,13 @@ void Genesis::InitializeGlobal(Handle<JSGlobalObject> global_object,
Builtins::kStringPrototypeLastIndexOf, 1, false);
SimpleInstallFunction(isolate_, prototype, "link",
Builtins::kStringPrototypeLink, 1, true);
#ifdef V8_INTL_SUPPORT
SimpleInstallFunction(isolate_, prototype, "localeCompare",
Builtins::kStringPrototypeLocaleCompare, 1, false);
#else
SimpleInstallFunction(isolate_, prototype, "localeCompare",
Builtins::kStringPrototypeLocaleCompare, 1, true);
#endif // V8_INTL_SUPPORT
SimpleInstallFunction(isolate_, prototype, "match",
Builtins::kStringPrototypeMatch, 1, true);
#ifdef V8_INTL_SUPPORT

View File

@ -7,6 +7,9 @@
#include "src/conversions.h"
#include "src/counters.h"
#include "src/objects-inl.h"
#ifdef V8_INTL_SUPPORT
#include "src/objects/intl-objects.h"
#endif
#include "src/regexp/regexp-utils.h"
#include "src/string-builder.h"
#include "src/string-case.h"
@ -190,10 +193,18 @@ BUILTIN(StringPrototypeLastIndexOf) {
//
// This function is implementation specific. For now, we do not
// do anything locale specific.
// If internationalization is enabled, then intl.js will override this function
// and provide the proper functionality, so this is just a fallback.
BUILTIN(StringPrototypeLocaleCompare) {
HandleScope handle_scope(isolate);
#ifdef V8_INTL_SUPPORT
TO_THIS_STRING(str1, "String.prototype.localeCompare");
Handle<String> str2;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, str2, Object::ToString(isolate, args.atOrUndefined(isolate, 1)));
RETURN_RESULT_OR_FAILURE(
isolate, Intl::StringLocaleCompare(isolate, str1, str2,
args.atOrUndefined(isolate, 2),
args.atOrUndefined(isolate, 3)));
#else
DCHECK_EQ(2, args.length());
TO_THIS_STRING(str1, "String.prototype.localeCompare");
@ -235,6 +246,7 @@ BUILTIN(StringPrototypeLocaleCompare) {
}
return Smi::FromInt(str1_length - str2_length);
#endif // !V8_INTL_SUPPORT
}
#ifndef V8_INTL_SUPPORT

View File

@ -95,6 +95,8 @@ enum ContextLookupFlags {
V(PROMISE_FUNCTION_INDEX, JSFunction, promise_function) \
V(RANGE_ERROR_FUNCTION_INDEX, JSFunction, range_error_function) \
V(REFERENCE_ERROR_FUNCTION_INDEX, JSFunction, reference_error_function) \
V(CACHED_OR_NEW_SERVICE_LOCALE_FUNCTION_INDEX, JSFunction, \
cached_or_new_service) \
V(RESOLVE_LOCALE_FUNCTION_INDEX, JSFunction, resolve_locale) \
V(SET_ADD_INDEX, JSFunction, set_add) \
V(SET_DELETE_INDEX, JSFunction, set_delete) \

View File

@ -1908,23 +1908,11 @@ function cachedOrNewService(service, locales, options, defaults) {
return new savedObjects[service](locales, useOptions);
}
/**
* Compares this and that, and returns less than 0, 0 or greater than 0 value.
* Overrides the built-in method.
*/
DEFINE_METHOD(
GlobalString.prototype,
localeCompare(that) {
if (IS_NULL_OR_UNDEFINED(this)) {
throw %make_type_error(kMethodInvokedOnNullOrUndefined);
}
var locales = arguments[1];
var options = arguments[2];
var collator = cachedOrNewService('collator', locales, options);
return compare(collator, this, that);
}
);
// TODO(ftang) remove the %InstallToContext once
// cachedOrNewService is available in C++
%InstallToContext([
"cached_or_new_service", cachedOrNewService
]);
/**
* Formats a Number object (this) using locale and options values.

View File

@ -1800,6 +1800,12 @@ namespace {
// Remove the following after we port InitializeLocaleList from src/js/intl.js
// to c++ https://bugs.chromium.org/p/v8/issues/detail?id=7987
// The following are temporary function calling back into js code in
// src/js/intl.js to call pre-existing functions until they are all moved to C++
// under src/objects/*.
// TODO(ftang): remove these temp function after bstell move them from js into
// C++
MaybeHandle<JSObject> InitializeLocaleList(Isolate* isolate,
Handle<Object> list) {
Handle<Object> result;
@ -1818,6 +1824,22 @@ bool IsAToZ(char ch) {
return (('A' <= ch) && (ch <= 'Z')) || (('a' <= ch) && (ch <= 'z'));
}
MaybeHandle<JSObject> CachedOrNewService(Isolate* isolate,
Handle<String> service,
Handle<Object> locales,
Handle<Object> options) {
Handle<Object> result;
Handle<Object> undefined_value(ReadOnlyRoots(isolate).undefined_value(),
isolate);
Handle<Object> args[] = {service, locales, options};
ASSIGN_RETURN_ON_EXCEPTION(
isolate, result,
Execution::Call(isolate, isolate->cached_or_new_service(),
undefined_value, arraysize(args), args),
JSArray);
return Handle<JSObject>::cast(result);
}
} // namespace
// Verifies that the input is a well-formed ISO 4217 currency code.
@ -1927,5 +1949,52 @@ MaybeHandle<String> Intl::StringLocaleConvertCase(Isolate* isolate,
return Object::ToString(isolate, obj);
}
MaybeHandle<Object> Intl::StringLocaleCompare(Isolate* isolate,
Handle<String> string1,
Handle<String> string2,
Handle<Object> locales,
Handle<Object> options) {
Factory* factory = isolate->factory();
Handle<JSObject> collator_holder;
ASSIGN_RETURN_ON_EXCEPTION(
isolate, collator_holder,
CachedOrNewService(isolate, factory->NewStringFromStaticChars("collator"),
locales, options),
Object);
DCHECK(Intl::IsObjectOfType(isolate, collator_holder, Intl::kCollator));
return Intl::InternalCompare(isolate, collator_holder, string1, string2);
}
Handle<Object> Intl::InternalCompare(Isolate* isolate,
Handle<JSObject> collator_holder,
Handle<String> string1,
Handle<String> string2) {
Factory* factory = isolate->factory();
icu::Collator* collator = Collator::UnpackCollator(collator_holder);
CHECK_NOT_NULL(collator);
string1 = String::Flatten(isolate, string1);
string2 = String::Flatten(isolate, string2);
UCollationResult result;
UErrorCode status = U_ZERO_ERROR;
{
DisallowHeapAllocation no_gc;
int32_t length1 = string1->length();
int32_t length2 = string2->length();
String::FlatContent flat1 = string1->GetFlatContent();
String::FlatContent flat2 = string2->GetFlatContent();
std::unique_ptr<uc16[]> sap1;
std::unique_ptr<uc16[]> sap2;
icu::UnicodeString string_val1(
FALSE, GetUCharBufferFromFlat(flat1, &sap1, length1), length1);
icu::UnicodeString string_val2(
FALSE, GetUCharBufferFromFlat(flat2, &sap2, length2), length2);
result = collator->compare(string_val1, string_val2, status);
}
DCHECK(U_SUCCESS(status));
return factory->NewNumberFromInt(result);
}
} // namespace internal
} // namespace v8

View File

@ -322,6 +322,14 @@ class Intl {
V8_WARN_UNUSED_RESULT static MaybeHandle<String> StringLocaleConvertCase(
Isolate* isolate, Handle<String> s, bool is_upper,
Handle<Object> locales);
V8_WARN_UNUSED_RESULT static MaybeHandle<Object> StringLocaleCompare(
Isolate* isolate, Handle<String> s1, Handle<String> s2,
Handle<Object> locales, Handle<Object> options);
V8_WARN_UNUSED_RESULT static Handle<Object> InternalCompare(
Isolate* isolate, Handle<JSObject> collator, Handle<String> s1,
Handle<String> s2);
};
} // namespace internal

View File

@ -264,32 +264,7 @@ RUNTIME_FUNCTION(Runtime_InternalCompare) {
CONVERT_ARG_HANDLE_CHECKED(JSObject, collator_holder, 0);
CONVERT_ARG_HANDLE_CHECKED(String, string1, 1);
CONVERT_ARG_HANDLE_CHECKED(String, string2, 2);
icu::Collator* collator = Collator::UnpackCollator(collator_holder);
CHECK_NOT_NULL(collator);
string1 = String::Flatten(isolate, string1);
string2 = String::Flatten(isolate, string2);
UCollationResult result;
UErrorCode status = U_ZERO_ERROR;
{
DisallowHeapAllocation no_gc;
int32_t length1 = string1->length();
int32_t length2 = string2->length();
String::FlatContent flat1 = string1->GetFlatContent();
String::FlatContent flat2 = string2->GetFlatContent();
std::unique_ptr<uc16[]> sap1;
std::unique_ptr<uc16[]> sap2;
icu::UnicodeString string_val1(
FALSE, GetUCharBufferFromFlat(flat1, &sap1, length1), length1);
icu::UnicodeString string_val2(
FALSE, GetUCharBufferFromFlat(flat2, &sap2, length2), length2);
result = collator->compare(string_val1, string_val2, status);
}
if (U_FAILURE(status)) return isolate->ThrowIllegalOperation();
return *isolate->factory()->NewNumberFromInt(result);
return *Intl::InternalCompare(isolate, collator_holder, string1, string2);
}
RUNTIME_FUNCTION(Runtime_CreatePluralRules) {