diff options
author | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2022-09-29 16:16:15 +0200 |
---|---|---|
committer | Allan Sandfeld Jensen <allan.jensen@qt.io> | 2022-11-09 10:04:06 +0000 |
commit | a95a7417ad456115a1ef2da4bb8320531c0821f1 (patch) | |
tree | edcd59279e486d2fd4a8f88a7ed025bcf925c6e6 /chromium/styleguide | |
parent | 33fc33aa94d4add0878ec30dc818e34e1dd3cc2a (diff) | |
download | qtwebengine-chromium-a95a7417ad456115a1ef2da4bb8320531c0821f1.tar.gz |
BASELINE: Update Chromium to 106.0.5249.126
Change-Id: Ib0bb21c437a7d1686e21c33f2d329f2ac425b7ab
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/438936
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Diffstat (limited to 'chromium/styleguide')
-rw-r--r-- | chromium/styleguide/c++/c++-features.md | 132 | ||||
-rw-r--r-- | chromium/styleguide/c++/c++.md | 60 | ||||
-rw-r--r-- | chromium/styleguide/java/java.md | 5 | ||||
-rw-r--r-- | chromium/styleguide/python/python.md | 31 | ||||
-rw-r--r-- | chromium/styleguide/web/OWNERS | 2 |
5 files changed, 172 insertions, 58 deletions
diff --git a/chromium/styleguide/c++/c++-features.md b/chromium/styleguide/c++/c++-features.md index d546b426163..a6140a247b1 100644 --- a/chromium/styleguide/c++/c++-features.md +++ b/chromium/styleguide/c++/c++-features.md @@ -38,6 +38,7 @@ The current status of existing standards and Abseil features is: features below * absl::StatusOr: Initially supported September 3, 2020 * absl::Cleanup: Initially supported February 4, 2021 + * absl::AnyInvocable: Initially supported June 20, 2022 [TOC] @@ -423,6 +424,26 @@ Usage is governed by the [Google Style Guide](https://google.github.io/styleguide/cppguide.html#CTAD). *** +### Fold expressions <sup>[allowed]</sup> + +```c++ +template <typename... Args> +auto sum(Args... args) { + return (... + args); +} +``` + +**Description:** A fold expression performs a fold of a template parameter pack +over a binary operator. + +**Documentation:** +[Fold expression](https://en.cppreference.com/w/cpp/language/fold) + +**Notes:** +*** promo +[Discussion thread](https://groups.google.com/a/chromium.org/g/cxx/c/4DTm3idXz0w/m/g_JjOh0wAgAJ) +*** + ### Selection statements with initializer <sup>[allowed]</sup> ```c++ @@ -972,26 +993,6 @@ deduced from the types of its arguments. None *** -### Fold expressions <sup>[tbd]</sup> - -```c++ -template <typename... Args> -auto sum(Args... args) { - return (... + args); -} -``` - -**Description:** A fold expression performs a fold of a template parameter pack -over a binary operator. - -**Documentation:** -[Fold expression](https://en.cppreference.com/w/cpp/language/fold) - -**Notes:** -*** promo -None -*** - ### constexpr lambda <sup>[tbd]</sup> ```c++ @@ -1622,6 +1623,24 @@ None The following Abseil library features are allowed in the Chromium codebase. +### Attribute macros <sup>[allowed]</sup> + +```c++ +const char* name() ABSL_ATTRIBUTE_RETURNS_NONNULL { return "hello world"; } +``` + +**Description:** Macros that conditionally resolve to attributes. Prefer to use +standard C++ attributes, such as `[[fallthrough]]`. Use these macros for +non-standard attributes, which may not be present in all compilers. + +**Documentation:** +[attributes.h](https://source.chromium.org/chromium/chromium/src/+/main:third_party/abseil-cpp/absl/base/attributes.h) + +**Notes:** +*** promo +[Discussion thread](https://groups.google.com/a/chromium.org/g/cxx/c/vQmaBfbyBGM/m/HHOYUZ5YAwAJ) +*** + ### 128bit integer <sup>[allowed]</sup> ```c++ @@ -1790,12 +1809,68 @@ Banned due to only working with 8-bit characters. Keep using `base::StringPiece` from `base/strings/`. *** +### FunctionRef <sup>[banned]</sup> + +```c++ +absl::FunctionRef +``` + +**Description:** Type for holding a non-owning reference to an object of any +invocable type. + +**Documentation:** +[function_ref.h](https://source.chromium.org/chromium/chromium/src/+/main:third_party/abseil-cpp/absl/functional/function_ref.h) + +**Notes:** +*** promo +- `absl::FunctionRef` is banned due to allowing implicit conversions between + function signatures in potentially surprising ways. For example, a callable + with the signature `int()` will bind to `absl::FunctionRef<void()>`: the + return value from the callable will be silently discarded. +- In Chromium, use `base::FunctionRef` instead. +- Unlike `base::OnceCallback` and `base::RepeatingCallback`, `base::FunctionRef` + supports capturing lambdas. +- Useful when passing an invocable object to a function that synchronously calls + the invocable object, e.g. `ForEachFrame(base::FunctionRef<void(Frame&)>)`. + This can often result in clearer code than code that is templated to accept + lambdas, e.g. with `template <typename Invocable> void + ForEachFrame(Invocable invocable)`, it is much less obvious what arguments + will be passed to `invocable`. +- For now, `base::OnceCallback` and `base::RepeatingCallback` intentionally + disallow conversions to `base::FunctionRef`, under the theory that the + callback should be a capturing lambda instead. Attempting to use this + conversion will trigger a `static_assert` requesting additional feedback for + use cases where this conversion would be valuable. +- *Important:* `base::FunctionRef` must not outlive the function call. Like + `base::StringPiece`, `base::FunctionRef` is a *non-owning* reference. Using a + `base::FunctionRef` as a return value or class field is dangerous and likely + to result in lifetime bugs. +- [Discussion thread](https://groups.google.com/a/chromium.org/g/cxx/c/JVN4E4IIYA0/m/V0EVUVLiBwAJ) +*** + ## Abseil TBD Features {#absl-review} The following Abseil library features are not allowed in the Chromium codebase. See the top of this page on how to propose moving a feature from this list into the allowed or banned sections. +### AnyInvocable <sup>[tbd]</sup> + +```c++ +absl::AnyInvocable +``` + +**Description:** An equivalent of the C++23 std::move_only_function. + +**Documentation:** +* [any_invocable.h](https://source.chromium.org/chromium/chromium/src/+/main:third_party/abseil-cpp/absl/functional/any_invocable.h) +* [std::move_only_function](https://en.cppreference.com/w/cpp/utility/functional/move_only_function/move_only_function) + +**Notes:** +*** promo +Overlaps with `base::RepeatingCallback`, `base::OnceCallback`. +*** + ### bind_front <sup>[tbd]</sup> ```c++ @@ -1875,23 +1950,6 @@ standard library. Overlaps with `base/ranges/algorithm.h`. *** -### FunctionRef <sup>[tbd]</sup> - -```c++ -absl::FunctionRef -``` - -**Description:** Type for holding a non-owning reference to an object of any -invocable type. - -**Documentation:** -[function_ref.h](https://source.chromium.org/chromium/chromium/src/+/main:third_party/abseil-cpp/absl/functional/function_ref.h) - -**Notes:** -*** promo -None -*** - ### Random <sup>[tbd]</sup> ```c++ diff --git a/chromium/styleguide/c++/c++.md b/chromium/styleguide/c++/c++.md index 93662bb0b0b..284ab800dec 100644 --- a/chromium/styleguide/c++/c++.md +++ b/chromium/styleguide/c++/c++.md @@ -36,7 +36,7 @@ status of Chromium's C++ support is covered in more detail in * Functions used only for testing should be restricted to test-only usages with the testing suffixes supported by - [PRESUMBIT.py](https://chromium.googlesource.com/chromium/src/+/main/PRESUBMIT.py). + [PRESUBMIT.py](https://chromium.googlesource.com/chromium/src/+/main/PRESUBMIT.py). `ForTesting` is the conventional suffix although similar patterns, such as `ForTest`, are also accepted. These suffixes are checked at presubmit time to ensure the functions are called only by test files. @@ -228,18 +228,24 @@ code when you find it, or at least not make such usage any more widespread. ## Non-owning pointers in class fields -Use `raw_ptr<T>` for class and struct fields in place of a raw C++ pointer `T*` -whenever possible, except in paths that include `/renderer/` or -`blink/public/web/`. `raw_ptr<T>` is a non-owning smart pointer that has -improved memory-safety over raw pointers, and can prevent exploitation of a -significant percentage of Use-after-Free bugs. - -Using `raw_ptr<T>` may not be possible in rare cases for -[performance reasons](../../base/memory/raw_ptr.md#Performance). -Additionally, `raw_ptr<T>` doesn’t support some C++ scenarios (e.g. `constexpr`, -ObjC pointers). Tooling will help to encourage use of `raw_ptr<T>`. See -[raw_ptr.md](../../base/memory/raw_ptr.md#When-to-use-raw_ptr_T) -for how to add exclusions. +Use `const raw_ref<T>` or `raw_ptr<T>` for class and struct fields in place of a +raw C++ reference `T&` or pointer `T*` whenever possible, except in paths that include +`/renderer/` or `blink/public/web/`. These a non-owning smart pointers that +have improved memory-safety over raw pointers and references, and can prevent +exploitation of a significant percentage of Use-after-Free bugs. + +Prefer `const raw_ref<T>` whenever the held pointer will never be null, and it's +ok to drop the `const` if the internal reference can be reassigned to point to a +different `T`. Use `raw_ptr<T>` in order to express that the pointer _can_ be +null. Only `raw_ptr<T>` can be default-constructed, since `raw_ref<T>` disallows +nullness. + +Using `raw_ref<T>` or `raw_ptr<T>` may not be possible in rare cases for +[performance reasons](../../base/memory/raw_ptr.md#Performance). Additionally, +`raw_ptr<T>` doesn’t support some C++ scenarios (e.g. `constexpr`, ObjC +pointers). Tooling will help to encourage use of these types in the future. See +[raw_ptr.md](../../base/memory/raw_ptr.md#When-to-use-raw_ptr_T) for how to add +exclusions. ## Forward declarations vs. #includes @@ -259,7 +265,7 @@ All files in Chromium start with a common license header. That header should look like this: ```c++ -// Copyright $YEAR The Chromium Authors. All rights reserved. +// Copyright $YEAR The Chromium Authors. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. ``` @@ -268,8 +274,8 @@ Some important notes about this header: * There is no `(c)` after `Copyright`. * `$YEAR` should be set to the current year at the time a file is created, and not changed thereafter. - * For files specific to Chromium OS, replace the word Chromium with the phrase - Chromium OS. + * For files specific to ChromiumOS, replace the word Chromium with the phrase + ChromiumOS. * If the style changes, don't bother to update existing files to comply with the new style. For the same reason, don't just blindly copy an existing file's header when creating a new file, since the existing file may use an @@ -337,6 +343,28 @@ these: so as early as possible can help with debugging, though our styleguide now recommends using a reference instead of a pointer when it cannot be null. +## Test-only code paths in production code + +Try to avoid test-only code paths in production code. Such code paths make +production code behave differently in tests. This makes both tests and +production code hard to reason about. Consider dependency injection, fake +classes, etc to avoid such code paths. + +However, if a test-only path in production code cannot be avoided, instrument +that code path with `CHECK_IS_TEST();` to assert that the code is only run in +tests. + +```c++ +// `profile_manager` may not be available in tests. +if (!profile_manager) { + CHECK_IS_TEST(); + return std::string(); +} +``` + +`CHECK_IS_TEST();` will crash outside of tests. This asserts that the test-only +code path is not accidentally or maliciously taken in production. + ## Miscellany * Use UTF-8 file encodings and LF line endings. diff --git a/chromium/styleguide/java/java.md b/chromium/styleguide/java/java.md index 1cabacba595..4d529e5888b 100644 --- a/chromium/styleguide/java/java.md +++ b/chromium/styleguide/java/java.md @@ -256,6 +256,11 @@ with the testing suffixes supported [PRESUMBIT.py](https://chromium.googlesource `ForTest`, are also accepted. These suffixes are checked at presubmit time to ensure the functions are called only by test files. +It's generally bad practice to directly call test-only methods from +non-test-only code. However, occasionally it has to be done, and if so, you +should guard the check with an `if (BuildConfig.IS_FOR_TEST)` so that our Java +optimizer can still remove the call in non-test builds. + ## Location "Top level directories" are defined as directories with a GN file, such as [//base](https://chromium.googlesource.com/chromium/src/+/main/base/) diff --git a/chromium/styleguide/python/python.md b/chromium/styleguide/python/python.md index 6f8bcd081de..b06c88088d3 100644 --- a/chromium/styleguide/python/python.md +++ b/chromium/styleguide/python/python.md @@ -14,9 +14,11 @@ will use), but we are increasingly seeing people running 3.9 locally as well. We (often) use a tool called [vpython] to manage Python packages; vpython is a wrapper around virtualenv. However, it is not safe to use vpython regardless of context, as it can have performance issues. All tests are -run under vpython, so it is safe there, but you should not use vpython -during PRESUBMIT checks, gclient runhooks, or during the build unless -a [//build/OWNER](../../build/OWNERS) has told you that it is okay to do so. +run under vpython, so it is safe there, and vpython is the default for +running scripts during PRESUBMIT checks (input_api.python3_executable points to +vpython3 and is used in GetPythonUnitTests), but you should not use vpython +during gclient runhooks, or during the build unless a +[//build/OWNER](../../build/OWNERS) has told you that it is okay to do so. Also, there is some performance overhead to using vpython, so prefer not to use vpython unless you need it (to pick up packages not available in the @@ -26,7 +28,9 @@ source tree). aren't as useful as you might think in Chromium, because most of our python invocations come from other tools like Ninja or the swarming infrastructure, and they also don't work on Windows. -So, don't expect them to help you. +So, don't expect them to help you. That said, a python 3 shebang is one way to +indicate to the presubmit system that test scripts should be run under Python 3 +rather than Python 2. However, if your script is executable, you should still use one, and for Python you should use `#!/usr/bin/env python3` or `#!/usr/bin/env vpython3` @@ -66,6 +70,25 @@ can request review for a change to this file. If there's no consensus, [`//styleguide/python/OWNERS`](https://chromium.googlesource.com/chromium/src/+/main/styleguide/python/OWNERS) get to decide. +## Portability + +There are a couple of differences in how text files are handled on Windows that +can lead to portability problems. These differences are: + +* The default encoding when reading/writing text files is cp1252 on Windows and +utf-8 on Linux, which can lead to Windows-only test failures. These can be +avoided by always specifying `encoding='utf-8'` when opening text files. + +* The default behavior when writing text files on Windows is to emit \r\n +(carriage return line feed) line endings. This can lead to cryptic Windows-only +test failures and is generally undesirable. This can be avoided by always +specifying `newline=''` when opening text files for writing. + +That is, use these forms when opening text files in Python: + +* reading: with open(filename, 'r', encoding='utf-8') as f: +* writing: with open(filename, 'w', encoding='utf-8', newline='') as f: + ## Tools ### pylint diff --git a/chromium/styleguide/web/OWNERS b/chromium/styleguide/web/OWNERS index d7e4f49972e..90531dbdfd1 100644 --- a/chromium/styleguide/web/OWNERS +++ b/chromium/styleguide/web/OWNERS @@ -1,3 +1,3 @@ # Interested in helping maintain this? Let us know! dpapad@chromium.org -michaelpg@chromium.org +rbpotter@chromium.org |