admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Enhance the readability of the style guide.

Browse Source 
* Make links more accessible for screen readers.
* Markup codeblocks with the source language.
* Markup tables with vertical bars on the leftmost and rightmost sides.
* Remove explicit formatting of headings.
* Update broken links according to the current location of the sites.
* Use a line width of 80 characters where possible.
* Use backticks to markup code.

Bug: None
Change-Id: I6177b379fc7a4f457de7729cbec1a25ba0390234
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/215974
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33820}
This commit is contained in:
Fanny Linderborg 2021-04-23 14:59:29 +00:00 committed by Commit Bot
parent e8080f4a43
commit 70efbb839b
1 changed files with 131 additions and 127 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

258
style-guide.md
View File

@ -1,20 +1,20 @@
# WebRTC coding style guide
## **General advice**
## General advice
Some older parts of the code violate the style guide in various ways.
* If making small changes to such code, follow the style guide when
it’s reasonable to do so, but in matters of formatting etc., it is
often better to be consistent with the surrounding code.
* If making large changes to such code, consider first cleaning it up
in a separate CL.
* If making small changes to such code, follow the style guide when it's
reasonable to do so, but in matters of formatting etc., it is often better to
be consistent with the surrounding code.
* If making large changes to such code, consider first cleaning it up in a
separate CL.
## **C++**
## C++
WebRTC follows the [Chromium][chr-style] and [Google][goog-style] C++
style guides. In cases where they conflict, the Chromium style guide
trumps the Google style guide, and the rules in this file trump them
WebRTC follows the [Chromium C++ style guide][chr-style] and the
[Google C++ style guide][goog-style]. In cases where they conflict, the Chromium
style guide trumps the Google style guide, and the rules in this file trump them
both.
[chr-style]: https://chromium.googlesource.com/chromium/src/+/HEAD/styleguide/c++/c++.md
@ -24,10 +24,10 @@ both.
WebRTC is written in C++14, but with some restrictions:
* We only allow the subset of C++14 (language and library) that is not
banned by Chromium; see [this page][chromium-cpp].
* We only allow the subset of C++14 that is also valid C++17;
otherwise, users would not be able to compile WebRTC in C++17 mode.
* We only allow the subset of C++14 (language and library) that is not banned by
Chromium; see the [list of banned C++ features in Chromium][chromium-cpp].
* We only allow the subset of C++14 that is also valid C++17; otherwise, users
would not be able to compile WebRTC in C++17 mode.
[chromium-cpp]: https://chromium-cpp.appspot.com/
@ -37,39 +37,39 @@ do not yet support them.
### Abseil
You may use a subset of the utilities provided by the [Abseil][abseil]
library when writing WebRTC C++ code. [Details](abseil-in-webrtc.md).
You may use a subset of the utilities provided by the [Abseil][abseil] library
when writing WebRTC C++ code; see the
[instructions on how to use Abseil in WebRTC](abseil-in-webrtc.md).
[abseil]: https://abseil.io/about/
### <a name="h-cc-pairs"></a>`.h` and `.cc` files come in pairs
`.h` and `.cc` files should come in pairs, with the same name (except
for the file type suffix), in the same directory, in the same build
target.
`.h` and `.cc` files should come in pairs, with the same name (except for the
file type suffix), in the same directory, in the same build target.
* If a declaration in `path/to/foo.h` has a definition in some `.cc`
file, it should be in `path/to/foo.cc`.
* If a definition in `path/to/foo.cc` file has a declaration in some
`.h` file, it should be in `path/to/foo.h`.
* Omit the `.cc` file if it would have been empty, but still list the
`.h` file in a build target.
* Omit the `.h` file if it would have been empty. (This can happen
with unit test `.cc` files, and with `.cc` files that define
`main`.)
* If a declaration in `path/to/foo.h` has a definition in some `.cc` file, it
should be in `path/to/foo.cc`.
* If a definition in `path/to/foo.cc` file has a declaration in some `.h` file,
it should be in `path/to/foo.h`.
* Omit the `.cc` file if it would have been empty, but still list the `.h` file
in a build target.
* Omit the `.h` file if it would have been empty. (This can happen with unit
test `.cc` files, and with `.cc` files that define `main`.)
This makes the source code easier to navigate and organize, and
precludes some questionable build system practices such as having
build targets that don’t pull in definitions for everything they
declare.
See also the
[examples and exceptions on how to treat `.h` and `.cpp` files](style-guide/h-cc-pairs.md).
[Examples and exceptions](style-guide/h-cc-pairs.md).
This makes the source code easier to navigate and organize, and precludes some
questionable build system practices such as having build targets that don't pull
in definitions for everything they declare.
### TODO comments
### `TODO` comments
Follow the [Google style][goog-style-todo]. When referencing a WebRTC bug,
prefer the url form, e.g.
```
Follow the [Google styleguide for `TODO` comments][goog-style-todo]. When
referencing a WebRTC bug, prefer the url form, e.g.
```cpp
// TODO(bugs.webrtc.org/12345): Delete the hack when blocking bugs are resolved.
```
@ -77,22 +77,23 @@ prefer the url form, e.g.
### Deprecation
Annotate the declarations of deprecated functions and classes with
[ABSL_DEPRECATED][ABSL_DEPRECATED] to cause an error when they're used inside
webrtc and a compiler warning when they're used by dependant projects. Like so:
Annotate the declarations of deprecated functions and classes with the
[`ABSL_DEPRECATED` macro][ABSL_DEPRECATED] to cause an error when they're used
inside WebRTC and a compiler warning when they're used by dependant projects.
Like so:
```
```cpp
ABSL_DEPRECATED("bugs.webrtc.org/12345")
std::pony PonyPlz(const std::pony_spec& ps);
```
NOTE 1: The annotation goes on the declaration in the .h file, not the
definition in the .cc file!
NOTE 1: The annotation goes on the declaration in the `.h` file, not the
definition in the `.cc` file!
NOTE 2: In order to have unit tests that use the deprecated function without
getting errors, do something like this:
```
```cpp
std::pony DEPRECATED_PonyPlz(const std::pony_spec& ps);
ABSL_DEPRECATED("bugs.webrtc.org/12345")
inline std::pony PonyPlz(const std::pony_spec& ps) {
@ -102,153 +103,156 @@ inline std::pony PonyPlz(const std::pony_spec& ps) {
In other words, rename the existing function, and provide an inline wrapper
using the original name that calls it. That way, callers who are willing to
call it using the DEPRECATED_-prefixed name don't get the warning.
call it using the `DEPRECATED_`-prefixed name don't get the warning.
[ABSL_DEPRECATED]: https://source.chromium.org/chromium/chromium/src/+/master:third_party/abseil-cpp/absl/base/attributes.h?q=ABSL_DEPRECATED
### ArrayView
When passing an array of values to a function, use `rtc::ArrayView`
whenever possible—that is, whenever you’re not passing ownership of
the array, and don’t allow the callee to change the array size.
whenever possible—that is, whenever you're not passing ownership of
the array, and don't allow the callee to change the array size.
For example,
instead of | use
------------------------------------|---------------------
`const std::vector<T>&` | `ArrayView<const T>`
`const T* ptr, size_t num_elements` | `ArrayView<const T>`
`T* ptr, size_t num_elements` | `ArrayView<T>`
| instead of | use |
|-------------------------------------|----------------------|
| `const std::vector<T>&` | `ArrayView<const T>` |
| `const T* ptr, size_t num_elements` | `ArrayView<const T>` |
| `T* ptr, size_t num_elements` | `ArrayView<T>` |
See [the source](api/array_view.h) for more detailed docs.
See the [source code for `rtc::ArrayView`](api/array_view.h) for more detailed
docs.
### sigslot
SIGSLOT IS DEPRECATED.
Prefer webrtc::CallbackList, and manage thread safety yourself.
Prefer `webrtc::CallbackList`, and manage thread safety yourself.
### Smart pointers
The following smart pointer types are recommended:
* std::unique_ptr for all singly-owned objects
* rtc::scoped_refptr for all objects with shared ownership
* `std::unique_ptr` for all singly-owned objects
* `rtc::scoped_refptr` for all objects with shared ownership
Use of std::shared_ptr is *not permitted*. It is
[banned](https://chromium-cpp.appspot.com/#library-blocklist) in the Chromium
style guide (overriding the Google style guide), and offers no compelling
advantage over rtc::scoped_refptr (which is cloned from the corresponding
Chromium type).
Use of `std::shared_ptr` is *not permitted*. It is banned in the Chromium style
guide (overriding the Google style guide), and offers no compelling advantage
over `rtc::scoped_refptr` (which is cloned from the corresponding Chromium
type). See the
[list of banned C++ library features in Chromium][chr-std-shared-ptr] for more
information.
In most cases, one will want to explicitly control lifetimes, and therefore
use std::unique_ptr, but in some cases, for instance where references have
to exist both from the API users and internally, with no way to
invalidate pointers held by the API user, rtc::scoped_refptr can be
appropriate.
In most cases, one will want to explicitly control lifetimes, and therefore use
`std::unique_ptr`, but in some cases, for instance where references have to
exist both from the API users and internally, with no way to invalidate pointers
held by the API user, `rtc::scoped_refptr` can be appropriate.
### std::bind
[chr-std-shared-ptr]: https://chromium-cpp.appspot.com/#library-blocklist
Don’t use `std::bind`—there are pitfalls, and lambdas are almost as
succinct and already familiar to modern C++ programmers.
### `std::bind`
### std::function
Don't use `std::bind`—there are pitfalls, and lambdas are almost as succinct and
already familiar to modern C++ programmers.
`std::function` is allowed, but remember that it’s not the right tool
for every occasion. Prefer to use interfaces when that makes sense,
and consider `rtc::FunctionView` for cases where the callee will not
save the function object.
### `std::function`
`std::function` is allowed, but remember that it's not the right tool for every
occasion. Prefer to use interfaces when that makes sense, and consider
`rtc::FunctionView` for cases where the callee will not save the function
object.
### Forward declarations
WebRTC follows the [Google][goog-forward-declarations] C++ style guide
with respect to forward declarations. In summary: avoid using forward
declarations where possible; just `#include` the headers you need.
WebRTC follows the
[Google C++ style guide on forward declarations][goog-forward-declarations].
In summary: avoid using forward declarations where possible; just `#include` the
headers you need.
[goog-forward-declarations]: https://google.github.io/styleguide/cppguide.html#Forward_Declarations
## **C**
## C
There’s a substantial chunk of legacy C code in WebRTC, and a lot of
it is old enough that it violates the parts of the C++ style guide
that also applies to C (naming etc.) for the simple reason that it
pre-dates the use of the current C++ style guide for this code base.
There's a substantial chunk of legacy C code in WebRTC, and a lot of it is old
enough that it violates the parts of the C++ style guide that also applies to C
(naming etc.) for the simple reason that it pre-dates the use of the current C++
style guide for this code base.
* If making small changes to C code, mimic the style of the
surrounding code.
* If making large changes to C code, consider converting the whole
thing to C++ first.
* If making small changes to C code, mimic the style of the surrounding code.
* If making large changes to C code, consider converting the whole thing to C++
first.
## **Java**
## Java
WebRTC follows the [Google Java style guide][goog-java-style].
[goog-java-style]: https://google.github.io/styleguide/javaguide.html
## **Objective-C and Objective-C++**
## Objective-C and Objective-C++
WebRTC follows the
[Chromium Objective-C and Objective-C++ style guide][chr-objc-style].
[chr-objc-style]: https://chromium.googlesource.com/chromium/src/+/HEAD/styleguide/objective-c/objective-c.md
## **Python**
## Python
WebRTC follows [Chromium’s Python style][chr-py-style].
WebRTC follows [Chromium's Python style][chr-py-style].
[chr-py-style]: https://chromium.googlesource.com/chromium/src/+/HEAD/styleguide/styleguide.md#python
[chr-py-style]: https://chromium.googlesource.com/chromium/src/+/HEAD/styleguide/python/python.md
## **Build files**
## Build files
The WebRTC build files are written in [GN][gn], and we follow
the [Chromium GN style guide][chr-gn-style]. Additionally, there are
some WebRTC-specific rules below; in case of conflict, they trump the
Chromium style guide.
The WebRTC build files are written in [GN][gn], and we follow the
[GN style guide][gn-style]. Additionally, there are some
WebRTC-specific rules below; in case of conflict, they trump the Chromium style
guide.
[gn]: https://chromium.googlesource.com/chromium/src/tools/gn/
[chr-gn-style]: https://chromium.googlesource.com/chromium/src/tools/gn/+/HEAD/docs/style_guide.md
[gn]: https://gn.googlesource.com/gn/
[gn-style]: https://gn.googlesource.com/gn/+/HEAD/docs/style_guide.md
### <a name="webrtc-gn-templates"></a>WebRTC-specific GN templates
Use the following [GN templates][gn-templ] to ensure that all
our [targets][gn-target] are built with the same configuration:
Use the following [GN templates][gn-templ] to ensure that all our
[GN targets][gn-target] are built with the same configuration:
instead of | use
-----------------|---------------------
`executable` | `rtc_executable`
`shared_library` | `rtc_shared_library`
`source_set` | `rtc_source_set`
`static_library` | `rtc_static_library`
`test` | `rtc_test`
| instead of | use |
|------------------|----------------------|
| `executable` | `rtc_executable` |
| `shared_library` | `rtc_shared_library` |
| `source_set` | `rtc_source_set` |
| `static_library` | `rtc_static_library` |
| `test` | `rtc_test` |
[gn-templ]: https://chromium.googlesource.com/chromium/src/tools/gn/+/HEAD/docs/language.md#Templates
[gn-target]: https://chromium.googlesource.com/chromium/src/tools/gn/+/HEAD/docs/language.md#Targets
[gn-templ]: https://gn.googlesource.com/gn/+/HEAD/docs/language.md#Templates
[gn-target]: https://gn.googlesource.com/gn/+/HEAD/docs/language.md#Targets
### Target visibility and the native API
The [WebRTC-specific GN templates](#webrtc-gn-templates) declare build
targets whose default `visibility` allows all other targets in the
WebRTC tree (and no targets outside the tree) to depend on them.
The [WebRTC-specific GN templates](#webrtc-gn-templates) declare build targets
whose default `visibility` allows all other targets in the WebRTC tree (and no
targets outside the tree) to depend on them.
Prefer to restrict the visibility if possible:
Prefer to restrict the `visibility` if possible:
* If a target is used by only one or a tiny number of other targets,
prefer to list them explicitly: `visibility = [ ":foo", ":bar" ]`
* If a target is used by only one or a tiny number of other targets, prefer to
list them explicitly: `visibility = [ ":foo", ":bar" ]`
* If a target is used only by targets in the same `BUILD.gn` file:
`visibility = [ ":*" ]`.
Setting `visibility = [ "*" ]` means that targets outside the WebRTC
tree can depend on this target; use this only for build targets whose
headers are part of the [native API](native-api.md).
Setting `visibility = [ "*" ]` means that targets outside the WebRTC tree can
depend on this target; use this only for build targets whose headers are part of
the [native WebRTC API](native-api.md).
### Conditional compilation with the C preprocessor
Avoid using the C preprocessor to conditionally enable or disable
pieces of code. But if you can’t avoid it, introduce a GN variable,
and then set a preprocessor constant to either 0 or 1 in the build
targets that need it:
Avoid using the C preprocessor to conditionally enable or disable pieces of
code. But if you can't avoid it, introduce a GN variable, and then set a
preprocessor constant to either 0 or 1 in the build targets that need it:
```
```gn
if (apm_debug_dump) {
defines = [ "WEBRTC_APM_DEBUG_DUMP=1" ]
} else {
@ -259,7 +263,7 @@ if (apm_debug_dump) {
In the C, C++, or Objective-C files, use `#if` when testing the flag,
not `#ifdef` or `#if defined()`:
```
```c
#if WEBRTC_APM_DEBUG_DUMP
// One way.
#else
@ -267,6 +271,6 @@ not `#ifdef` or `#if defined()`:
#endif
```
When combined with the `-Wundef` compiler option, this produces
compile time warnings if preprocessor symbols are misspelled, or used
without corresponding build rules to set them.
When combined with the `-Wundef` compiler option, this produces compile time
warnings if preprocessor symbols are misspelled, or used without corresponding
build rules to set them.