tools,src: forbid usage of v8::Persistent

`v8::Persistent` comes with the surprising catch that it requires manual cleanup. `v8::Global` doesn’t, making it easier to use, and additionally provides move semantics. New code should always use `v8::Global`. PR-URL: #31018 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Stephen Belanger <[email protected]>
nodejs · Jan 14, 2020 · b9c057c · b9c057c
1 parent ea5be29
commit b9c057c
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 0 deletions.
diff --git a/src/node_object_wrap.h b/src/node_object_wrap.h
@@ -65,6 +65,7 @@ class ObjectWrap {
   }
 
 
+  // NOLINTNEXTLINE(runtime/v8_persistent)
   inline v8::Persistent<v8::Object>& persistent() {
     return handle_;
   }
@@ -122,6 +123,7 @@ class ObjectWrap {
     delete wrap;
   }
 
+  // NOLINTNEXTLINE(runtime/v8_persistent)
   v8::Persistent<v8::Object> handle_;
 };
 

diff --git a/tools/cpplint.py b/tools/cpplint.py
@@ -321,6 +321,7 @@
     'runtime/string',
     'runtime/threadsafe_fn',
     'runtime/vlog',
+    'runtime/v8_persistent',
     'whitespace/blank_line',
     'whitespace/braces',
     'whitespace/comma',
@@ -627,6 +628,8 @@
 
 _NULL_TOKEN_PATTERN = re.compile(r'\bNULL\b')
 
+_V8_PERSISTENT_PATTERN = re.compile(r'\bv8::Persistent\b')
+
 _RIGHT_LEANING_POINTER_PATTERN = re.compile(r'[^=|(,\s><);&?:}]'
                                             r'(?<!(sizeof|return))'
                                             r'\s\*[a-zA-Z_][0-9a-zA-Z_]*')
@@ -4547,6 +4550,28 @@ def CheckNullTokens(filename, clean_lines, linenum, error):
     error(filename, linenum, 'readability/null_usage', 2,
           'Use nullptr instead of NULL')
 
+def CheckV8PersistentTokens(filename, clean_lines, linenum, error):
+  """Check v8::Persistent usage.
+
+  Args:
+    filename: The name of the current file.
+    clean_lines: A CleansedLines instance containing the file.
+    linenum: The number of the line to check.
+    error: The function to call with any errors found.
+  """
+  line = clean_lines.elided[linenum]
+
+  # Avoid preprocessor lines
+  if Match(r'^\s*#', line):
+    return
+
+  if line.find('/*') >= 0 or line.find('*/') >= 0:
+    return
+
+  for match in _V8_PERSISTENT_PATTERN.finditer(line):
+    error(filename, linenum, 'runtime/v8_persistent', 2,
+          'Use v8::Global instead of v8::Persistent')
+
 def CheckLeftLeaningPointer(filename, clean_lines, linenum, error):
   """Check for left-leaning pointer placement.
 
@@ -4723,6 +4748,7 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state,
   CheckCheck(filename, clean_lines, linenum, error)
   CheckAltTokens(filename, clean_lines, linenum, error)
   CheckNullTokens(filename, clean_lines, linenum, error)
+  CheckV8PersistentTokens(filename, clean_lines, linenum, error)
   CheckLeftLeaningPointer(filename, clean_lines, linenum, error)
   classinfo = nesting_state.InnermostClass()
   if classinfo: