From 54ca31f9593b818a2cc5f7fdcbb05395f2ce5959 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Mon, 21 Mar 2022 14:33:12 -0500 Subject: [PATCH] Update code style guide for header include updates Signed-off-by: Michael Richters --- docs/codebase/code-style.md | 88 ++++++++++++++++++++++--------------- 1 file changed, 53 insertions(+), 35 deletions(-) diff --git a/docs/codebase/code-style.md b/docs/codebase/code-style.md index df3063c2..f7562d05 100644 --- a/docs/codebase/code-style.md +++ b/docs/codebase/code-style.md @@ -21,9 +21,11 @@ Our style guide is based on the [Google C++ style guide][goog:c++-guide] which w - [Header Files](#header-files) - [Self-contained Headers](#self-contained-headers) - [Header Guards](#header-guards) + - [Include What You Use](#include-what-you-use) - [Forward Declarations](#forward-declarations) - [Inline Functions](#inline-functions) - - [Names and Order of Includes](#names-and-order-of-includes) + - [Organization of Includes](#organization-of-includes) + - [Top-level Arduino Library Headers](#top-level-arduino-library-headers) - [Scoping](#scoping) - [Namespaces](#namespaces) - [Unnamed Namespaces and Static Variables](#unnamed-namespaces-and-static-variables) @@ -258,6 +260,12 @@ There are rare cases where a file designed to be included is not self-contained. All header files should have `#pragma once` guards at the top to prevent multiple inclusion. +### Include What You Use + +If a source or header file refers to a symbol defined elsewhere, the file should directly include a header file which provides a declaration or definition of that symbol. + +Do not rely on transitive inclusions. This allows maintainers to remove no-longer-needed `#include` statements from their headers without breaking clients code. This also applies to directly associated headers - `foo.cpp` should include `bar.h` if it uses a symbol defined there, even if `foo.h` (currently) includes `bar.h`. + ### Forward Declarations > Avoid using forward declarations where possible. Just `#include` the headers you need. @@ -311,65 +319,75 @@ Another useful rule of thumb: it's typically not cost effective to inline functi It is important to know that functions are not always inlined even if they are declared as such; for example, virtual and recursive functions are not normally inlined. Usually recursive functions should not be inline. The main reason for making a virtual function inline is to place its definition in the class, either for convenience or to document its behavior, e.g., for accessors and mutators. -### Names and Order of Includes +### Organization of Includes - +> Use standard order for readability and to avoid hidden dependencies: +> - The header associated with this source file, if any +> - System headers and Arduino library headers (including other Kaleidoscope plugins, but not Kaleidoscope itself) +> - Kaleidoscope headers and headers for the individual plugin (other than the associated header above) -> Use standard order for readability and to avoid hidden dependencies: Related header, Arduino libraries, other libraries' `.h`, your project's `.h`. +These three sections should be separated by single blank lines, and should be sorted alphabetically. -All libraries must have at least one header in their top-level `src/` directory, to be included without any path components. This is the way Arduino finds libraries, and a limitation we must adhere to. These headers should - in general - include any other headers they may need, so that the consumer of the library only has to include one header. The name of this header must be the same as the name of the library. +When including system headers and Arduino library headers (including Kaleidoscope plugins), use angle brackets to indicate that those sources are external. -The recommended naming is to prefix the library name with `Kaleidoscope-`. +For headers inside the current library and for Kaleidoscope core headers, use double quotes and a full pathname (starting below the `src/` directory). This applies to the source file's associated header, as well; don't use a pathname relative to the source file's directory. -If there is more than one header, they should be listed as descendants of the project's source directory without use of UNIX directory shortcuts `.` (the current directory) or `..` (the parent directory), and live in a `Kaleidoscope` subdirectory. For example, if we have a plugin named `Kaleidoscope-Something`, which has an additional header file other than `Kaleidoscope-Something.h`, it should be in `src/Kaleidoscope/Something-Other.h`, and be included as: +For the sake of clarity, the above sections can be further divided to make it clear where each included header file can be found, but this is probably not necessary in most cases, because the path name of a header usually indicates which library it is located in. + +For example, the includes in `Kaleidoscope-Something/src/kaleidoscope/Something.cpp` might look like this: ```c++ -#include "Kaleidoscope-Something.h" -#include "Kaleidoscope/Something-Other.h" +#include "kaleidoscope/Something.h" + +#include +#include +#include + +#include "kaleidoscope/KeyAddr.h" +#include "kaleidoscope/KeyEvent.h" +#include "kaleidoscope/key_defs.h" +#include "kaleidoscope/plugin/something/utils.h" ``` -Having more than one level of subdirectories is not recommended. +**Exception** -In `dir/foo.cpp` or `dir/foo_test.cpp`, whose main purpose is to implement or test the stuff in `dir2/foo2.h`, order your includes as follows: +Sometimes, system-specific code needs conditional includes. Such code can put conditional includes after other includes. Of course, keep your system-specific code small and localized. Example: -1. `dir2/foo2.h` -2. Arduino libraries. -3. Other libraries' `.h` files. -4. Your project's `.h` files. +```c++ +#if defined(ARDUINO_AVR_MODEL01) +#include "kaleidoscope/Something-AVR-Model01.h" +#endif -With the preferred ordering, if `dir2/foo2.h` omits any necessary includes, the build of `dir/foo.cpp` or `dir/foo_test.cpp` will break. Thus, this rule ensures that build breakages show up first for the people working on these files, not for innocent people in other packages. +#if defined(ARDUINO_AVR_SHORTCUT) +#include "kaleidoscope/Something-AVR-Shortcut.h" +#endif +``` -`dir/foo.cc` and `dir2/foo2.h` are usually in the same directory (e.g. `Kaleidoscope/Something_test.cpp` and `Kaleidoscope/Something.h`), but may sometimes be in different directories too. +### Top-level Arduinio Library Headers -Within each section the includes should be ordered alphabetically. +All libraries must have at least one header in their top-level `src/` directory, to be included without any path components. This is the way Arduino finds libraries, and a limitation we must adhere to. These headers should - in general - include any other headers they may need, so that the consumer of the library only has to include one header. The name of this header must be the same as the name of the library. -You should include all the headers that define the symbols you rely upon, except in the unusual case of [forward declarations](#forward-declarations). If you rely on symbols from `bar.h`, don't count on the fact that you included `foo.h` which (currently) includes `bar.h`: include `bar.h` yourself, unless `foo.h` explicitly demonstrates its intent to provide you the symbols of `bar.h`. However, any includes present in the related header do not need to be included again in the related `cc` (i.e., `foo.cc` can rely on `foo.h`'s includes). +The naming convention for Kaleidoscope plugins is to use the `Kaleidoscope-` prefix: e.g. `Kaleidoscope-Something`, which would have a top-level header named `Kaleidoscope-Something.h` in its `src/` directory. -For example, the includes in `Kaleidoscope-Something/src/Kaleidoscope/Something.cpp` might look like this: +In the case of Kaleidoscope plugin libraries, the number of source and header files tends to be very small (usually just one `*.cpp` file and its associated header, in addition to the library's top-level header). When one plugin depends on another, we therefore only include the top-level header of the dependency. For example, if `Kaleidoscope-OtherThing` depends on `Kaleidoscope-Something`, the file `kaleidoscope/plugin/OtherThing.h` will contain the line: ```c++ -#include "Kaleidoscope/Something.h" +#include +``` -#include "Arduino.h" +…and `Kaleidoscope-Something.h` will look like this: -#include "Kaleidoscope-LEDControl.h" -#include "Kaleidoscope-Focus.h" +```c++ +#include "kaleidoscope/plugin/Something.h" ``` -**Exception** +This both makes it clearer where to find the included code, and allows the restructuring of that code without breaking the dependent library (assuming the symbols haven't changed as well). -Sometimes, system-specific code needs conditional includes. Such code can put conditional includes after other includes. Of course, keep your system-specific code small and localized. Example: +If a plugin library has symbols meant to be exported, and more than one header file in which those symbols are defined, all such header files should be included in the top-level header for the library. For example, if `Kaleidoscope-Something` defines types `kaleidoscope::plugin::Something` and `kaleidoscope::plugin::something::Helper`, both of which are meant to be accessible by `Kaleidoscope-OtherThing`, the top-level header `Kaleidoscope-Something.h` should look like this: ```c++ -#include "Kaleidoscope.h" - -#if defined(ARDUINO_AVR_MODEL01) -#include "Kaleidoscope/Something-AVR-Model01.h" -#endif - -#if defined(ARDUINO_AVR_SHORTCUT) -#include "Kaleidoscope/Something-AVR-Shortcut.h" -#endif +#include "kaleidoscope/plugin/Something.h" +#include "kaleidoscope/plugin/something/Helper.h" ```