Allow most of <ranges> but explicitly ban views
Change-Id: I67befc6d5cc18ab3ce43e6e3a1ad68f8dba878fd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5668999 Commit-Queue: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Cr-Commit-Position: refs/heads/main@{#1323174}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
e437f89711
commit
8971922e9c
195
PRESUBMIT.py
195
PRESUBMIT.py
@ -1301,11 +1301,196 @@ _BANNED_CPP_FUNCTIONS: Sequence[BanRule] = (
|
||||
[_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
|
||||
),
|
||||
BanRule(
|
||||
r'/#include <ranges>',
|
||||
('<ranges> is not yet allowed. Use base/ranges/algorithm.h instead.',
|
||||
),
|
||||
True,
|
||||
[_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
|
||||
pattern='std::views',
|
||||
explanation=(
|
||||
'Use of std::views is banned in Chrome. If you need this '
|
||||
'functionality, please contact cxx@chromium.org.',
|
||||
),
|
||||
treat_as_error=True,
|
||||
excluded_paths=[
|
||||
# Don't warn in third_party folders.
|
||||
_THIRD_PARTY_EXCEPT_BLINK
|
||||
],
|
||||
),
|
||||
BanRule(
|
||||
# Ban everything except specifically allowlisted constructs.
|
||||
pattern=r'/std::ranges::(?!' + '|'.join((
|
||||
# From https://en.cppreference.com/w/cpp/ranges:
|
||||
# Range access
|
||||
'begin',
|
||||
'end',
|
||||
'cbegin',
|
||||
'cend',
|
||||
'rbegin',
|
||||
'rend',
|
||||
'crbegin',
|
||||
'crend',
|
||||
'size',
|
||||
'ssize',
|
||||
'empty',
|
||||
'data',
|
||||
'cdata',
|
||||
# Range primitives
|
||||
'iterator_t',
|
||||
'const_iterator_t',
|
||||
'sentinel_t',
|
||||
'const_sentinel_t',
|
||||
'range_difference_t',
|
||||
'range_size_t',
|
||||
'range_value_t',
|
||||
'range_reference_t',
|
||||
'range_const_reference_t',
|
||||
'range_rvalue_reference_t',
|
||||
'range_common_reference_t',
|
||||
# Dangling iterator handling
|
||||
'dangling',
|
||||
'borrowed_iterator_t',
|
||||
# Banned: borrowed_subrange_t
|
||||
# Range concepts
|
||||
'range',
|
||||
'borrowed_range',
|
||||
'sized_range',
|
||||
'view',
|
||||
'input_range',
|
||||
'output_range',
|
||||
'forward_range',
|
||||
'bidirectional_range',
|
||||
'random_access_range',
|
||||
'contiguous_range',
|
||||
'common_range',
|
||||
'viewable_range',
|
||||
'constant_range',
|
||||
# Banned: Views
|
||||
# Banned: Range factories
|
||||
# Banned: Range adaptors
|
||||
# From https://en.cppreference.com/w/cpp/algorithm/ranges:
|
||||
# Constrained algorithms: non-modifying sequence operations
|
||||
'all_of',
|
||||
'any_of',
|
||||
'none_of',
|
||||
'for_each',
|
||||
'for_each_n',
|
||||
'count',
|
||||
'count_if',
|
||||
'mismatch',
|
||||
'equal',
|
||||
'lexicographical_compare',
|
||||
'find',
|
||||
'find_if',
|
||||
'find_if_not',
|
||||
'find_end',
|
||||
'find_first_of',
|
||||
'adjacent_find',
|
||||
'search',
|
||||
'search_n',
|
||||
# Constrained algorithms: modifying sequence operations
|
||||
'copy',
|
||||
'copy_if',
|
||||
'copy_n',
|
||||
'copy_backward',
|
||||
'move',
|
||||
'move_backward',
|
||||
'fill',
|
||||
'fill_n',
|
||||
'transform',
|
||||
'generate',
|
||||
'generate_n',
|
||||
'remove',
|
||||
'remove_if',
|
||||
'remove_copy',
|
||||
'remove_copy_if',
|
||||
'replace',
|
||||
'replace_if',
|
||||
'replace_copy',
|
||||
'replace_copy_if',
|
||||
'swap_ranges',
|
||||
'reverse',
|
||||
'reverse_copy',
|
||||
'rotate',
|
||||
'rotate_copy',
|
||||
'shuffle',
|
||||
'sample',
|
||||
'unique',
|
||||
'unique_copy',
|
||||
# Constrained algorithms: partitioning operations
|
||||
'is_partitioned',
|
||||
'partition',
|
||||
'partition_copy',
|
||||
'stable_partition',
|
||||
'partition_point',
|
||||
# Constrained algorithms: sorting operations
|
||||
'is_sorted',
|
||||
'is_sorted_until',
|
||||
'sort',
|
||||
'partial_sort',
|
||||
'partial_sort_copy',
|
||||
'stable_sort',
|
||||
'nth_element',
|
||||
# Constrained algorithms: binary search operations (on sorted ranges)
|
||||
'lower_bound',
|
||||
'upper_bound',
|
||||
'binary_search',
|
||||
'equal_range',
|
||||
# Constrained algorithms: set operations (on sorted ranges)
|
||||
'merge',
|
||||
'inplace_merge',
|
||||
'includes',
|
||||
'set_difference',
|
||||
'set_intersection',
|
||||
'set_symmetric_difference',
|
||||
'set_union',
|
||||
# Constrained algorithms: heap operations
|
||||
'is_heap',
|
||||
'is_heap_until',
|
||||
'make_heap',
|
||||
'push_heap',
|
||||
'pop_heap',
|
||||
'sort_heap',
|
||||
# Constrained algorithms: minimum/maximum operations
|
||||
'max',
|
||||
'max_element',
|
||||
'min',
|
||||
'min_element',
|
||||
'minmax',
|
||||
'minmax_element',
|
||||
'clamp',
|
||||
# Constrained algorithms: permutation operations
|
||||
'is_permutation',
|
||||
'next_permutation',
|
||||
'prev_premutation',
|
||||
# Constrained uninitialized memory algorithms
|
||||
'uninitialized_copy',
|
||||
'uninitialized_copy_n',
|
||||
'uninitialized_fill',
|
||||
'uninitialized_fill_n',
|
||||
'uninitialized_move',
|
||||
'uninitialized_move_n',
|
||||
'uninitialized_default_construct',
|
||||
'uninitialized_default_construct_n',
|
||||
'uninitialized_value_construct',
|
||||
'uninitialized_value_construct_n',
|
||||
'destroy',
|
||||
'destroy_n',
|
||||
'destroy_at',
|
||||
'construct_at',
|
||||
# Return types
|
||||
'in_fun_result',
|
||||
'in_in_result',
|
||||
'in_out_result',
|
||||
'in_in_out_result',
|
||||
'in_out_out_result',
|
||||
'min_max_result',
|
||||
'in_found_result',
|
||||
)) + r')\w+',
|
||||
explanation=(
|
||||
'Use of range views and associated helpers is banned in Chrome. '
|
||||
'If you need this functionality, please contact cxx@chromium.org.',
|
||||
),
|
||||
treat_as_error=True,
|
||||
excluded_paths=[
|
||||
# Don't warn in third_party folders.
|
||||
_THIRD_PARTY_EXCEPT_BLINK
|
||||
],
|
||||
),
|
||||
BanRule(
|
||||
r'/#include <source_location>',
|
||||
|
@ -3134,6 +3134,10 @@ class BannedTypeCheckTest(unittest.TestCase):
|
||||
MockFile('some/cpp/problematic/file5.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('views_usage.cc', ['std::views::all(vec)']),
|
||||
]
|
||||
|
||||
results = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())
|
||||
@ -3152,6 +3156,10 @@ class BannedTypeCheckTest(unittest.TestCase):
|
||||
self.assertFalse('some/cpp/nocheck/file.cc' in results[1].message)
|
||||
self.assertFalse('some/cpp/comment/file.cc' in results[0].message)
|
||||
self.assertFalse('some/cpp/comment/file.cc' in results[1].message)
|
||||
self.assertFalse('allowed_ranges_usage.cc' in results[0].message)
|
||||
self.assertFalse('allowed_ranges_usage.cc' in results[1].message)
|
||||
self.assertTrue('banned_ranges_usage.cc' in results[1].message)
|
||||
self.assertTrue('views_usage.cc' in results[1].message)
|
||||
|
||||
def testBannedCppRandomFunctions(self):
|
||||
banned_rngs = [
|
||||
|
@ -1065,6 +1065,54 @@ machinery in `<type_traits>`.
|
||||
None
|
||||
***
|
||||
|
||||
### Range algorithms <sup>[allowed]</sup>
|
||||
|
||||
```c++
|
||||
constexpr int kArr[] = {2, 4, 6, 8, 10, 12};
|
||||
constexpr auto is_even = [] (auto x) { return x % 2 == 0; };
|
||||
static_assert(std::ranges::all_of(kArr, is_even));
|
||||
```
|
||||
|
||||
**Description:** Provides versions of most algorithms that accept either an
|
||||
iterator-sentinel pair or a single range argument.
|
||||
|
||||
**Documentation:**
|
||||
[Ranges algorithms](https://en.cppreference.com/w/cpp/algorithm/ranges)
|
||||
|
||||
**Notes:**
|
||||
*** promo
|
||||
Supersedes `//base`'s backports in `//base/ranges/algorithm.h`.
|
||||
|
||||
[Discussion thread](https://groups.google.com/a/chromium.org/g/cxx/c/ZnIbkfJ0Glw)
|
||||
***
|
||||
|
||||
### Range access, range primitives, dangling iterator handling, and range concepts <sup>[allowed]</sup>
|
||||
|
||||
```c++
|
||||
// Range access:
|
||||
constexpr int kArr[] = {2, 4, 6, 8, 10, 12};
|
||||
static_assert(std::ranges::size(kArr) == 6);
|
||||
|
||||
// Range primitives:
|
||||
static_assert(
|
||||
std::same_as<std::ranges::iterator_t<decltype(kArr)>, const int*>);
|
||||
|
||||
// Range concepts:
|
||||
static_assert(std::ranges::contiguous_range<decltype(kArr)>);
|
||||
```
|
||||
|
||||
**Description:** Various helper functions and types for working with ranges.
|
||||
|
||||
**Documentation:**
|
||||
[Ranges library](https://en.cppreference.com/w/cpp/ranges)
|
||||
|
||||
**Notes:**
|
||||
*** promo
|
||||
Supersedes `//base`'s backports in `//base//ranges/ranges.h`.
|
||||
|
||||
[Discussion thread](https://groups.google.com/a/chromium.org/g/cxx/c/ZnIbkfJ0Glw)
|
||||
***
|
||||
|
||||
### Library feature-test macros and <version> <sup>[allowed]</sup>
|
||||
|
||||
```c++
|
||||
@ -1449,6 +1497,34 @@ encoded using the current C locale.
|
||||
Chromium functionality should not vary with the C locale.
|
||||
***
|
||||
|
||||
### Views, range factories, and range adaptors <sup>[banned]</sup>
|
||||
|
||||
```c++
|
||||
constexpr int kArr[] = {6, 2, 8, 4, 4, 2};
|
||||
constexpr auto plus_one = std::views::transform([](int n){ return n + 1; });
|
||||
static_assert(std::ranges::equal(kArr | plus_one, {7, 3, 9, 5, 5, 3}));
|
||||
|
||||
// Prints 1, 2, 3, 4, 5, 6.
|
||||
for (auto i : std::ranges::iota_view(1, 7)) {
|
||||
std::cout << i << '\n';
|
||||
}
|
||||
```
|
||||
|
||||
**Description:** Lightweight objects that represent iterable sequences.
|
||||
Provides facilities for lazy operations on ranges, along with composition into
|
||||
pipelines.
|
||||
|
||||
**Documentation:**
|
||||
[Ranges library](https://en.cppreference.com/w/cpp/ranges)
|
||||
|
||||
**Notes:**
|
||||
*** promo
|
||||
Banned in Chrome due to questions about the design, impact on build time, and
|
||||
runtime performance.
|
||||
|
||||
[Discussion thread](https://groups.google.com/a/chromium.org/g/cxx/c/ZnIbkfJ0Glw)
|
||||
***
|
||||
|
||||
### std::to_address <sup>[banned]</sup>
|
||||
|
||||
```c++
|
||||
@ -1596,28 +1672,6 @@ Has both pros and cons compared to `absl::StrFormat` (which we don't yet use).
|
||||
Migration would be nontrivial.
|
||||
***
|
||||
|
||||
### <ranges> <sup>[tbd]</sup>
|
||||
|
||||
```c++
|
||||
constexpr int arr[] = {6, 2, 8, 4, 4, 2};
|
||||
constexpr auto plus_one = std::views::transform([](int n){ return n + 1; });
|
||||
static_assert(std::ranges::equal(arr | plus_one, {7, 3, 9, 5, 5, 3}));
|
||||
```
|
||||
|
||||
**Description:** Generalizes algorithms using range views, which are lightweight
|
||||
objects that represent iterable sequences. Provides facilities for eager and
|
||||
lazy operations on ranges, along with composition into pipelines.
|
||||
|
||||
**Documentation:**
|
||||
[Ranges library](https://en.cppreference.com/w/cpp/ranges)
|
||||
|
||||
**Notes:**
|
||||
*** promo
|
||||
Significant concerns expressed internally. We should consider whether there are
|
||||
clearly-safe pieces to allow (e.g. to replace `base/ranges/algorithm.h`) and
|
||||
engage with the internal library team.
|
||||
***
|
||||
|
||||
### <source_location> <sup>[tbd]</sup>
|
||||
|
||||
```c++
|
||||
|
Reference in New Issue
Block a user