From 70a3527bf72337506ea2ec251f74d241451799e2 Mon Sep 17 00:00:00 2001
From: Daniel Cheng <dcheng@chromium.org>
Date: Tue, 6 May 2025 09:41:34 -0700
Subject: [PATCH] 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}
---
 PRESUBMIT.py      |  8 ++++++--
 PRESUBMIT_test.py | 44 +++++++++++++++++++++++++++++---------------
 2 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index 35d3e0257ad82..574fe38aa0e3c 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -1236,7 +1236,7 @@ _BANNED_CPP_FUNCTIONS: Sequence[BanRule] = (
     ),
     BanRule(
         # Ban everything except specifically allowlisted constructs.
-        pattern=r'/std::ranges::(?!' + '|'.join((
+        pattern=r'/std::ranges::(?!(?:' + '|'.join((
             # From https://en.cppreference.com/w/cpp/ranges:
             # Range access
             'begin',
@@ -1419,7 +1419,11 @@ _BANNED_CPP_FUNCTIONS: Sequence[BanRule] = (
             'distance',
             'next',
             '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=(
             'Use of range views and associated helpers is banned in Chrome. '
             'If you need this functionality, please contact cxx@chromium.org.',
diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py
index e82fda59f6fde..a2ff946d49552 100755
--- a/PRESUBMIT_test.py
+++ b/PRESUBMIT_test.py
@@ -3029,23 +3029,35 @@ class BannedTypeCheckTest(unittest.TestCase):
             MockFile('some/cpp/problematic/file4.cc', [
                 'Browser* browser = chrome::FindBrowserWithTab(web_contents)'
             ]),
-            MockFile('allowed_ranges_usage.cc', ['std::ranges::begin(vec)']),
-            MockFile('banned_ranges_usage.cc',
-                     ['std::ranges::subrange(first, last)']),
+            MockFile(
+                'allowed_ranges_usage.cc',
+                [
+                    '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('content/desktop_android.cc',
-                     [
-                         '// some first line',
-                         '#if BUILDFLAG(IS_DESKTOP_ANDROID)',
-                         '// some third line',
-                     ]),
+            MockFile('content/desktop_android.cc', [
+                '// some first line',
+                '#if BUILDFLAG(IS_DESKTOP_ANDROID)',
+                '// some third line',
+            ]),
         ]
 
         results = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())
 
         # Each entry in results corresponds to a BanRule with a violation, in
         # 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(
             '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('allowed_ranges_usage.cc' not in r.message for r in results))
         self.assertTrue('banned_ranges_usage.cc' in results[5].message)
-        self.assertTrue('views_usage.cc' in results[6].message)
-        self.assertTrue('content/desktop_android.cc' in results[7].message)
+        self.assertTrue('banned_ranges_usage.cc' in results[6].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.
-        self.assertEqual(results[7].locations[0].file_path, 'content/desktop_android.cc')
-        self.assertEqual(results[7].locations[0].start_line, 2)
-        self.assertEqual(results[7].locations[0].end_line, 2)
+        self.assertEqual(results[8].locations[0].file_path,
+                         'content/desktop_android.cc')
+        self.assertEqual(results[8].locations[0].start_line, 2)
+        self.assertEqual(results[8].locations[0].end_line, 2)
 
     def testBannedCppRandomFunctions(self):
         banned_rngs = [