Closed Bug 1338928 Opened 4 years ago Closed 4 years ago

Const data tables should go in a read-only data section (intl/)

Categories

(Core :: Internationalization, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: away, Assigned: away)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Attached patch Add constexprSplinter Review
Please see bug 1334254 comment 0 for background.

These changes move 4.5k of tables from the .data section to the read-only .rdata section.
Attachment #8836520 - Flags: review?(VYV03354)
I forgot to mention: the change in nsUNIXCharset.cpp is, of course, unnecessary for the Visual Studio workaround. But I added it anyway for consistency.
Comment on attachment 8836520 [details] [diff] [review]
Add constexpr

Review of attachment 8836520 [details] [diff] [review]:
-----------------------------------------------------------------

Are you sure this change has no side-effects on other compilers? But 1255655 added the const for this very reason.
Attachment #8836520 - Flags: review?(VYV03354) → review+
> But 1255655
Bug 1255655
Prior to bug 1255655, the arrays were not const at all, so that bug's change makes sense. 

Constexpr is like const but even stronger (if you try to say "const constexpr", the compiler complains about "duplicate const") so this shouldn't disturb existing const data.
Thanks for explaining.
Pushed by dmajor@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a93ad0b96cc
Add constexpr to static data under intl/ for better codegen on Windows. r=emk
https://hg.mozilla.org/mozilla-central/rev/2a93ad0b96cc
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.