Bug 129612 - Rename DEFINE_STATIC_LOCAL to DEPRECATED_DEFINE_STATIC_LOCAL
Summary: Rename DEFINE_STATIC_LOCAL to DEPRECATED_DEFINE_STATIC_LOCAL
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sergio Villar Senin
URL:
Keywords:
Depends on:
Blocks: 130185
  Show dependency treegraph
 
Reported: 2014-03-03 11:17 PST by Sergio Villar Senin
Modified: 2014-03-14 01:31 PDT (History)
2 users (show)

See Also:


Attachments
Patch (435.19 KB, patch)
2014-03-12 11:01 PDT, Sergio Villar Senin
no flags Details | Formatted Diff | Diff
Patch (434.34 KB, patch)
2014-03-13 03:37 PDT, Sergio Villar Senin
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sergio Villar Senin 2014-03-03 11:17:28 PST
As suggested by Darin in https://bugs.webkit.org/show_bug.cgi?id=128173#c2. The recommended alternative is static NeverDestroyed.
Comment 1 Sergio Villar Senin 2014-03-12 11:01:23 PDT
Created attachment 226533 [details]
Patch

I removed the list of changes in the WebCore ChangeLog because it was huge
Comment 2 Sergio Villar Senin 2014-03-13 03:31:03 PDT
BTW I opened bug 130185 to start replacing them with static NeverDestroyed<T>
Comment 3 Sergio Villar Senin 2014-03-13 03:37:49 PDT
Created attachment 226588 [details]
Patch
Comment 4 Darin Adler 2014-03-13 09:58:59 PDT
Comment on attachment 226588 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=226588&action=review

Did you use the do-webcore-rename script? It’s really good at doing changes like this one automatically.

> Source/WTF/wtf/StdLibExtras.h:38
> -#ifndef DEFINE_STATIC_LOCAL
> +#ifndef DEPRECATED_DEFINE_STATIC_LOCAL

Should update the comment above to talk about using NeverDestroyed instead.

> Tools/DumpRenderTree/JavaScriptThreading.cpp:49
> +    DEPRECATED_DEFINE_STATIC_LOCAL(Mutex, staticMutex, ());

Note for the future: I’m surprised we need this at all in test tools. A plain old static should be OK in all the test tool cases. It’s only the frameworks themselves that have the “no static destructor” constraint.

> Tools/Scripts/check-for-exit-time-destructors:100
> +    print "ERROR: Use DEPRECATED_DEFINE_STATIC_LOCAL from <wtf/StdLibExtras.h>\n";

This is no longer correct advice. The message should mention NeverDestroyed instead.

> Tools/TestWebKitAPI/Tests/WebKit2/SeccompFilters.cpp:45
> +DEPRECATED_DEFINE_STATIC_LOCAL(String, rootDir, (ASCIILiteral("/")));

Same thought here about test tools not needing this.
Comment 5 Sergio Villar Senin 2014-03-13 10:19:13 PDT
(In reply to comment #4)
> (From update of attachment 226588 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=226588&action=review
> 
> Did you use the do-webcore-rename script? It’s really good at doing changes like this one automatically.

Oh, didn't know about it, I just cooked my own.

> > Source/WTF/wtf/StdLibExtras.h:38
> > -#ifndef DEFINE_STATIC_LOCAL
> > +#ifndef DEPRECATED_DEFINE_STATIC_LOCAL
> 
> Should update the comment above to talk about using NeverDestroyed instead.

Good point

> > Tools/DumpRenderTree/JavaScriptThreading.cpp:49
> > +    DEPRECATED_DEFINE_STATIC_LOCAL(Mutex, staticMutex, ());
> 
> Note for the future: I’m surprised we need this at all in test tools. A plain old static should be OK in all the test tool cases. It’s only the frameworks themselves that have the “no static destructor” constraint.

Yep, I'll open a bug for that.

> > Tools/Scripts/check-for-exit-time-destructors:100
> > +    print "ERROR: Use DEPRECATED_DEFINE_STATIC_LOCAL from <wtf/StdLibExtras.h>\n";
> 
> This is no longer correct advice. The message should mention NeverDestroyed instead.

Indeed
Comment 6 Sergio Villar Senin 2014-03-14 01:31:37 PDT
Committed r165607: <http://trac.webkit.org/changeset/165607>