CodingStyle.md

   1 # Coding style guide
   2
   3 It is said that there is no accounting for taste. However, when it comes to code, we are of the opinion that a _consistent_ style makes it easier to collaborate in a shared code base.
   4
   5 Style guidelines are bound to be controversial. Some conventions laid out in this guide have objective merit. However, most boil down to personal preferences of the original authors.
   6
   7 As such, this guide attempts to lay out the conventions used in the project so that new contributors can easily conform to them and minimize time lost during code review.
   8
   9 Contributions are expected to adhere to these guidelines.
  10
  11 ## Migration from C
  12
  13 As the LTTng-tools project aims at supporting a broad range of compilers -- currently starting from GCC 4.8 and Clang 3.3 -- its build system is configured to use the C++11 standard.
  14
  15 LTTng-tools has historically been developped in C99 with liberal uses of GNU extensions. Since the release of LTTng 2.13, it has started a migration to C++.
  16
  17 In order to ease the transition, it underwent an initial migration phase which had a limited scope: build all existing C code as C++11.
  18
  19 As such, most of the project's code does not qualify as idiomatic C++ code. This migration is ongoing and expected to span across multiple release cycles.
  20
  21 However, new contributions are expected to conform the C++ style described in this guide. Some exceptions are allowed for small fixes which have to be back-ported to stable branches.
  22
  23 ## Automated formatting
  24
  25 All the project's C++ files follow the [clang-format](https://clang.llvm.org/docs/ClangFormat.html) [style](https://clang.llvm.org/docs/ClangFormatStyleOptions.html) of the `.clang-format` file for whitespaces, indentation, and line breaks.
  26
  27 You _must_ format your changes with clang-format before you contribute a patch.
  28
  29 Note that clang-format 14 is required to use the project's `.clang-format` file.
  30
  31 Most text editors allow you to format a sub-section of a source file using clang-format to ensure it adheres to the project's style.
  32
  33 If you are submitting a change to existing source files, __do not run clang-format on the whole file_ as this may introduce more changes than you intended and _will_ cause your changes to be rejected.
  34
  35 ## Tabs VS Spaces
  36
  37 While our founding mothers and fathers eschewed any consideration for non-English languages when designing the ASCII character encoding they, in a rare moment of technical decadence, decided to dedicate a character to the sole purpose of expressing tabulations.
  38
  39 This project makes use of this character to express indentations in its source files.
  40
  41 Note that while tab characters are used for semantic indentation purposes, spaces are perfectly fine to use for _visual_ alignment (e.g. ascii diagrams).
  42
  43 ## Single line control flow statements
  44
  45 Single line control flow statements (if/for/while) are required to use braces.
  46
  47 ```cpp
  48 /* bad */
  49 if  (my_thingy)
  50         do_the_thing();
  51
  52 /* good */
  53 if  (my_thingy)  {
  54         do_the_thing();
  55 }
  56 ```
  57
  58 ## Naming
  59
  60 - Use snake case (e.g. `a_snake_case_name`) except for template parameters, which use camel case and end with `Type` (e.g. `ACamelCaseNameType`).
  61
  62 - Prefer using explicit and verbose names. For instance:
  63   - When naming a variable that indicates a count of bananas, prefer `banana_count` to `bananas`, `count`, or `n`.
  64   - When naming a function or method that validates and initializes a user profile, prefer `validate_and_initialize_user_profile()` to `set_up()`, `viup()`, `do_user_profile()`, `init()`.
  65
  66 - Avoid the use of overly generic terms like `data`, `ptr`, and `buffer`.
  67
  68 - Use an underscore prefix for private or protected methods and members, and member type names: `_try_connect()`, `class _user_count`, `int _version`.
  69
  70 - Name trivial setters and getters like the property name, without a verb (e.g. `set` and `get` prefixes).
  71
  72   ```cpp
  73   /* good, gets the session's name. */
  74   session.name();
  75   /* good, sets the session's name. */
  76   session.name("my new name");
  77
  78   /* good, non-trivial accessor */
  79   session.add_channel(my_channel);
  80   ```
  81
  82 - Use the `is` or `has` prefixes to name boolean properties or functions which return a `bool` type.
  83
  84 - Do not make-up abbreviations to shorten names. Term of art abbreviations are, however, acceptable. For example: `mpeg`, `ctf`, `cfg`, `init` are accepted. A notable exception to this rule applies to namespaces, see the "Use of namespaces/Aliases" section.
  85
  86 ## Comments
  87
  88 In general, comments should focus on _why_ something is done and document the assumptions the code was built upon. They should not repeat what it does in plain english except if the code is particularily complex. Keep in mind that what may be obvious to you right now may not be obvious to reviewers... or your future self.
  89
  90 Also, write comments in grammatically-sound English and avoid writing using telegraph style:
  91
  92 ```cpp
  93 /* Bad: init cfg */
  94
  95 /* Bad: init cfg before reply */
  96
  97 /* Good: The configuration must be initialized before replying since it initializes the user's credentials. */
  98 ```
  99
 100 ## Include guards
 101
 102 Header files must use include guards to prevent multiple inclusion issues. To avoid collisions, the name of include guards must be as specific as possible and include the name of the file.
 103
 104 ```cpp
 105 /* In file thingy-handler.hpp */
 106
 107 #ifndef LTTNG_CONSUMERD_THINGY_HANDLER
 108 #define LTTNG_CONSUMERD_THINGY_HANDLER
 109
 110 /* Some briliant code. */
 111
 112 #endif  /* LTTNG_CONSUMERD_THINGY_HANDLER */
 113 ```
 114
 115
 116 ## Use of namespaces
 117
 118 Make liberal use of namespaces. Very little should be available in the `lttng`,
 119 let alone global, namespace.
 120
 121 Moreover, prefer to use anonymous namespaces to the `static`  keyword to restrict the visibility of a symbol to its translation unit.
 122
 123 ### Do not pollute the global namespace
 124
 125 Never use the `using` directive to import the contents of a namespace. If a namespace is used often in a file, define an alias.
 126
 127 ### Aliases
 128
 129 Within a translation unit, it is acceptable to abbreviate commonly-used namespace names to define an alias. For instance, the file containing the implementation of the `food::fruits::citrus::grapefruit` can use the `ffc` namespace alias for brievety.
 130
 131 ```cpp
 132 /* In file grapefruit.cpp */
 133
 134 namespace ffc = food::fruits::citrus;
 135
 136 ffc::grapefruit::grapefruit()
 137 {
 138     // ...
 139 }
 140 ```
 141
 142 ## File layout example
 143
 144 ```cpp
 145 /*
 146  * Copyright (C) 20xx Robert Binette <bob@codebleu.qc.ca>
 147  *
 148  * SPDX-License-Identifier: GPL-2.0-only
 149  *
 150  */
 151
 152 #ifndef LTTNG_THING_DOER_H
 153 #define LTTNG_THING_DOER_H
 154
 155 /* Mind the order of inclusions, in alphabetical order per category. */
 156
 157 /* Project-local headers. */
 158 #include "my-local-stuff.hpp"
 159 #include "utils.hpp"
 160
 161 /* Project-wide headers. */
 162 #include <vendor/optional.hpp>
 163
 164 /* System headers. */
 165 #include <functional>
 166 #include <string>
 167 #include
 168
 169 namespace lttng {
 170 namespace sessiond {
 171
 172 class things;
 173
 174 using on_new_name_function = std::function<void(const std::string& name)>;
 175
 176 class thing_doer : public lttng::sessiond::doer {
 177 public:
 178         explicit thing_doer(const std::string& name);
 179
 180         virtual void do() override final;
 181         const std::string& name() const;
 182
 183 private:
 184         unsigned int _count_things(std::vector) const noexcept;
 185
 186         const std::string _name;
 187 };
 188
 189 } /* namespace sessiond */
 190 } /* namespace lttng */
 191
 192 #endif /* LTTNG_THING_DOER_H */
 193 ```
 194
 195 ## Miscelaneous guidelines
 196
 197 In general, the project’s contributors make an effort to follow, for C++11 code:
 198
 199 [The C++ Core Guidelines](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md)
 200
 201 [Scott Meyers’s “Effective Modern C++”](https://www.oreilly.com/library/view/effective-modern-c/9781491908419/)
 202
 203 Here are a couple of reminders:
 204 * When defining a class, put constructors as the first methods, whatever their access (public/protected/private), then the destructor, and then the rest.
 205
 206 * Declare variables as close to where they are used as possible.
 207
 208 * Use auto when possible.
 209
 210 * Use const as much as possible, even for pointer (const char* const) and numeric values (const unsigned int) which never need to change.
 211
 212 * Make methods const noexcept or const as much as possible.
 213
 214 * Make constructors explicit unless you really need an implicit constructor (which is rare).
 215
 216 * Use `std::unique_ptr` to manage memory when possible.
 217   * However, use references (`*my_unique_ptr`) and raw pointers (`my_unique_ptr.get()`) when not transferring ownership.
 218   * Only use `std::shared_ptr` when ownership is conceptually shared.
 219
 220 * Use `nullptr`, not `NULL` nor `0`.
 221
 222 * Return by value (rvalue) instead of by output parameter (non-const lvalue reference), even complex objects, unless you can prove that the performance is improved when returning by parameter.
 223
 224 * For a function parameter or a return value of which the type needs to be a reference or pointer, use:
 225   * **If the value is mandatory**, a reference.
 226   * **If the value is optional**, a raw pointer.
 227
 228 * Don't use `std::move()` when you already have an rvalue, which means:
 229   * Don't write `return std::move(...);` as this can interfere with RVO (Return Value Optimization).
 230   * Don't use `std::move()` with a function call (`std::move(func())`).
 231
 232 * For each possible move/copy constructor or assignment operator, do one of:
 233   * Write a custom one.
 234   * Mark it as defaulted (`default`)
 235   * Mark it as deleted (`delete`).
 236
 237 * Use scoped enumerations (`enum class`).
 238
 239 * Mark classes known to be final with the `final` keyword.
 240
 241 * Use type aliases (`using`), not type definitions (`typedef`).
 242
 243 * Use anonymous namespaces for local functions instead of `static`.
 244
 245 * Return a structure with named members instead of a generic container such as `std::pair` or `std::tuple`.
 246
 247 * When a class inherits a base class with virtual methods, use the `override` keyword to mark overridden virtual methods, and do not use the `virtual` keyword again (as the method is already known to be virtual).
 248
 249 * Define overloaded operators only if their meaning is obvious, unsurprising, and consistent with the corresponding built-in operators.
 250
 251   For example, use `|` as a bitwise or logical-or, not as a shell-style pipe.
 252
 253 * Use RAII wrappers when managing system resources or interacting with C libraries.
 254
 255   In other words, don't rely on ``goto``s and error labels to clean up as you would do in C.
 256
 257 * Throw an exception when there's an unexpected, exceptional condition,
 258   [including from a constructor](https://isocpp.org/wiki/faq/exceptions#ctors-can-throw), instead of returning a status code.
 259
 260   However, be mindful of the exception-safety of your users. For instance, `liblttng-ctl` exposes a C interface meaning that is must catch and handle all exceptions, most likely by returning a suitable error code.
 261
 262 * Accept a by-value parameter and move it (when it's moveable) when you intend to copy it anyway. You can do this with most STL containers.
 263
 264 ## C Style (historical)
 265
 266 The coding style used for this project follows the the Linux kernel guide lines, except that brackets `{`, `}` should typically be used even for single-line if/else statements. Please refer to:
 267
 268 - doc/kernel-CodingStyle.txt (copied from the Linux 3.4.4 tree).
 269
 270 - Linux kernel scripts/checkpatch.pl for a script which verify the patch
 271   coding style.
 272
 273 For header files, please declare the following in this order:
 274
 275 1) `#define`
 276
 277  - Default values should go in: src/common/defaults.h
 278  - Macros used across the project: src/common/macros.h
 279
 280 2) Variables
 281
 282  - No _static_ in a header file! This is madness.
 283  - Use _extern_ if the global variable is set else where.
 284
 285 3) Function prototype
 286
 287 Furthermore, respect the name spacing of files for each non-static symbol visiable outside the scope of the C file. For instance, for the utils.c file in libcommon, every call should be prefixed by "utils_*".
 288
 289 ### Error handling
 290
 291 In legacy C-style code, we ask to use one single return point in a function. For that, we uses the "goto" statement for the error handling creating one single point for error handling and return code. See the following example:
 292
 293 ```c
 294 int some_function(...)
 295 {
 296         int ret;
 297         [...]
 298
 299         if (ret != 0) {
 300                 goto error;
 301         }
 302
 303         [...]
 304 error:
 305         return ret;
 306 }
 307 ```