Changeset View
Changeset View
Standalone View
Standalone View
doc/developer-notes.md
Show All 9 Lines | |||||
- Basic rules specified in [src/.clang-format](/src/.clang-format). | - Basic rules specified in [src/.clang-format](/src/.clang-format). | ||||
- Braces on new lines for namespaces, classes, functions, methods. | - Braces on new lines for namespaces, classes, functions, methods. | ||||
- Braces on the same line for everything else. | - Braces on the same line for everything else. | ||||
- 4 space indentation (no tabs) for every block except namespaces. | - 4 space indentation (no tabs) for every block except namespaces. | ||||
- No indentation for `public`/`protected`/`private` or for `namespace`. | - No indentation for `public`/`protected`/`private` or for `namespace`. | ||||
- No extra spaces inside parenthesis; don't do ( this ) | - No extra spaces inside parenthesis; don't do ( this ) | ||||
- No space after function names; one space after `if`, `for` and `while`. | - No space after function names; one space after `if`, `for` and `while`. | ||||
- If an `if` only has a single-statement then-clause, it can appear | - Always add braces for `if`, `for`, `while`, etc., statements. | ||||
jasonbcox: These are called "block statements". We can clarify this by saying:
- Always add braces for… | |||||
on the same line as the if, without braces. In every other case, | |||||
braces are required, and the then and else clauses must appear | |||||
correctly indented on a new line. | |||||
- `++i` is preferred over `i++`. | - `++i` is preferred over `i++`. | ||||
- Use CamelCase for functions/methods, and lowerCamelCase for variables. | |||||
- GLOBAL_CONSTANTS should use UPPER_SNAKE_CASE. | |||||
- namespaces should use lower_snake_case. | |||||
- Function names should generally start with an English command-form verb | |||||
(e.g. `ValidateTransaction`, `AddTransactionToMempool`, `ConnectBlock`) | |||||
- Variable names should generally be nouns or past/future tense verbs. | |||||
(e.g. `canDoThing`, `signatureOperations`, `didThing`) | |||||
- Avoid using globals, remove existing globals whenever possible. | |||||
- Class member variable names should be prepended with `m_` | |||||
- DO choose easily readable identifier names. | |||||
- DO favor readability over brevity. | |||||
- DO NOT use Hungarian notation. | |||||
- DO NOT use abbreviations or contractions within identifiers. | |||||
- WRONG: mempool | |||||
- RIGHT: MemoryPool | |||||
- WRONG: ChangeDir | |||||
- RIGHT: ChangeDirectory | |||||
- DO NOT use obscure acronyms, DO uppercase any acronyms. | |||||
- FINALLY, do not migrate existing code unless refactoring. It makes | |||||
forwarding-porting from Bitcoin Core more difficult. | |||||
Block style example: | The naming convention roughly mirrors [Microsoft Naming Conventions](https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/general-naming-conventions) | ||||
C++ Coding Standards should strive to follow the [LLVM Coding Standards](https://llvm.org/docs/CodingStandards.html) | |||||
Code style example: | |||||
```c++ | ```c++ | ||||
namespace foo | // namespaces should be lower_snake_case | ||||
{ | namespace foo_bar_bob { | ||||
class Class | |||||
{ | /** | ||||
bool Function(const std::string& s, int n) | * Class is used for doing classy things. All classes should | ||||
{ | * have a doxygen comment describing their PURPOSE. That is to say, | ||||
// Comment summarising what this section of code does | * why they exist. Functional details can be determined from the code. | ||||
* @see PerformTask() | |||||
*/ | |||||
class Class { | |||||
private: | |||||
//! memberVariable's name should be lowerCamelCase, and be a noun. | |||||
int m_memberVariable; | |||||
public: | |||||
/** | |||||
* The documentation before a function or class method should follow Doxygen | |||||
* spec. The name of the function should start with an english verb which | |||||
* indicates the intended purpose of this code. | |||||
* | |||||
* The function name should be should be CamelCase. | |||||
* | |||||
* @param[in] s A description | |||||
* @param[in] n Another argument description | |||||
* @pre Precondition for function... | |||||
*/ | |||||
bool PerformTask(const std::string& s, int n) { | |||||
// Use lowerChamelCase for local variables. | |||||
bool didMore = false; | |||||
// Comment summarizing the intended purpose of this section of code | |||||
for (int i = 0; i < n; ++i) { | for (int i = 0; i < n; ++i) { | ||||
// When something fails, return early | if (!DidSomethingFail()) { | ||||
if (!Something()) return false; | return false; | ||||
} | |||||
... | ... | ||||
if (SomethingElse()) { | if (IsSomethingElse()) { | ||||
DoMore(); | DoMore(); | ||||
didMore = true; | |||||
} else { | } else { | ||||
DoLess(); | DoLess(); | ||||
} | } | ||||
} | } | ||||
// Success return is usually at the end | return didMore; | ||||
return true; | |||||
} | } | ||||
} | } | ||||
} | } | ||||
``` | ``` | ||||
Doxygen comments | Doxygen comments | ||||
----------------- | ----------------- | ||||
To facilitate the generation of documentation, use doxygen-compatible comment blocks for functions, methods and fields. | To facilitate the generation of documentation, use doxygen-compatible comment blocks for functions, methods and fields. | ||||
For example, to describe a function use: | For example, to describe a function use: | ||||
```c++ | ```c++ | ||||
/** | /** | ||||
▲ Show 20 Lines • Show All 164 Lines • ▼ Show 20 Lines | |||||
and commit them. | and commit them. | ||||
Development guidelines | Development guidelines | ||||
============================ | ============================ | ||||
A few non-style-related recommendations for developers, as well as points to | A few non-style-related recommendations for developers, as well as points to | ||||
pay attention to for reviewers of Bitcoin Core code. | pay attention to for reviewers of Bitcoin Core code. | ||||
General Bitcoin Core | |||||
---------------------- | |||||
- New features should be exposed on RPC first, then can be made available in the GUI | |||||
- *Rationale*: RPC allows for better automatic testing. The test suite for | |||||
the GUI is very limited | |||||
- Make sure pull requests pass Travis CI before merging | |||||
- *Rationale*: Makes sure that they pass thorough testing, and that the tester will keep passing | |||||
on the master branch. Otherwise all new pull requests will start failing the tests, resulting in | |||||
confusion and mayhem | |||||
- *Explanation*: If the test suite is to be updated for a change, this has to | |||||
be done first | |||||
Wallet | Wallet | ||||
------- | ------- | ||||
- Make sure that no crashes happen with run-time option `-disablewallet`. | - Make sure that no crashes happen with run-time option `-disablewallet`. | ||||
- *Rationale*: In RPC code that conditionally uses the wallet (such as | - *Rationale*: In RPC code that conditionally uses the wallet (such as | ||||
`validateaddress`) it is easy to forget that global pointer `pwalletMain` | `validateaddress`) it is easy to forget that global pointer `pwalletMain` | ||||
can be NULL. See `test/functional/disablewallet.py` for functional tests | can be NULL. See `test/functional/disablewallet.py` for functional tests | ||||
▲ Show 20 Lines • Show All 60 Lines • ▼ Show 20 Lines | |||||
- Prefer explicit constructions over implicit ones that rely on 'magical' C++ behavior | - Prefer explicit constructions over implicit ones that rely on 'magical' C++ behavior | ||||
- *Rationale*: Easier to understand what is happening, thus easier to spot mistakes, even for those | - *Rationale*: Easier to understand what is happening, thus easier to spot mistakes, even for those | ||||
that are not language lawyers | that are not language lawyers | ||||
Strings and formatting | Strings and formatting | ||||
------------------------ | ------------------------ | ||||
- Be careful of `LogPrint` versus `LogPrintf`. `LogPrint` takes a `category` argument, `LogPrintf` does not. | |||||
- *Rationale*: Confusion of these can result in runtime exceptions due to | |||||
formatting mismatch, and it is easy to get wrong because of subtly similar naming | |||||
- Use `std::string`, avoid C string manipulation functions | - Use `std::string`, avoid C string manipulation functions | ||||
- *Rationale*: C++ string handling is marginally safer, less scope for | - *Rationale*: C++ string handling is marginally safer, less scope for | ||||
buffer overflows and surprises with `\0` characters. Also some C string manipulations | buffer overflows and surprises with `\0` characters. Also some C string manipulations | ||||
tend to act differently depending on platform, or even the user locale | tend to act differently depending on platform, or even the user locale | ||||
- Use `ParseInt32`, `ParseInt64`, `ParseUInt32`, `ParseUInt64`, `ParseDouble` from `utilstrencodings.h` for number parsing | - Use `ParseInt32`, `ParseInt64`, `ParseUInt32`, `ParseUInt64`, `ParseDouble` from `utilstrencodings.h` for number parsing | ||||
- *Rationale*: These functions do overflow checking, and avoid pesky locale issues | - *Rationale*: These functions do overflow checking, and avoid pesky locale issues | ||||
- For `strprintf`, `LogPrint`, `LogPrintf` formatting characters don't need size specifiers | |||||
- *Rationale*: Bitcoin Core uses tinyformat, which is type safe. Leave them out to avoid confusion | |||||
Variable names | Variable names | ||||
-------------- | -------------- | ||||
The shadowing warning (`-Wshadow`) is enabled by default. It prevents issues rising | The shadowing warning (`-Wshadow`) is enabled by default. It prevents issues rising | ||||
from using a different variable with the same name. | from using a different variable with the same name. | ||||
Please name variables so that their names do not shadow variables defined in the source code. | Please name variables so that their names do not shadow variables defined in the source code. | ||||
E.g. in member initializers, prepend `_` to the argument name shadowing the | |||||
member name: | |||||
```c++ | |||||
class AddressBookPage | |||||
{ | |||||
Mode mode; | |||||
} | |||||
AddressBookPage::AddressBookPage(Mode _mode) : | |||||
mode(_mode) | |||||
... | |||||
``` | |||||
When using nested cycles, do not name the inner cycle variable the same as in | |||||
upper cycle etc. | |||||
Threads and synchronization | Threads and synchronization | ||||
---------------------------- | ---------------------------- | ||||
- Build and run tests with `-DDEBUG_LOCKORDER` to verify that no potential | - Build and run tests with `-DDEBUG_LOCKORDER` to verify that no potential | ||||
deadlocks are introduced. As of 0.12, this is defined by default when | deadlocks are introduced. As of 0.12, this is defined by default when | ||||
configuring with `--enable-debug` | configuring with `--enable-debug` | ||||
- When using `LOCK`/`TRY_LOCK` be aware that the lock exists in the context of | - When using `LOCK`/`TRY_LOCK` be aware that the lock exists in the context of | ||||
▲ Show 20 Lines • Show All 127 Lines • Show Last 20 Lines |
These are called "block statements". We can clarify this by saying: