diff --git a/doc/contributing/source/coding-style.rst b/doc/contributing/source/coding-style.rst index 88aba880d..4ef3ac654 100644 --- a/doc/contributing/source/coding-style.rst +++ b/doc/contributing/source/coding-style.rst @@ -45,8 +45,8 @@ to produce consistent output among themselves. Integration with IDEs ===================== -Clang-format automatically integrates with modern IDEs (e.g., VS Code) that read the -``.clang-format`` file and automatically formats the code on save or on typing. +Clang-format can be integrated with modern IDEs (e.g., VS Code) that are able to +read the ``.clang-format`` file and automatically format the code on save or on typing. Please refer to the documentation of your IDE for more information. Some examples of IDE integration are provided in @@ -63,22 +63,24 @@ and on type by enabling the following settings: "editor.formatOnType": true, } -Manual usage in the terminal -============================ +Clang-format usage in the terminal +================================== -Clang-format can be manually run on the terminal by applying the following commands: +In addition to IDE support, clang-format can be manually run on the terminal. + +To format a file in-place, run the following command: .. sourcecode:: console clang-format -i $FILE -To check that a file is properly formatted, run the following command on the terminal. -If the code is well formatted, the process will exit with code 0; if is not, it will -exit with code of 1 and indicate the lines that should be formatted. +To check that a file is well formatted, run the following command on the terminal. +If the file is not well formatted, clang-format indicates the lines that need to be +formatted. .. sourcecode:: console - clang-format --dry-run --Werror $FILE + clang-format --dry-run $FILE Clang-format Git integration ============================ @@ -104,7 +106,7 @@ C++ comments [example adopted from ... // clang-format on -To exclude the whole file from being formatted, surround the whole file with the +To exclude an entire file from being formatted, surround the whole file with the special comments. check-style-clang-format.py @@ -146,11 +148,11 @@ For quick-reference, the most used commands are listed below: .. sourcecode:: console - # Entire codebase (using paths relative to the ns-3 root) + # Entire codebase (using paths relative to the ns-3 main directory) ./utils/check-style-clang-format.py [--fix] [--no-formatting] [--no-whitespace] [--no-tabs] . # Entire codebase (using absolute paths) - /path/to/utils/check-style-clang-format.py [--fix] [--no-formatting] [--no-whitespace] [--no-tabs] /path/to/ns3/root + /path/to/utils/check-style-clang-format.py [--fix] [--no-formatting] [--no-whitespace] [--no-tabs] /path/to/ns3 # Specific directory /path/to/utils/check-style-clang-format.py [--fix] [--no-formatting] [--no-whitespace] [--no-tabs] absolute_or_relative/path/to/directory @@ -166,6 +168,10 @@ The |ns3| project uses `clang-tidy `_ to statically analyze (lint) C++ code and help developers write better code. Clang-tidy can be easily integrated with modern IDEs or run manually on the command-line. +The list of clang-tidy checks currently enabled in the |ns3| project is saved on the +``.clang-tidy`` file. The full list of clang-tidy checks and their detailed explanations +can be seen in `Clang-tidy checks `_. + Clang-tidy installation ======================= @@ -197,22 +203,31 @@ Please refer to the documentation of your IDE for more information. Some examples of IDE integration are provided in `clang-tidy documentation `_ -Manual usage in the terminal -============================ +Clang-tidy usage +================ -In order to use clang-tidy on the terminal, |ns3| must first be configured by running -the following command on the terminal: +In order to use clang-tidy, |ns3| must first be configured with the flag +``--enable-clang-tidy``. To configure |ns3| with tests, examples and clang-tidy enabled, +run the following command: .. sourcecode:: console - ./ns3 configure --enable-clang-tidy + ./ns3 configure --enable-examples --enable-tests --enable-clang-tidy -Then, clang-tidy can be manually run on the terminal by applying the following commands -in the |ns3| root directory: +Due to the integration of clang-tidy with CMake, clang-tidy can be run while building +|ns3|. In this way, clang-tidy errors will be shown alongside build errors on the terminal. +To build |ns3| and run clang-tidy, run the the following command: .. sourcecode:: console - # Analyze (and fix) single file with clang-tidy + ./ns3 build + +To run clang-tidy without building |ns3|, use the following commands on the ns-3 +main directory: + +.. sourcecode:: console + + # Analyze (and fix) a single file with clang-tidy clang-tidy -p cmake-cache/ [--fix] [--format-style=file] [--quiet] $FILE # Analyze (and fix) multiple files in parallel @@ -221,27 +236,19 @@ in the |ns3| root directory: # Analyze (and fix) the entire ns-3 codebase in parallel run-clang-tidy -p cmake-cache/ [-fix] [-format] [-quiet] -Please note that clang-tidy only analyzes implementation files (i.e., ``*.cc`` files). -Header files are analyzed when they are included by implementation files, using the -``#include "..."`` directive. +When running clang-tidy, please note that: -Integration with CMake -====================== +- Clang-tidy only analyzes implementation files (i.e., ``*.cc`` files). + Header files are analyzed when they are included by implementation files, with the + ``#include "..."`` directive. -CMake provides native integration for clang-tidy. This allows CMake to simultaneously -build |ns3| and scan the codebase with clang-tidy. To enable this process, please -use the following commands on the |ns3| root directory: +- Not all clang-tidy checks provide automatic fixes. For those cases, a manual fix + must be made by the developer. -.. sourcecode:: console +- Enabling clang-tidy will add time to the build process (in the order of minutes). - ./ns3 configure --enable-clang-tidy - ./ns3 build - -Please note that enabling clang-tidy scanning will add time to the build process -(in the order of minutes). - -Disable analysis in specific lines -================================== +Disable clang-tidy analysis in specific lines +============================================= To disable clang-tidy analysis of a particular rule in a specific function, specific clang-tidy comments have to be added to the corresponding function. @@ -266,8 +273,9 @@ following two "special comment" syntaxes: void func(); // NOLINT(modernize-use-override) -To disable ``modernize-use-override`` checking on a block of code, use the -following "special comment" syntax: +To disable a specific clang-tidy check on a block of code, for instance the +``modernize-use-override`` check, surround the code block with the following +"special comments": .. sourcecode:: cpp @@ -277,8 +285,8 @@ following "special comment" syntax: // NOLINTEND(modernize-use-override) -To disable all clang-tidy checks on a block of code, use the following -"special comment" syntax: +To disable all clang-tidy checks on a block of code, surround it with the +following "special comments": .. sourcecode:: cpp @@ -287,8 +295,8 @@ To disable all clang-tidy checks on a block of code, use the following void func2(); // NOLINTEND -To exclude the whole file from being formatted, surround the whole file with the -special comments. +To exclude an entire file from being checked, surround the whole file with the +"special comments". Source code formatting ********************** @@ -432,7 +440,7 @@ Separate each block with a new line. class MyClass { public: - static counter = 0; + static int m_counter = 0; MyClass(int x, int y); @@ -463,8 +471,8 @@ The constructor initializers are always declared one per line, with a trailing c void MyClass::MyClass(int x, int y) - : m_x (x), - m_y (y) + : m_x(x), + m_y(y) { } @@ -496,7 +504,7 @@ declaration: .. sourcecode:: cpp template - void func(T t); + void Func(T t); Naming ====== @@ -1029,23 +1037,23 @@ the |ns3| smart pointer class ``Ptr`` should be used in boolean comparisons as f .. sourcecode:: cpp - for Ptr<> p, do not use: use instead: - ======================== ================================= - if (p != nullptr) {...} if (p) {...} + for Ptr<> p, do not use: use instead: + ======================== ================================= + if (p != nullptr) {...} if (p) {...} if (p != NULL) {...} - if (p != 0) {...} if (p) {...} + if (p != 0) {...} if (p) {...} - if (p == nullptr) {...} if (!p) {...} + if (p == nullptr) {...} if (!p) {...} if (p == NULL) {...} if (p == 0) {...} - NS_ASSERT... (p != 0, ...) NS_ASSERT... (p, ...) - NS_ABORT... (p != 0, ...) NS_ABORT... (p, ...) + NS_ASSERT... (p != nullptr, ...) NS_ASSERT... (p, ...) + NS_ABORT... (p != nullptr, ...) NS_ABORT... (p, ...) - NS_ASSERT... (p == 0, ...) NS_ASSERT... (!p, ...) - NS_ABORT... (p == 0, ...) NS_ABORT... (!p, ...) + NS_ASSERT... (p == nullptr, ...) NS_ASSERT... (!p, ...) + NS_ABORT... (p == nullptr, ...) NS_ABORT... (!p, ...) - NS_TEST... (p, 0, ...) NS_TEST... (p, nullptr, ...) + NS_TEST... (p, nullptr, ...) NS_TEST... (p, nullptr, ...) C++ standard ============ @@ -1062,8 +1070,8 @@ Miscellaneous items - ``NS_LOG_COMPONENT_DEFINE("log-component-name");`` statements should be placed within namespace ns3 (for module code) and after the - ``using namespace ns3;``. In examples. - ``NS_OBJECT_ENSURE_REGISTERED()`` should also be placed within namespace ns3. + ``using namespace ns3;``. In examples, + ``NS_OBJECT_ENSURE_REGISTERED()`` should also be placed within namespace ``ns3``. - Pointers and references are left-aligned: @@ -1081,10 +1089,10 @@ Miscellaneous items void MySub(T const& t); // Not OK - Do not include inline implementations in header files; put all - implementation in a .cc file (unless implementation in the header file + implementation in a ``.cc`` file (unless implementation in the header file brings demonstrable and significant performance improvement). -- Do not use ``nil``, ``NULL`` or ``0`` constants; use ``nullptr`` (improves portability) +- Do not use ``NULL``, ``nil`` or ``0`` constants; use ``nullptr`` (improves portability) - Consider whether you want the default constructor, copy constructor, or assignment operator in your class. If not, explicitly mark them as deleted and make the @@ -1092,28 +1100,35 @@ Miscellaneous items .. sourcecode:: cpp - public: - // Explain why these are not supported - ClassName() = delete; - ClassName(const ClassName&) = delete; - ClassName& operator=(const ClassName&) = delete; + class MyClass + { + public: + // Allowed constructors + MyClass(int i); + + // Deleted constructors. + // Explain why they are not supported. + MyClass() = delete; + MyClass(const MyClass&) = delete; + MyClass& operator=(const MyClass&) = delete; + }; - Avoid returning a reference to an internal or local member of an object: .. sourcecode:: cpp - a_type& foo(); // should be avoided, return a pointer or an object. - const a_type& foo(); // same as above + MyType& foo(); // Avoid. Prefer to return a pointer or an object. + const MyType& foo(); // Same as above. This guidance does not apply to the use of references to implement operators. - Expose class members through access functions, rather than direct access to a public object. The access functions are typically named ``Get`` and - ``Set``. For example, a member ``m_delayTime`` might have accessor functions - ``GetDelayTime()`` and ``SetDelayTime()``. + ``Set`` followed by the member's name. For example, a member ``m_delayTime`` + might have accessor functions ``GetDelayTime()`` and ``SetDelayTime()``. - Do not bring the C++ standard library namespace into |ns3| source files by - using the ``using`` directive; i.e., avoid ``using namespace std;``. + using the ``using namespace std;`` directive. - Do not use the C++ ``goto`` statement. @@ -1128,7 +1143,7 @@ of rules that should be observed while developing code. - Prefer to use ``.emplace_back()`` over ``.push_back()`` to optimize performance. - When creating STL smart pointers, prefer to use ``std::make_shared`` or - ``std::make_unique``, instead of creating the pointer with ``new``. + ``std::make_unique``, instead of creating the smart pointer with ``new``. .. sourcecode:: cpp @@ -1140,7 +1155,7 @@ of rules that should be observed while developing code. .. sourcecode:: cpp - std::vector myVector {1, 2, 3}; + std::vector myVector{1, 2, 3}; for (const auto& v : myVector) { ... } // Prefer for (int i = 0; i < myVector.size(); i++) { ... } // Avoid @@ -1150,7 +1165,7 @@ of rules that should be observed while developing code. .. sourcecode:: cpp - std::vector myVector {1, 2, 3}; + std::vector myVector{1, 2, 3}; for (const auto& v : myVector) { ... } // OK for (auto v : myVector) { ... } // Avoid @@ -1160,21 +1175,23 @@ of rules that should be observed while developing code. .. sourcecode:: cpp - int N_ITEMS = 5; + constexpr int N_ITEMS = 5; + std::vector myVector; - myVector.reserve(N_ITEMS); // Reserve memory required to store all items + myVector.reserve(N_ITEMS); // Reserve memory to store all items + for (int i = 0; i < N_ITEMS; i++) { myVector.emplace_back(i); } - Prefer to initialize STL containers (e.g., ``std::vector``, ``std::map``, etc.) - directly with lists, instead of pushing elements one-by-one. + directly with a braced-init-list, instead of pushing elements one-by-one. .. sourcecode:: cpp // OK - std::vector myVector = {1, 2, 3}; + std::vector myVector{1, 2, 3}; // Avoid std::vector myVector; @@ -1186,6 +1203,20 @@ of rules that should be observed while developing code. - Avoid unnecessary calls to the functions ``.c_str()`` and ``.data()`` of ``std::string``. +- Avoid unnecessarily dereferencing std smart pointers (``std::shared_ptr``, + ``std::unique_ptr``) with calls to their member function ``.get()``. + Prefer to use the std smart pointer directly where needed. + + .. sourcecode:: cpp + + auto ptr = std::make_shared(); + + // OK + if (ptr) { ... } + + // Avoid + if (ptr.get()) { ... } + - Avoid declaring trivial destructors, to optimize performance. - Prefer to use ``static_assert()`` over ``NS_ASSERT()`` when conditions can be