Modified: trunk/Tools/ChangeLog (260733 => 260734)
--- trunk/Tools/ChangeLog 2020-04-26 22:54:45 UTC (rev 260733)
+++ trunk/Tools/ChangeLog 2020-04-26 23:04:20 UTC (rev 260734)
@@ -1,3 +1,21 @@
+2020-04-26 Yusuke Suzuki <[email protected]>
+
+ Warn when NeverDestroyed<Lock> is used
+ https://bugs.webkit.org/show_bug.cgi?id=211054
+
+ Reviewed by Darin Adler.
+
+ WTF::Lock and WTF::Condition are designed to be constant-initialize compliant. So NeverDestroyed<> for these types are not necessary,
+ or rather, introducing race condition issue while `static Lock` doesn't have that issue. This patch adds lint rules which prevent
+ us from using `NeverDestroyed<Lock>` and `LazyNeverDestroyed<Lock>`.
+
+ * Scripts/webkitpy/style/checkers/cpp.py:
+ (check_wtf_never_destroyed):
+ (check_style):
+ (CppChecker):
+ * Scripts/webkitpy/style/checkers/cpp_unittest.py:
+ (WebKitStyleTest.test_wtf_never_destroyed):
+
2020-04-25 Darin Adler <[email protected]>
[Cocoa] Deal with another round of Xcode upgrade checks
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py (260733 => 260734)
--- trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py 2020-04-26 22:54:45 UTC (rev 260733)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/cpp.py 2020-04-26 23:04:20 UTC (rev 260734)
@@ -2544,6 +2544,28 @@
"Use 'WTF::makeUnique<{typename}>' instead of 'std::make_unique<{typename}>'.".format(typename=typename))
+def check_wtf_never_destroyed(clean_lines, line_number, file_state, error):
+ """Looks for use of 'NeverDestroyed<Lock/Condition>' which should be replaced with 'Lock/Condition'.
+
+ Args:
+ clean_lines: A CleansedLines instance containing the file.
+ line_number: The number of the line to check.
+ file_state: A _FileState instance which maintains information about
+ the state of things in the file.
+ error: The function to call with any errors found.
+ """
+
+ line = clean_lines.elided[line_number] # Get rid of comments and strings.
+
+ using_wtf_never_destroyed_search = search(r'\b(?:Lazy)?NeverDestroyed\s*<([^(>]+)>', line) # LazyNeverDestroyed is also caught.
+ if not using_wtf_never_destroyed_search:
+ return
+
+ typename = using_wtf_never_destroyed_search.group(1).strip()
+ if search(r'(Lock|Condition)', typename):
+ error(line_number, 'runtime/wtf_never_destroyed', 4, "Use 'static Lock/Condition' instead of 'NeverDestroyed<Lock/Condition>'.")
+
+
def check_lock_guard(clean_lines, line_number, file_state, error):
"""Looks for use of 'std::lock_guard<>' which should be replaced with 'WTF::Locker'.
@@ -3124,6 +3146,7 @@
check_wtf_move(clean_lines, line_number, file_state, error)
check_wtf_optional(clean_lines, line_number, file_state, error)
check_wtf_make_unique(clean_lines, line_number, file_state, error)
+ check_wtf_never_destroyed(clean_lines, line_number, file_state, error)
check_lock_guard(clean_lines, line_number, file_state, error)
check_ctype_functions(clean_lines, line_number, file_state, error)
check_switch_indentation(clean_lines, line_number, error)
@@ -4305,6 +4328,7 @@
'runtime/wtf_optional',
'runtime/wtf_make_unique',
'runtime/wtf_move',
+ 'runtime/wtf_never_destroyed',
'security/assertion',
'security/missing_warn_unused_return',
'security/printf',
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py (260733 => 260734)
--- trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py 2020-04-26 22:54:45 UTC (rev 260733)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py 2020-04-26 23:04:20 UTC (rev 260734)
@@ -5486,6 +5486,75 @@
" [runtime/wtf_move] [4]",
'foo.mm')
+ def test_wtf_never_destroyed(self):
+ self.assert_lint(
+ 'static NeverDestroyed<Foo> foo;',
+ '',
+ 'foo.cpp')
+
+ self.assert_lint(
+ 'static LazyNeverDestroyed<Foo> foo;',
+ '',
+ 'foo.cpp')
+
+ self.assert_lint(
+ 'static NeverDestroyed<Lock> lock;',
+ "Use 'static Lock/Condition' instead of 'NeverDestroyed<Lock/Condition>'."
+ " [runtime/wtf_never_destroyed] [4]",
+ 'foo.cpp')
+
+ self.assert_lint(
+ 'static NeverDestroyed<Condition> condition;',
+ "Use 'static Lock/Condition' instead of 'NeverDestroyed<Lock/Condition>'."
+ " [runtime/wtf_never_destroyed] [4]",
+ 'foo.cpp')
+
+ self.assert_lint(
+ 'static LazyNeverDestroyed<Lock> lock;',
+ "Use 'static Lock/Condition' instead of 'NeverDestroyed<Lock/Condition>'."
+ " [runtime/wtf_never_destroyed] [4]",
+ 'foo.cpp')
+
+ self.assert_lint(
+ 'static LazyNeverDestroyed<Condition> condition;',
+ "Use 'static Lock/Condition' instead of 'NeverDestroyed<Lock/Condition>'."
+ " [runtime/wtf_never_destroyed] [4]",
+ 'foo.cpp')
+
+ self.assert_lint(
+ 'static NeverDestroyed<Foo> foo;',
+ '',
+ 'foo.mm')
+
+ self.assert_lint(
+ 'static LazyNeverDestroyed<Foo> foo;',
+ '',
+ 'foo.mm')
+
+ self.assert_lint(
+ 'static NeverDestroyed<Lock> lock;',
+ "Use 'static Lock/Condition' instead of 'NeverDestroyed<Lock/Condition>'."
+ " [runtime/wtf_never_destroyed] [4]",
+ 'foo.mm')
+
+ self.assert_lint(
+ 'static NeverDestroyed<Condition> condition;',
+ "Use 'static Lock/Condition' instead of 'NeverDestroyed<Lock/Condition>'."
+ " [runtime/wtf_never_destroyed] [4]",
+ 'foo.mm')
+
+ self.assert_lint(
+ 'static LazyNeverDestroyed<Lock> lock;',
+ "Use 'static Lock/Condition' instead of 'NeverDestroyed<Lock/Condition>'."
+ " [runtime/wtf_never_destroyed] [4]",
+ 'foo.mm')
+
+ self.assert_lint(
+ 'static LazyNeverDestroyed<Condition> condition;',
+ "Use 'static Lock/Condition' instead of 'NeverDestroyed<Lock/Condition>'."
+ " [runtime/wtf_never_destroyed] [4]",
+ 'foo.mm')
+
def test_wtf_optional(self):
self.assert_lint(
'Optional<int> a;',
