Better handling of internal failure modes and fix section bug #81
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| # Guidelines for writing C++ code for *snitch* | ||
|
|
||
| Unless otherwise stated, follow the [C++ Core Guidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines). Below are exceptions to these guidelines, or more opinionated choices. | ||
|
|
||
|
|
||
| ## `noexcept` | ||
|
|
||
| Follow the [Lakos Rule](https://quuxplusone.github.io/blog/2018/04/25/the-lakos-rule/) (with some tweaks): | ||
| - Functions with a wide contract (i.e., with no precondition) should be marked as unconditionally `noexcept`. | ||
| - Functions with a narrow contract (i.e., with preconditions), should not be marked as `noexcept`. | ||
| - If a template function is conditionally-wide, (e.g., its purpose is only to be a wrapper or adapter over another function), then it may be marked conditionally `noexcept`. | ||
|
|
||
| In particular: | ||
| - Do not mark a function `noexcept` just because it happens not to throw. The decision should be based on the *interface* of the function (which includes its pre-condition), and not its implementation. | ||
|
|
||
| Rationale: | ||
| - Easy transition to using contracts when they come to C++. | ||
| - Enable testing for pre-condition violations by conditionally throwing. | ||
|
|
||
|
|
||
| ## Heap allocations | ||
|
|
||
| *snitch* code must not directly or indirectly allocate heap (or "free store") memory while running tests. This means that a number of common C++ STL classes cannot be used (at least not with the default allocator): | ||
| - `std::string`: use `std::string_view` (for constant strings) or `snitch::small_string` (for variable strings) instead. | ||
| - `std::vector`, `std::map`, `std::set`, and their variants: use `std::array` (for fixed size arrays) or `snitch::small_vector` (for variable size arrays) instead. | ||
| - `std::function`: use `snitch::small_function` instead. | ||
| - `std::unique_ptr`, `std::shared_ptr`: use values on the stack, and raw pointers for non-owning references. | ||
|
|
||
| Unfortunately, the standard does not generally specify if a function or class allocates heap memory or not. We can make reasonable guesses for simple cases; in particular the following are fine to use: | ||
| - `std::string_view`, | ||
| - `std::array`, | ||
| - `std::span`, | ||
| - `std::variant` with `std::get_if` and `std::visit`, | ||
| - `std::optional`, | ||
| - `std::tuple`, | ||
| - Functions in `<algorithm>` ([except `std::stable_sort`, `std::stable_partition`, and `std::inplace_merge`](https://stackoverflow.com/a/46714875/1565581)). | ||
| - Concepts and type traits. | ||
|
|
||
| Any type or function not listed above *should* be assumed to use heap memory unless demonstrated otherwise. One way to monitor this is on Linux is to use [valgrind/massif](https://valgrind.org/docs/manual/ms-manual.html). | ||
|
|
||
| Note: If necessary, it is acceptable to allocate/de-allocate heap memory when a test is not running, i.e., either at the start or end of the program, or (if single-threaded) in between test cases. For example, as of writing this, with GCC 10 on Linux and the default reporter, *snitch* performs heap allocations on two occasions: | ||
| - several allocations at program startup, generated by [`libstdc++` initialisation](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68606). | ||
| - one at the first console output, generated by `glibc` for its internal I/O buffer. | ||
|
|
||
|
|
||
| ## Heavy headers and compilation time | ||
|
|
||
| One of the advantages of *snitch* over competing testing framework is fast compilation of tests. To preserve this advantage, ["heavy" STL headers](https://artificial-mind.net/projects/compile-health/) should not be included in *snitch* headers unless absolutely necessary. However, they can be included in the *snitch* implementation `*.cpp` files. | ||
|
|
||
| Therefore: | ||
| - Place as much code as possible in the `*.cpp` files rather than in headers. | ||
| - When not possible (templates, constexpr, etc.), consider if you can use a short and clear hand-written alternative instead. For example, `std::max(a, b)` requires `<algorithm>`, but can also be written as `a > b ? a : b`. Some of the simplest algorithms in `<algorithm>`, like `std::copy`, can also be written with an explicit loop. | ||
|
There was a problem hiding this comment. -and clear hand-written alternative instead
+and clear handwritten alternative insteadThere was a problem hiding this comment. The more you know :) Thanks |
||
| - Finally, consider if you really need the full feature from the STL, or just a small subset. For example, if you need a metaprogramming type list and don't need to instanciate the types, don't use `std::tuple<...>`: use a custom `template<typename ... Args> struct type_list {}` instead. | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good spot, thanks!