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

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Nicholas Nethercote [inactive]]

Bug 1056479 introduced a new static constructor.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Move gPrefLangToLangGroups[] inside a function to avoid a 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.

Comment 2

[John Daggett (:jtd)]

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.

Comment 3

[Nicholas Nethercote [inactive]]

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.

Comment 4

[John Daggett (:jtd)]

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 5

[John Daggett (:jtd)]

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.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/807da62a1d3e

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/807da62a1d3e

Comment 8

[Karl Tomlinson (:karlt)]

(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.