Closed Bug 1319464 Opened 3 years ago Closed 3 years ago

Replace flattened strings with linear strings in Intl.cpp

Categories

(Core :: JavaScript: Internationalization API, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: anba, Assigned: anba)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch bug1319464.patch (obsolete) — Splinter Review
NewUNumberFormat():
We're already explicitly passing uCurrency's length to unum_setTextAttribute, so we don't need to null-terminate uCurrency.

Time zone lookups:
Again no null-termination required because we're always passing the string's length (HashStringIgnoreCaseASCII, EqualCharsIgnoreCaseASCII).

js::intl_patternForSkeleton():
I've changed this method so it matches the normal ICU call pattern. As a side-effect the null-termination is no longer required.

NewUDateFormat():
Also changed to the normal ICU call pattern, so we no longer need to null-terminate the strings.


I've also replaced some "int" with "int32_t" and added assertions for consistency with other calls to ICU functions.
Attachment #8813309 - Flags: review?(jwalden+bmo)
Attachment #8813309 - Flags: review?(jwalden+bmo) → review+
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7251919f6a23
Replace flattened with linear strings in Intl.cpp file. r=Waldo
Keywords: checkin-needed
Also backed out (in addition to bug 1319465) for failures in Intl.h in Sm-tc(nu):

https://hg.mozilla.org/integration/mozilla-inbound/rev/9caa83bd1f8353ca9c61f663d5023f61326e18e1

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=aabf76e98464f96bcee92c111286ffe939b98700
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=39721183&repo=mozilla-inbound

[task 2016-11-23T16:02:36.957660Z] In file included from /home/worker/workspace/build/src/js/src/builtin/Intl.cpp:12:0:
[task 2016-11-23T16:02:36.957731Z] /home/worker/workspace/build/src/js/src/builtin/Intl.h:85:43: error: expected ')' before '*' token
[task 2016-11-23T16:02:36.957756Z]              explicit Lookup(JSLinearString* timeZone);
[task 2016-11-23T16:02:36.957788Z]                                            ^
[task 2016-11-23T16:02:38.971359Z] /home/worker/workspace/build/src/js/src/builtin/Intl.cpp:2045:1: error: prototype for 'js::SharedIntlData::TimeZoneHasher::Lookup::Lookup(JSLinearString*)' does not match any in class 'js::SharedIntlData::TimeZoneHasher::Lookup'
[task 2016-11-23T16:02:38.971428Z]  js::SharedIntlData::TimeZoneHasher::Lookup::Lookup(JSLinearString* timeZone)
[task 2016-11-23T16:02:38.971442Z]  ^
[task 2016-11-23T16:02:38.971473Z] In file included from /home/worker/workspace/build/src/js/src/builtin/Intl.cpp:12:0:
[task 2016-11-23T16:02:38.971542Z] /home/worker/workspace/build/src/js/src/builtin/Intl.h:74:16: error: candidates are: constexpr js::SharedIntlData::TimeZoneHasher::Lookup::Lookup(js::SharedIntlData::TimeZoneHasher::Lookup&&)
[task 2016-11-23T16:02:38.971566Z]          struct Lookup
[task 2016-11-23T16:02:38.971584Z]                 ^
[task 2016-11-23T16:02:38.971633Z] /home/worker/workspace/build/src/js/src/builtin/Intl.h:74:16: error:                 constexpr js::SharedIntlData::TimeZoneHasher::Lookup::Lookup(const js::SharedIntlData::TimeZoneHasher::Lookup&)
[task 2016-11-23T16:02:38.971686Z] /home/worker/workspace/build/src/js/src/builtin/Intl.h:74:16: error:                 js::SharedIntlData::TimeZoneHasher::Lookup::Lookup()
[task 2016-11-23T16:02:38.971746Z] /home/worker/workspace/build/src/js/src/builtin/Intl.h:72:12: error: 'struct js::SharedIntlData::TimeZoneHasher' is private
[task 2016-11-23T16:02:38.971775Z]      struct TimeZoneHasher
[task 2016-11-23T16:02:38.971797Z]             ^
...
Flags: needinfo?(andrebargull)
Looks like a missing include for vm/String.h.
Flags: needinfo?(andrebargull)
Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/00df72027708
Backed out changeset 7251919f6a23 to fix Sm-Tc(nu). r=backout
Attached patch bug1319464.patchSplinter Review
Added forward declaration to fix non-unified build bustage. Carrying r+ from Waldo.
Attachment #8813309 - Attachment is obsolete: true
Attachment #8813795 - Flags: review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/968fe7fa5d93
Replace flattened with linear strings in Intl.cpp file. r=Waldo
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/968fe7fa5d93
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.