summaryrefslogtreecommitdiff
path: root/CPP_STYLE_GUIDE.md
blob: 30a2ec99fa087b9a76df730cb3f8e31fa1a1c79c (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
# C++ Style Guide

## Table of Contents

* [Guides and References](#guides-and-references)
* [Formatting](#formatting)
  * [Left-leaning (C++ style) asterisks for pointer declarations](#left-leaning-c-style-asterisks-for-pointer-declarations)
  * [C++ style comments](#c-style-comments)
  * [2 spaces of indentation for blocks or bodies of conditionals](#2-spaces-of-indentation-for-blocks-or-bodies-of-conditionals)
  * [4 spaces of indentation for statement continuations](#4-spaces-of-indentation-for-statement-continuations)
  * [Align function arguments vertically](#align-function-arguments-vertically)
  * [Initialization lists](#initialization-lists)
  * [CamelCase for methods, functions, and classes](#camelcase-for-methods-functions-and-classes)
  * [snake\_case for local variables and parameters](#snake_case-for-local-variables-and-parameters)
  * [snake\_case\_ for private class fields](#snake_case_-for-private-class-fields)
  * [snake\_case for C-like structs](#snake_case-for-c-like-structs)
  * [Space after `template`](#space-after-template)
* [Memory Management](#memory-management)
  * [Memory allocation](#memory-allocation)
  * [Use `nullptr` instead of `NULL` or `0`](#use-nullptr-instead-of-null-or-0)
  * [Use explicit pointer comparisons](#use-explicit-pointer-comparisons)
  * [Ownership and Smart Pointers](#ownership-and-smart-pointers)
  * [Avoid non-const references](#avoid-non-const-references)
  * [Use AliasedBuffers to manipulate TypedArrays](#use-aliasedbuffers-to-manipulate-typedarrays)
* [Others](#others)
  * [Type casting](#type-casting)
  * [Using `auto`](#using-auto)
  * [Do not include `*.h` if `*-inl.h` has already been included](#do-not-include-h-if--inlh-has-already-been-included)
  * [Avoid throwing JavaScript errors in C++ methods](#avoid-throwing-javascript-errors-in-c)
    * [Avoid throwing JavaScript errors in nested C++ methods](#avoid-throwing-javascript-errors-in-nested-c-methods)

## Guides and References

The Node.js C++ codebase strives to be consistent in its use of language
features and idioms, as well as have some specific guidelines for the use of
runtime features.

Coding guidelines are based on the following guides (highest priority first):

1. This document
2. The [Google C++ Style Guide][]
3. The ISO [C++ Core Guidelines][]

In general code should follow the C++ Core Guidelines, unless overridden by the
Google C++ Style Guide or this document. At the moment these guidelines are
checked manually by reviewers, with the goal to validate this with automatic
tools.

## Formatting

Unfortunately, the C++ linter (based on [Google’s `cpplint`][]), which can be
run explicitly via `make lint-cpp`, does not currently catch a lot of rules that
are specific to the Node.js C++ code base. This document explains the most
common of these rules:

### Left-leaning (C++ style) asterisks for pointer declarations

`char* buffer;` instead of `char *buffer;`

### C++ style comments

Use C++ style comments (`//`) for both single-line and multi-line comments.
Comments should also start with uppercase and finish with a dot.

Examples:

```c++
// A single-line comment.

// Multi-line comments
// should also use C++
// style comments.
```

The codebase may contain old C style comments (`/* */`) from before this was the
preferred style. Feel free to update old comments to the preferred style when
working on code in the immediate vicinity or when changing/improving those
comments.

### 2 spaces of indentation for blocks or bodies of conditionals

```c++
if (foo)
  bar();
```

or

```c++
if (foo) {
  bar();
  baz();
}
```

Braces are optional if the statement body only has one line.

`namespace`s receive no indentation on their own.

### 4 spaces of indentation for statement continuations

```c++
VeryLongTypeName very_long_result = SomeValueWithAVeryLongName +
    SomeOtherValueWithAVeryLongName;
```

Operators are before the line break in these cases.

### Align function arguments vertically

```c++
void FunctionWithAVeryLongName(int parameter_with_a_very_long_name,
                               double other_parameter_with_a_very_long_name,
                               ...);
```

If that doesn’t work, break after the `(` and use 4 spaces of indentation:

```c++
void FunctionWithAReallyReallyReallyLongNameSeriouslyStopIt(
    int okay_there_is_no_space_left_in_the_previous_line,
    ...);
```

### Initialization lists

Long initialization lists are formatted like this:

```c++
HandleWrap::HandleWrap(Environment* env,
                       Local<Object> object,
                       uv_handle_t* handle,
                       AsyncWrap::ProviderType provider)
    : AsyncWrap(env, object, provider),
      state_(kInitialized),
      handle_(handle) {
```

### CamelCase for methods, functions, and classes

Exceptions are simple getters/setters, which are named `property_name()` and
`set_property_name()`, respectively.

```c++
class FooBar {
 public:
  void DoSomething();
  static void DoSomethingButItsStaticInstead();

  void set_foo_flag(int flag_value);
  int foo_flag() const;  // Use const-correctness whenever possible.
};
```

### snake\_case for local variables and parameters

```c++
int FunctionThatDoesSomething(const char* important_string) {
  const char* pointer_into_string = important_string;
}
```

### snake\_case\_ for private class fields

```c++
class Foo {
 private:
  int counter_ = 0;
};
```

### snake\_case for C-like structs
For plain C-like structs snake_case can be used.

```c++
struct foo_bar {
  int name;
}
```

### Space after `template`

```c++
template <typename T>
class FancyContainer {
 ...
}
```

## Memory Management

### Memory allocation

* `Malloc()`, `Calloc()`, etc. from `util.h` abort in Out-of-Memory situations
* `UncheckedMalloc()`, etc. return `nullptr` in OOM situations

### Use `nullptr` instead of `NULL` or `0`

Further reading in the [C++ Core Guidelines][ES.47].

### Use explicit pointer comparisons

Use explicit comparisons to `nullptr` when testing pointers, i.e.
`if (foo == nullptr)` instead of `if (foo)` and
`foo != nullptr` instead of `!foo`.

### Ownership and Smart Pointers

* [R.20]: Use `std::unique_ptr` or `std::shared_ptr` to represent ownership
* [R.21]: Prefer `unique_ptr` over `shared_ptr` unless you need to share
  ownership

Use `std::unique_ptr` to make ownership transfer explicit. For example:

```cpp
std::unique_ptr<Foo> FooFactory();
void FooConsumer(std::unique_ptr<Foo> ptr);
```

Since `std::unique_ptr` has only move semantics, passing one by value transfers
ownership to the callee and invalidates the caller's instance.

Don't use `std::auto_ptr`, it is deprecated ([Reference][cppref_auto_ptr]).

### Avoid non-const references

Using non-const references often obscures which values are changed by an
assignment. Consider using a pointer instead, which requires more explicit
syntax to indicate that modifications take place.

```c++
class ExampleClass {
 public:
  explicit ExampleClass(OtherClass* other_ptr) : pointer_to_other_(other_ptr) {}

  void SomeMethod(const std::string& input_param,
                  std::string* in_out_param);  // Pointer instead of reference

  const std::string& get_foo() const { return foo_string_; }
  void set_foo(const std::string& new_value) { foo_string_ = new_value; }

  void ReplaceCharacterInFoo(char from, char to) {
    // A non-const reference is okay here, because the method name already tells
    // users that this modifies 'foo_string_' -- if that is not the case,
    // it can still be better to use an indexed for loop, or leave appropriate
    // comments.
    for (char& character : foo_string_) {
      if (character == from)
        character = to;
    }
  }

 private:
  std::string foo_string_;
  // Pointer instead of reference. If this object 'owns' the other object,
  // this should be a `std::unique_ptr<OtherClass>`; a
  // `std::shared_ptr<OtherClass>` can also be a better choice.
  OtherClass* pointer_to_other_;
};
```

### Use AliasedBuffers to manipulate TypedArrays

When working with typed arrays that involve direct data modification
from C++, use an `AliasedBuffer` when possible. The API abstraction and
the usage scope of `AliasedBuffer` are documented in [aliased_buffer.h][].

```c++
// Create an AliasedBuffer.
AliasedBuffer<uint32_t, v8::Uint32Array> data;
...

// Modify the data through natural operator semantics.
data[0] = 12345;
```

## Others

### Type casting

* Use `static_cast<T>` if casting is required, and it is valid
* Use `reinterpret_cast` only when it is necessary
* Avoid C-style casts (`(type)value`)
* `dynamic_cast` does not work because Node.js is built without
  [Run Time Type Information][]

Further reading:

* [ES.48]: Avoid casts
* [ES.49]: If you must use a cast, use a named cast

### Using `auto`

Being explicit about types is usually preferred over using `auto`.

Use `auto` to avoid type names that are noisy, obvious, or unimportant. When
doing so, keep in mind that explicit types often help with readability and
verifying the correctness of code.

```cpp
for (const auto& item : some_map) {
  const KeyType& key = item.first;
  const ValType& value = item.second;
  // The rest of the loop can now just refer to key and value,
  // a reader can see the types in question, and we've avoided
  // the too-common case of extra copies in this iteration.
}
```

### Do not include `*.h` if `*-inl.h` has already been included

Do

```cpp
#include "util-inl.h"  // already includes util.h
```

instead of

```cpp
#include "util.h"
#include "util-inl.h"
```

### Avoid throwing JavaScript errors in C++

When there is a need to throw errors from a C++ binding method, try to
return the data necessary for constructing the errors to JavaScript,
then construct and throw the errors [using `lib/internal/errors.js`][errors].

In general, type-checks on arguments should be done in JavaScript
before the arguments are passed into C++. Then in the C++ binding, simply using
`CHECK` assertions to guard against invalid arguments should be enough.

If the return value of the binding cannot be used to signal failures or return
the necessary data for constructing errors in JavaScript, pass a context object
to the binding and put the necessary data inside in C++. For example:

```cpp
void Foo(const FunctionCallbackInfo<Value>& args) {
  Environment* env = Environment::GetCurrent(args);
  // Let the JavaScript handle the actual type-checking,
  // only assertions are placed in C++
  CHECK_EQ(args.Length(), 2);
  CHECK(args[0]->IsString());
  CHECK(args[1]->IsObject());

  int err = DoSomethingWith(args[0].As<String>());
  if (err) {
    // Put the data inside the error context
    Local<Object> ctx = args[1].As<Object>();
    Local<String> key = FIXED_ONE_BYTE_STRING(env->isolate(), "code");
    ctx->Set(env->context(), key, err).FromJust();
  } else {
    args.GetReturnValue().Set(something_to_return);
  }
}

// In the initialize function
env->SetMethod(target, "foo", Foo);
```

```js
exports.foo = function(str) {
  // Prefer doing the type-checks in JavaScript
  if (typeof str !== 'string') {
    throw new errors.codes.ERR_INVALID_ARG_TYPE('str', 'string');
  }

  const ctx = {};
  const result = binding.foo(str, ctx);
  if (ctx.code !== undefined) {
    throw new errors.codes.ERR_ERROR_NAME(ctx.code);
  }
  return result;
};
```

#### Avoid throwing JavaScript errors in nested C++ methods

When you need to throw a JavaScript exception from C++ (i.e.
`isolate()->ThrowException()`) prefer to do it as close to the return to JS as
possible, and not inside of nested C++ calls. Since this changes the JS
execution state doing it closest to where it is consumed reduces the chances of
side effects.

Node.js is built [without C++ exception handling][], so code using `throw` or
even `try` and `catch` **will** break.

[C++ Core Guidelines]: http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines
[Google C++ Style Guide]: https://google.github.io/styleguide/cppguide.html
[Google’s `cpplint`]: https://github.com/google/styleguide
[errors]: https://github.com/nodejs/node/blob/master/doc/guides/using-internal-errors.md
[ES.47]: http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-nullptr
[ES.48]: http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-casts
[ES.49]: http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-casts-named
[R.20]: http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-owner
[R.21]: http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-unique
[Run Time Type Information]: https://en.wikipedia.org/wiki/Run-time_type_information
[cppref_auto_ptr]: https://en.cppreference.com/w/cpp/memory/auto_ptr
[without C++ exception handling]: https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_exceptions.html#intro.using.exception.no
[aliased_buffer.h]: https://github.com/nodejs/node/blob/master/src/aliased_buffer.h#L12