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)
Core
Graphics: Text
Tracking
()
RESOLVED
FIXED
mozilla41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
Attachments
(1 file)
(deleted),
patch
|
jtd
:
review+
|
Details | Diff | Splinter Review |
Bug 1056479 introduced a new static constructor.
Assignee | ||
Comment 1•10 years ago
|
||
|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 2•10 years ago
|
||
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.
Assignee | ||
Comment 3•10 years ago
|
||
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•10 years ago
|
||
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•10 years ago
|
||
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+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment 8•10 years ago
|
||
(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.
Description
•