Bug 134794 - Migrate the usage of DEPRECATED_DEFINE_STATIC_LOCAL(T, name, args) to "static NeverDestroyed<T> name(args)"
Summary: Migrate the usage of DEPRECATED_DEFINE_STATIC_LOCAL(T, name, args) to "static...
Status: RESOLVED DUPLICATE of bug 130185
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Vivek Galatage
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-09 20:49 PDT by Vivek Galatage
Modified: 2017-07-21 20:22 PDT (History)
3 users (show)

See Also:


Attachments
Patch (372.54 KB, patch)
2014-07-09 20:57 PDT, Vivek Galatage
no flags Details | Formatted Diff | Diff
Patch (315.48 KB, patch)
2014-07-10 00:02 PDT, Vivek Galatage
no flags Details | Formatted Diff | Diff
Patch (315.39 KB, patch)
2014-07-10 00:15 PDT, Vivek Galatage
no flags Details | Formatted Diff | Diff
Patch (314.86 KB, patch)
2014-07-10 00:29 PDT, Vivek Galatage
no flags Details | Formatted Diff | Diff
Patch (314.56 KB, patch)
2014-07-10 01:35 PDT, Vivek Galatage
darin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vivek Galatage 2014-07-09 20:49:14 PDT
Splitting DEFINE_STATIC_LOCAL into two for better readability of the code. In this, one is without 
any arguments whereas the other macro takes argument as macro param. This CL makes these calls 
more explicit. 

The macros DEFINE_STATIC_LOCAL and DEFINE_STATIC_LOCAL_WITH_ARG are defined in Source/wtf/StdLibExtras.h 

The following commands are executed in order to change this: 
git ls-files | xargs sed -i 's/DEFINE_STATIC_LOCAL\((.*\)\(, ()\)/DEFINE_STATIC_LOCAL_WITHOUT_ARG\1/g' 
git ls-files | xargs sed -i 's/DEFINE_STATIC_LOCAL(/DEFINE_STATIC_LOCAL_WITH_ARG(/g' 
git ls-files | xargs sed -i 's/DEFINE_STATIC_LOCAL_WITHOUT_ARG(/DEFINE_STATIC_LOCAL(/g'
Comment 1 Vivek Galatage 2014-07-09 20:57:09 PDT
Created attachment 234686 [details]
Patch
Comment 2 WebKit Commit Bot 2014-07-09 20:59:08 PDT
Attachment 234686 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/StdLibExtras.h:57:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 295 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Sam Weinig 2014-07-09 21:08:45 PDT
Instead of doing this, we should probably be moving code over to WTF::NeverDestroyed.
Comment 4 Vivek Galatage 2014-07-09 21:13:26 PDT
(In reply to comment #3)
> Instead of doing this, we should probably be moving code over to WTF::NeverDestroyed.

Yeah sure, would give it a try. Thanks for reviewing :)
Comment 5 Vivek Galatage 2014-07-10 00:02:54 PDT
Created attachment 234692 [details]
Patch
Comment 6 Vivek Galatage 2014-07-10 00:15:40 PDT
Created attachment 234693 [details]
Patch
Comment 7 Vivek Galatage 2014-07-10 00:29:26 PDT
Created attachment 234694 [details]
Patch
Comment 8 Vivek Galatage 2014-07-10 01:35:25 PDT
Created attachment 234695 [details]
Patch
Comment 9 Darin Adler 2014-08-19 09:13:13 PDT
Comment on attachment 234695 [details]
Patch

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

Great idea, but this giant patch doesn’t build. How about a sequence of smaller patches to achieve the same thing? I’d like to carefully review each one to spot mistakes.

For String and AtomicString, it might be better to get rid of some of these rather than converting to NeverDestroyed; I think we’re overdoing the global AtomicString pattern.

A good idea for as first patch is something that only deals with clearly-correct cases such as maps and mutexes, and sticks to a manageable smaller list. Then we can tackle the trickier cases like strings. Also need a patch that successfully applies and compiles.

> Source/WebCore/bindings/scripts/test/JS/JSTestActiveDOMObject.h:86
> +    static NeverDestroyed<JSTestActiveDOMObjectOwner> jsTestActiveDOMObjectOwner;

These are generated files, so they should not be changed manually. Instead we need to change the script and regenerate.
Comment 10 Darin Adler 2017-07-21 20:22:22 PDT

*** This bug has been marked as a duplicate of bug 130185 ***