Bug 149807 - [Intl] Change the return type of canonicalizeLocaleList() from JSArray* to Vector<String>
Summary: [Intl] Change the return type of canonicalizeLocaleList() from JSArray* to Ve...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-05 08:18 PDT by Sukolsak Sakshuwong
Modified: 2015-10-05 16:37 PDT (History)
4 users (show)

See Also:

Attachments
Patch (11.97 KB, patch)
2015-10-05 08:26 PDT, Sukolsak Sakshuwong 		no flags Details | Formatted Diff | Diff
Use the LHS Vector<String> instead of const Vector<String>& (11.95 KB, patch)
2015-10-05 15:05 PDT, Sukolsak Sakshuwong 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Sukolsak Sakshuwong 2015-10-05 08:18:11 PDT 
From ECMA-402, 9.2.1, the abstract operation CanonicalizeLocaleList returns a List of Strings. From the spec, we never modify the result from CanonicalizeLocaleList(). We never expose it to the user either. This patch changes the return type of canonicalizeLocaleList() from JSArray* to Vector<String>. This should ease the workload of the GC and make the code a bit easier to read.

http://www.ecma-international.org/ecma-402/1.0/ECMA-402.pdf
Comment 1 Sukolsak Sakshuwong 2015-10-05 08:26:49 PDT 
Created attachment 262439 [details]
Patch
Comment 2 Benjamin Poulain 2015-10-05 14:44:12 PDT 
Comment on attachment 262439 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=262439&action=review

Ok

> Source/JavaScriptCore/runtime/IntlCollatorConstructor.cpp:152
> +    const Vector<String>& requestedLocales = canonicalizeLocaleList(exec, exec->argument(0));

IMHO, using Vector<String> makes the lifetime clearer.
Comment 3 Sukolsak Sakshuwong 2015-10-05 15:05:24 PDT 
Created attachment 262467 [details]
Use the LHS Vector<String> instead of const Vector<String>&
Comment 4 Sukolsak Sakshuwong 2015-10-05 15:18:47 PDT 
(In reply to comment #2)
> Comment on attachment 262439 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=262439&action=review
> 
> Ok
> 
> > Source/JavaScriptCore/runtime/IntlCollatorConstructor.cpp:152
> > +    const Vector<String>& requestedLocales = canonicalizeLocaleList(exec, exec->argument(0));
> 
> IMHO, using Vector<String> makes the lifetime clearer.

Thanks! Fixed.
Comment 5 WebKit Commit Bot 2015-10-05 16:37:42 PDT 
Comment on attachment 262467 [details]
Use the LHS Vector<String> instead of const Vector<String>&

Clearing flags on attachment: 262467

Committed r190591: <http://trac.webkit.org/changeset/190591>
Comment 6 WebKit Commit Bot 2015-10-05 16:37:45 PDT 
All reviewed patches have been landed.  Closing bug.