blob: 71d1196df256f846380bd0f6074e17fbc56b435d [file] [log] [blame] [view]
Artem Titova6178672023-01-30 10:51:011<!-- go/cmark -->
2<!--* freshness: {owner: 'danilchap' reviewed: '2022-01-17'} *-->
Danil Chapovalov46f5c112021-05-12 12:05:483
Artem Titova6178672023-01-30 10:51:014# WebRTC coding style guide
Artem Titov0f2ce5c2023-01-26 20:18:465
Fanny Linderborg70efbb82021-04-23 14:59:296## General advice
Karl Wiberg241d7102017-09-08 13:07:157
8Some older parts of the code violate the style guide in various ways.
Danil Chapovalov80569ea2022-01-17 15:11:029If making large changes to such code, consider first cleaning it up in a
Fanny Linderborg70efbb82021-04-23 14:59:2910 separate CL.
Karl Wiberg241d7102017-09-08 13:07:1511
Fanny Linderborg70efbb82021-04-23 14:59:2912## C++
Karl Wibergbb821e22017-09-03 03:02:1213
Fanny Linderborg70efbb82021-04-23 14:59:2914WebRTC follows the [Chromium C++ style guide][chr-style] and the
15[Google C++ style guide][goog-style]. In cases where they conflict, the Chromium
16style guide trumps the Google style guide, and the rules in this file trump them
Karl Wiberge1ed2412017-09-06 09:10:2317both.
Karl Wibergbb821e22017-09-03 03:02:1218
Fanny Linderborg0d2dc1f2021-07-14 14:02:1119[chr-style]: https://chromium.googlesource.com/chromium/src/+/main/styleguide/c++/c++.md
Karl Wibergbb821e22017-09-03 03:02:1220[goog-style]: https://google.github.io/styleguide/cppguide.html
21
Karl Wiberg180d9922018-03-15 10:21:0522### C++ version
23
Mirko Bonadei28cd1642021-12-24 08:55:2224WebRTC is written in C++17, but with some restrictions:
Karl Wiberg180d9922018-03-15 10:21:0525
Mirko Bonadei28cd1642021-12-24 08:55:2226* We only allow the subset of C++17 (language and library) that is not banned by
Joe Mason4893dbe2021-09-16 23:11:5627 Chromium; see the [list of banned C++ features in Chromium][chr-style-cpp].
Mirko Bonadei28cd1642021-12-24 08:55:2228* We only allow the subset of C++17 that is also valid C++20; otherwise, users
29 would not be able to compile WebRTC in C++20 mode.
Karl Wiberg180d9922018-03-15 10:21:0530
Danil Chapovalov8ad0be02022-01-24 12:58:3031[chr-style-cpp]: https://chromium.googlesource.com/chromium/src/+/main/styleguide/c++/c++-features.md
Karl Wiberg180d9922018-03-15 10:21:0532
Karl Wibergc3af97d2018-08-27 02:26:1833### Abseil
34
Fanny Linderborg70efbb82021-04-23 14:59:2935You may use a subset of the utilities provided by the [Abseil][abseil] library
36when writing WebRTC C++ code; see the
37[instructions on how to use Abseil in WebRTC](abseil-in-webrtc.md).
Karl Wibergc3af97d2018-08-27 02:26:1838
39[abseil]: https://abseil.io/about/
40
Karl Wiberg08c5cb02018-03-14 11:23:1241### <a name="h-cc-pairs"></a>`.h` and `.cc` files come in pairs
42
Fanny Linderborg70efbb82021-04-23 14:59:2943`.h` and `.cc` files should come in pairs, with the same name (except for the
44file type suffix), in the same directory, in the same build target.
Karl Wiberg08c5cb02018-03-14 11:23:1245
Fanny Linderborg70efbb82021-04-23 14:59:2946* If a declaration in `path/to/foo.h` has a definition in some `.cc` file, it
47 should be in `path/to/foo.cc`.
48* If a definition in `path/to/foo.cc` file has a declaration in some `.h` file,
49 it should be in `path/to/foo.h`.
50* Omit the `.cc` file if it would have been empty, but still list the `.h` file
51 in a build target.
52* Omit the `.h` file if it would have been empty. (This can happen with unit
53 test `.cc` files, and with `.cc` files that define `main`.)
Karl Wiberg08c5cb02018-03-14 11:23:1254
Fanny Linderborg70efbb82021-04-23 14:59:2955See also the
56[examples and exceptions on how to treat `.h` and `.cpp` files](style-guide/h-cc-pairs.md).
Karl Wiberg08c5cb02018-03-14 11:23:1257
Fanny Linderborg70efbb82021-04-23 14:59:2958This makes the source code easier to navigate and organize, and precludes some
59questionable build system practices such as having build targets that don't pull
60in definitions for everything they declare.
Karl Wiberg08c5cb02018-03-14 11:23:1261
Fanny Linderborg70efbb82021-04-23 14:59:2962### `TODO` comments
Danil Chapovalov375eff42019-12-11 11:45:0863
Fanny Linderborg70efbb82021-04-23 14:59:2964Follow the [Google styleguide for `TODO` comments][goog-style-todo]. When
Emil Lundmark11174e72022-07-01 08:00:1165referencing a WebRTC bug, prefer using the URL form (excluding the scheme part):
Fanny Linderborg70efbb82021-04-23 14:59:2966
67```cpp
Danil Chapovalov375eff42019-12-11 11:45:0868// TODO(bugs.webrtc.org/12345): Delete the hack when blocking bugs are resolved.
69```
70
Emil Lundmark11174e72022-07-01 08:00:1171The short form used in commit messages, e.g. `webrtc:12345`, is discouraged.
72
Danil Chapovalov375eff42019-12-11 11:45:0873[goog-style-todo]: https://google.github.io/styleguide/cppguide.html#TODO_Comments
74
Danil Chapovalov7013b3b2021-02-22 13:31:2675### Deprecation
76
Fanny Linderborg70efbb82021-04-23 14:59:2977Annotate the declarations of deprecated functions and classes with the
Harald Alvestrand832657f2022-04-27 08:59:3278[`[[deprecated]]` attribute][DEPRECATED] to cause an error when they're used
Fanny Linderborg70efbb82021-04-23 14:59:2979inside WebRTC and a compiler warning when they're used by dependant projects.
80Like so:
Danil Chapovalov7013b3b2021-02-22 13:31:2681
Fanny Linderborg70efbb82021-04-23 14:59:2982```cpp
Harald Alvestrand832657f2022-04-27 08:59:3283[[deprecated("bugs.webrtc.org/12345")]]
Danil Chapovalov7013b3b2021-02-22 13:31:2684std::pony PonyPlz(const std::pony_spec& ps);
85```
86
Fanny Linderborg70efbb82021-04-23 14:59:2987NOTE 1: The annotation goes on the declaration in the `.h` file, not the
88definition in the `.cc` file!
Danil Chapovalov7013b3b2021-02-22 13:31:2689
90NOTE 2: In order to have unit tests that use the deprecated function without
91getting errors, do something like this:
92
Fanny Linderborg70efbb82021-04-23 14:59:2993```cpp
Danil Chapovalov7013b3b2021-02-22 13:31:2694std::pony DEPRECATED_PonyPlz(const std::pony_spec& ps);
Harald Alvestrand832657f2022-04-27 08:59:3295[[deprecated("bugs.webrtc.org/12345")]]
Danil Chapovalov7013b3b2021-02-22 13:31:2696inline std::pony PonyPlz(const std::pony_spec& ps) {
97 return DEPRECATED_PonyPlz(ps);
98}
99```
100
101In other words, rename the existing function, and provide an inline wrapper
102using the original name that calls it. That way, callers who are willing to
Fanny Linderborg70efbb82021-04-23 14:59:29103call it using the `DEPRECATED_`-prefixed name don't get the warning.
Danil Chapovalov7013b3b2021-02-22 13:31:26104
Harald Alvestrand832657f2022-04-27 08:59:32105NOTE 3: Occasionally, with long descriptions, `git cl format` will do the wrong
106thing with the attribute. In that case, you can use the
107[`ABSL_DEPRECATED` macro][ABSL_DEPRECATED], which is formatted in a more
108readable way.
109
110[DEPRECATED]: https://en.cppreference.com/w/cpp/language/attributes/deprecated
Tony Herreb0ed1202021-07-22 15:40:44111[ABSL_DEPRECATED]: https://source.chromium.org/chromium/chromium/src/+/main:third_party/abseil-cpp/absl/base/attributes.h?q=ABSL_DEPRECATED
Danil Chapovalov7013b3b2021-02-22 13:31:26112
Karl Wiberge1ed2412017-09-06 09:10:23113### ArrayView
Karl Wibergbb821e22017-09-03 03:02:12114
Karl Wiberge1ed2412017-09-06 09:10:23115When passing an array of values to a function, use `rtc::ArrayView`
Fanny Linderborg70efbb82021-04-23 14:59:29116whenever possible—that is, whenever you're not passing ownership of
117the array, and don't allow the callee to change the array size.
Karl Wiberge1ed2412017-09-06 09:10:23118
119For example,
120
Fanny Linderborg70efbb82021-04-23 14:59:29121| instead of | use |
122|-------------------------------------|----------------------|
123| `const std::vector<T>&` | `ArrayView<const T>` |
124| `const T* ptr, size_t num_elements` | `ArrayView<const T>` |
125| `T* ptr, size_t num_elements` | `ArrayView<T>` |
Karl Wiberge1ed2412017-09-06 09:10:23126
Fanny Linderborg70efbb82021-04-23 14:59:29127See the [source code for `rtc::ArrayView`](api/array_view.h) for more detailed
128docs.
Karl Wibergfb4e6772017-09-04 09:27:33129
Taylor Brandstetterefc5fbd2017-12-08 19:42:15130### sigslot
131
Harald Alvestrandf703ed12021-04-19 13:28:21132SIGSLOT IS DEPRECATED.
Taylor Brandstetterefc5fbd2017-12-08 19:42:15133
Fanny Linderborg70efbb82021-04-23 14:59:29134Prefer `webrtc::CallbackList`, and manage thread safety yourself.
Taylor Brandstetterefc5fbd2017-12-08 19:42:15135
Harald Alvestrandf703ed12021-04-19 13:28:21136### Smart pointers
Taylor Brandstetterefc5fbd2017-12-08 19:42:15137
Harald Alvestrandf703ed12021-04-19 13:28:21138The following smart pointer types are recommended:
Taylor Brandstetterefc5fbd2017-12-08 19:42:15139
Fanny Linderborg70efbb82021-04-23 14:59:29140 * `std::unique_ptr` for all singly-owned objects
141 * `rtc::scoped_refptr` for all objects with shared ownership
Taylor Brandstetterefc5fbd2017-12-08 19:42:15142
Fanny Linderborg70efbb82021-04-23 14:59:29143Use of `std::shared_ptr` is *not permitted*. It is banned in the Chromium style
Danil Chapovalov80569ea2022-01-17 15:11:02144guide (overriding the Google style guide). See the
Fanny Linderborg70efbb82021-04-23 14:59:29145[list of banned C++ library features in Chromium][chr-std-shared-ptr] for more
146information.
Taylor Brandstetterefc5fbd2017-12-08 19:42:15147
Fanny Linderborg70efbb82021-04-23 14:59:29148In most cases, one will want to explicitly control lifetimes, and therefore use
149`std::unique_ptr`, but in some cases, for instance where references have to
150exist both from the API users and internally, with no way to invalidate pointers
151held by the API user, `rtc::scoped_refptr` can be appropriate.
Taylor Brandstetterefc5fbd2017-12-08 19:42:15152
Danil Chapovalov8ad0be02022-01-24 12:58:30153[chr-std-shared-ptr]: https://chromium.googlesource.com/chromium/src/+/main/styleguide/c++/c++-features.md#shared-pointers-banned
Karl Wiberg0d446252018-03-19 22:27:35154
Fanny Linderborg70efbb82021-04-23 14:59:29155### `std::bind`
Karl Wiberg0d446252018-03-19 22:27:35156
Fanny Linderborg70efbb82021-04-23 14:59:29157Don't use `std::bind`—there are pitfalls, and lambdas are almost as succinct and
158already familiar to modern C++ programmers.
Karl Wiberg0d446252018-03-19 22:27:35159
Fanny Linderborg70efbb82021-04-23 14:59:29160### `std::function`
161
162`std::function` is allowed, but remember that it's not the right tool for every
163occasion. Prefer to use interfaces when that makes sense, and consider
164`rtc::FunctionView` for cases where the callee will not save the function
165object.
Karl Wiberg0d446252018-03-19 22:27:35166
Taylor Brandstetter212a2062017-10-12 18:22:48167### Forward declarations
168
Fanny Linderborg70efbb82021-04-23 14:59:29169WebRTC follows the
170[Google C++ style guide on forward declarations][goog-forward-declarations].
171In summary: avoid using forward declarations where possible; just `#include` the
172headers you need.
Taylor Brandstetter212a2062017-10-12 18:22:48173
174[goog-forward-declarations]: https://google.github.io/styleguide/cppguide.html#Forward_Declarations
175
Harald Alvestrandf88487c2022-11-04 12:13:04176### RTTI and dynamic_cast
177
178The Google style guide [permits the use of dynamic_cast](https://google.github.io/styleguide/cppguide.html#Run-Time_Type_Information__RTTI_).
179
180However, WebRTC does not permit it. WebRTC (and Chrome) is compiled with the
181-fno-rtti flag, and the overhead of enabling RTTI it is on the order of 220
182Kbytes (for Android Arm64).
183
184Use static_cast and take your own steps to ensure type safety.
185
Fanny Linderborg70efbb82021-04-23 14:59:29186## C
Karl Wibergfb4e6772017-09-04 09:27:33187
Fanny Linderborg70efbb82021-04-23 14:59:29188There's a substantial chunk of legacy C code in WebRTC, and a lot of it is old
189enough that it violates the parts of the C++ style guide that also applies to C
190(naming etc.) for the simple reason that it pre-dates the use of the current C++
Danil Chapovalov80569ea2022-01-17 15:11:02191style guide for this code base. If making large changes to C code, consider
192converting the whole thing to C++ first.
Karl Wibergee3d7602017-09-04 12:46:47193
Fanny Linderborg70efbb82021-04-23 14:59:29194## Java
Karl Wiberg241d7102017-09-08 13:07:15195
196WebRTC follows the [Google Java style guide][goog-java-style].
197
198[goog-java-style]: https://google.github.io/styleguide/javaguide.html
199
Fanny Linderborg70efbb82021-04-23 14:59:29200## Objective-C and Objective-C++
Karl Wiberg241d7102017-09-08 13:07:15201
202WebRTC follows the
203[Chromium Objective-C and Objective-C++ style guide][chr-objc-style].
204
Fanny Linderborg0d2dc1f2021-07-14 14:02:11205[chr-objc-style]: https://chromium.googlesource.com/chromium/src/+/main/styleguide/objective-c/objective-c.md
Karl Wiberg241d7102017-09-08 13:07:15206
Fanny Linderborg70efbb82021-04-23 14:59:29207## Python
Karl Wiberg241d7102017-09-08 13:07:15208
Fanny Linderborg70efbb82021-04-23 14:59:29209WebRTC follows [Chromium's Python style][chr-py-style].
Karl Wiberg241d7102017-09-08 13:07:15210
Fanny Linderborg0d2dc1f2021-07-14 14:02:11211[chr-py-style]: https://chromium.googlesource.com/chromium/src/+/main/styleguide/python/python.md
Karl Wiberg241d7102017-09-08 13:07:15212
Fanny Linderborg70efbb82021-04-23 14:59:29213## Build files
Karl Wibergee3d7602017-09-04 12:46:47214
Fanny Linderborg70efbb82021-04-23 14:59:29215The WebRTC build files are written in [GN][gn], and we follow the
216[GN style guide][gn-style]. Additionally, there are some
217WebRTC-specific rules below; in case of conflict, they trump the Chromium style
218guide.
Karl Wiberg91d0ab72017-09-07 15:05:31219
Fanny Linderborg70efbb82021-04-23 14:59:29220[gn]: https://gn.googlesource.com/gn/
221[gn-style]: https://gn.googlesource.com/gn/+/HEAD/docs/style_guide.md
Karl Wiberg91d0ab72017-09-07 15:05:31222
Per Kjellandera7f2d842018-01-10 15:54:53223### <a name="webrtc-gn-templates"></a>WebRTC-specific GN templates
Karl Wiberg91d0ab72017-09-07 15:05:31224
Mirko Bonadeiab397dd2022-12-16 10:08:01225As shown in the table below, for library targets (`source_set` and
226`static_library`), you should default on using `rtc_library` (which abstracts
227away the complexity of using the correct target type for Chromium component
228builds).
Karl Wiberg91d0ab72017-09-07 15:05:31229
Mirko Bonadeiab397dd2022-12-16 10:08:01230The general rule is for library targets is:
2311. Use `rtc_library`.
2322. If the library is a header only target use `rtc_source_set`.
2333. If you really need to generate a static library, use `rtc_static_library`
234 (same for shared libraries, in such case use `rtc_shared_library`).
235
236To ensure that all our [GN targets][gn-target] are built with the same
237configuration, only use the following [GN templates][gn-templ].
238
239| instead of | use |
240|------------------|-----------------------------------------------------------------------------------------|
241| `executable` | `rtc_executable` |
242| `shared_library` | `rtc_shared_library` |
243| `source_set` | `rtc_source_set` (only for header only libraries, for everything else use `rtc_library` |
244| `static_library` | `rtc_static_library` (use `rtc_library` unless you really need `rtc_static_library` |
245| `test` | `rtc_test` |
Karl Wiberg91d0ab72017-09-07 15:05:31246
Fanny Linderborg70efbb82021-04-23 14:59:29247
248[gn-templ]: https://gn.googlesource.com/gn/+/HEAD/docs/language.md#Templates
249[gn-target]: https://gn.googlesource.com/gn/+/HEAD/docs/language.md#Targets
Karl Wiberg91d0ab72017-09-07 15:05:31250
Per Kjellandera7f2d842018-01-10 15:54:53251### Target visibility and the native API
252
Fanny Linderborg70efbb82021-04-23 14:59:29253The [WebRTC-specific GN templates](#webrtc-gn-templates) declare build targets
254whose default `visibility` allows all other targets in the WebRTC tree (and no
255targets outside the tree) to depend on them.
Per Kjellandera7f2d842018-01-10 15:54:53256
Fanny Linderborg70efbb82021-04-23 14:59:29257Prefer to restrict the `visibility` if possible:
Per Kjellandera7f2d842018-01-10 15:54:53258
Fanny Linderborg70efbb82021-04-23 14:59:29259* If a target is used by only one or a tiny number of other targets, prefer to
260 list them explicitly: `visibility = [ ":foo", ":bar" ]`
Per Kjellandera7f2d842018-01-10 15:54:53261* If a target is used only by targets in the same `BUILD.gn` file:
262 `visibility = [ ":*" ]`.
263
Fanny Linderborg70efbb82021-04-23 14:59:29264Setting `visibility = [ "*" ]` means that targets outside the WebRTC tree can
265depend on this target; use this only for build targets whose headers are part of
266the [native WebRTC API](native-api.md).
Per Kjellandera7f2d842018-01-10 15:54:53267
Karl Wibergee3d7602017-09-04 12:46:47268### Conditional compilation with the C preprocessor
269
Fanny Linderborg70efbb82021-04-23 14:59:29270Avoid using the C preprocessor to conditionally enable or disable pieces of
271code. But if you can't avoid it, introduce a GN variable, and then set a
272preprocessor constant to either 0 or 1 in the build targets that need it:
Karl Wibergee3d7602017-09-04 12:46:47273
Fanny Linderborg70efbb82021-04-23 14:59:29274```gn
Karl Wibergee3d7602017-09-04 12:46:47275if (apm_debug_dump) {
276 defines = [ "WEBRTC_APM_DEBUG_DUMP=1" ]
277} else {
278 defines = [ "WEBRTC_APM_DEBUG_DUMP=0" ]
279}
280```
281
282In the C, C++, or Objective-C files, use `#if` when testing the flag,
283not `#ifdef` or `#if defined()`:
284
Fanny Linderborg70efbb82021-04-23 14:59:29285```c
Karl Wibergee3d7602017-09-04 12:46:47286#if WEBRTC_APM_DEBUG_DUMP
287// One way.
288#else
289// Or another.
290#endif
291```
292
Fanny Linderborg70efbb82021-04-23 14:59:29293When combined with the `-Wundef` compiler option, this produces compile time
294warnings if preprocessor symbols are misspelled, or used without corresponding
295build rules to set them.