docs/Coding_Conventions_and_Standards.md
Google style from https://google.github.io/styleguide/cppguide.html with a few minor alterations:
nullptr so the API clearly advertises the intentshared_ptr and unique_ptr) is expected. A non-const reference equates to "this is a non-null object that you can change but are not being given ownership of".gsl::span<const T> by value (or std::span when supported) as input arguments when passing const references to containers with contiguous storage (like std::vector). This allows the function to be container independent, and the argument to represent arbitrary memory spans or sub-spans. The below examples allow the client code to use either std::vector or InlinedVector. An instance of a gsl::span would be created automatically./// Instead of
void foo(const std::vector<int64_t>&);
/// Use to pass any contiguous const container containing int64_t
// Now you can seamlessly pass either `std::vector`, `InlinedVector`, `std::array` or `gsl::span` as an argument.
void foo(gsl::span<const int64_t>);
// Example with pointer to const data. Instead of
void foo(const std::vector<const Node*>&);
// Use
void foo(gsl::span<const Node* const>);
gsl::span<const T> by value instead of a const reference to a contiguous member container. Prefer returning gsl::span instead of a pointer referring to a chunk of memory. The size is also included in the span.
For example,// Instead of
const std::vector<int64_t>& foo();
// Return a span by value
gsl::span<const int64_t> foo();
// Instead of
const int64_t* foo();
// Return a span by value
gsl::span<const int64_t> foo();
std::initializer_list<T> is not automatically convertible to a gsl::span<const T>. Use AsSpan({1, 2, 3}) defined at core/common/span_utils.h to convert std::initializer_list<T> to a span. You can also use std::array. For example,// Original code
void foo(const std::vector<std::string>&);
foo({"abc", "dbf"}); // Works
// After refactoring to gsl::span it would no longer compile. Use AsSpan().
void foo(gsl::span<const std::string>);
foo(AsSpan<std::string>{"abc", "dbf"}); // Works
Prefer passing std::string_view by value instead of const std::string&. Make sure that the lifespan of a std::string instance ecplises the lifespan of the corresponding std::string_view instance.
using namespace permitted with limited scope
using namespace at all is overly restrictive. Follow the C++ Core Guidelines:
Onnxruntime aims to reduce latency and latency variance by minimizing the amount of dynamic memory allocations.
typedefs to reduce memory allocations is required:
TensorShapeVector typedef to build or modify shapes from core/framework/tensor_shape.h. It is based on a vector implementation that features small buffer optimization. Its small buffer size is the same to that of in TensorShape.InlinedVector<T> typedef instead of std::vector defined at core/common/inlined_containers_fwd.h. By default, it provides 64 bytes of inlined storage. You can customize inlined size with the second template non-type parameter N.InlinedHashSet<T> and InlinedHashMap<T> typedefs from core/common/inlined_containers.h. These are drop-in replacements for std::unordered_set/map that store their keys and values in one continuous buffer and reduce the number of allocations. They also do not allocate an end node when default constructed. Note, that these Hash containers do not provide pointer stability. std::map and std::set can often be replaced by hash containers as well.NodeHashSet and NodeHashMap. Although node based, they are more cache friendly.core/common/inlined_containers_fwd.h to forward declare any of the above container types.std::string_view for use in containers to reduce the number of allocations and avoid string duplication. Keep in mind that the lifespan of the objects being referred to must eclipse the lifespan of the corresponding std::string_view.Abseil library for the above typedefs. Abseil container documentation is here.Abseil library or absl namespace directly. We should be able to build Onnxruntime without Abseil.onnxruntime/tools/natvis/abseil-cpp.natvis for the above containers visualizations and debugging help in VS Studio and VS Code.reserve() and not resize() on vectors. resize() default constructs all the elements for the size which can be expensive/noticeable even if the type is trivial. Default values are rarely used in practice and it becomes a waste. Construction like std::vector<int>(10, 0) is the same as resize() and is potentially wasteful.reserve() on hash containers and vectors. For example,#include "core/common/inlined_containers.h"
void foo(gsl::span<const std::string> names) {
// For local processing, names are still valid
// use std::string_view to avoid duplicate memory allocations.
// same code would work with std::unordered_set if built without Abseil
InlinedHashSet<std::string_view> unique_names;
unique_names.reserve(names.size());
unique_names.insert(names.cbegin(), names.cend());
}
auto with const, *, & and && where applicable to more clearly express the intentORT_DISALLOW_COPY_ASSIGNMENT_AND_MOVE initiallyORT_DISALLOW_* macros in https://github.com/microsoft/onnxruntime/blob/main/include/onnxruntime/core/common/common.hstd::unique_ptr might be considered for delayed or optional construction of objects or members of classes. Instead, use std::optional as appropriate to reduce the number of allocations.else after return. see: https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-returnstd::shared_ptr. Use std::shared_ptr only if it's not clear when and where the object will be de-allocated. See also: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-shared_ptrlong type, which could be either 32 bits or 64 bits.std::make_unique(). References for the reasoning:
#include "core/common/safeint.h"SafeInt<size_t> in the code for examplesClang-format will handle automatically formatting code to these rules. There’s a Visual Studio plugin that can format on save at https://marketplace.visualstudio.com/items?itemName=LLVMExtensions.ClangFormat, or alternatively the latest versions of Visual Studio 2017 include clang-format support.
There is a .clang-format file in the root directory that has the max line length override and defaults to the google rules. This should be automatically discovered by the clang-format tools.
Visual Studio Code Analysis with C++ Core guidelines rules enabled is configured to run on build for the onnxruntime_common, onnxruntime_graph and onnxruntime_util libraries. Updating the onnxruntime_framework and onnxruntime_provider libraries to enable Code Analysis and build warning free is pending.
Code changes should build with no Code Analysis warnings, however this is somewhat difficult to achieve consistently as the Code Analysis implementation is in fairly constant flux. Different minor releases may have less false positives (a build with the latest version may be warning free, and a build with an earlier version may not), or detect additional problems (an earlier version builds warning free and a later version doesn't).
We use BinSkim Binary Analyzer to scan our binaries.
There should be unit tests that cover the core functionality of the product, expected edge cases, and expected errors. Code coverage from these tests should aim at maintaining over 80% coverage.
All changes should be covered by new or existing unit tests.
In order to check that all the code you expect to be covered by testing is covered, run code coverage in Visual Studio using 'Analyze Code Coverage' under the Test menu.
There is a configuration file in onnxruntime/VSCodeCoverage.runsettings that can be used to configure code coverage so that it reports numbers for just the onnxruntime code. Select that file in Visual Studio via the Test menu: Test -> Test Settings -> Select Test Settings File.
Using Show Code Coverage Coloring will allow you to visually inspect which lines were hit by the tests. See https://docs.microsoft.com/en-us/visualstudio/test/using-code-coverage-to-determine-how-much-code-is-being-tested?view=vs-2017.
This project uses lintrunner for linting. It provides a consistent linting experience locally and in CI. You can install the dependencies and initialize with
pip install -r requirements-lintrunner.txt
lintrunner init
This will install lintrunner on your system and download all the necessary
dependencies to run linters locally.
If you want to see what lintrunner init will install, run
lintrunner init --dry-run.
To format local changes:
lintrunner -a
To format all files:
lintrunner -a --all-files
To show help text:
lintrunner -h
To read more about lintrunner, see wiki.
To update an existing linting rule or create a new one, modify .lintrunner.toml or create a
new adapter following examples in https://github.com/justinchuby/lintrunner-adapters.
Follow the Black formatter's coding style when possible. A maximum line length of 120 characters is allowed for consistency with the C++ code.
Please adhere to the PEP8 Style Guide. We use Google's python style guide as the style guide which is an extension to PEP8.
Use pyright, which is provided as a component of the pylance extension in VS Code for static type checking.
Auto-formatting is done with black and isort. The tools are configured in pyproject.toml. From the root of the repository, you can run
lintrunner f --all-files
to format Python files.
Use pydocstyle to lint documentation styles. pydocstyle is enabled in VS Code.
VS Code is automatically configured with workspace configurations.
For Python development is VS Code, read this tutorial for more information.
Follow black's documentation to set up the black formatter for PyCharm.
We use the Python built-in unittest framework for creating unit tests and pytest to run them. Use pytest to create tests only when unittest does not fit the need.
Test the behavior, instead of the implementation. To make what a test is testing clear, the test methods should be named following the pattern test_<method or function name>_<expected behavior>_[when_<condition>].
e.g. test_method_x_raises_error_when_dims_is_not_a_sequence
Please follow the Google Objective-C/C++ Style Guide with the following alteration(s):
Clang-format can be used to format Objective-C/C++ code. The .clang-format file is in the repository root directory.