From 9142ffd708cc640e904c4475b30f3d4b2827b616 Mon Sep 17 00:00:00 2001 From: Glen Robertson Date: Thu, 16 May 2024 01:37:47 +0000 Subject: [PATCH] Fix false-positive in unique_ptr presubmit check with mismatched <> Presubmit was preventing upload of crrev.com/c/5534806 due to changed line: ` std::unique_ptr>(` being interpreted as a call to the unique_ptr constructor. IIUC this was because the check only looks at the changed lines and missed the extra ">" indicating this was really part of a larger template expression. Added a condition that suppresses the check when selected brackets are mismatching (ie. number of < and > are not equal). Added tests for these cases. Change-Id: I260b7826eb1f55764a8a93b3402888afa5032eed Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5538411 Reviewed-by: Dirk Pranke Auto-Submit: Glen Robertson Commit-Queue: Glen Robertson Cr-Commit-Position: refs/heads/main@{#1301700} --- PRESUBMIT.py | 10 +++++++--- PRESUBMIT_test.py | 5 ++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/PRESUBMIT.py b/PRESUBMIT.py index db18cfe517db99..545de9d0b47e80 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -3548,8 +3548,9 @@ def CheckForAnonymousVariables(input_api, output_api): def CheckUniquePtrOnUpload(input_api, output_api): # Returns whether |template_str| is of the form for some types T - # and U. Assumes that |template_str| is already in the form <...>. - def HasMoreThanOneArg(template_str): + # and U, or is invalid due to mismatched angle bracket pairs. Assumes that + # |template_str| is already in the form <...>. + def HasMoreThanOneArgOrInvalid(template_str): # Level of <...> nesting. nesting = 0 for c in template_str: @@ -3559,6 +3560,9 @@ def HasMoreThanOneArg(template_str): nesting -= 1 elif c == ',' and nesting == 1: return True + if nesting != 0: + # Invalid. + return True return False file_inclusion_pattern = [r'.+%s' % _IMPLEMENTATION_EXTENSIONS] @@ -3615,7 +3619,7 @@ def HasMoreThanOneArg(template_str): # bar = std::unique_ptr(foo); local_path = f.LocalPath() return_construct_result = return_construct_pattern.search(line) - if return_construct_result and not HasMoreThanOneArg( + if return_construct_result and not HasMoreThanOneArgOrInvalid( return_construct_result.group('template_arg')): problems_constructor.append( '%s:%d\n %s' % (local_path, line_number, line.strip())) diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py index f6f9573f6d2b0b..9c13acf5b4bf45 100755 --- a/PRESUBMIT_test.py +++ b/PRESUBMIT_test.py @@ -3164,6 +3164,9 @@ def testFalsePositives(self): ]), MockFile('dir/nested.cc', ['set>();']), MockFile('dir/nested2.cc', ['map>();']), + # Changed line is inside a multiline template block. + MockFile('dir/template.cc', [' std::unique_ptr>(']), + MockFile('dir/template2.cc', [' std::unique_ptr>()']), # Two-argument invocation of std::unique_ptr is exempt because there is # no equivalent using std::make_unique. @@ -3172,7 +3175,7 @@ def testFalsePositives(self): ] results = PRESUBMIT.CheckUniquePtrOnUpload(mock_input_api, MockOutputApi()) - self.assertEqual(0, len(results)) + self.assertEqual([], results) class CheckNoDirectIncludesHeadersWhichRedefineStrCat(unittest.TestCase): def testBlocksDirectIncludes(self):