brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 1 | # Chromium C++ style guide |
| 2 | |
Peter Kasting | 777e130 | 2020-06-02 21:44:40 | [diff] [blame] | 3 | _For other languages, please see the |
John Palmer | be05130 | 2021-05-19 11:48:35 | [diff] [blame] | 4 | [Chromium style guides](https://chromium.googlesource.com/chromium/src/+/main/styleguide/styleguide.md)._ |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 5 | |
| 6 | Chromium follows the [Google C++ Style |
| 7 | Guide](https://google.github.io/styleguide/cppguide.html) unless an exception |
| 8 | is listed below. |
| 9 | |
| 10 | A checkout should give you |
John Palmer | be05130 | 2021-05-19 11:48:35 | [diff] [blame] | 11 | [clang-format](https://chromium.googlesource.com/chromium/src/+/main/docs/clang_format.md) |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 12 | to automatically format C++ code. By policy, Clang's formatting of code should |
| 13 | always be accepted in code reviews. |
| 14 | |
brettw | 196290a | 2016-07-07 03:52:16 | [diff] [blame] | 15 | You can propose changes to this style guide by sending an email to |
| 16 | `[email protected]`. Ideally, the list will arrive at some consensus and you can |
| 17 | request review for a change to this file. If there's no consensus, |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 18 | `src/styleguide/c++/OWNERS` get to decide. |
| 19 | |
Tom McKee | bfea2678 | 2020-02-18 20:57:47 | [diff] [blame] | 20 | Blink code in `third_party/blink` uses [Blink style](blink-c++.md). |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 21 | |
Jeremy Roman | 1bebbea | 2019-06-20 19:17:14 | [diff] [blame] | 22 | ## Modern C++ features |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 23 | |
Peter Kasting | 1865f277 | 2021-12-23 21:23:58 | [diff] [blame] | 24 | Google and Chromium style |
Peter Kasting | 777e130 | 2020-06-02 21:44:40 | [diff] [blame] | 25 | [targets C++17](https://google.github.io/styleguide/cppguide.html#C++_Version). |
Peter Kasting | 1865f277 | 2021-12-23 21:23:58 | [diff] [blame] | 26 | Additionally, some features of supported C++ versions remain forbidden. The |
| 27 | status of Chromium's C++ support is covered in more detail in |
Avi Drissman | 0aafa9e | 2022-01-18 21:41:01 | [diff] [blame] | 28 | [Modern C++ use in Chromium](c++-features.md). |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 29 | |
| 30 | ## Naming |
| 31 | |
| 32 | * "Chromium" is the name of the project, not the product, and should never |
| 33 | appear in code, variable names, API names etc. Use "Chrome" instead. |
| 34 | |
Dominic Farolino | f3d0307 | 2021-07-20 15:55:54 | [diff] [blame] | 35 | ## Tests and Test-only Code |
Tommy C. Li | 1a13764a | 2018-09-17 21:18:34 | [diff] [blame] | 36 | |
| 37 | * Functions used only for testing should be restricted to test-only usages |
Peter Kasting | 777e130 | 2020-06-02 21:44:40 | [diff] [blame] | 38 | with the testing suffixes supported by |
Joseph Koshy | 0b38a32 | 2022-07-13 15:01:40 | [diff] [blame^] | 39 | [PRESUBMIT.py](https://chromium.googlesource.com/chromium/src/+/main/PRESUBMIT.py). |
Caitlin Fischer | 210cfab | 2020-05-07 20:04:30 | [diff] [blame] | 40 | `ForTesting` is the conventional suffix although similar patterns, such as |
| 41 | `ForTest`, are also accepted. These suffixes are checked at presubmit time |
| 42 | to ensure the functions are called only by test files. |
Roland Bock | 1f21a73 | 2021-05-20 16:59:53 | [diff] [blame] | 43 | * Classes used only for testing should be in a GN build target that is |
| 44 | marked `testonly=true`. Tests can depend on such targets, but production |
| 45 | code can not. |
Dominic Farolino | 200a14ec | 2021-07-20 16:42:40 | [diff] [blame] | 46 | * While test files generally appear alongside the production code they test, |
| 47 | support code for `testonly` targets should be placed in a `test/` subdirectory. |
| 48 | For example, see `//mojo/core/core_unittest.cc` and |
| 49 | `//mojo/core/test/mojo_test_base.cc`. For test classes used across multiple |
| 50 | directories, it might make sense to move them into a nested `test` namespace for |
| 51 | clarity. |
Dominic Farolino | f3d0307 | 2021-07-20 15:55:54 | [diff] [blame] | 52 | * Despite the Google C++ style guide |
| 53 | [deprecating](https://google.github.io/styleguide/cppguide.html#File_Names) |
| 54 | the `_unittest.cc` suffix for unit test files, in Chromium we still use this |
| 55 | suffix to distinguish unit tests from browser tests, which are written in |
| 56 | files with the `_browsertest.cc` suffix. |
Tommy C. Li | 1a13764a | 2018-09-17 21:18:34 | [diff] [blame] | 57 | |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 58 | ## Code formatting |
| 59 | |
| 60 | * Put `*` and `&` by the type rather than the variable name. |
danakj | cd463f1 | 2021-02-12 19:53:13 | [diff] [blame] | 61 | * In class declarations, group function overrides together within each access |
Peter Kasting | 5e04bd3 | 2019-05-21 21:44:10 | [diff] [blame] | 62 | control section, with one labeled group per parent class. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 63 | * Prefer `(foo == 0)` to `(0 == foo)`. |
| 64 | |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 65 | ## Unnamed namespaces |
| 66 | |
| 67 | Items local to a .cc file should be wrapped in an unnamed namespace. While some |
| 68 | such items are already file-scope by default in C++, not all are; also, shared |
| 69 | objects on Linux builds export all symbols, so unnamed namespaces (which |
| 70 | restrict these symbols to the compilation unit) improve function call cost and |
| 71 | reduce the size of entry point tables. |
| 72 | |
| 73 | ## Exporting symbols |
| 74 | |
Peter Kasting | 5e04bd3 | 2019-05-21 21:44:10 | [diff] [blame] | 75 | Symbols can be exported (made visible outside of a shared library/DLL) by |
| 76 | annotating with a `<COMPONENT>_EXPORT` macro name (where `<COMPONENT>` is the |
| 77 | name of the component being built, e.g. BASE, NET, CONTENT, etc.). Class |
| 78 | annotations should precede the class name: |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 79 | ```c++ |
| 80 | class FOO_EXPORT Foo { |
| 81 | void Bar(); |
| 82 | void Baz(); |
| 83 | // ... |
| 84 | }; |
| 85 | ``` |
| 86 | |
| 87 | Function annotations should precede the return type: |
| 88 | ```c++ |
| 89 | class FooSingleton { |
| 90 | FOO_EXPORT Foo& GetFoo(); |
Jeremy Roman | 31220288 | 2019-02-19 21:43:50 | [diff] [blame] | 91 | FOO_EXPORT Foo& SetFooForTesting(Foo* foo); |
Peter Kasting | 5e04bd3 | 2019-05-21 21:44:10 | [diff] [blame] | 92 | void SetFoo(Foo* foo); // Not exported. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 93 | }; |
| 94 | ``` |
| 95 | |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 96 | ## Multiple inheritance |
| 97 | |
| 98 | Multiple inheritance and virtual inheritance are permitted in Chromium code, |
| 99 | but discouraged (beyond the "interface" style of inheritance allowed by the |
| 100 | Google style guide, for which we do not require classes to have the "Interface" |
| 101 | suffix). Consider whether composition could solve the problem instead. |
| 102 | |
| 103 | ## Inline functions |
| 104 | |
| 105 | Simple accessors should generally be the only inline functions. These should be |
Peter Kasting | 5e04bd3 | 2019-05-21 21:44:10 | [diff] [blame] | 106 | named using `snake_case()`. Virtual functions should never be declared this way. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 107 | |
| 108 | ## Logging |
| 109 | |
| 110 | Remove most logging calls before checking in. Unless you're adding temporary |
| 111 | logging to track down a specific bug, and you have a plan for how to collect |
| 112 | the logged data from user machines, you should generally not add logging |
| 113 | statements. |
| 114 | |
| 115 | For the rare case when logging needs to stay in the codebase for a while, |
| 116 | prefer `DVLOG(1)` to other logging methods. This avoids bloating the release |
| 117 | executable and in debug can be selectively enabled at runtime by command-line |
| 118 | arguments: |
Peter Kasting | 5e04bd3 | 2019-05-21 21:44:10 | [diff] [blame] | 119 | * `--v=n` sets the global log level to n (default 0). All log statements with |
| 120 | a log level less than or equal to the global level will be printed. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 121 | * `--vmodule=mod=n[,mod=n,...]` overrides the global log level for the module |
| 122 | mod. Supplying the string foo for mod will affect all files named foo.cc, |
| 123 | while supplying a wildcard like `*bar/baz*` will affect all files with |
| 124 | `bar/baz` in their full pathnames. |
| 125 | |
| 126 | ## Platform-specific code |
| 127 | |
| 128 | To `#ifdef` code for specific platforms, use the macros defined in |
| 129 | `build/build_config.h` and in the Chromium build config files, not other macros |
Tommy C. Li | 1a13764a | 2018-09-17 21:18:34 | [diff] [blame] | 130 | set by specific compilers or build environments (e.g. `WIN32`). |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 131 | |
| 132 | Place platform-specific #includes in their own section below the "normal" |
| 133 | `#includes`. Repeat the standard `#include` order within this section: |
| 134 | |
| 135 | ```c++ |
| 136 | #include "foo/foo.h" |
| 137 | |
| 138 | #include <stdint.h> |
| 139 | #include <algorithm> |
| 140 | |
| 141 | #include "base/strings/utf_string_conversions.h" |
Xiaohan Wang | 0727d88 | 2022-01-21 03:03:43 | [diff] [blame] | 142 | #include "build/build_config.h" |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 143 | #include "chrome/common/render_messages.h" |
| 144 | |
Xiaohan Wang | 0727d88 | 2022-01-21 03:03:43 | [diff] [blame] | 145 | #if BUILDFLAG(IS_WIN) |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 146 | #include <windows.h> |
Robert Liao | c7c9a1c | 2017-10-18 01:41:31 | [diff] [blame] | 147 | #include "base/win/com_init_util.h" |
Xiaohan Wang | 0727d88 | 2022-01-21 03:03:43 | [diff] [blame] | 148 | #elif BUILDFLAG(IS_POSIX) |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 149 | #include "base/posix/global_descriptors.h" |
| 150 | #endif |
| 151 | ``` |
| 152 | |
| 153 | ## Types |
| 154 | |
Dominick Ng | 71a699a | 2022-01-26 23:46:57 | [diff] [blame] | 155 | * Refer to the [Mojo style |
| 156 | guide](https://chromium.googlesource.com/chromium/src/+/main/docs/security/mojo.md) |
| 157 | when working with types that will be passed across network or process |
| 158 | boundaries. For example, explicitly-sized integral types must be used for |
| 159 | safety, since the sending and receiving ends may not have been compiled |
| 160 | with the same sizes for things like `int` and `size_t`. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 161 | * Use `size_t` for object and allocation sizes, object counts, array and |
Peter Kasting | 5e04bd3 | 2019-05-21 21:44:10 | [diff] [blame] | 162 | pointer offsets, vector indices, and so on. This prevents casts when |
| 163 | dealing with STL APIs, and if followed consistently across the codebase, |
| 164 | minimizes casts elsewhere. |
| 165 | * Occasionally classes may have a good reason to use a type other than |
| 166 | `size_t` for one of these concepts, e.g. as a storage space optimization. In |
| 167 | these cases, continue to use `size_t` in public-facing function |
| 168 | declarations, and continue to use unsigned types internally (e.g. |
| 169 | `uint32_t`). |
Dominick Ng | 71a699a | 2022-01-26 23:46:57 | [diff] [blame] | 170 | * Follow the [integer semantics |
| 171 | guide](https://chromium.googlesource.com/chromium/src/+/main/docs/security/integer-semantics.md) |
| 172 | for all arithmetic conversions and calculations used in memory management |
| 173 | or passed across network or process boundaries. In other circumstances, |
| 174 | follow [Google C++ casting |
Dana Fried | 74a1889 | 2018-09-13 19:10:01 | [diff] [blame] | 175 | conventions](https://google.github.io/styleguide/cppguide.html#Casting) |
| 176 | to convert arithmetic types when you know the conversion is safe. Use |
Chris Palmer | 8218b857 | 2019-02-26 00:19:16 | [diff] [blame] | 177 | `checked_cast<T>` (from `base/numerics/safe_conversions.h`) when you need to |
| 178 | `CHECK` that the source value is in range for the destination type. Use |
| 179 | `saturated_cast<T>` if you instead wish to clamp out-of-range values. |
Peter Kasting | 5e04bd3 | 2019-05-21 21:44:10 | [diff] [blame] | 180 | `CheckedNumeric` is an ergonomic way to perform safe arithmetic and casting |
| 181 | in many cases. |
Peter Kasting | 9e210a52 | 2021-03-20 15:45:42 | [diff] [blame] | 182 | * The Google Style Guide [bans |
| 183 | UTF-16](https://google.github.io/styleguide/cppguide.html#Non-ASCII_Characters). |
| 184 | For various reasons, Chromium uses UTF-16 extensively. Use `std::u16string` |
| 185 | and `char16_t*` for 16-bit strings, `u"..."` to declare UTF-16 literals, and |
| 186 | either the actual characters or the `\uXXXX` or `\UXXXXXXXX` escapes for |
| 187 | Unicode characters. Avoid `\xXX...`-style escapes, which can cause subtle |
| 188 | problems if someone attempts to change the type of string that holds the |
| 189 | literal. In code used only on Windows, it may be necessary to use |
| 190 | `std::wstring` and `wchar_t*`; these are legal, but note that they are |
| 191 | distinct types and are often not 16-bit on other platforms. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 192 | |
| 193 | ## Object ownership and calling conventions |
| 194 | |
| 195 | When functions need to take raw or smart pointers as parameters, use the |
| 196 | following conventions. Here we refer to the parameter type as `T` and name as |
| 197 | `t`. |
Peter Kasting | 5e04bd3 | 2019-05-21 21:44:10 | [diff] [blame] | 198 | * If the function does not modify `t`'s ownership, declare the param as `T*`. |
| 199 | The caller is expected to ensure `t` stays alive as long as necessary, |
| 200 | generally through the duration of the call. Exception: In rare cases (e.g. |
| 201 | using lambdas with STL algorithms over containers of `unique_ptr<>`s), you |
| 202 | may be forced to declare the param as `const std::unique_ptr<T>&`. Do this |
| 203 | only when required. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 204 | * If the function takes ownership of a non-refcounted object, declare the |
| 205 | param as `std::unique_ptr<T>`. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 206 | * If the function (at least sometimes) takes a ref on a refcounted object, |
| 207 | declare the param as `scoped_refptr<T>`. The caller can decide |
| 208 | whether it wishes to transfer ownership (by calling `std::move(t)` when |
| 209 | passing `t`) or retain its ref (by simply passing t directly). |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 210 | * In short, functions should never take ownership of parameters passed as raw |
| 211 | pointers, and there should rarely be a need to pass smart pointers by const |
| 212 | ref. |
| 213 | |
Gabriel Charette | b62806f4a | 2019-01-29 21:08:41 | [diff] [blame] | 214 | Conventions for return values are similar with an important distinction: |
| 215 | * Return raw pointers if-and-only-if the caller does not take ownership. |
| 216 | * Return `std::unique_ptr<T>` or `scoped_refptr<T>` by value when the impl is |
| 217 | handing off ownership. |
| 218 | * **Distinction**: Return `const scoped_refptr<T>&` when the impl retains |
| 219 | ownership so the caller isn't required to take a ref: this avoids bumping |
| 220 | the reference count if the caller doesn't need ownership and also |
| 221 | [helps binary size](https://crrev.com/c/1435627)). |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 222 | |
| 223 | A great deal of Chromium code predates the above rules. In particular, some |
| 224 | functions take ownership of params passed as `T*`, or take `const |
| 225 | scoped_refptr<T>&` instead of `T*`, or return `T*` instead of |
| 226 | `scoped_refptr<T>` (to avoid refcount churn pre-C++11). Try to clean up such |
| 227 | code when you find it, or at least not make such usage any more widespread. |
| 228 | |
Lukasz Anforowicz | 269e41d | 2021-11-30 18:32:01 | [diff] [blame] | 229 | ## Non-owning pointers in class fields |
| 230 | |
danakj | 2c78205 | 2022-06-30 02:13:48 | [diff] [blame] | 231 | Use `const raw_ref<T>` or `raw_ptr<T>` for class and struct fields in place of a |
| 232 | raw C++ reference `T&` or pointer `T*` whenever possible, except in paths that include |
| 233 | `/renderer/` or `blink/public/web/`. These a non-owning smart pointers that |
| 234 | have improved memory-safety over raw pointers and references, and can prevent |
| 235 | exploitation of a significant percentage of Use-after-Free bugs. |
Lukasz Anforowicz | 269e41d | 2021-11-30 18:32:01 | [diff] [blame] | 236 | |
danakj | 2c78205 | 2022-06-30 02:13:48 | [diff] [blame] | 237 | Prefer `const raw_ref<T>` whenever the held pointer will never be null, and it's |
| 238 | ok to drop the `const` if the internal reference can be reassigned to point to a |
| 239 | different `T`. Use `raw_ptr<T>` in order to express that the pointer _can_ be |
| 240 | null. Only `raw_ptr<T>` can be default-constructed, since `raw_ref<T>` disallows |
| 241 | nullness. |
| 242 | |
| 243 | Using `raw_ref<T>` or `raw_ptr<T>` may not be possible in rare cases for |
| 244 | [performance reasons](../../base/memory/raw_ptr.md#Performance). Additionally, |
| 245 | `raw_ptr<T>` doesn’t support some C++ scenarios (e.g. `constexpr`, ObjC |
| 246 | pointers). Tooling will help to encourage use of these types in the future. See |
| 247 | [raw_ptr.md](../../base/memory/raw_ptr.md#When-to-use-raw_ptr_T) for how to add |
| 248 | exclusions. |
Lukasz Anforowicz | 269e41d | 2021-11-30 18:32:01 | [diff] [blame] | 249 | |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 250 | ## Forward declarations vs. #includes |
| 251 | |
| 252 | Unlike the Google style guide, Chromium style prefers forward declarations to |
| 253 | `#includes` where possible. This can reduce compile times and result in fewer |
| 254 | files needing recompilation when a header changes. |
| 255 | |
| 256 | You can and should use forward declarations for most types passed or returned |
| 257 | by value, reference, or pointer, or types stored as pointer members or in most |
| 258 | STL containers. However, if it would otherwise make sense to use a type as a |
| 259 | member by-value, don't convert it to a pointer just to be able to |
| 260 | forward-declare the type. |
| 261 | |
| 262 | ## File headers |
| 263 | |
Peter Kasting | 5e04bd3 | 2019-05-21 21:44:10 | [diff] [blame] | 264 | All files in Chromium start with a common license header. That header should |
| 265 | look like this: |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 266 | |
| 267 | ```c++ |
| 268 | // Copyright $YEAR The Chromium Authors. All rights reserved. |
| 269 | // Use of this source code is governed by a BSD-style license that can be |
| 270 | // found in the LICENSE file. |
| 271 | ``` |
| 272 | |
| 273 | Some important notes about this header: |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 274 | * There is no `(c)` after `Copyright`. |
Peter Kasting | 5e04bd3 | 2019-05-21 21:44:10 | [diff] [blame] | 275 | * `$YEAR` should be set to the current year at the time a file is created, and |
| 276 | not changed thereafter. |
| 277 | * For files specific to Chromium OS, replace the word Chromium with the phrase |
| 278 | Chromium OS. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 279 | * If the style changes, don't bother to update existing files to comply with |
| 280 | the new style. For the same reason, don't just blindly copy an existing |
| 281 | file's header when creating a new file, since the existing file may use an |
| 282 | outdated style. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 283 | * The Chromium project hosts mirrors of some upstream open-source projects. |
| 284 | When contributing to these portions of the repository, retain the existing |
| 285 | file headers. |
| 286 | |
| 287 | Use standard `#include` guards in all header files (see the Google style guide |
| 288 | sections on these for the naming convention). Do not use `#pragma once`; |
| 289 | historically it was not supported on all platforms, and it does not seem to |
| 290 | outperform #include guards even on platforms which do support it. |
| 291 | |
| 292 | ## CHECK(), DCHECK(), and NOTREACHED() |
| 293 | |
| 294 | The `CHECK()` macro will cause an immediate crash if its condition is not met. |
| 295 | `DCHECK()` is like `CHECK()` but is only compiled in when `DCHECK_IS_ON` is true |
| 296 | (debug builds and some bot configurations, but not end-user builds). |
| 297 | `NOTREACHED()` is equivalent to `DCHECK(false)`. Here are some rules for using |
| 298 | these: |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 299 | * Use `DCHECK()` or `NOTREACHED()` as assertions, e.g. to document pre- and |
| 300 | post-conditions. A `DCHECK()` means "this condition must always be true", |
| 301 | not "this condition is normally true, but perhaps not in exceptional |
| 302 | cases." Things like disk corruption or strange network errors are examples |
| 303 | of exceptional circumstances that nevertheless should not result in |
| 304 | `DCHECK()` failure. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 305 | * A consequence of this is that you should not handle DCHECK() failures, even |
Peter Kasting | 5e04bd3 | 2019-05-21 21:44:10 | [diff] [blame] | 306 | if failure would result in a crash. Attempting to handle a `DCHECK()` |
| 307 | failure is a statement that the `DCHECK()` can fail, which contradicts the |
| 308 | point of writing the `DCHECK()`. In particular, do not write code like the |
| 309 | following: |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 310 | ```c++ |
| 311 | DCHECK(foo); |
Peter Kasting | 5e04bd3 | 2019-05-21 21:44:10 | [diff] [blame] | 312 | if (!foo) // Eliminate this code. |
| 313 | ... |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 314 | |
Peter Kasting | 5e04bd3 | 2019-05-21 21:44:10 | [diff] [blame] | 315 | if (!bar) { // Replace this whole conditional with "DCHECK(bar);". |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 316 | NOTREACHED(); |
Peter Kasting | 5e04bd3 | 2019-05-21 21:44:10 | [diff] [blame] | 317 | return; |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 318 | } |
| 319 | ``` |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 320 | * Use `CHECK()` if the consequence of a failed assertion would be a security |
| 321 | vulnerability, where crashing the browser is preferable. Because this takes |
| 322 | down the whole browser, sometimes there are better options than `CHECK()`. |
| 323 | For example, if a renderer sends the browser process a malformed IPC, an |
| 324 | attacker may control the renderer, but we can simply kill the offending |
| 325 | renderer instead of crashing the whole browser. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 326 | * You can temporarily use `CHECK()` instead of `DCHECK()` when trying to |
| 327 | force crashes in release builds to sniff out which of your assertions is |
| 328 | failing. Don't leave these in the codebase forever; remove them or change |
| 329 | them back once you've solved the problem. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 330 | * Don't use these macros in tests, as they crash the test binary and leave |
| 331 | bots in a bad state. Use the `ASSERT_xx()` and `EXPECT_xx()` family of |
| 332 | macros, which report failures gracefully and can continue running other |
| 333 | tests. |
danakj | cd463f1 | 2021-02-12 19:53:13 | [diff] [blame] | 334 | * Dereferencing a null pointer in C++ is generally UB (undefined behavior) as |
| 335 | the compiler is free to assume a dereference means the pointer is not null |
| 336 | and may apply optimizations based on that. As such, there is sometimes a |
| 337 | strong opinion to `CHECK()` pointers before dereference. Chromium builds |
| 338 | with the `no-delete-null-pointer-checks` Clang/GCC flag which prevents such |
| 339 | optimizations, meaning the side effect of a null dereference would just be |
| 340 | the use of 0x0 which will lead to a crash on all the platforms Chromium |
| 341 | supports. As such we do not use `CHECK()` to guard pointer deferences. A |
| 342 | `DCHECK()` can be used to document that a pointer is never null, and doing |
| 343 | so as early as possible can help with debugging, though our styleguide now |
| 344 | recommends using a reference instead of a pointer when it cannot be null. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 345 | |
Roland Bock | d662a2d | 2022-07-12 20:55:27 | [diff] [blame] | 346 | ## Test-only code paths in production code |
| 347 | |
| 348 | Try to avoid test-only code paths in production code. Such code paths make |
| 349 | production code behave differently in tests. This makes both tests and |
| 350 | production code hard to reason about. Consider dependency injection, fake |
| 351 | classes, etc to avoid such code paths. |
| 352 | |
| 353 | However, if a test-only path in production code cannot be avoided, instrument |
| 354 | that code path with `CHECK_IS_TEST();` to assert that the code is only run in |
| 355 | tests. |
| 356 | |
| 357 | ```c++ |
| 358 | // `profile_manager` may not be available in tests. |
| 359 | if (!profile_manager) { |
| 360 | CHECK_IS_TEST(); |
| 361 | return std::string(); |
| 362 | } |
| 363 | ``` |
| 364 | |
| 365 | `CHECK_IS_TEST();` will crash outside of tests. This asserts that the test-only |
| 366 | code path is not accidentally or maliciously taken in production. |
| 367 | |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 368 | ## Miscellany |
| 369 | |
| 370 | * Use UTF-8 file encodings and LF line endings. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 371 | * Unit tests and performance tests should be placed in the same directory as |
| 372 | the functionality they're testing. |
Xiaohan Wang | cc517f08 | 2019-01-16 01:58:14 | [diff] [blame] | 373 | * The [C++ Dos and Don'ts](c++-dos-and-donts.md) page has more helpful |
| 374 | information. |