| Summary: | Use static global variables instead of static NeverDestroyed inside function if possible | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||
| Component: | New Bugs | Assignee: | Yusuke Suzuki <ysuzuki> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | cgarcia, darin, mcatanzaro, saam, sam, simon.fraser, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Yusuke Suzuki
2018-07-29 06:56:32 PDT
Created attachment 346028 [details]
Patch
Comment on attachment 346028 [details]
Patch
Change seems fine.
Bug title is a bit inaccurate.
Having a constexpr constructor does not affect whether NeverDestroyed is needed. It does affect whether we can have a global outside a function. If the constructor is not constexpr it needs to be inside a function so it gets run the first time the function is installed rather than at load time.
What affects the need for NeverDestroyed is whether the class has a trivial destructor. If it does then NeverDestroyed is not needed.
In all these cases in this patch we changed both things but they are independent.
You can have a static without NeverDestroyed if the destructor is trivial. You can have a global outside a function that uses NeverDestroyed as long as the entire constructor is all constexpr. (Might need to fix NeverDestroyed implementation to be sure it’s constructors are constexpr.)
“Called” not “installed” (In reply to Darin Adler from comment #2) > Comment on attachment 346028 [details] > Patch > > Change seems fine. > > Bug title is a bit inaccurate. > > Having a constexpr constructor does not affect whether NeverDestroyed is > needed. It does affect whether we can have a global outside a function. If > the constructor is not constexpr it needs to be inside a function so it gets > run the first time the function is installed rather than at load time. > > What affects the need for NeverDestroyed is whether the class has a trivial > destructor. If it does then NeverDestroyed is not needed. > > In all these cases in this patch we changed both things but they are > independent. Right, changed. > > You can have a static without NeverDestroyed if the destructor is trivial. > You can have a global outside a function that uses NeverDestroyed as long as > the entire constructor is all constexpr. (Might need to fix NeverDestroyed > implementation to be sure it’s constructors are constexpr.) Currently, we cannot have constexpr NeverDestroyed, since it allocates an instance on aligned storage with placement `new`. This type of code cannot be achieved inside constexpr constructor unfortunately :( Committed r234347: <https://trac.webkit.org/changeset/234347> |