Commit 66451a6e authored by Clemens Hammacher's avatar Clemens Hammacher Committed by Commit Bot

[presubmit] Check for proper V8_NOEXCEPT annotations

Forgetting V8_NOEXCEPT annotations on copy constructors, move
constructors, copy assignment operators or move assignment operators
can cause subtle performance bugs or compilation failures, sometimes
only on specific architectures or compilers. Thus check that all those
special class members are marked V8_NOEXCEPT.

This check is only executed on modified files for now, and can be
bypassed. Please report any false positives on the associated bug.

Bug: v8:8616

R=jgruber@chromium.org, machenbach@chromium.org

Change-Id: Ieefd8e39fbb1b314dc8d72ee87f6138b784205af
Reviewed-on: https://chromium-review.googlesource.com/c/1386496Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Reviewed-by: 's avatarMichael Achenbach <machenbach@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58404}
parent 2dbe465a
......@@ -322,6 +322,7 @@ def _CommonChecks(input_api, output_api):
_CheckNoInlineHeaderIncludesInNormalHeaders(input_api, output_api))
results.extend(_CheckJSONFiles(input_api, output_api))
results.extend(_CheckMacroUndefs(input_api, output_api))
results.extend(_CheckNoexceptAnnotations(input_api, output_api))
results.extend(input_api.RunTests(
input_api.canned_checks.CheckVPythonSpec(input_api, output_api)))
return results
......@@ -442,6 +443,49 @@ def _CheckMacroUndefs(input_api, output_api):
return []
def _CheckNoexceptAnnotations(input_api, output_api):
"""
Checks that all user-defined constructors and assignment operators are marked
V8_NOEXCEPT.
This is required for standard containers to pick the right constructors. Our
macros (like MOVE_ONLY_WITH_DEFAULT_CONSTRUCTORS) add this automatically.
Omitting it at some places can result in weird compiler errors if this is
mixed with other classes that have the annotation.
TODO(clemensh): This check should eventually be enabled for all files via
tools/presubmit.py (https://crbug.com/v8/8616).
"""
# matches any class name.
class_name = r'\b([A-Z][A-Za-z0-9_]*)(?:::\1)?'
# initial class name is potentially followed by this to declare an assignment
# operator.
potential_assignment = r'(?:&\s+operator=)?\s*'
# matches an argument list that contains only a reference to a class named
# like the first capture group, potentially const.
single_class_ref_arg = r'\((?:const\s+)?\1(?:::\1)?&&?[^,;)]*\)'
# matches anything but a sequence of whitespaces followed by V8_NOEXCEPT.
not_followed_by_noexcept = r'(?!\s+V8_NOEXCEPT\b)'
full_pattern = r'^.*?' + class_name + potential_assignment + \
single_class_ref_arg + not_followed_by_noexcept + '.*?$'
regexp = input_api.re.compile(full_pattern, re.MULTILINE)
errors = []
for f in input_api.AffectedFiles(include_deletes=False):
with open(f.LocalPath()) as fh:
for match in re.finditer(regexp, fh.read()):
errors.append(match.group().strip())
if errors:
return [output_api.PresubmitPromptOrNotify(
'Copy constructors, move constructors, copy assignment operators and '
'move assignment operators should be marked V8_NOEXCEPT.\n'
'Please report false positives on https://crbug.com/v8/8616.',
errors)]
return []
def CheckChangeOnUpload(input_api, output_api):
results = []
results.extend(_CommonChecks(input_api, output_api))
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment