Skip to content

Commit

Permalink
Allow most of <ranges> but explicitly ban views
Browse files Browse the repository at this point in the history 
Change-Id: I67befc6d5cc18ab3ce43e6e3a1ad68f8dba878fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5668999
Commit-Queue: Daniel Cheng <[email protected]>
Reviewed-by: Peter Kasting <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1323174}
  • Loading branch information
@zetafunction
zetafunction authored and Chromium LUCI CQ committed Jul 4, 2024
1 parent e437f89 commit 8971922
Show file tree
Hide file tree
Showing 3 changed files with 274 additions and 27 deletions.
195 changes: 190 additions & 5 deletions PRESUBMIT.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1301,11 +1301,196 @@ class 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 [email protected].',
),
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 [email protected].',
),
treat_as_error=True,
excluded_paths=[
# Don't warn in third_party folders.
_THIRD_PARTY_EXCEPT_BLINK
],
),
BanRule(
r'/#include <source_location>',
Expand Down
8 changes: 8 additions & 0 deletions PRESUBMIT_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3134,6 +3134,10 @@ def testBannedCppFunctions(self):
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())
Expand All @@ -3152,6 +3156,10 @@ def testBannedCppFunctions(self):
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 = [
Expand Down
98 changes: 76 additions & 22 deletions styleguide/c++/c++-features.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 &lt;version&gt; <sup>[allowed]</sup>

```c++
Expand Down Expand Up @@ -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++
Expand Down Expand Up @@ -1596,28 +1672,6 @@ Has both pros and cons compared to `absl::StrFormat` (which we don't yet use).
Migration would be nontrivial.
***

### &lt;ranges&gt; <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.
***
### &lt;source_location&gt; <sup>[tbd]</sup>

```c++
Expand Down

0 comments on commit 8971922

Please sign in to comment.