Fix presubmit ban on std::ranges::views
This is tricky, because std::ranges::view is allowed but is a prefix of std::ranges::views, which was causing std::ranges::views to also be treated as allowed. Change-Id: I632bb34757ee5776efbc1fe03a9d3e2d72cc084f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6507488 Reviewed-by: Nico Weber <thakis@chromium.org> Commit-Queue: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/main@{#1456412}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
5faf0899a8
commit
70a3527bf7
@ -1236,7 +1236,7 @@ _BANNED_CPP_FUNCTIONS: Sequence[BanRule] = (
|
|||||||
),
|
),
|
||||||
BanRule(
|
BanRule(
|
||||||
# Ban everything except specifically allowlisted constructs.
|
# Ban everything except specifically allowlisted constructs.
|
||||||
pattern=r'/std::ranges::(?!' + '|'.join((
|
pattern=r'/std::ranges::(?!(?:' + '|'.join((
|
||||||
# From https://en.cppreference.com/w/cpp/ranges:
|
# From https://en.cppreference.com/w/cpp/ranges:
|
||||||
# Range access
|
# Range access
|
||||||
'begin',
|
'begin',
|
||||||
@ -1419,7 +1419,11 @@ _BANNED_CPP_FUNCTIONS: Sequence[BanRule] = (
|
|||||||
'distance',
|
'distance',
|
||||||
'next',
|
'next',
|
||||||
'prev',
|
'prev',
|
||||||
)) + r')\w+',
|
# Require a word boundary at the end of negative lookahead
|
||||||
|
# assertion, e.g. to ensure that even though `view` is allowed (and
|
||||||
|
# should not match this regex), `views` is still treated as
|
||||||
|
# disallowed (and matches the regex).
|
||||||
|
)) + r')\b)\w+',
|
||||||
explanation=(
|
explanation=(
|
||||||
'Use of range views and associated helpers is banned in Chrome. '
|
'Use of range views and associated helpers is banned in Chrome. '
|
||||||
'If you need this functionality, please contact cxx@chromium.org.',
|
'If you need this functionality, please contact cxx@chromium.org.',
|
||||||
|
@ -3029,23 +3029,35 @@ class BannedTypeCheckTest(unittest.TestCase):
|
|||||||
MockFile('some/cpp/problematic/file4.cc', [
|
MockFile('some/cpp/problematic/file4.cc', [
|
||||||
'Browser* browser = chrome::FindBrowserWithTab(web_contents)'
|
'Browser* browser = chrome::FindBrowserWithTab(web_contents)'
|
||||||
]),
|
]),
|
||||||
MockFile('allowed_ranges_usage.cc', ['std::ranges::begin(vec)']),
|
MockFile(
|
||||||
MockFile('banned_ranges_usage.cc',
|
'allowed_ranges_usage.cc',
|
||||||
['std::ranges::subrange(first, last)']),
|
[
|
||||||
|
'std::ranges::begin(vec);',
|
||||||
|
# std::ranges::view is a concept and allowed, but the views
|
||||||
|
# library itself is not (see below)
|
||||||
|
'static_assert(std::ranges::view<SomeType>);'
|
||||||
|
]),
|
||||||
|
MockFile(
|
||||||
|
'banned_ranges_usage.cc',
|
||||||
|
[
|
||||||
|
'std::ranges::subrange(first, last);',
|
||||||
|
# Edge case: make sure std::ranges::views is disallowed,
|
||||||
|
# even though std::ranges::view is allowed.
|
||||||
|
'std::ranges::views::take(first, count);'
|
||||||
|
]),
|
||||||
MockFile('views_usage.cc', ['std::views::all(vec)']),
|
MockFile('views_usage.cc', ['std::views::all(vec)']),
|
||||||
MockFile('content/desktop_android.cc',
|
MockFile('content/desktop_android.cc', [
|
||||||
[
|
'// some first line',
|
||||||
'// some first line',
|
'#if BUILDFLAG(IS_DESKTOP_ANDROID)',
|
||||||
'#if BUILDFLAG(IS_DESKTOP_ANDROID)',
|
'// some third line',
|
||||||
'// some third line',
|
]),
|
||||||
]),
|
|
||||||
]
|
]
|
||||||
|
|
||||||
results = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())
|
results = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())
|
||||||
|
|
||||||
# Each entry in results corresponds to a BanRule with a violation, in
|
# Each entry in results corresponds to a BanRule with a violation, in
|
||||||
# the order they were encountered.
|
# the order they were encountered.
|
||||||
self.assertEqual(8, len(results))
|
self.assertEqual(9, len(results))
|
||||||
self.assertTrue('some/cpp/problematic/file.cc' in results[0].message)
|
self.assertTrue('some/cpp/problematic/file.cc' in results[0].message)
|
||||||
self.assertTrue(
|
self.assertTrue(
|
||||||
'third_party/blink/problematic/file.cc' in results[1].message)
|
'third_party/blink/problematic/file.cc' in results[1].message)
|
||||||
@ -3057,13 +3069,15 @@ class BannedTypeCheckTest(unittest.TestCase):
|
|||||||
self.assertTrue(all('some/cpp/comment/file.cc' not in r.message for r in results))
|
self.assertTrue(all('some/cpp/comment/file.cc' not in r.message for r in results))
|
||||||
self.assertTrue(all('allowed_ranges_usage.cc' not in r.message for r in results))
|
self.assertTrue(all('allowed_ranges_usage.cc' not in r.message for r in results))
|
||||||
self.assertTrue('banned_ranges_usage.cc' in results[5].message)
|
self.assertTrue('banned_ranges_usage.cc' in results[5].message)
|
||||||
self.assertTrue('views_usage.cc' in results[6].message)
|
self.assertTrue('banned_ranges_usage.cc' in results[6].message)
|
||||||
self.assertTrue('content/desktop_android.cc' in results[7].message)
|
self.assertTrue('views_usage.cc' in results[7].message)
|
||||||
|
self.assertTrue('content/desktop_android.cc' in results[8].message)
|
||||||
|
|
||||||
# Check ResultLocation data. Line nums start at 1.
|
# Check ResultLocation data. Line nums start at 1.
|
||||||
self.assertEqual(results[7].locations[0].file_path, 'content/desktop_android.cc')
|
self.assertEqual(results[8].locations[0].file_path,
|
||||||
self.assertEqual(results[7].locations[0].start_line, 2)
|
'content/desktop_android.cc')
|
||||||
self.assertEqual(results[7].locations[0].end_line, 2)
|
self.assertEqual(results[8].locations[0].start_line, 2)
|
||||||
|
self.assertEqual(results[8].locations[0].end_line, 2)
|
||||||
|
|
||||||
def testBannedCppRandomFunctions(self):
|
def testBannedCppRandomFunctions(self):
|
||||||
banned_rngs = [
|
banned_rngs = [
|
||||||
|
Reference in New Issue
Block a user