Closed Bug 1164735 Opened 10 years ago Closed 10 years ago

Move gPrefLangToLangGroups[] inside a function to avoid a static constructor

Categories

(Core :: Graphics: Text, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox41 --- fixed

People

(Reporter: n.nethercote, Assigned: n.nethercote)

References

Details

Attachments

(1 file)

Bug 1056479 introduced a new static constructor.
|static| variables at the top level are initialized at start-up. |static| variables in a function are initialized the first time that function is called.
Attachment #8605601 - Flags: review?(jdaggett)
Comment on attachment 8605601 [details] [diff] [review] Move gPrefLangToLangGroups[] inside a function to avoid a static constructor Review of attachment 8605601 [details] [diff] [review]: ----------------------------------------------------------------- There are other arrays within the gfxPlatform code that don't appear to produce static initialization code because they're labeled as const arrays. That's what this array should be but that'll take some code reworking. Karl already commented that he wanted a different way of doing this so I think I'll hang a patch for this on bug 1163488. I'll confirm that none of the arrays involved produce static initialization code.
Depends on: 1163488
A static variable needs a static constructor if (a) it has a non-trivial constructor, or (b) it has virtual methods, or (c) it has a destructor, or (d) the compiler is stupid. This also applies to the elements of static arrays. I figure that one or more of (a), (b), or (c) is true for nsGkAtom. It's not true for, say, |const char*|, which all the other arrays appear to be.
I was actually thinking more of the MozLangGroups array in gfxFontconfigUtils.cpp. It looks like this is used as a static array without initialization: http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxFontconfigUtils.cpp#376 But I think the idea of replacing the pref lang mappings with #include's is a better way.
Comment on attachment 8605601 [details] [diff] [review] Move gPrefLangToLangGroups[] inside a function to avoid a static constructor Review of attachment 8605601 [details] [diff] [review]: ----------------------------------------------------------------- Might as well take this as it appears to fix a regression caused by the static array not getting initialized correctly with the current code. This will get cleaned up on bug 1163488.
Attachment #8605601 - Flags: review?(jdaggett) → review+
Blocks: 1165788
Blocks: 1165590
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
(In reply to John Daggett (:jtd) from comment #4) > I was actually thinking more of the MozLangGroups array in > gfxFontconfigUtils.cpp. It looks like this is used as a static array without > initialization: MozLangGroups could be done better, yes (critiquing my own code here IIRC). If function-static, assuming it is called after nsGkAtoms::AddRefAtoms() as assumed for PrefLangToLangGroups() here, it could similarly use nsIAtom* instead of nsIAtom*const&. I assume that an array of addresses of (or references to) static storage duration variables (as MozLangGroups is) in PIC requires relocations, which are performed at startup. I expect merely taking the nsIAtom* value of the variables can use relative addressing.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: