brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 1 | # Chromium C++ style guide |
| 2 | |
| 3 | _For other languages, please see the [Chromium style guides](https://chromium.googlesource.com/chromium/src/+/master/styleguide/styleguide.md)._ |
| 4 | |
| 5 | Chromium follows the [Google C++ Style |
| 6 | Guide](https://google.github.io/styleguide/cppguide.html) unless an exception |
| 7 | is listed below. |
| 8 | |
| 9 | A checkout should give you |
| 10 | [clang-format](https://chromium.googlesource.com/chromium/src/+/master/docs/clang_format.md) |
| 11 | to automatically format C++ code. By policy, Clang's formatting of code should |
| 12 | always be accepted in code reviews. |
| 13 | |
brettw | 196290a | 2016-07-07 03:52:16 | [diff] [blame] | 14 | You can propose changes to this style guide by sending an email to |
| 15 | `[email protected]`. Ideally, the list will arrive at some consensus and you can |
| 16 | request review for a change to this file. If there's no consensus, |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 17 | `src/styleguide/c++/OWNERS` get to decide. |
| 18 | |
Daniel Cheng | 7ecc1d6 | 2017-06-12 20:24:30 | [diff] [blame] | 19 | Blink code in `third_party/WebKit` uses [Blink style](blink-c++.md). |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 20 | |
| 21 | ## C++11 features |
| 22 | |
| 23 | Google style has adopted most C++11 features, but Chromium has a more |
| 24 | restricted set. The status of C++11 features in Chromium is tracked in the |
| 25 | separate [C++11 use in Chromium](https://chromium-cpp.appspot.com/) page. |
| 26 | |
| 27 | ## Naming |
| 28 | |
| 29 | * "Chromium" is the name of the project, not the product, and should never |
| 30 | appear in code, variable names, API names etc. Use "Chrome" instead. |
| 31 | |
Tommy C. Li | 1a13764a | 2018-09-17 21:18:34 | [diff] [blame] | 32 | ## Test-only Code |
| 33 | |
| 34 | * Functions used only for testing should be restricted to test-only usages |
| 35 | with the `ForTesting` suffix. This is checked at presubmit time to ensure |
| 36 | these functions are only called by test files. |
| 37 | |
| 38 | * Test-only constructors cannot have the `ForTesting` suffix. Instead, they |
| 39 | should be declared protected with a test-only subclass, or private with a |
| 40 | test-only friend class. They should be commented as `For testing only`. |
| 41 | |
| 42 | * Test-only free functions should generally live within a test_support |
| 43 | target. |
| 44 | |
| 45 | * `#if defined(UNIT_TEST)` is problematic and discouraged. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 46 | |
| 47 | ## Code formatting |
| 48 | |
| 49 | * Put `*` and `&` by the type rather than the variable name. |
| 50 | |
| 51 | * When you derive from a base class, group any overriding functions in your |
| 52 | header file in one labeled section. Use the override specifier on all these |
| 53 | functions. |
| 54 | |
| 55 | * Prefer `(foo == 0)` to `(0 == foo)`. |
| 56 | |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 57 | * Prefer putting delegate classes in their own header files. Implementors of |
| 58 | the delegate interface will often be included elsewhere, which will often |
| 59 | cause more coupling with the header of the main class. |
| 60 | |
| 61 | * Don't use else after return. So use: |
| 62 | ```c++ |
| 63 | if (foo) |
| 64 | return 1; |
| 65 | return 2; |
| 66 | ``` |
| 67 | instead of: |
| 68 | ```c++ |
| 69 | if (foo) |
| 70 | return 1; |
| 71 | else |
| 72 | return 2; |
| 73 | ``` |
| 74 | |
| 75 | ## Unnamed namespaces |
| 76 | |
| 77 | Items local to a .cc file should be wrapped in an unnamed namespace. While some |
| 78 | such items are already file-scope by default in C++, not all are; also, shared |
| 79 | objects on Linux builds export all symbols, so unnamed namespaces (which |
| 80 | restrict these symbols to the compilation unit) improve function call cost and |
| 81 | reduce the size of entry point tables. |
| 82 | |
| 83 | ## Exporting symbols |
| 84 | |
| 85 | When building shared libraries and DLLs, we need to indicate which functions |
| 86 | and classes should be visible outside of the library, and which should only be |
| 87 | visible inside the library. |
| 88 | |
| 89 | Symbols can be exported by annotating with a `<COMPONENT>_EXPORT` macro name |
| 90 | (where `<COMPONENT>` is the name of the component being built, e.g. BASE, NET, |
| 91 | CONTENT, etc.). Class annotations should precede the class name: |
| 92 | ```c++ |
| 93 | class FOO_EXPORT Foo { |
| 94 | void Bar(); |
| 95 | void Baz(); |
| 96 | // ... |
| 97 | }; |
| 98 | ``` |
| 99 | |
| 100 | Function annotations should precede the return type: |
| 101 | ```c++ |
| 102 | class FooSingleton { |
| 103 | FOO_EXPORT Foo& GetFoo(); |
| 104 | FOO_EXPORT Foo& SetFooForTesting(Foo& foo); |
| 105 | void SetFoo(Foo& foo); |
| 106 | }; |
| 107 | ``` |
| 108 | |
| 109 | These examples result in `Foo::Bar()`, `Foo::Baz()`, `FooSingleton::GetFoo()`, |
| 110 | and `FooSingleton::SetFooForTesting()` all being available outside of the DLL, |
| 111 | but not `FooSingleton::SetFoo()`. |
| 112 | |
| 113 | Whether something is exported is distinct from whether it is public or private, |
| 114 | or even whether it would normally be considered part of the external API. For |
| 115 | example, if part of the external API is an inlined function that calls a |
| 116 | private function, that private function must be exported as well. |
| 117 | |
| 118 | ## Multiple inheritance |
| 119 | |
| 120 | Multiple inheritance and virtual inheritance are permitted in Chromium code, |
| 121 | but discouraged (beyond the "interface" style of inheritance allowed by the |
| 122 | Google style guide, for which we do not require classes to have the "Interface" |
| 123 | suffix). Consider whether composition could solve the problem instead. |
| 124 | |
| 125 | ## Inline functions |
| 126 | |
| 127 | Simple accessors should generally be the only inline functions. These should be |
| 128 | named `unix_hacker_style()`. Virtual functions should never be declared this way. |
Xiaohan Wang | cc517f08 | 2019-01-16 01:58:14 | [diff] [blame] | 129 | For more detail, consult the [C++ Dos and Don'ts](c++-dos-and-donts.md) section |
| 130 | on inlining. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 131 | |
| 132 | ## Logging |
| 133 | |
| 134 | Remove most logging calls before checking in. Unless you're adding temporary |
| 135 | logging to track down a specific bug, and you have a plan for how to collect |
| 136 | the logged data from user machines, you should generally not add logging |
| 137 | statements. |
| 138 | |
| 139 | For the rare case when logging needs to stay in the codebase for a while, |
| 140 | prefer `DVLOG(1)` to other logging methods. This avoids bloating the release |
| 141 | executable and in debug can be selectively enabled at runtime by command-line |
| 142 | arguments: |
| 143 | |
| 144 | * `--v=n` sets the global log level to n (default 0). All log statements with a |
| 145 | log level less than or equal to the global level will be printed. |
| 146 | |
| 147 | * `--vmodule=mod=n[,mod=n,...]` overrides the global log level for the module |
| 148 | mod. Supplying the string foo for mod will affect all files named foo.cc, |
| 149 | while supplying a wildcard like `*bar/baz*` will affect all files with |
| 150 | `bar/baz` in their full pathnames. |
| 151 | |
| 152 | ## Platform-specific code |
| 153 | |
| 154 | To `#ifdef` code for specific platforms, use the macros defined in |
| 155 | `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] | 156 | set by specific compilers or build environments (e.g. `WIN32`). |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 157 | |
| 158 | Place platform-specific #includes in their own section below the "normal" |
| 159 | `#includes`. Repeat the standard `#include` order within this section: |
| 160 | |
| 161 | ```c++ |
| 162 | #include "foo/foo.h" |
| 163 | |
| 164 | #include <stdint.h> |
| 165 | #include <algorithm> |
| 166 | |
| 167 | #include "base/strings/utf_string_conversions.h" |
| 168 | #include "chrome/common/render_messages.h" |
| 169 | |
| 170 | #if defined(OS_WIN) |
| 171 | #include <windows.h> |
Robert Liao | c7c9a1c | 2017-10-18 01:41:31 | [diff] [blame] | 172 | #include "base/win/com_init_util.h" |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 173 | #elif defined(OS_POSIX) |
| 174 | #include "base/posix/global_descriptors.h" |
| 175 | #endif |
| 176 | ``` |
| 177 | |
| 178 | ## Types |
| 179 | |
| 180 | * Use `size_t` for object and allocation sizes, object counts, array and |
| 181 | pointer offsets, vector indices, and so on. The signed types are incorrect |
| 182 | and unsafe for these purposes (e.g. integer overflow behavior for signed |
| 183 | types is undefined in the C and C++ standards, while the behavior is |
| 184 | defined for unsigned types.) The C++ STL is a guide here: they use `size_t` |
| 185 | and `foo::size_type` for very good reasons. |
| 186 | |
| 187 | * Use `size_t` directly in preference to `std::string::size_type` and similar. |
| 188 | |
| 189 | * Occasionally classes may have a good reason to use a type other than `size_t` |
| 190 | for one of these concepts, e.g. as a storage space optimization. In these |
| 191 | cases, continue to use `size_t` in public-facing function declarations. |
| 192 | |
| 193 | * Be aware that `size_t` (object sizes and indices), `off_t` (file offsets), |
| 194 | `ptrdiff_t` (the difference between two pointer values), `intptr_t` (an |
| 195 | integer type large enough to hold the value of a pointer), `uint32_t`, |
| 196 | `uint64_t`, and so on are not necessarily the same. Use the right type for |
| 197 | your purpose. |
| 198 | |
Dana Fried | 74a1889 | 2018-09-13 19:10:01 | [diff] [blame] | 199 | * Follow [Google C++ casting |
| 200 | conventions](https://google.github.io/styleguide/cppguide.html#Casting) |
| 201 | to convert arithmetic types when you know the conversion is safe. Use |
| 202 | `checked_cast<>()` (from `base/numerics/safe_conversions.h`) when you |
| 203 | need to enforce via `CHECK()` that the source value is in-range for the |
| 204 | destination type. Use `saturated_cast<>()` (from the same file) if you |
| 205 | instead wish to clamp out-of-range values. |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 206 | |
| 207 | * Do not use unsigned types to mean "this value should never be < 0". For |
| 208 | that, use assertions or run-time checks (as appropriate). |
| 209 | |
| 210 | * In cases where the exact size of the type matters (e.g. a 32-bit pixel |
| 211 | value, a bitmask, or a counter that has to be a particular width), use one |
| 212 | of the sized types from `<stdint.h>`, e.g. `uint32_t`. |
| 213 | |
| 214 | * When passing values across network or process boundaries, use |
| 215 | explicitly-sized types for safety, since the sending and receiving ends may |
| 216 | not have been compiled with the same sizes for things like int and |
| 217 | `size_t`. However, to the greatest degree possible, avoid letting these |
| 218 | sized types bleed through the APIs of the layers in question. |
| 219 | |
| 220 | * Don't use `std::wstring`. Use `base::string16` or `base::FilePath` instead. |
| 221 | (Windows-specific code interfacing with system APIs using `wstring` and |
| 222 | `wchar_t` can still use `string16` and `char16`; it is safe to assume that |
| 223 | these are equivalent to the "wide" types.) |
| 224 | |
| 225 | ## Object ownership and calling conventions |
| 226 | |
| 227 | When functions need to take raw or smart pointers as parameters, use the |
| 228 | following conventions. Here we refer to the parameter type as `T` and name as |
| 229 | `t`. |
| 230 | |
| 231 | * If the function does not modify `t`'s ownership, declare the param as `T*`. The |
| 232 | caller is expected to ensure `t` stays alive as long as necessary, generally |
| 233 | through the duration of the call. Exception: In rare cases (e.g. using |
claudiomagni | e85f90c | 2017-05-10 05:40:29 | [diff] [blame] | 234 | lambdas with STL algorithms over containers of `unique_ptr<>`s), you may be |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 235 | forced to declare the param as `const std::unique_ptr<T>&`. Do this only when |
| 236 | required. |
| 237 | |
| 238 | * If the function takes ownership of a non-refcounted object, declare the |
| 239 | param as `std::unique_ptr<T>`. |
| 240 | |
| 241 | * If the function (at least sometimes) takes a ref on a refcounted object, |
| 242 | declare the param as `scoped_refptr<T>`. The caller can decide |
| 243 | whether it wishes to transfer ownership (by calling `std::move(t)` when |
| 244 | passing `t`) or retain its ref (by simply passing t directly). |
| 245 | |
| 246 | * In short, functions should never take ownership of parameters passed as raw |
| 247 | pointers, and there should rarely be a need to pass smart pointers by const |
| 248 | ref. |
| 249 | |
Gabriel Charette | b62806f4a | 2019-01-29 21:08:41 | [diff] [blame^] | 250 | Conventions for return values are similar with an important distinction: |
| 251 | * Return raw pointers if-and-only-if the caller does not take ownership. |
| 252 | * Return `std::unique_ptr<T>` or `scoped_refptr<T>` by value when the impl is |
| 253 | handing off ownership. |
| 254 | * **Distinction**: Return `const scoped_refptr<T>&` when the impl retains |
| 255 | ownership so the caller isn't required to take a ref: this avoids bumping |
| 256 | the reference count if the caller doesn't need ownership and also |
| 257 | [helps binary size](https://crrev.com/c/1435627)). |
brettw | f0e606a5 | 2016-07-06 21:17:20 | [diff] [blame] | 258 | |
| 259 | A great deal of Chromium code predates the above rules. In particular, some |
| 260 | functions take ownership of params passed as `T*`, or take `const |
| 261 | scoped_refptr<T>&` instead of `T*`, or return `T*` instead of |
| 262 | `scoped_refptr<T>` (to avoid refcount churn pre-C++11). Try to clean up such |
| 263 | code when you find it, or at least not make such usage any more widespread. |
| 264 | |
| 265 | ## Forward declarations vs. #includes |
| 266 | |
| 267 | Unlike the Google style guide, Chromium style prefers forward declarations to |
| 268 | `#includes` where possible. This can reduce compile times and result in fewer |
| 269 | files needing recompilation when a header changes. |
| 270 | |
| 271 | You can and should use forward declarations for most types passed or returned |
| 272 | by value, reference, or pointer, or types stored as pointer members or in most |
| 273 | STL containers. However, if it would otherwise make sense to use a type as a |
| 274 | member by-value, don't convert it to a pointer just to be able to |
| 275 | forward-declare the type. |
| 276 | |
| 277 | ## File headers |
| 278 | |
| 279 | All files in Chromium start with a common license header. That header should look like this: |
| 280 | |
| 281 | ```c++ |
| 282 | // Copyright $YEAR The Chromium Authors. All rights reserved. |
| 283 | // Use of this source code is governed by a BSD-style license that can be |
| 284 | // found in the LICENSE file. |
| 285 | ``` |
| 286 | |
| 287 | Some important notes about this header: |
| 288 | |
| 289 | * There is no `(c)` after `Copyright`. |
| 290 | |
| 291 | * `$YEAR` should be set to the current year at the time a file is created, and not changed thereafter. |
| 292 | |
| 293 | * For files specific to Chromium OS, replace the word Chromium with the phrase Chromium OS. |
| 294 | |
| 295 | * If the style changes, don't bother to update existing files to comply with |
| 296 | the new style. For the same reason, don't just blindly copy an existing |
| 297 | file's header when creating a new file, since the existing file may use an |
| 298 | outdated style. |
| 299 | |
| 300 | * The Chromium project hosts mirrors of some upstream open-source projects. |
| 301 | When contributing to these portions of the repository, retain the existing |
| 302 | file headers. |
| 303 | |
| 304 | Use standard `#include` guards in all header files (see the Google style guide |
| 305 | sections on these for the naming convention). Do not use `#pragma once`; |
| 306 | historically it was not supported on all platforms, and it does not seem to |
| 307 | outperform #include guards even on platforms which do support it. |
| 308 | |
| 309 | ## CHECK(), DCHECK(), and NOTREACHED() |
| 310 | |
| 311 | The `CHECK()` macro will cause an immediate crash if its condition is not met. |
| 312 | `DCHECK()` is like `CHECK()` but is only compiled in when `DCHECK_IS_ON` is true |
| 313 | (debug builds and some bot configurations, but not end-user builds). |
| 314 | `NOTREACHED()` is equivalent to `DCHECK(false)`. Here are some rules for using |
| 315 | these: |
| 316 | |
| 317 | * Use `DCHECK()` or `NOTREACHED()` as assertions, e.g. to document pre- and |
| 318 | post-conditions. A `DCHECK()` means "this condition must always be true", |
| 319 | not "this condition is normally true, but perhaps not in exceptional |
| 320 | cases." Things like disk corruption or strange network errors are examples |
| 321 | of exceptional circumstances that nevertheless should not result in |
| 322 | `DCHECK()` failure. |
| 323 | |
| 324 | * A consequence of this is that you should not handle DCHECK() failures, even |
| 325 | if failure would result in a crash. Attempting to handle a `DCHECK()` failure |
| 326 | is a statement that the `DCHECK()` can fail, which contradicts the point of |
| 327 | writing the `DCHECK()`. In particular, do not write code like the following: |
| 328 | ```c++ |
| 329 | DCHECK(foo); |
| 330 | if (!foo) ... // Can't succeed! |
| 331 | |
| 332 | if (!bar) { |
| 333 | NOTREACHED(); |
| 334 | return; // Replace this whole conditional with "DCHECK(bar);" and keep going instead. |
| 335 | } |
| 336 | ``` |
| 337 | |
| 338 | * Use `CHECK()` if the consequence of a failed assertion would be a security |
| 339 | vulnerability, where crashing the browser is preferable. Because this takes |
| 340 | down the whole browser, sometimes there are better options than `CHECK()`. |
| 341 | For example, if a renderer sends the browser process a malformed IPC, an |
| 342 | attacker may control the renderer, but we can simply kill the offending |
| 343 | renderer instead of crashing the whole browser. |
| 344 | |
| 345 | * You can temporarily use `CHECK()` instead of `DCHECK()` when trying to |
| 346 | force crashes in release builds to sniff out which of your assertions is |
| 347 | failing. Don't leave these in the codebase forever; remove them or change |
| 348 | them back once you've solved the problem. |
| 349 | |
| 350 | * Don't use these macros in tests, as they crash the test binary and leave |
| 351 | bots in a bad state. Use the `ASSERT_xx()` and `EXPECT_xx()` family of |
| 352 | macros, which report failures gracefully and can continue running other |
| 353 | tests. |
| 354 | |
| 355 | ## Miscellany |
| 356 | |
| 357 | * Use UTF-8 file encodings and LF line endings. |
| 358 | |
| 359 | * Unit tests and performance tests should be placed in the same directory as |
| 360 | the functionality they're testing. |
| 361 | |
Xiaohan Wang | cc517f08 | 2019-01-16 01:58:14 | [diff] [blame] | 362 | * The [C++ Dos and Don'ts](c++-dos-and-donts.md) page has more helpful |
| 363 | information. |