scanf() and fscanf() format strings should specify a field width for the %s string placeholder
scanf() and fscanf() format strings should specify a field width for the %s string placeholder
By default, there is no limit on the length of the string being read. The scanf
family of functions will continue to read characters into the buffer until they encounter a whitespace character.
If the input contains a string that is long enough and lacks whitespace characters, it can result in memory beyond the end of the buffer being overwritten. This situation is known as a buffer overflow vulnerability.
using enum should be used in scopes with high concentration of enum constants
using enum should be used in scopes with high concentration of enum constants
C++20 extends the `using declaration to class enum. using enum introduces all the enum constants into the current scope.
As with other using declarations, using enum improves readability when used in small scopes yet might generate confusion in large scopes.
The switch statement, as in the example above, when applied to a class enum value, is a natural scope for using enum.
This rule reports scopes that use a particular class enum extensively and could benefit from using enum declaration. For example, it reports most switch statements applied to an enum` value.
Results of dynamic_cast on pointers should always be tested
Results of dynamic_cast on pointers should always be tested
`dynamic_cast allow to convert pointers from one class to another, following the class inheritance hierarchy (note that in many cases, the need to perform a dynamic_cast is an indicator of poor class design, for instance not following the Liskov substitution principle).
If the requested conversion is not possible, the result of the conversion is a null pointer to the requested class.
An implication of this is that result of dynamic_cast` should be tested before using it to avoid dereferencing a null pointer.
Unnecessary expensive copy should be avoided when using auto as a placeholder type
Unnecessary expensive copy should be avoided when using auto as a placeholder type
Unintentional expensive copy should be avoided when using `auto as a placeholder type.
When using const auto as a placeholder type, you might unintentionally forget to add an ampersand(&) after the auto keyword. This can silently create a pointless copy and possibly have a bad impact on the performance of your code depending on the size of the created object and its context.
For example, if it happens in a range-based for loop context, it is going to lead to creating as many useless objects as the size of the range.
This rule will detect a declaration of an unmodified local variable with expensive to copy type and auto` as a placeholder type that is initialized with a non-temporary object.
Jump statements should not be redundant
Jump statements should not be redundant
Jump statements, such as return, break, goto, and continue
let you change the default flow of program execution, but jump statements that direct the control flow to the original direction are just a waste of keystrokes.
There should not be unused parameters (named or unnamed) in the set of parameters for a virtual function and all the functions that override it
There should not be unused parameters (named or unnamed) in the set of parameters for a virtual function and all the functions that override it
Unused function parameters are often due to design changes and can lead to mismatched parameter lists.
<cstdio> should not be used
<cstdio> should not be used
This includes file and I/O functions fgetpos, fopen, ftell, gets, perror, remove, rename
, etc.
Streams and file I/O have a large number of unspecified, undefined and implementation-defined behaviors associated with them.
Account validity should be verified when authenticating users with PAM
Account validity should be verified when authenticating users with PAM
When authenticating users, if the validity of the account is not checked (not locked, not expired …), it may lead to unauthorized access to resources.
std::jthread should be used instead of std::thread
std::jthread should be used instead of std::thread
`std::jthread, introduced in C++20, is a wrapper around std::thread. This way, it has the same functionalities as std::thread, making the substitution straightforward while adding two interesting behaviors:
It joins by default in its destructor. If a std::thread was not joined or detached before being destroyed, a call to std::terminate was made. This behavior can’t happen with std::jthread.
It can be canceled or stopped in some situations by calling request_stop().
This rule raises an issue when std::thread` is used.
Type specifiers should be listed in a standard order
Type specifiers should be listed in a standard order
Shared coding conventions allow teams to collaborate efficiently. This rule checks that type specifiers always appear in the following order:
`typedef
type name, spelling of built-in types with more than one type-specifier:
signedness - signed or unsigned
last single type-specifier or
short int
long int
long long int
long double
Since the positioning of the const` keyword is controversial, this rule does not check it.
Octal and hexadecimal escape sequences should be terminated
Octal and hexadecimal escape sequences should be terminated
There is potential for confusion if an octal or hexadecimal escape sequence is immediately followed by other characters. Instead, such sequences shall be terminated by either:
The start of another escape sequence.
The end of the character constant or the end of a string literal.
volatile types should not be used in compound operations
volatile types should not be used in compound operations
In C++, it usually does not matter how many times you access a variable as long as the variable value is the right one. However, this is not the case when the variable is in a memory region mapped to external hardware. In that case, for instance, several successive reads can yield different values (if the memory is updated by the hardware in-between), and several writes of the same value may be significant (some hardware trigger events each time a memory location is written to).
To specify that every read and write has an impact outside of the abstract machine of the language, access to a variable may be qualified as volatile: it will oblige the program to perform all specified reads and writes operations without optimizing anything away.
When a variable appears in a compound expression (for instance, a++ or a+=2), the variable is accessed twice even if it is only named once. The standard was not explicit on this topic up until C++23, but this detail matters for volatile
variables for which every access to the variable matters.
Consequently, it is usually clearer to rewrite the code so that the variable appears twice, matching the number of accesses that will happen.
Note: In C++20, compound expressions on volatile variables were deprecated. This deprecation was removed in C++23, and the number of accesses was made explicit. The reason for removing the deprecation is that such operations are commonly used in embedded code, especially to access specific variable bits. However, using a function with a dedicated name instead of direct bit manipulation usually leads to code that is easier to read.
Expressions with type char or wchar_t should not be used as operands to built-in operators other than =, ==, !=, and &
Expressions with type char or wchar_t should not be used as operands to built-in operators other than =, ==, !=, and &
Manipulation of character data may generate results that are contrary to developer expectations. For example, ISO/IEC 14882:2003 §2.2(3) only requires that the digits “0” to “9” have consecutive numerical values.
Non-exception types should not be caught
Non-exception types should not be caught
Throwing as an exception an object that is not derived from `std::exception is a bad practice. It is usually unreliable, meaningless, and a source of type clashes.
For the same reason, catching a non-exception type is a sign that your application has a bad exception-handling design. You should use standard exception types or create your own exception types that inherit at some level from std::exception`.
if constexpr should be preferred to overloading for metaprogramming
if constexpr should be preferred to overloading for metaprogramming
C++17 version of the standards introduces `if constexpr. If the constexpr keyword follows the if keyword in an if statement, then the if condition must be a constant and the then or else block is discarded at compile time, depending on the value of the constant.
More precisely, if constexpr branches that are discarded are not going to be instantiated. This behavior enables us to write some overloaded function templates in a more readable way: you don’t need to use complex patterns (eg: by using std::enable_if) to make code compile.
This rule points out where a complex overloaded functions template could simply be replaced by if constexpr`.
Macros should not be used to define constants
Macros should not be used to define constants
A macro is a textual replacement, which means that it’s not respecting the type system, it’s not respecting scoping rules… There is no reason not to use a constant instead.
Most of the time, a macro can be replaced by a `constexpr declaration (a constant that is guaranteed to be computed during compilation). If your compiler is too old to properly handle constexpr, you may use const instead.
If you have a series of related integer macros, you might also consider replacing them by an enum`.
try_lock, lock and unlock should not be directly used for mutexes
try_lock, lock and unlock should not be directly used for mutexes
Mutexes are synchronization primitives that allow managing concurrency using a mechanism of `lock/unlock.
While explicitly locking or unlocking a mutex is possible, it is error-prone. This is particularly true in complex code paths (or with exceptions) where it is easy to have a mismatch between locks and unlocks.
As a result, mutexes should not be locked or unlocked manually.
Adopting the C++ RAII (Resource Acquisition Is Initialization) idiom solves this problem by creating an object that will lock the mutex on creation and unlock it on destruction. Furthermore, using this idiom can also greatly improve the readability of the code.
Several classes are available as RAII wrappers:
std::scoped_lock is the default, most efficient wrapper for simple cases (only available since C++17)
std::lock_guard is similar to std::scoped_lock, but with fewer features. It should only be used if you don’t have access to std::scoped_lock.
std::unique_lock` allows more manual unlocking/locking again and should only be used when these features are needed, for instance, with condition variables.
typedefs that indicate size and signedness should be used in place of the basic types
typedefs that indicate size and signedness should be used in place of the basic types
The basic numeric types char, int, short, long, float, double, and long double
should not be used. Instead, specific-length typedefs should be. This rule helps to clarify the size of the storage, but does not guarantee portability because of the asymmetric behavior of integral promotion.
Note that it is still important to understand the integer size of the implementation, and developers should be aware of the actual implementation of the typedefs under these definitions.
Switch labels should not be nested inside non-switch blocks
Switch labels should not be nested inside non-switch blocks
A switch-label can be placed anywhere within the statements that form the body of a switch statement, potentially leading to unstructured code. To prevent this from happening, the scope of a case-label or default-label shall be the statement forming the body of a switch statement. All case-clauses and the default-clause shall be at the same scope.
Aggregates should be initialized with braces in non-generic code
Aggregates should be initialized with braces in non-generic code
With C++20, it is now possible to initialize aggregate types using parentheses. This language feature was introduced to simplify writing generic code and consistently initialize objects, whether they are aggregates or not.
For the sake of simplicity, aggregate types include arrays, unions, and structures without user-declared constructors and with only public non-static data members and public bases.
Initializing objects with parentheses has several downsides compared to braces.
Mainly, they allow narrowing conversion of arithmetic types that can result in unexpected program behaviors. See also S5276.
Secondly, they can result in the most vexing parse. For example, `Aggregate a(std::string()); declares function, while Aggregate a{std::string()}; declares a variable.
Furthermore, using braces is idiomatic and consistent with C.
For all these reasons, braces should be preferred for non-generic code when initializing aggregates. And the fix is often trivial: replace the parentheses () with braces {}`.
Here is a noncompliant example:
Polymorphic classes should suppress copying
Polymorphic classes should suppress copying
A `polymorphic class defines or inherits at least one virtual function. In some circumstances, copy-construction or assignment of such a class can inadvertently lead to casting from the derived class to the base class, which is a cause of slicing.
In most cases, you will prevent copying in a whole class hierarchy by preventing it in the base class only:
You can define an =delete copy constructor and assignment operator in the base class.
You can define an =delete` move assignment operator, which will disable all other copy/move operations (this is less explicit, but since you only have to explicitly disable one function, some authors recommend this approach)
Copy can be implicitly disabled by some existing members of the base class
This rule reports an issue if a polymorphic class is publicly copyable.
Logical expressions should not be used as array subscription index
Logical expressions should not be used as array subscription index
It is not common practice to use a logical expression as an array index, the user probably made a typo and misplaced the closing square bracket. This rule flags all array subscriptions where the index is a logical expression.
Signed and unsigned types should not be mixed in expressions
Signed and unsigned types should not be mixed in expressions
Some signed to unsigned conversions may lead to implementation-defined behavior. This behavior may not be consistent with developer expectations.
If you need to mix signed and unsigned types, you should make your intent explicit by using explicit casts and avoiding implicit casts.
This rule will detect implicit conversions that change the signedness.
Switch statement conditions should not have essentially boolean type
Switch statement conditions should not have essentially boolean type
When there is only a single condition to test, you have the option of using either a switch statement or an if-else if-else statement. For a larger set of potential values, a switch can be easier to read, but when the condition being tested is essentially boolean, then an if/else
statement should be used instead.
false should not be assigned to pointer types
false should not be assigned to pointer types
Assignment of false to a pointer type is implicitly converted to a NULL
assignment.
Global static initializers should not be used
Global static initializers should not be used
The execution order of `static initializers is unspecified when they are in different compilation units (files). Relying on a particular initialization order can have nasty repercussions and should therefore be avoided. Even if the code works now, it could change in the future without notice.
If you need to use static` globals, you should put them inside the function that uses them, or create a getter and declare them inside that getter.
Function template parameters should be named if reused
Function template parameters should be named if reused
C++20 introduces full template support for lambda functions on par with the regular template functions. The full template syntax for a lambda adds a template-arguments clause after the capture clause completing the panoply of brackets: []<>(){}. For example:
[]<typename T>(T arg) \{ return arg; \}
Although more verbose than using `auto for the types of the arguments, this syntax enables you to name the types for the parameters, constrain these types (see Concepts), and reuse these types for multiple arguments.
One common use case for the named template argument is a lambda with multiple arguments of the same type. Pre-C++20 code had to resort to the use of decltype: [](auto arg1, decltype(arg1) arg2) … . Not only is it obscure it also only approximates our goal: it requires the second-argument type to be convertible to the first-argument type.
Moreover, similar issues may appear for normal functions, that declare parameters with auto in place of type using C++20 abbreviated template syntax.
This rule reports the use of decltype(arg) for parameters introduced with auto`.
enum members other than the first one should not be explicitly initialized unless all members are explicitly initialized
enum members other than the first one should not be explicitly initialized unless all members are explicitly initialized
If an enumerator list is given with no explicit initialization of members, then C/C++ allocates a sequence of integers starting at zero for the first element and increasing by one for each subsequent element.
An explicit initialization of the first element, as permitted by this rule, forces the allocation of integers to start at the given value. When adopting this approach it is essential to ensure that the initialization value used is small enough that no subsequent value in the list will exceed the int
storage used by enumeration constants.
Explicit initialization of all items in the list, which is also permissible, prevents the mixing of automatic and manual allocation, which is error prone.
However, it is then the responsibility of the developer to ensure that all values are in the required range, and that values are not unintentionally duplicated.
#include directives in a file should only be preceded by other preprocessor directives or comments
#include directives in a file should only be preceded by other preprocessor directives or comments
To aid code readability, all the `#include directives in a particular code file should be grouped together near the top of the file. The only items which may precede an #include in a file are other preprocessor directives or comments.
Additionally, an #include may appear within an extern “C”` block, this can be used for instance to include a C file from a C++ file.
strerror should not be used
strerror should not be used
The `strerror function returns a pointer to a buffer that is only valid until the function is called again, including from another thread. Which means that in practice, for any multithread program, it’s not possible to use it properly.
One safe alternative is strerror_s, provided in annex K of C11. To have access to it, you need a standard library that supports it (this can be tested with the macro STDC_LIB_EXT1), and you need to enable it by defining the macro STDC_WANT_LIB_EXT1 before including <string.h>. strerror_s takes as an argument a buffer that will store the error message. Iworks together with the strerrorlen_s function, which can tell you the required buffer size to store the error.
Some environment also provide the strerror_r function, which works in a way similar to strerror_s`, except there is now function that can provide you with the needed buffer size (but the return value will tell you if the buffer was large enough): Either you accept to have a truncated message if the message is too long, or you should call this function in a loop with increasing buffer size until it succeeds.
Identifiers should not be longer than 31 characters
Identifiers should not be longer than 31 characters
In addition to being difficult to use, too-long variable names can limit code portability. The ISO standard requires that variable, type, function and label names be no more than 31 characters long.
Note that 31 characters is an upper bound, rather than a length recommendation. Shorter names are better, as long as they’re still communicative.
^ should not be confused with exponentiation
^ should not be confused with exponentiation
In C and its family of languages, the `^ operator performs the exclusive or (xor) operation. This can be misleading since ^ is also commonly used to designate the exponentiation operation, for instance, in BASIC or R.
This rule will flag uses of ^` in places where exponentiation is suspected to be the intended operation, i.e., on expressions that attempt to xor 2 or 10 with a constant expression.
Size of variable length arrays should be greater than zero
Size of variable length arrays should be greater than zero
Variable length arrays are used to allocate a stack size for the number of elements that are known at the runtime, for example:
Partial specialization syntax should not be used for function templates
Partial specialization syntax should not be used for function templates
Class templates can be explicitly or partially specialized. But according to the C++ standard, function templates cannot be partially specialized. Under certain conditions, the Microsoft® compiler will silently ignore the confusing application of partial specialization syntax to a function, but other compilers will raise an error for it and fail compilation.
GNU attributes should be used correctly
GNU attributes should be used correctly
`attribute is a GNU extension that allows to decorate functions, parameters, variables… with some attributes. It may help for compiler optimizations or for the writer of some code to better state his intent (and have the compiler check it).
If this extension is used incorrectly, it will usually not break the build, but it still means that the code may not behave as the developer expects. This rule reports such occurrences of bad use of attribute`.
Members should be used instead of private bases
Members should be used instead of private bases
When inheritance is used, the code of the class becomes less clear, as the declarations of members used in the implementation are not visible from the class declaration and requires inspection of the class hierarchy. This is even more prominent in the case of templates that derive from their parameters, as the members of the base are unknown until the template is instantiated.
In the case of private inheritance, those drawbacks are usually not offset by the benefits of runtime polymorphism: this class cannot be passed to functions accepting base class, and children of this class cannot access the base class features. As a consequence, replacing a private base class with a named member of the same type often improves the clarity of the code.
However, one of the reasons for using private inheritance instead of composition is that it allows the compiler to not allocate additional space for the empty member - this is known as Empty Base Optimization (EBO). Since C++20, the same effect can be achieved, more clearly, by declaring members with the [[no_unique_address]]
attribute. Therefore, using private inheritance just for this space optimization is no longer required.
This rule reports an issue on classes that has private bases class that can be replaced with a member.
Width, alignment, and padding format options should be used consistently
Width, alignment, and padding format options should be used consistently
In formatting functions like std::format, replacement fields can add format specifications as a list of options. For example, std::format(”{:>5}”, d); will display d aligned to the right with a padding of '' to its left so that the display is always at least five characters wide.
Some of these options work together, and mentioning one without the other can lead to confusing code where it is unclear to the reader how the output will look. The same can happen if the options have incompatible values.
This rule raises an issue when:
The alignment or the padding options are set, but the width option is not specified, leading to no actual padding.
Both a character padding and a 0 padding are specified on a numerical value, leading to the 0 padding being ignored.
Nonsensical implicit casts should not be made
Nonsensical implicit casts should not be made
Implicit casts which do not make sense are likely to be programming errors.
The rule reports an issue for the following implicit casts:
Boolean to pointer
Float to boolean
nullptr
constant to integer
Comma operator should not be used
Comma operator should not be used
The comma operator takes two expressions, executes them from left to right, and returns the result of the second one. The use of this operator is generally detrimental to the readability and reliability of code, and the same effect can be achieved by other means.
There should not be more than one definition of a virtual function on a path through the inheritance hierarchy
There should not be more than one definition of a virtual function on a path through the inheritance hierarchy
The main aim of this rule is clarity for maintainers and reviewers, by ensuring that the version of a function that can be executed from any point in a class hierarchy is unambiguous.
Additionally, where classes form a diamond hierarchy, call by dominance ([1] §10.3(11)) may occur resulting in a call to a function that is inconsistent with developer expectations. This rule also prevents call by dominance.
Coroutine parameters should not become dangling references
Coroutine parameters should not become dangling references
Coroutines, introduced in C++20, are functions in which execution can be suspended and resumed. When a coroutine resumes, it takes over where it left thanks to the coroutine state.
A coroutine state is an object which contains all the information a coroutine needs to resume its execution correctly: local variables, copy of the parameters…
This means that if the coroutine has a parameter that is a reference to an object, this object must exist as long as the coroutine is not destroyed. Otherwise, the reference stored in the coroutine state will become a dangling reference and will lead to undefined behavior when the coroutine resumes.
This rule detects when a coroutine parameter becomes a dangling reference.
To fix this, you can either pass the parameter by value or extend the lifetime of the parameter.
auto should be used for non-type template parameter
auto should be used for non-type template parameter
Starting `C++17, you can use auto and decltype(auto) to declare non-type template parameters. This new feature provides a way to write generic code for non-type parameters of different types. Also, it allows, by using variadic templates, to make a template take a list of non-type template parameters of different types: template<auto… VS> class A.
If the type is used in the template definition, you can replace it with auto, or decltype if you want to underline that the type is the same as of the template parameter. Note, that you can use template <class T> T packed_t(T…); to get the type of arguments in the auto…` pack (see the “Compliant Solution” section below).
This rule detects the common pattern where a type template parameter is introduced only to be used as a type for the next non-type template parameter(s).
static_assert with no message should be used over static_assert with empty or redundant message
static_assert with no message should be used over static_assert with empty or redundant message
C++11 introduced `static_assert(expr, message) to check that the compile-time constant expression expr is true.
C++17 has made the second argument message optional. This rule flags occurrences of std::static_assert where the second argument message is empty or a substring of expr`.
#pragma once should not be used
#pragma once should not be used
`#pragma once is a preprocessor directive meant to ensure that a file is only included once in a compilation unit. However, there are several reasons to avoid it:
It is not part of the standard, which prevents its use in some contexts.
Even if it is supported by virtually all compilers, since its behavior is not defined, it may differ between compilers, especially for some corner cases when determining if two files are identical (for instance, in the presence of symbolic links).
Its semantic is slightly different from the semantic of an include guard. For instance, if a file is duplicated in two different locations, #pragma once will not prevent multiple inclusion of this file.
Note: There used to be a build performance improvement when using #pragma once instead of an include guard because naive implementations of include guards need to parse the full file to get the #endif matching the #if. But most modern compilers specifically detect the include guard pattern and use a dedicated optimization that makes it about as fast as #pragma once`.
A pointer operand and any pointer resulting from pointer arithmetic using that operand should both address elements of the same array
A pointer operand and any pointer resulting from pointer arithmetic using that operand should both address elements of the same array
This rule applies to expressions of the form:
integer_expression + pointer_expression
pointer_expression + integer_expression
pointer_expression - integer_expression
++pointer_expression
pointer_expression++
—pointer_expression
pointer_expression—
pointer_expression [ integer_expression ]
where pointer_expression is a pointer to an array element.
It is undefined behaviour if the result obtained from one of the above expressions is not a pointer to an element of the array pointed to by pointer_expression or an element one beyond the end of that array.
Bit fields should not be used
Bit fields should not be used
The real need for bit fields is narrow and highly specialized. Previously, they were used to save memory, but that’s less a concern in modern systems than are the extra instructions required to interact with them. Today, they may be needed in direct hardware interaction, but since their behavior is platform-dependent, getting them right can be tricky, and since their use is increasingly rare these days, they’re likely to confuse maintainers. For these reasons, it’s simpler and more performant to use another field type instead of bit fields.
Declaration specifiers should not be redundant
Declaration specifiers should not be redundant
Redundant declaration specifiers should be removed or corrected. Typically, they represent bugs. A specifier modifies the type or pointer to its left. Only when it is at the far left does it apply to the right.
Pointer conversions should be restricted to a safe subset
Pointer conversions should be restricted to a safe subset
Casting an object pointer can very easily lead to undefined behavior. Only a few cases are supported, for instance casting an object pointer to a large enough integral type (and back again), casting an object pointer to a pointer to void (and back again)… Using a pointer cast to access an object as if it was of another type than its real type is not supported in general.
This rule detects casts between object pointers and incompatible types.
Comment styles // and /* ... */ should not be mixed within a file
Comment styles // and /* ... */ should not be mixed within a file
Template argument lists should end with >>
Template argument lists should end with >>
C++11 improves the specification of the parser so that at the end of a template, multiple >s will be interpreted as closing the template argument list rather than as the right shift operator or stream extraction operator. It is no longer required to place a space between the two >
s.
This rule raises an issue when such useless spaces exist.
std::byte should be used when you need byte-oriented memory access
std::byte should be used when you need byte-oriented memory access
C++17 introduced std::byte. It allows you to have byte-oriented access to memory in a type-safe, unambiguous manner. Before, you had to use either `char, signed char, or unsigned char to access memory as bytes. The previous approach was error-prone as the char type allows you to accidentally perform arithmetic operations. Also, it was confusing since char, signed char, and unsigned char are also used to represent actual characters and arithmetic values.
std::byte is simply a scoped enumeration with bit-wise operators and a helper function to_integer<T> to convert byte object to integral type T.
This rule will detect byte-like usage of char, signed char, and unsigned char and suggest replacing them by std::byte`.
Type-constraints should not be used for forwarding reference parameters
Type-constraints should not be used for forwarding reference parameters
Type-constraints provide a concise way to express constraints on the type deduced for a given template parameter or auto placeholder. In a situation when a type-constraint is applied to a forwarding reference parameter (T&&), the corresponding concept will be checked against the lvalue reference (if the argument is an lvalue) or the plain type (if the argument is an rvalue):
Types should be token-for-token identical in all declarations and re-declarations
Types should be token-for-token identical in all declarations and re-declarations
If a re-declaration has compatible types but not types which are token-for-token identical, it may not be clear to which declaration that re-declaration refers.
Atomic types should be used instead of volatile types
Atomic types should be used instead of volatile types
The main intended use-case for `volatile in C and C++ is to access data that can be modified by something external to the program, typically some hardware register. In contrast with other languages that provide a volatile keyword, it does not provide any useful guarantees related to atomicity, memory ordering, or inter-thread synchronization. It is only really needed for the kind of low-level code found in kernels or embedded software, i.e. using memory-mapped I/O registers to manipulate hardware directly.
According to the C standard:
Only C11/C++11 “atomic types” are free from data races, and you should use them or synchronization primitives if you want to avoid race conditions.
This rule raises an issue when a local variable or class data member is declared as volatile` (at the top level of the type, pointers to volatile are not reported).
Switches should be used for sequences of simple tests
Switches should be used for sequences of simple tests
When a single primitive is tested against three or more values in an if/else if
structure, it should be converted to a switch instead for greater readability.
Access specifiers should not be redundant
Access specifiers should not be redundant
Redundant access specifiers should be removed because they needlessly clutter the code.
Functions should not be defined with a variable number of arguments
Functions should not be defined with a variable number of arguments
Passing arguments via an ellipsis bypasses the type checking performed by the compiler. Additionally, passing an argument with non-POD class type leads to undefined behavior. Note that the rule specifies “defined” (and not “declared”) so as to permit the use of existing library functions.
WIP: An array of instances of a class should not be converted to a pointer to a base class
WIP: An array of instances of a class should not be converted to a pointer to a base class
A base class and its derived class often differ in size.
Accessing an array of a derived class through a pointer to the base class leads to wrong pointer arithmetic and can then corrupt memory.
NULL should not be thrown
NULL should not be thrown
throw(NULL) is equivalent to throw(0), and is therefore caught by an integer handler. However, since NULL is typically used in the context of pointers, developers may expect it to be caught by a pointer-to-type handler. Thus to avoid confusion, zero should be thrown instead of NULL
.
Object and function types should be explicitly stated in their declarations and definitions
Object and function types should be explicitly stated in their declarations and definitions
The C90 standard allows implicit typing of variables and functions, and some C compilers still support legacy code by allowing implicit typing. But it should not be used for new code because it might lead to confusion.
C++ macros should only be used for include guards, type qualifiers, or storage class specifiers
C++ macros should only be used for include guards, type qualifiers, or storage class specifiers
These are the only permitted uses of macros. C++ offers const
variable and function templates, which provide a type-safe alternative to the preprocessor.
Unused variables should be removed
Unused variables should be removed
Variables declared and never used in a project constitute noise and may indicate that the wrong variable name has been used somewhere. Removing these declarations reduces the possibility that they may later be used instead of the correct variable.
If padding is used within bit-fields, then the padding members should be unnamed to avoid violation of this rule.
Dereferenced null pointers should not be bound to references
Dereferenced null pointers should not be bound to references
Dereferencing a null pointer has undefined behavior, and it is particularly harmful if a reference is then bound to the result, because a reference is assumed to refer to a valid object.
Error information returned from functions should be tested
Error information returned from functions should be tested
A function (whether it is part of the standard library, a third party library or a user defined function) may provide some means of indicating the occurrence of an error. This may be via a global error flag, a parametric error flag, a special return value or some other means. Whenever such a mechanism is provided by a function the calling program shall check for the indication of an error as soon as the function returns.
Note, however, that the checking of input values to functions is considered a more robust means of error prevention than trying to detect errors after the function has completed.
Named bit-fields with signed integer type should have a length of more than one bit
Named bit-fields with signed integer type should have a length of more than one bit
The values which may be represented by a bit-field of length one may not meet developer expectations. Anonymous signed bit-fields of any length are allowed.
Containers should be accessed with valid iterator arguments
Containers should be accessed with valid iterator arguments
Container member functions that modify the containers often take an iterator as input to specify a precise location work on. If this iterator comes from a different container than the one calling the function, the result will be undefined.
Classes should explicitly specify the access level when specifying base classes
Classes should explicitly specify the access level when specifying base classes
When specifying base classes for a class, the access level is private by default. While private inheritance has its uses, it is far less used than public inheritance. Therefore:
Either you want public inheritance, and you have to specify it
Or you want private inheritance, and you should be explicit about this design decision, by specifying it too.
Default parameters should not be defined
Default parameters should not be defined
Setting method parameter defaults seems like a tidy way to make a method more usable. However, function pointers to methods with defaulted parameters can be confusing, because the function signature may not seem to match the call signature. Therefore, the use of multiple, overloaded methods is preferred.
Functions should be declared explicitly
Functions should be declared explicitly
The use of prototypes enables the compiler to check the integrity of function definitions and calls. Without prototypes the compiler is not obliged to pick up certain errors in function calls (e.g. different number of arguments from the function body, mismatch in types of arguments between call and definition). Function interfaces have been shown to be a cause of considerable problems, and therefore this rule is considered very important.
The recommended method of implementing function prototypes for external functions is to declare the function (i.e. give the function prototype) in a header file, and then include the header file in all those code files that need the prototype (see MISRA C 2004, Rule 8.8).
Return value of nodiscard functions should not be ignored
Return value of nodiscard functions should not be ignored
If a function is defined with a [[nodiscard]] attribute or if it returns an object which is [[nodiscard]]
, its return value is very important and should not be silently ignored.
Unevaluated operands should not have side effects
Unevaluated operands should not have side effects
Operands of `sizeof, noexcept and decltype are unevaluated. So side effects in these operands (which are all the effects an expression can have in addition to producing a value), will not be applied. And that may be surprising to the reader.
Additionally, the operand of typeid` may or may not be evaluated, depending on its type: it will be evaluated if it is a function call that returns reference to a polymorphic type, but it will not be evaluated in all the other cases. This difference of behavior is tricky to apprehend and that is why both cases are reported here.
This rules reports an issue when operands of such operators have side-effects.
Bit-fields should not have enum type
Bit-fields should not have enum type
The use of enum as a bit-field type is prohibited because ISO/IEC 14882:2003 does not explicitly define the underlying representation as signed or unsigned
. It is therefore not possible to determine the exact number of bits required to represent all values in the enumeration.
A call to wait() on a std::condition_variable should have a condition
A call to wait() on a std::condition_variable should have a condition
A `condition variable is a synchronization primitive that can be used to block a thread, or multiple threads at the same time, until another thread both modifies a shared variable (the condition), and notifies the condition variable.
Waiting for a condition variable` without a condition can lead to spurious wake-ups or to wait forever.
Coroutines should have well-defined exception behavior
Coroutines should have well-defined exception behavior
When a regular function throws an exception, stack unwinding occurs. This applies to exceptions thrown within the function body or from an inner function call.
C++20 introduced coroutines, a stackless method to invoke functions that can be suspended and resumed. As coroutines have no stack, exceptions behave differently across a coroutine boundary.
The Promise object of any coroutine is required to have an unhandled_exception function. If an exception escapes the coroutine function body, the unhandled_exception function is called, and the coroutine reaches the final-suspend point. Resuming the coroutine after this point is undefined behavior.
The unhandled_exception method is used to define such behavior. The exception can be obtained with std::current_exception and can be logged, rethrown, or stored:
If rethrown, the exception will be received in any thread that resumes the coroutine.
If stored, it can be propagated through the Promise object to the awaiter.
If no exceptions were expected from the coroutine, the program can be terminated.
Choosing an approach depends on the coroutine use-case. Also, keep in mind the following:
Rethrowing in unhandled_exception will cause the coroutine to reach the final-suspend point without calling final_suspend first.
A noexcept specified coroutine will only terminate the program if an exception is thrown from the Promise type’s construction. This happens because the coroutine internal mechanisms wrap the coroutine body in a try-catch block. To enforce noexcept on a coroutine, the program should be terminated in the promise_type unhandled_exception function.
Control should not be transferred into a complex logic block using a goto or a switch statement
Control should not be transferred into a complex logic block using a goto or a switch statement
Having a `switch and its cases wholly encompassed by a control structure such as a try, @try, catch, @catch, or a loop is perfectly acceptable. (try and catch are used hereafter to refer to both variants.) It is also acceptable to have a goto and its target label wholly encompassed in a control structure.
What is not acceptable is using a goto or case to suddenly jump into the body of a try, catch, Objective-C @finally, or loop structure. Tangling labels or switch blocks with other control structures results in code that is difficult, if not impossible to understand. More importantly, when it compiles (some of these constructs won’t compile under ISO-conformant compilers), it can lead to unexpected results. Therefore this usage should be strictly avoided.
This C++ code sample, which is also applicable to Objective-C if try and catch are converted to @try and @catch, demonstrates jumping into a switch and into a try and catch` :
Copy constructors should only initialize their classes base classes and non-static members
Copy constructors should only initialize their classes base classes and non-static members
If a compiler implementation detects that a call to a copy constructor is redundant, then it is permitted to omit that call, even if the copy constructor has a side effect other than the construction of a copy of the object. This is called “copy elision”.
It is therefore important to ensure that a copy constructor does not modify the program state, since the number of such modifications may be indeterminate.
Concise syntax should be used for concatenatable namespaces
Concise syntax should be used for concatenatable namespaces
Namespaces represent a cross-file named scope. They are very useful to organize code and interfaces without cluttering a unique namespace. For instance, they provide a much cleaner way to avoid name collisions than using bad long names.
Namespaces can be nested to provide even more structure to type and symbol names. In that case, namespaces can be nested one inside another like scopes would with curly braces.
In C++17, a new concise syntax was introduced to increase the readability of nested namespaces. It is much less verbose and involves much less curly braces-delimited scopes. Whereas declaring a nested namespace of depth N requires N pairs of curly braces with the original syntax, this new syntax requires only one pair of curly braces. This syntax is much more readable and less error-prone. When possible, non-inlined or inlined (since C++20) named namespaces should be concatenated.
Explicit argument indexing in std::format should be used only for non-trivial ordering
Explicit argument indexing in std::format should be used only for non-trivial ordering
Calls to std::format and std::vformat
can receive either format strings whose replacement fields are fully indexed or fully non-indexed.
Explicitly indexing replacement fields is useful when arguments are not used in order or are used multiple times. Implicit indexing has the advantage of being terse and simple.
Using explicit indexing in a context where implicit indexing would have the same behavior is more verbose and error-prone. It might confuse the reader, who might expect the arguments not to be used in order.
This rule raises an issue on explicit indexing that should be replaced by implicit indexing.
Single-bit named bit fields should not be of a signed type
Single-bit named bit fields should not be of a signed type
The values that can be represented by a signed bit field with a length of one bit may not meet developer expectations. For example, according to the C99 Standard, a single-bit signed bit-field has a single (one) sign bit and no (zero) value bits.
This rule does not apply to unnamed bit fields, as their values cannot be accessed.
final classes should not have protected members
final classes should not have protected members
The difference between private and protected visibility is that child classes can see and use protected members but cannot see private ones. Since a final class will have no children, marking the members of a final class protected
is confusingly pointless.
Boolean operations should not have numeric operands, and vice versa
Boolean operations should not have numeric operands, and vice versa
There are several constructs in the language that work with boolean:
If statements: `if (b) …
Conditional operator: int i = b ? 0 : 42;
Logical operators: (b1 || b2) && !b3
Those operations would also work with arithmetic or enum values operands, because there is a conversion from those types to bool. However, this conversion might not always be obvious, for instance, an integer return code might use the value 0 to indicate that everything worked as expected, but converted to boolean, this value would be false, which often denotes failure. Conversion from integer to bool should be explicit.
Moreover, a logical operation with integer types might also be a confusion with the bitwise operators (&, | and ~).
Converting a pointer to bool to check if it is null is idiomatic and is allowed by this rule. We also allow the use of any user-defined type convertible to bool (for instance std::ostream), since they were specifically designed to be used in such situations. What this rule really detects is the use or arithmetic types (int, long…) and of enum types.
On the other hand, arithmetic operations are defined with booleans, but usually make little sense (think of adding two booleans). Booleans should not be used in an arithmetic context.
Finally, comparing a boolean with the literals true or false` is unnecessarily verbose, and should be avoided.
Transparent function objects should be used with associative std::string containers
Transparent function objects should be used with associative std::string containers
Transparent function objects are function-like types that support heterogeneous operations. There are essentially two kinds of such types: transparent comparators and transparent hashers. For instance, a transparent comparator for strings would support comparing a std::string with string-like types (such as char const* or std::string_view).
These transparent function objects are interesting for search-optimized containers such as std::set and std::map, including their multi and unordered variants. When transparent comparators/hashers are used, the containers enable additional overloads for many operations that support types different from their key_type.
For example, std::set<std::string> is not using transparent comparators. Invoking many member functions with a non-std::string argument leads to, implicitly or explicitly, creating a temporary std::string object because the functions only support an argument of the key_type`.
main should return an int
main should return an int
Even if the compiler allows it, `main should not return void. The return value of main is used by callers to determine whether the program executed successfully or not. A 0 return value indicates that the program completed successfully. Anything else indicates an error.
Since both standards and conventions dictate that main return an int, any caller that evaluates the return value of a void main method will believe the program executed successfully, regardless of the actual outcome.
Further, main’s return type should not be left to default to int, as happens when it is not expressly listed. Instead, it should be set explicitly to int`.
Use std::variant instead of unions with non-trivial types.
Use std::variant instead of unions with non-trivial types.
In order to save memory, unions allow you to use the same memory to store objects from a list of possible types as long as one object is stored at a time.
In C and in C++ prior to C++11, unions are restricted to trivial types.
Starting from C++11, it is possible to use unions with non-trivial types with the following limitations :
You have to manually handle the lifetime of the active member, using placement new and explicit object destruction.
You have to define special members like destructor and copy-constructor while taking into consideration the active member.
In some cases, code that fails to perfectly follow those rules may still compile, but lead to memory corruption.
C++17 introduced std::variant
which can replace unions while removing this burden and the associated risk. As a safer and more readable alternative, they should be preferred.
Loops should not have more than one break or goto statement
Loops should not have more than one break or goto statement
Restricting the number of exits from a loop is done in the interests of good structured programming. One break or goto
statement is acceptable in a loop since this allows, for example, for dual-outcome loops or optimal coding.
Unused assignments should be removed
Unused assignments should be removed
Computing or retrieving a value only to then immediately overwrite it or throw it away indicates a serious logic error in the code.
Assigning a value to a local variable that is not read by any subsequent instruction is called a dead store. The following code snippet depicts a few dead stores.
The pre-processor should only be used for file inclusion and include guards
The pre-processor should only be used for file inclusion and include guards
C++ provides safer ways of achieving what is often done using the pre-processor, by way of inline functions and constant declarations.
Pointers should not be cast to integral types
Pointers should not be cast to integral types
The size of integer required to hold a memory address is implementation-dependent. Therefore, casting a pointer (i.e. a memory address) to any integral data type may result in data loss because the integral type is too small to hold the full address value.
When treating a memory address as integer type is absolutely required, you should be sure to use a large enough type to hold all the data.
Result of the standard remove algorithms should not be ignored
Result of the standard remove algorithms should not be ignored
Despite their names, the standard remove algorithms (std::remove, std::remove_if, std::unique) do not erase elements from a given range. Instead, they shift the preserved (not removed) elements to the beginning of the range and return an iterator after the last preserved element. The “removed” elements have unspecified values.
C++20 introduced functions in the std::ranges namespace with the same names. Aside from returning a subrange instead of an iterator, they exhibit the same behavior.
Ignoring the result of any of these functions indicates a bug: It is impossible to distinguish removed elements in the container from the others. As a result, any further operations on the container may access elements with unspecified values. And this may lead to invalid program states, data corruption, or crashes.
atof, atoi and atol from <stdlib.h> should not be used
atof, atoi and atol from <stdlib.h> should not be used
<stdlib.h>‘s atof, atoi, and atol
functions, which convert strings to numbers, have undefined behavior when the strings cannot be converted, and should therefore be avoided.
any_of, all_of and none_of should be used
any_of, all_of and none_of should be used
Using `<algorithm> header non-modifying operations all_of, none_of and any_of allows to simplify the code and make it less error-prone.
When using versions of the standard between C++11 and C++17 included you should use:
std::all_of: return true if all elements in the given range are matching the given predicate, false otherwise
std::none_of: return true if no elements in the given range are matching the given predicate, false otherwise
std::any_of: return true if at least one element in the given range is matching the given predicate, false otherwise
In C++20, range-based alternatives have been introduced:
std::ranges::all_of
std::ranges::none_of
std::ranges::any_of`
This rule will detect common patterns that can be replaced by these three STL algorithms.
Setting capabilities is security-sensitive
Setting capabilities is security-sensitive
n lead to privilege escalation.
Linux capabilities allow you to assign narrow slices of `root’s permissions to files or processes. A thread with capabilities bypasses the normal kernel security checks to execute high-privilege actions such as mounting a device to a directory, without requiring (additional) root privileges.
Member functions that can be made static or const should be made static or const respectively
Member functions that can be made static or const should be made static or const respectively
Declaring a member function static or const limits its access to the non-static data members.
This helps to prevent unintentional modification of the data, and facilitates compliance with MISRA C++ 2008 Rule 7–1–1 (A variable which is not modified shall be const qualified).
emplace should be prefered over insert with std::set and std::unordered_set
emplace should be prefered over insert with std::set and std::unordered_set
`emplace enables you to avoid copying or moving the value you are about to insert and, instead, it constructs it in-place with the arguments provided.
Prefer using emplace, or emplace_hint if all the conditions hold:
You are inserting a single value.
You are constructing a fresh temporary value just to insert it into the set.
You expect that the key is not in the set.
You should keep the insert in any of the cases below:
You are inserting multiple values in one shot.
You are inserting a pre-existing value that is constructed for another purpose.
You are inserting an object that is cheap to move or to copy (e.g., an integer).
The key you are inserting is likely to be in the set (in this case by using insert you avoid creating a useless temporary node).
This rule detects calls to insert that lead to the creation of a large temporary object that can be avoided by using the emplace` member function.
Comparision operators (<=>, ==) should be defaulted unless non-default behavior is required
Comparision operators (<=>, ==) should be defaulted unless non-default behavior is required
Comparison operator implementations like `== or <=>, despite not being hard to write, remain a source of bugs as they must be updated with every change in the class’s member list. For instance, if the operation does not consider a newly introduced member in the class, the issue will only manifest if two instances are identical, except for the freshly introduced member. As a consequence, this type of bug is usually hard to spot.
C++20 introduced the ability to define both operator<=> and operator== as defaulted (= default) to indicate that they should consider all members in the order of their declaration. This makes code concise and makes all the comparison operators resilient to the changes to the list of members. Thanks to operator rewriting, all other comparison operations (!=, <, >, <=, =>) can also rely on these robust operators.
Furthermore, when operator<=> is defined as defaulted, the compiler will generate a defaulted version of operator==` if no other version is declared.
Subscript operator should be const-overloaded
Subscript operator should be const-overloaded
Usually, it is the container data structures that define `operator[] to mimic the traditional C-array interface. The subscript operator looks up an element in the container. It can return the element by value or by (optionally const) reference. You need a non-const subscript operator returning non-const reference to the element to be able to modify it. You need a const subscript operator to make a const-qualified instance of the container useful and enable the read-access with a consistent lookup interface.
This rule raises an issue on a class that has a non-const operator[] overload that does not modify *this object but lacks a const` overload.
<ctime> should not be used
<ctime> should not be used
Various aspects of ctime
are implementation-defined or unspecified, such as the formats of times.
The address of an automatic object should not be assigned to another object that may persist after the first object has ceased to exist
The address of an automatic object should not be assigned to another object that may persist after the first object has ceased to exist
An automatic object is an object whose lifetime is automatically managed. The storage for an automatic object, e.g. a local variable, is allocated at the beginning of the enclosing code block and is deallocated at the end. This is commonly referred to as “allocated on the stack”.
If the address of an automatic object is assigned to another automatic object of larger scope, a static or extern object, or if it is returned from a function (using return or an output parameter), then there will be a point where the address will point to an object that ceased to exist. In that case, the address becomes invalid, and attempts to dereference the invalid address — trying to access the object that ceased to exist — result in undefined behavior.
Using strncat or wcsncat is security-sensitive
Using strncat or wcsncat is security-sensitive
a buffer of characters, normally using the `null character as a sentinel for the end of the string. This means that the developer has to be aware of low-level details such as buffer sizes or having an extra character to store the final null character. Doing that correctly and consistently is notoriously difficult and any error can lead to a security vulnerability, for instance, giving access to sensitive data or allowing arbitrary code execution.
The function char *strncat( char *restrict dest, const char *restrict src, size_t count ); appends the characters of string src at the end of dest, but only add count characters max. dest will always be null-terminated. The wcsncat` does the same for wide characters, and should be used with the same guidelines.
Non-const global variables should not be used
Non-const global variables should not be used
A global variable can be modified from anywhere in the program. At first, this might look convenient, but it makes programs harder to understand. When you see a function call, you cannot know if the function will affect the value of the variable or not. You have lost the ability to reason locally about your code and must always have the whole program in mind.
Additionally, global variables are often subject to race conditions in multi-threaded environments.
Some global variables defined in external libraries (such as std::cout, std::cin, std::cerr
) are acceptable to use, but you should have a good reason to create your own. If you use a global variable, ensure they can be safely accessed concurrently.
This rule detects all declarations of global variables (in the global namespace or any namespace) that are not constant.
The plain char type shall be used only for the storage and use of character values
The plain char type shall be used only for the storage and use of character values
The char type within C++ is defined for use with the implementation character set. It is implementation-defined if char
is signed or unsigned, and it is therefore unsuitable for use with numeric data.
Character values consist of character literals or strings. A character set maps text characters onto numeric values; the character value is the text itself.
The values of expressions should not be implicitly converted to a different underlying type
The values of expressions should not be implicitly converted to a different underlying type
The value of an expression shall not be implicitly converted to a different type if:
it is not a conversion to a wider type, or
the expression is complex, or
the expression is a function argument, or
the expression is a return expression.
The intention when restricting implicit conversion of complex expressions is to require that in a sequence of arithmetic operations within an expression, all operations should be conducted in exactly the same
arithmetic type. Notice that this does not imply that all operands in an expression are of the same type. The expression u32a + u16b + u16c is compliant – both additions will notionally be performed in type U32.
The expression u16a + u16b + u32c is not compliant – the first addition is notionally performed in type U16 and the second in type U32. The word “notionally” is used because, in practice, the type in which arithmetic will be conducted will depend on the implemented size of an int
. By observing the principle whereby all operations are performed in a consistent (underlying) type, it is possible to avoid programmer confusion and some of the dangers associated with integral promotion.
using should be preferred for type aliasing
using should be preferred for type aliasing
Since C++11, type aliases can be declared via `using or typedef. using should be preferred as more readable because you see the new name/alias first.
In addition, using can be templated, which makes it applicable to more situations than typedef`.
<signal.h> should not be used
<signal.h> should not be used
Signal handling contains implementation-defined and undefined behavior.
Inline variables should be used to declare global variables in header files
Inline variables should be used to declare global variables in header files
C++17
introduced inline variables. They provide a proper way to define global variables in header files. Before inline variables, it wasn’t possible to simply define global variables without compile or link errors:
Instead, you had to resort to less readable inconvenient workarounds like variable templates or functions that return a static object. These workarounds will initialize the variables when used instead of the start of the program, which might be inconvenient depending on the program.
This rule will detect these workarounds and suggest using inline variables instead.
std::auto_ptr should not be used
std::auto_ptr should not be used
`std::auto_ptr was a pre-C++11 attempt to do what std::unique_ptr now does. Unfortunately, the move semantics needed to make it work properly weren’t in place, so copying a std::auto_ptr has the very surprising behavior of invalidating the source of the copy.
That problem has been fixed with std::unique_ptr, so std::auto_ptr has been deprecated in C++11 and removed in C++17.
If your compiler allows it, you should replace all use of std::auto_ptr with std::unique_ptr`. Otherwise, define your own (non-copyable) smart pointer.
The unary & operator should not be overloaded
The unary & operator should not be overloaded
Taking the address of an object of incomplete type, where the complete type contains a user declared operator &
leads to undefined behavior.
STL algorithms and range-based for loops should be preferred to traditional for loops
STL algorithms and range-based for loops should be preferred to traditional for loops
`for-loops are a very powerful and versatile tool that can be used for many purposes. This flexibility comes with drawbacks:
It is very easy to make a small mistake when writing them,
They are relatively verbose to write,
They do not express the intent of the code, the reader has to look at loop details to understand what the loop does.
There are algorithms that encapsulate a for-loop and give it some meaning (std::all_of, std::count_if, std::remove_if…). These algorithms are well tested, efficient, and explicit and therefore should be your first choice.
This rule detects loops that go through all consecutive elements of a sequence (eg: containers, objects with begin() and end() member functions), and deal only with the current element without side-effects on the rest of the sequence.
This rule suggests using one of the supported STL algorithm patterns corresponding to your C++ standard when a loop matches it.
Currently, this rule supports:
std::all_of (since C++11) and std::ranges::all_of (since C++20): returns true if all elements in the given range are matching the given predicate, false otherwise
std::none_of (since C++11) and std::ranges::none_of (since C++20): returns true if no elements in the given range are matching the given predicate, false otherwise
std::any_of (since C++11) and std::ranges::any_of (since C++20): returns true if at least one element in the given range is matching the given predicate, false otherwise
std::ranges::contains (since C++23): returns true if at least one element in the given range is equal to the given value, false otherwise
This rule suggests two options below when the loop doesn’t match any of the supported STL algorithm patterns and you just want to iterate over all elements of a sequence:
Range-based for-loops, which were introduced in C++11 and will run through all elements of a sequence
std::for_each, an algorithm that performs the same operation between two iterators (allowing more flexibility, for instance by using reverse_iterator`s, or with a variant that can loop in parallel on several elements at a time).
Type and object identifiers should be defined in blocks that minimize their visibility
Type and object identifiers should be defined in blocks that minimize their visibility
Defining variables in the minimum block scope possible reduces the visibility of those variables and therefore reduces the possibility that these identifiers will be used accidentally. A corollary of this is that global objects (including singleton function objects) shall be used in more than one function.
Digit separators should be used
Digit separators should be used
C++14 adds the ability to write numbers with digit separators for better readability. Splitting a number that has more than 4 consecutive digits improves readability.
This rule verifies that numbers are written using digit separators when they have more than 4 consecutive digits.
For any given template specialization, an explicit instantiation of the template with the template- arguments used in the specialization should not render the program ill-formed
For any given template specialization, an explicit instantiation of the template with the template- arguments used in the specialization should not render the program ill-formed
An implicit template specialization does not instantiate every member of the template. Where instantiation of a member would result in an ill-formed program it is not clear that the template should be used with the supplied template-arguments.
reinterpret_cast should not be used
reinterpret_cast should not be used
Because `reinterpret_cast does not perform any type safety validations, it is capable of performing dangerous conversions between unrelated types, often leading to undefined behavior.
In some cases, reinterpret_cast can be simply replaced by a more focused cast, such as static_cast.
If the goal is to access the binary representation of an object, reinterpret_cast leads to undefined behavior. Before C++20, the correct way is to use memcpy to copy the object’s bits. Since C++20, a better option is available: std::bit_cast allows to reinterpret a value as being of a different type of the same length preserving its binary representation (see also S6181).
This rule raises an issue when reinterpret_cast` is used.
Member functions should not return non-const handles to class data
Member functions should not return non-const handles to class data
By implementing class interfaces with member functions the implementation retains more control over how the object state can be modified and helps to allow a class to be maintained without affecting clients. Returning a handle to class-data allows for clients to modify the state of the object without using any interfaces.
Literal zero (0) should not be used as the null-pointer-constant
Literal zero (0) should not be used as the null-pointer-constant
In C++, the literal 0 is both an integer type and the null-pointer-constant. To meet developer expectations, NULL
should be used as the null-pointer-constant, and 0 for the integer zero.
All declarations of the same function (in other translation units) should have the same set of type-ids
All declarations of the same function (in other translation units) should have the same set of type-ids
It is undefined behaviour if a function has different exception-specifications in different translation units.
const references to numbers should not be made
const references to numbers should not be made
There is no point in creating a const
reference to a literal numeric value. Most likely the intent was not to create a reference, but a constant value.
The sizeof and alignof operator should not be used with operands of a void type
The sizeof and alignof operator should not be used with operands of a void type
Although some compilers will allow it, the use of sizeof and alignof with arguments that have a void type is forbidden by both the C and C++ standards.
Argument of printf should be a format string
Argument of printf should be a format string
It is a security vulnerability to call printf with a unique string argument that is not a string literal. Indeed, if this argument comes from a user input, this user can:
make the program crash by executing code equivalent to: `printf(“%s%s%s%s%s%s%s%s”)
view the stack or memory at any location by executing code equivalent to: printf(“%08x %08x %08x %08x %08x\n”)
Starting with C++23, std::print` should be preferred: its arguments are validated at compile-time, making it more secure.
Function-like macros should not be used
Function-like macros should not be used
It is tempting to treat function-like macros as functions, but the two things work differently. For instance, the use of functions offers parameter type-checking, while the use of macros does not. Additionally, with macros, there is the potential for a macro to be evaluated multiple times. In general, functions offer a safer, more robust mechanism than function-like macros, and that safety usually outweighs the speed advantages offered by macros. Therefore functions should be used instead when possible.
A for loop shall contain a single loop-counter which shall not have floating type
A for loop shall contain a single loop-counter which shall not have floating type
When using a floating-point loop counter, accumulation of rounding errors may result in a mismatch between the expected and actual number of iterations. This can happen when a loop step that is not a power of the floating-point radix is rounded to a value that can be represented.
Even if a loop with a floating-point loop counter appears to behave correctly on one implementation, it may give a different number of iterations on another implementation.
No object or function identifier with static storage duration should be reused
No object or function identifier with static storage duration should be reused
Regardless of scope, no identifier with static
storage duration should be re-used across any source files in the project. This includes objects or functions with external linkage and any objects or functions with the static storage class specifier.
While the compiler can understand this and is in no way confused, the possibility exists for the developer to incorrectly associate unrelated variables with the same name.
Constructors that are callable with a single argument of fundamental type should be explicit
Constructors that are callable with a single argument of fundamental type should be explicit
The explicit keyword prevents the constructor from being used to implicitly convert from a fundamental type to the class type.
std::move should only be used where moving can happen
std::move should only be used where moving can happen
When calling std::move on an object, we usually expect the resulting operation to be fast, using move semantic to rip data off the source object. If, despite the call to std::move, the source object ends up being copied, the code might be unexpectedly slow.
This can happen:
When std::move is called on an object which does not provide a specific move constructor and will resort to copying when requested to move.
When calling std::move with a const argument.
When passing the result of std::move as a const reference argument. In this case, no object will be moved since it’s impossible to call the move constructor from within the function. std::move should only be used when the argument is passed by value or by r-value reference.
constexpr should be used for an unmodified constinit variable
constexpr should be used for an unmodified constinit variable
C++20 introduces a new keyword `constinit that requires initialization of the variable to be done at compile time.
The similarity in the names might raise confusion between constexpr, const, and constinit:
const prohibits the changes to a variable after its initialization. It does not restrict the initializer in any way.
constinit does not prohibit variable changes after initialization, it only restricts the initializer expression to compile time.
constexpr is const constinit + “constant destruction”: it requires that the variable is initialized at compile time and that it is never changed, plus it requires the type to have constant destruction, i.e., a plain type or a class type with a constexpr destructor.
To address the confusion this rule reports constinit variables that are const or effectively const` (i.e., never modified) and have constant destruction.
Parameters in a function prototype should be named
Parameters in a function prototype should be named
Naming the parameters in a function prototype helps identify how they’ll be used by the function, thereby acting as a thin layer of documentation for the function.
Free functions should be preferred to member functions when accessing a container in a generic context
Free functions should be preferred to member functions when accessing a container in a generic context
It is often considered a better style to access objects in generic code with free functions than with member functions because it allows one to adapt an object to a template without modifying it just by adding the right overload of the free function. This is especially true with containers, which can come in a wide variety (and some of them can’t even have member functions, for instance, C-style arrays).
Therefore, the C++ standard library provides free functions that can be applied on any standard container and that can be adapted for user-defined containers. They are:
Since C++11: `std::begin, std::end, std::cbegin, std::cend
Since C++14: std::rbegin, std::rend, std::crbegin, std::crend
Since C++17: std::size, std::empty, std::data
Since C++20: std::ssize`
When writing generic code, you should prefer using those functions for objects that depend on the template arguments: it will allow your code to work with a wider variety of containers.
String literals should not be concatenated implicitly
String literals should not be concatenated implicitly
While in C, and derived languages, it is legal to concatenate two literals by putting them next to each other, this is only justified in a few cases. For instance if one is a macro or if the layout makes it clearer.
Octal values should not be used
Octal values should not be used
Integer literals starting with a zero are octal rather than decimal values. While using octal values is fully supported, most developers do not have experience with them. They may not recognize octal values as such, mistaking them instead for decimal values.
Hexadecimal literals (`0xdeadbeef) and binary literals (0b0101’0110’00011, available since C++14), on the other hand, have a clear marker (0x or 0b) and can be used to define the binary representation of a value.
Character literals starting with \ and followed by one to three digits are octal escaped literals. Character literals starting with \x` and followed by one or more hexits are hexadecimal escaped literals, and are usually more readable.
case ranges should not be empty
case ranges should not be empty
The GNU compiler gives the possibility to specify a range of consecutive values in a case label, for example: case: 1 … 5.
However, if the values are written in decreasing order, i.e., from the larger value to the smaller one, the range will evaluate as empty. So the case body will never be executed.
make_unique and make_shared should be used to construct unique_ptr and shared_ptr
make_unique and make_shared should be used to construct unique_ptr and shared_ptr
Appropriate size arguments should be passed to strncat and strlcpy
Appropriate size arguments should be passed to strncat and strlcpy
The string manipulation functions `strncat, strlcat and strlcpy require a size argument that describes how many bytes from the source buffer are used at most. In many situations the size of the source buffer is unknown, which is why the size argument for these functions should be based on the size of the destination buffer. This helps to prevent buffer overflows.
Note that strncat` always adds a terminating null character at the end of the appended characters; therefore, the size argument should be smaller than the size of the destination to leave enough space for the null character.
Facilities in <random> should be used instead of srand, rand and random_shuffle
Facilities in <random> should be used instead of srand, rand and random_shuffle
The use of `srand together with rand to seed the random number generator and then generate numbers usually produces low-quality randomness. Further, rand can only provide a number between 0 and RAND_MAX, and it is left to the caller to transform the result into what is actually required (E.G. a float between 0 and 1 for a random percentage, an int between 1 and 6 for a dice game, …), and that transformation might introduce additional biases.
C++11 introduced the <random> library, which contains several high quality random value generators as well as statistical distributions you can use to put the results in the form you need. Those mechanisms should be used instead of rand and srand.
Additionally, std::random_shuffle, which is deprecated in C++14 and removed in C++17, uses rand and should be replaced by std::shuffle, which uses the random number generators provided by <random>`.
Class members should not be initialized with dangling references
Class members should not be initialized with dangling references
Special attention should be paid when initializing class members: it is easy to get it wrong and initialize them with references that are going to be invalidated at the end of the constructor, known as dangling references.
A pointer parameter in a function prototype should be declared as pointer to const if the pointer is not used to modify the addressed object
A pointer parameter in a function prototype should be declared as pointer to const if the pointer is not used to modify the addressed object
This rule leads to greater precision in the definition of the function interface. The const qualification should be applied to the object pointed to, not to the pointer, since it is the object itself that is being protected.
cin and cout should not be used for files
cin and cout should not be used for files
Redirecting standard in and standard out to/from files is bad practice because it contravenes the standard expectation that cin and cout do not relate to files. Additionally, it is less efficient than using file streams such as ifstream and ofstream
. For both of these reasons, this practice should be avoided.
Casts should not convert pointers to functions to any other pointer type, including pointers to functions
Casts should not convert pointers to functions to any other pointer type, including pointers to functions
Conversion of a function pointer to a non-function pointer type causes undefined behaviour. Undefined behaviour may arise if a function call is made using a pointer that is the result of a function pointer conversion.
All overloads of virtual member functions should be virtual
All overloads of virtual member functions should be virtual
Virtual and non-virtual functions are not dispatched the same way: for virtual functions, the resolution will be done dynamically while for non-virtual ones, it will be done statically by the compiler based on the type it sees.
Thus, overloading functions with both virtual and non-virtual functions adds a level of complexity to the code because the resolution of these functions will be completely different even though they share the same name. So the code becomes more confusing and more error-prone (for example if a virtual function is mistaken with a non-virtual one).
Types, objects and functions used in multiple translation units should be declared in only one file
Types, objects and functions used in multiple translation units should be declared in only one file
Having a single declaration of a type, object or function allows the compiler to detect incompatible types for the same entity.
Normally, this will mean declaring an external identifier in a header file that will be included in any file where the identifier is defined or used.
Unused functions and methods should be removed
Unused functions and methods should be removed
Functions or methods that are not called may be symptomatic of a serious problem, such as missing paths.
A pointer to a virtual base class shall only be cast to a pointer to a derived class by means of dynamic_cast
A pointer to a virtual base class shall only be cast to a pointer to a derived class by means of dynamic_cast
Casting from a virtual base to a derived class, using any means other than dynamic_cast has undefined behavior. The behavior for dynamic_cast is defined.
Note: As of C++17, the program is considered ill-formed, and an error is reported.
Most compilers emit an error for previous versions of C++ as well.
Alternative operators should not be used
Alternative operators should not be used
Even though the C++ standard defines both “Primary” and “Alternative” operators, it is not a good idea to use the alternatives. Developers seeing an alphabetical name expect a variable, a function, a class, a namespace… in short, anything but an operator, and they will be confused at best by code that uses such operators.
Primary | Alternative |
---|---|
&& | and |
&= | and_eq |
& | bitand |
| | bitor |
~ | compl |
! | not |
!= | not_eq |
|| | or |
|= | or_eq |
^ | xor |
^= | xor_eq |
Member data in non-POD class types should be private
Member data in non-POD class types should be private
By implementing class interfaces with member functions, the implementation retains more control over how the object state can be modified, and helps to allow a class to be maintained without affecting clients.
Copy assignment operators should be declared when there are template assignment operators with generic parameters
Copy assignment operators should be declared when there are template assignment operators with generic parameters
Contrary to possible developer expectations, a template assignment operator will not suppress the compiler generated copy assignment operator. This may lead to incorrect copy semantics for members requiring deep copies.
std::scoped_lock should be used instead of std::lock_guard
std::scoped_lock should be used instead of std::lock_guard
std::scoped_lock basically provides the same feature as std::lock_guard, but is more generic: It can lock several mutexes at the same time, with a deadlock prevention mechanism (see S5524). The equivalent code to perform simultaneous locking with std::lock_guard is significantly more complex. Therefore, it is simpler to use std::scoped_lock
all the time, even when locking only one mutex (there will be no performance impact).
Inherited functions should not be hidden
Inherited functions should not be hidden
std::visit should be used to switch on the type of the current value in a std::variant
std::visit should be used to switch on the type of the current value in a std::variant
`std::variant is a type-safe union that can hold values of a type out of a fixed list of types.
Depending on the current alternative inside a variant, it is common to execute dedicated code. There are basically two ways to achieve that:
Writing code that checks the current alternative, then getting it and running specific code
Letting std::visit perform the check and select the code to run by using overload resolution with the different alternatives
The second option is usually preferable:
It requires less boilerplate code.
It is easy to handle multiple similar alternatives together if desired.
It is usually more robust: if a new alternative is added to the variant, but the visitor does not support it, it will not compile.
This rule raises an issue when variant::index is called, or when variant::holds_alternative or variant::get_if is used in a series of if - else if (calling one of these functions in isolation can be an acceptable lightweight alternative to std::visit` in some cases).
Note: When defining the visitor of a variant, it can be nicer to use a series of lambdas by making use of the overloaded pattern
Binary operators should be overloaded as friend functions
Binary operators should be overloaded as friend functions
Member functions can only be used with an instance of a class. But `friend functions can be used with an implicitly converted type. So loosening access privileges to friend on overloaded binary operators makes them more flexible. Specifically, with a friend function, the class instance can be on either the right or the left of the operator, but with a member function, it can only be on the left.
This rule raises an issue for all non-friend overloaded binary operators except:
”=”, ”[ ]”, and ”->”, which cannot be overloaded as friend functions.
”+=”, ”-=”, ”*=”, ”/=”, ”%=”, ”^=”, ”&=”, ”|=”, ”<<=”, and ”>>=`”, which are not symmetric operators.
try_emplace should be used with std::map and std::unordered_map
try_emplace should be used with std::map and std::unordered_map
`emplace and insert in std::map and std::unordered_map might construct the (key, value) pair, including the value object, even when it is not necessary.
emplace destroys the constructed pair if the key is already present, wasting the effort on construction and destruction of the value.
If insert was called with a temporary, it leads to an extra copy or move construction and destruction of the temporary.
C++17 introduced try_emplace that does not construct the value if the key is already present in the map and constructs the value in place if necessary.
In most cases, you should use try_emplace. In particular, if two conditions hold:
You are inserting a single object at a time.
You are creating a new mapped-to value and/or (key, value) pair just to insert it into the map.
You should keep the insert if one of the conditions holds:
The (key, value) pair is already constructed (for another purpose).
You want to insert multiple (key, value) pairs with a single call.
You should keep emplace and emplace_hint if
You use piecewise construction with std::piecewise_construct.
This rule detects calls to insert that lead to the construction of a large temporary object, as well as calls to emplace and emplace_hint` with no piecewise construction.
Track parsing failures
Track parsing failures
When the analysis succeeds, it doesn’t mean that the analyzer was able to understand all the analyzed code. If the analyzer fails to parse some parts of your code, it will ignore them during the analysis. This rule will help you track these parsing failures.
There are many reasons why parsing failures can happen, here are the common ones:
Compiler extensions: Your compiler might allow you to write code that isn’t standard-conforming.
Bad analysis environment. This usually means that the environment during the build is different than the one during the analysis. For example, files or symbolic links that were available during the build are not available during the analysis.
Use of new language features that are not yet supported by our analyzer.
Limitation in our analyzer. We are always working on improving this.
How do they impact analysis? We cannot judge without looking at specific examples, as they contain a broad range of types of errors. On our side, we will make sure that you get notified through the analysis logs when they have an impact on the quality of the analysis.
There are three recommended ways to deal with parsing failures:
Fix them when it is possible. It should be obvious from the message if you can do it. For example, by replacing the use of a compiler extension with the standard-conforming equivalent.
If you cannot fix them and the analysis logs state that they have a bad impact on the analysis results, Report them.
If you cannot fix them and the analysis logs don’t state anything explicit about their impact, ignore them by resolving them as “won’t fix”.
Member functions that dont mutate their objects should be declared const
Member functions that dont mutate their objects should be declared const
No member function can be invoked on a const-qualified object unless the member function is declared “const”.
Qualifying member functions that don’t mutate their object with the “const” qualifier makes your interface easier to understand; you can deduce without diving into implementation if a member function is going to mutate its object.
Also, const-qualified member functions make working with const-qualified objects possible. The compiler ensures that only member functions that are declared “const” can be invoked on “const” objects. Avoiding declaring non-mutating member functions const might break const-correctness: it will not be possible to invoke such non-mutating functions on const-qualified objects.
The Rule-of-Zero should be followed
The Rule-of-Zero should be followed
Most classes should not directly handle resources, but instead, use members that perform resource handling for them:
For memory, it can be `std::unique_ptr, std::shared_ptr, std::vector…
For files, it can be std::ofstream, std::ifstream…
…
Classes that avoid directly handling resources don’t need to define any of the special member functions required to properly handle resources: destructor, copy constructor, move constructor, copy-assignment operator, move-assignment operator. That’s because the versions of those functions provided by the compiler do the right thing automatically, which is especially useful because writing these functions correctly is typically tricky and error-prone.
Omitting all of these functions from a class is known as the Rule of Zero because no special function should be defined.
In some cases, this rule takes a slightly different shape, while respecting the fact that no definition of those functions will be provided:
For the base class of a polymorphic hierarchy, the destructor should be declared as public and virtual, and defaulted (=default). The copy-constructor and copy-assignment operator should be deleted. (If you want to copy classes in a polymorphic hierarchy, use the clone idiom.) The move operation will be automatically deleted by the compiler.
For other kinds of base classes, the destructor should be protected and non-virtual, and defaulted (=default`).
Values of different enum types should not be compared
Values of different enum types should not be compared
Just as comparing apples and oranges is seen as a classic folly, comparing values from different enumerations against each other or converting them into one another is nonsensical. True, at root enums are simply named numbers, and it’s certainly valid to compare numbers. But an added layer of meaning is created by an enum
, one that goes beyond simple numerical values.
Ignoring that extra layer of meaning is at best a trap for maintainers, who are likely to be hopelessly confused by the code. At worst, it is a bug, which will lead to unexpected results.
C declarations should not be made inside Objective-C structures
C declarations should not be made inside Objective-C structures
C-style definitions should not be made inside Objective-C structures such as @interface
s. Doing so appears to limit their scope to the interface, but in fact, it imposes no such restriction. Such symbols are available globally, and may result in future confusion. Instead, such definitions should be moved to the top level, to make it clear that they’re globally available.
Unused type declarations should be removed
Unused type declarations should be removed
If a type is declared but not used, then it is unclear to a reviewer if the type is redundant or it has been left unused by mistake.
std::uncaught_exception should not be used
std::uncaught_exception should not be used
`bool std::uncaught_exception() allows you to know whether a thread is in an exception stack unwinding context. However, its practical functionality was restricted.
C++17 deprecates bool std::uncaught_exception() and introduces int std::uncaught_exceptions() which returns the number of uncaught exceptions. The code example below shows how you can benefit from this new improved function.
std::uncaught_exception has been removed in C++20.
This rule will flag any usage of std::uncaught_exception`.
operator delete should be written along with operator new
operator delete should be written along with operator new
The operator new allocates memory for objects, and the operator delete frees the memory allocated by the matching operator new. When a class needs to customize memory allocation, it can override the operator new to use a custom memory allocation strategy and override the operator delete accordingly.
If the operator delete is not overridden alongside the operator new, the program will call its default implementation, which may not be suitable for the custom memory allocation strategy used by the overridden operator new.
For instance, if the operator new draws memory from a preallocated buffer instead of allocating memory, the operator delete should not call the free function to release the memory. Reciprocally, if the operator new allocate memory with malloc, the operator delete must call free.
On the other hand, if the operator delete is overridden without overriding the operator new, it is suspicious because it may not correctly release the memory allocated by the default operator new.
By defining the operator delete along with the operator new, the memory is deallocated in a way consistent with the custom allocation strategy used by the operator new.
Identifiers that refers to types, should not also refer to objects or functions in the same scope
Identifiers that refers to types, should not also refer to objects or functions in the same scope
For C compatibility, it is possible in C++ for a name to refer to both a type and object or a type and function. This can lead to confusion.
The function chosen by overload resolution should resolve to a function declared previously in the translation unit
The function chosen by overload resolution should resolve to a function declared previously in the translation unit
Argument-dependent lookup (ADL) adds additional associated namespaces to the set of scopes searched when lookup is performed for the names of called functions. For function templates, ADL is performed at the point of instantiation of the function template, and so it is possible that a function declared after the template may be called.
To ensure that ADL does not take place when calling a function with a dependent argument, the postfix-expression denoting the called function can either be a qualified name or a parenthesized expression.
Operators with dependent types may also have this problem. In order to avoid ADL in these examples, operators should not be overloaded, or the calls should be changed to use explicit function call syntax and a qualified name or parenthesized expression used, as above.
Virtual functions should be declared with the virtual keyword
Virtual functions should be declared with the virtual keyword
For code compliant with C++98 or C++03 standards, declaring overriding virtual functions with the virtual
keyword removes the need to check the base class to determine whether a function is virtual.
Iterators should not be used after invalidation
Iterators should not be used after invalidation
Iterators are useful to deal with data inside a container: they point to one of its element and can be incremented or decremented to access other elements of this container. However, they can be invalidated when their container is modified.
For example, iterators of std::vector are invalidated after an insertion which changed the capacity of the container, or if they point after an element of the std::vector
which has just been erased.
Once an iterator has been invalidated, you can only assign a new value to it, but you should not increment, decrement or dereference it.
There shall be at most one occurrence of the # or ## operators in a single macro definition
There shall be at most one occurrence of the # or ## operators in a single macro definition
Because the evaluation order of # and ## are not specified, the results of using them both in the same macro could be unpredictable. Therefore macros should contain at most once instance of either # or ##
.
Classes should not be derived from virtual bases
Classes should not be derived from virtual bases
The use of virtual base classes can introduce a number of undefined and potentially confusing behaviours. The use of virtual bases is not recommended.
Calls to swap should not be qualified
Calls to swap should not be qualified
When you make an unqualified call to swap, argument dependent lookup will ensure that overloads will also be searched in the namespace where the types of the arguments of the call are declared.
However, argument dependent lookup won’t happen if you explicitly qualify the call `std::swap (as a reminder, overrides of swap should not be written in the standard namespace - see S3470) so the overload will not be found and the result of the swap may be different than expected.
If you want your code to work both with std::swap and with user-defined swap (for instance in a template), you should use a using declaration using std::swap; before calling swap` without qualification.
Function parameters should not be of type std::unique_ptr<T> const &
Function parameters should not be of type std::unique_ptr<T> const &
If you use `std::unique_ptr<T> const & for a function parameter type, it means that the function will not be able to alter the ownership of the pointed-to object by the unique_ptr:
It cannot acquire ownership of the pointed-to object (this would require a parameter of type std::unique_ptr<T>)
It cannot transfer the object ownership to someone else (this would require a std::unique_ptr<T> &).
That means the function can only observe the pointed-to object, and in this case, passing a T* (if the unique_ptr can be null) or a T& (if it cannot) provides the same features, while also allowing the function to work with objects that are not handled by a unique_ptr (e.g., objects on the stack, in a vector`, or in another kind of smart pointer), thus making the function more general-purpose.
Moved-from objects should not be relied upon
Moved-from objects should not be relied upon
After a move took place, the object that has been moved-from is left in a valid but unspecified state. Even if in a valid state, the fact of an object being in an unspecified state may lead to undefined behavior.
Move construction and its respective move semantics has been introduced in C++11. Moving objects becomes interesting if one wishes to get an object into a different scope, while no longer requiring the original object. While one would previously need to make a potentially expensive copy to get an object into another scope and then destroy the original, move constructors allow one to move objects without performing a copy. Move constructors are typically implemented by “stealing” the resources held by another object specified as the move constructor’s parameter, rather than making a copy. “Stealing” resources (e.g. memory) from another object is oftentimes much more efficient than making a copy and destroying the original, and can frequently be implemented by reassigning a few pointer variables.
Move-assignment operators behave analogously, except that they are used once the object that is moved-to has already been constructed. In contrast to copy-assignment operators, a move-assignment operator too “steals” the moved-from object’s resources without the need for making a potentially expensive copy.
The comma operator, &&, and || should not be overloaded
The comma operator, &&, and || should not be overloaded
Overloaded versions of the comma and logical conjunction operators have the semantics of function calls whose sequence point and ordering semantics are different from those of the built-in versions. It may not be clear at the point of use that these operators are overloaded, and so developers may be unaware which semantics apply.
Exception: Starting from C++17, the order of evaluation of the comma operator is defined and identical for the builtin and the overloaded versions. In such circumstances, the comma operator can safely be overloaded.
An exception object should not have pointer type
An exception object should not have pointer type
When an exception is a pointer, it is difficult for the code that catches the exception to determine whether or not it needs to delete the pointed-to object. It is even more complicated than a traditional manual memory management scenario because the throw and the corresponding catch can be far apart.
Keywords introduced in later specifications should not be used as identifiers
Keywords introduced in later specifications should not be used as identifiers
While keywords introduced in later standards can legally be used as identifiers in code compiled to earlier standards, doing so will eventually cause problems. Such code will cause compile errors if (when) the compiler is upgraded, and fixing those errors could be difficult and painful.
Additionally, such misuse of keywords has the potential to thoroughly confuse people who are unfamiliar with the code base, possibly leading them to introduce additional errors.
For these reasons, the earlier this practice is stopped, the better.
This rule flags instances of the following keywords used as identifiers:
C99
`inline, restrict, _Bool, _Complex, _Noreturn, _Static_assert, _Thread_local
C11
_Alignas, _Alignof, _Atomic, _Generic, _Imaginary
C++11
alignas, alignof, char16_t, char32_t, constexpr, decltype, noexcept, nullptr, static_assert, thread_local
C++20
concept, requires, constinit, consteval, co_await, co_return, co_yield, char8_t`
Member data should be initialized in-class or in a constructor initialization list
Member data should be initialized in-class or in a constructor initialization list
There are three ways to initialize a non-static data member in a class:
With an in-class initializer
In the initialization list of a constructor
In the constructor body
You should use those methods in that order of preference. When applicable, in-class initializers are best, because they apply automatically to all constructors of the class (except for default copy/move constructors and constructors where an explicit initialization for this member is provided). But they can only be used for initialization with constant values.
If your member value depends on a parameter, you can initialize it in the constructor’s initialization list. If the initialization is complex, you can define a function to compute the value, and use this function in the initializer list.
Initialization in the constructor body has several issues. First, it’s not an initialization, but an assignment. Which means it will not work with all data types (const-qualified members, members of reference type, member of a type without default constructor…). And even if it works, the member will first be initialized, then assigned to, which means useless operations will take place. To prevent “use-before-set” errors, it’s better to immediately initialize the member with its real value.
It’s hard to find a good example where setting the value of a member in the constructor would be appropriate. One case might be when you assign to several data members in one operation. As a consequence constructor bodies are empty in many situations.
This rules raises an issue in two conditions:
When you assign a value to a member variable in the body of a constructor.
When you initialize a member variable in the initializer list of a constructor, but could have done so directly in the class:
The initial value does not depend on a constructor parameter
The variable has either no in-class initializer, or an in-class initializer with the same value as in the constructor
Iterators should not be used out of bounds
Iterators should not be used out of bounds
Iterators are useful to deal with data inside a container: they point to one of its element and can be incremented or decremented to access other elements of this container. However, it will be undefined behavior if they try to access data out of bounds:
You should not try to get an iterator which would be before `begin()
You should not try to get an iterator which would be after end()
You should not try to dereference end(): ranges are semi open, which means that begin() is the location of the first element, but end()` is a location past-the-end of the container, and does not correspond to any data.
Size of allocated memory should be compatible with receiver type size
Size of allocated memory should be compatible with receiver type size
When allocating memory with malloc, calloc and realloc
it is important to make sure that the size of the allocated memory is compatible with the receiver type.
Network addresses should be converted to, or from network byte order
Network addresses should be converted to, or from network byte order
Network addresses have to be encoded in the network byte order, as specified by RFC-1700, which may be different from that of the host running the code, depending on the endianness of its architecture. This is usually done by using `ntohs, ntohl, htons or htonl.
This rule raises an issue when the following fields are assigned to or from without proper conversion:
struct sockaddr_in.sin_port
struct sockaddr_in.sin_addr.s_addr
struct sockaddr_in6.sin6_port`
Changing working directories without verifying the success is security-sensitive
Changing working directories without verifying the success is security-sensitive
the current working directory is to modify the base path when the process performs relative path resolutions. When the working directory cannot be changed, the process keeps the directory previously defined as the active working directory. Thus, verifying the success of chdir() type of functions is important to prevent unintended relative paths and unauthorized access.
Using strcpy or wcscpy is security-sensitive
Using strcpy or wcscpy is security-sensitive
a buffer of characters, normally using the `null character as a sentinel for the end of the string. This means that the developer has to be aware of low-level details such as buffer sizes or having an extra character to store the final null character. Doing that correctly and consistently is notoriously difficult and any error can lead to a security vulnerability, for instance, giving access to sensitive data or allowing arbitrary code execution.
The function char *strcpy(char * restrict dest, const char * restrict src); copies characters from src to dest. The wcscpy does the same for wide characters and should be used with the same guidelines.
Note: the functions strncpy and wcsncpy might look like attractive safe replacements for strcpy and wcscpy`, but they have their own set of issues (see S5816), and you should probably prefer another more adapted alternative.
Pointer and reference local variables should be const if the corresponding object is not modified
Pointer and reference local variables should be const if the corresponding object is not modified
This rule leads to greater precision in the definition of local variables by making the developer intention about modifying the variable explicit. The const
qualification shall be applied to the object pointed to, not to the pointer, since it is the object itself that is being protected.
The delete operator should only be used for pointers
The delete operator should only be used for pointers
The delete
operator expects a pointer argument. Passing an object to it may compile and seem to run (with an implicit cast to pointer type), but it can result in unexpected behavior at runtime.
Standard groupings should be used with digit separators
Standard groupings should be used with digit separators
C++14 introduced the ability to use a digit separator (’
) to split a literal number into groups of digits for better readability.
To ensure that readability is really improved by using digit separators, this rule verifies:
Homogeneity
Except for the left-most group, which can be smaller, all groups in a number should contain the same number of digits. Mixing group sizes is at best confusing for maintainers, and at worst a typographical error that is potentially a bug.
Standardization
It is also confusing to regroup digits using a size that is not standard. This rule enforce the following standards:
Decimal numbers should be separated using groups of 3 digits.
Hexadecimal numbers should be separated using groups of 2 or 4 digits.
Octal and Binary should be separated using groups of 2, 3 or 4 digits.
Furthermore, using groups with more than 4 consecutive digits is not allowed because they are difficult for maintainers to read.
errno should not be used
errno should not be used
`errno is a facility of C++ which should in theory be useful, but which in practice is poorly defined by ISO/IEC 14882:2003. A non-zero value may or may not indicate that a problem has occurred; therefore errno shall not be used.
Even for those functions for which the behaviour of errno is well defined, it is preferable to check the values of inputs before calling the function rather than relying on using errno` to trap errors.
The prefix increment/decrement form should be used
The prefix increment/decrement form should be used
Postfix increment and decrement typically involves making a copy of the object being incremented or decremented, whereas its prefix form does not. Therefore the prefix form is usually the more efficient form, and should be preferred.
This rule raises an issue if a postfix increment or decrement operator is used, but its return value is not read.
Names from dependent bases of class templates should be referred to using qualified-ids or this->
Names from dependent bases of class templates should be referred to using qualified-ids or this->
Using a qualified-id or prefixing the identifier with this->
ensures that the entity chosen is consistent with developer expectations.
Unary minus should not be applied to an unsigned expression
Unary minus should not be applied to an unsigned expression
Applying the unary minus operator to an unsigned variable or expression will always yield another unsigned expression. More plainly, in some cases the operation itself is meaningless, and in some other cases the result will be unexpected. In all cases it is bad practice. Therefore the unary minus operator should not be applied to unsigned variables or expressions.
Functions which do not return should be declared as noreturn
Functions which do not return should be declared as noreturn
The attribute noreturn
indicates that a function does not return. This information clarifies the behavior of the function and it allows the compiler to do optimizations.
It can also help the compiler (and static analyzer tools, i.e. us) provide better error messages:
Condition-specific catch handlers should not be used after the ellipsis (catch-all) handler
Condition-specific catch handlers should not be used after the ellipsis (catch-all) handler
The catch-all handler, written catch(…) in C++, or @catch(…) in Objective-C, catches every type of exception. If there is another catch statement for a specific exception after the catch-all handler, it will not be executed because the catch-all handler will already have handled the exception.
std::source_location should be used instead of __FILE__, __LINE__, and __func__ macros
std::source_location should be used instead of __FILE__, __LINE__, and __func__ macros
C++20 introduced `std::source_location to represent the information about the code point. This class exposes the same information as FILE, LINE, and func and makes passing this information as a single argument possible.
Furthermore, the std::source_location::current() function, when used as the default argument of the function parameter, will collect information from the call site. Consequently, this class enables the replacement of various logging macros, with functions accepting std::source_location as a parameter.
This rule reports the use of source location-related macros like FILE, LINE, and func which can be replaced by std::source_location`.
inline should not be used redundantly
inline should not be used redundantly
Since C++03, a member function that is contained within a class definition is by definition inline, so an using an inline
specifier on such functions is redundant.
Header files should not contain unnamed namespaces
Header files should not contain unnamed namespaces
An unnamed namespace will be unique within each translation unit. Any declarations appearing in an unnamed namespace in a header will refer to a different entity in each translation unit, which is probably not the expected behavior.
extern shouldnt be used on member definitions
extern shouldnt be used on member definitions
Data members and member functions cannot be defined as external, although entire objects can. When a member is declared as extern
, the compiler simply ignores the keyword, making it both extraneous and confusing.
pthread_mutex_t should be properly initialized and destroyed
pthread_mutex_t should be properly initialized and destroyed
Mutexes are synchronization primitives that allow managing concurrency.
Their use requires following a well-defined life cycle:
Mutexes need to be initialized (using `pthread_mutex_init) before being used. Once it is initialized, a mutex is in an unlocked state.
Mutexes need to be destroyed (using pthread_mutex_destroy) to free the associated internal resources. Only unlocked mutexes can be safely destroyed.
Before initialization and after destruction, a mutex is in an uninitialized state.
During a mutex’ life cycle, the following patterns should be avoided as they result in undefined behavior:
trying to initialize an already initialized mutex
trying to destroy an initialized mutex that is in a locked state
trying to destroy an uninitialized mutex
trying to lock an uninitialized mutex
trying to unlock an uninitialized mutex
In C++11 and higher, std::mutex` is less error-prone and is supported by more platforms.
In C++03, it is recommended to wrap mutex creation/destruction in an RAII class, as well as mutex lock/unlock. Those RAII classes will perform the right operations, even in the presence of exceptions.
Macros should not be used as replacements for typedef and using
Macros should not be used as replacements for typedef and using
C provides a way of defining or aliasing a type through `typedef. On top of it, C++ adds using that can do the same and more.
Using a macro to define a type is inferior to the previous ways for two reasons:
macros cannot be enclosed into scopes. Or at least, doing so is cumbersome and error-prone, as in that case, the macro needs to be defined and undefined manually.
macros are handled by the preprocessor and are not understood by the compiler. They can easily pollute the code in places where types are not expected. typedef and using are known to the compiler to define types and can be more strictly checked.
As a result, macros should not be used as a replacement for typedef or using`.
nonnull pointers should not be set to null
nonnull pointers should not be set to null
A function’s return value and parameters may be decorated with attributes to convey additional information to the compiler and/or other developers.
A commonly used attribute is nonnull which can be used to mark a function’s return value and parameters as shown in the following:
All accessible entity names within a multiple inheritance hierarchy should be unique
All accessible entity names within a multiple inheritance hierarchy should be unique
If the names are ambiguous, the compiler should report the name clash and not generate arbitrary or unexpectedly resolved code. However, this ambiguity may not be obvious to a developer.
There is also a specific concern that if the member function is virtual, resolving the ambiguity by explicitly referencing the base class in effect removes the virtual behaviour from the function.
:: operator should be used to access global variables and functions
:: operator should be used to access global variables and functions
While it is possible to access a global variable or function without using the ::
operator, it can be considered to be misleading because it might imply to the readers of your code that this is a local or class variable/function and not a global one. Being explicit also allows more freedom in naming local variables without the chance of clashing with global names.
Preprocessor directives should not be indented
Preprocessor directives should not be indented
Indenting preprocessor directives reduces the code readability, because it make preprocessor directives harder to spot.
The right template argument should be specified for std::forward
The right template argument should be specified for std::forward
`std::forward forwards lvalues either as lvalues or as rvalues based on its template argument.
std::forward should always take as a non-template argument a forwarding reference which is defined by the standard as:
rvalue reference to a cv-unqualified template parameter that does not represent a template parameter of a class template.
If you don’t pass forwarding reference as an argument to std::forward S5417 will be triggered.
If you don’t pass the template parameter referred to by the forwarded reference or the decltype` of the forwarded expression this rule will be triggered.
Coroutine should have co_return on each execution path or provide return_void
Coroutine should have co_return on each execution path or provide return_void
When a regular, non-void function flows off the end of its body without returning a value, the behavior is undefined. With a coroutine, when flowing off the end of its body, return_void() is invoked on the promise for the said coroutine. If such invocation is not possible (e.g., because the function is not defined), the behavior is undefined.
In other words, a coroutine should either:
have all its execution paths reach a co_return statement or throw an exception;
or its promise type should provide return_void().
This rule raises an issue on coroutines that do not meet the above criteria.
Keywords shall not be used as macros identifiers
Keywords shall not be used as macros identifiers
In programming languages, keywords have a special meaning and are reserved for the language. Hence, it is a bad idea to define macros with keywords as macro identifiers as it can easily lead to undefined behavior:
The same object might be defined differently in different places, which violates the One Definition Rule
If you include any header from the standard library, it is undefined behavior to define such macros
Additionally, it is awkward for anyone reading the code to have a keyword that means something different.
Concept names should comply with a naming convention
Concept names should comply with a naming convention
Shared coding conventions allow teams to collaborate effectively. This rule checks that all C++ concept names match a provided regular expression.
The return value of std::move should be used in a function
The return value of std::move should be used in a function
std::move is not really moving anything, but tells the compiler that a value can be considered as no longer useful. It is technically a cast to a RValue, and allows overload resolution to select the version of a function that will perform destructive operations on that value (therefore actually moving from it).
As a consequence, calling std::move on an object and then not directly using the returned value as a function argument is not the typical pattern, and may be indicative of a bug. Note that calling a member function on the result of std::move is considered as passing it to a function (as the hidden this parameter), as well as using it as an operand (the called function is the overloaded operator) or initializing an object with it (the called function is the constructor).
Only valid arguments should be passed to stream functions
Only valid arguments should be passed to stream functions
The standard C library includes a number of functions for handling I/O streams. These functions put certain constraints on the values of their parameters. The constraints include the following:
The value for the `FILE*-typed parameter may not be NULL
The third argument of fseek must be either of SEEK_SET, SEEK_END, or SEEK_CUR`
Failing to pass correctly constrained parameters renders them invalid and will result in undefined behavior.
Zero should not be a possible denominator
Zero should not be a possible denominator
If the denominator to a division or modulo operation is zero, the behavior of the application is undefined.
Operator / is used for division and % for modulo operation. Division and modulo operations are susceptible to divide-by-zero (and signed integer overflow) errors.
operator= should check for assignment to self
operator= should check for assignment to self
In some cases, we might end up with some code that assigns an object to itself. Therefore, when writing an `operator=, we must ensure that this use case works correctly, which may require special care. One technique to achieve this is to explicitly check at the start of operator= if we are assigning to ourselves, and in that case, just do nothing.
It is usually a bad idea to perform this check for optimization purposes only, because it optimizes for a very rare case while adding an extra check for the more common case. But when it is necessary for correctness, it should be added.
This rule raises an issue when an operator=` does not check for assignment to self before proceeding with the assignment.
auto should be used to store a result of functions that conventionally return an iterator or a range
auto should be used to store a result of functions that conventionally return an iterator or a range
`auto is a type placeholder that may be used in variable declarations to instruct the compiler to infer the type from the initializer.
The use of auto reduces unnecessary boilerplate in situations where the type of the variable is apparent from the context (see rule S5827). In other situations, though, whether auto increases or decreases readability is a matter of personal taste.
In the case of variables initialized from a function that conventionally returns an iterator (e.g., begin, end, std::find), it is clear that the type of the variable is some iterator. Spelling the exact type of the iterator in such a situation does not improve the clarity of the code, especially considering the usual verbosity of such types. The same can be said for functions returning ranges.
This rule raises an issue on the declaration of a variable that is initialized with the return value of a function that conventionally returns an iterator when the variable is declared with an explicit type equal to the function’s return type. The detected functions are:
begin and end` functions and their const and reverse variants
standard algorithms that return iterators or ranges
All the elements of an aggregate should be provided with an initial value
All the elements of an aggregate should be provided with an initial value
In C or C++, it is possible to provide an initial value for the elements of an array. When fewer values are provided than the size of the array, the last elements of the array are zero-initialized for builtin-types (like int or pointers), and value-initialized otherwise. However, as soon as some values are provided, it is clearer to provide them all and not rely on these default initializations.
Empty class members should be marked as [[no_unique_address]]
Empty class members should be marked as [[no_unique_address]]
In C++, every independent object needs to have a unique address, which implies that its size cannot be null. Sub-objects of another object, however, do not have this constraint. Empty base class subobjects usually don’t take any space in the final object, but empty member variables, by default, take at least one byte. The impact on the object’s size may be even larger due to padding and alignment requirements.
C++20 introduces the `[[no_unique_address]] attribute. It indicates that preserving the uniqueness of the address guarantee is not important for the decorated member variable. If the variable type is empty, no storage needs to be reserved for it in the class.
If the type is not empty, this attribute is still valid and has no effect. This allows placing this attribute on dependent member variables in template classes and having the exact behavior depend on the template parameters.
This rule raises an issue on each member of a class that has an empty or potentially empty (in case of templates) type and does not have a [[no_unique_address]] attribute.
Note: This rule is disabled on Windows because [[no_unique_address]]` isn’t well supported by MSVC and Clang on this platform.
Identical expressions should not be used on both sides of a binary operator
Identical expressions should not be used on both sides of a binary operator
Using the same value on both sides of a binary operator is a code defect. In the case of logical operators, it is either a copy/paste error and, therefore, a bug, or it is simply duplicated code and should be simplified. In the case of most binary mathematical operators, having the same value on both sides of an operator yields predictable results and should be simplified as well.
Header guards should be followed by according #define macro
Header guards should be followed by according #define macro
Using include guards, wrapping around the entire content of a header file, is a best practice ensuring that no matter how many times the header is included in a translation unit, its content will only be seen once.
The include guard pattern is made up of four parts:
`#ifndef at the top of the file, with a unique macro name (usually, the name relates to the file’s name to ensure uniqueness).
#define with the same macro name.
The content of the file
#endif` at the end of the file
The rule raises an issue when the name in the second part differs from the first (usually because of a typo or a copy/paste issue).
Undefined macro identifiers should only be used in #if and #elif directives as operands to defined
Undefined macro identifiers should only be used in #if and #elif directives as operands to defined
If an attempt is made to use an identifier in a preprocessor directive, and that identifier has not been defined, the preprocessor will assume the value zero. #ifdef, #ifndef and defined()
are provided to test the existence of a macro, and are therefore excluded.
std::enable_if should not be used
std::enable_if should not be used
std::enable_if is a very important part of template meta-programming in C++ up to C++17. Based on SFINAE (“Substitution Failure Is Not An Error”), it can be used to subtly tune the behavior of overload resolution based on properties of types.
However, using std::enable_if correctly is not easy and requires skills and experience for a resulting code that is not straightforward and costly to maintain. Since C++20, new features replace complex uses of std::enable_if:
Concepts allow defining named constraints on types, using a terse syntax to specify that a template argument must adhere to a concept;
requires clauses can be directly written for one-shot constraints;
In some cases, using if constexpr (introduced in C++17) may replace an overload set with just one function (see S6017).
Additionally, since those features provide a higher level of abstraction, compilers understand them better and can provide more straightforward diagnostics when a constraint is violated.
Consequently, std::enable_if is no longer the right tool and should be replaced with those facilities. Note that the replacement is not always mechanical. For instance, reusing an existing concept defined in the standard is a better alternative than putting the std::enable_if expression in a requires clause.
This rule reports the use of std::enable_if.
C-style memory allocation routines should not be used
C-style memory allocation routines should not be used
The `malloc, realloc, calloc and free routines are used to dynamically allocate memory in the heap. But, in contrast to the new and delete operators introduced in C++, they allocate raw memory, which is not type-safe, and they do not correctly invoke object constructors. Additionally, mixing them with new/delete results in undefined behavior.
Note that directly replacing those functions with new/delete` is usually not a good idea (see S5025).
Assembly language should be encapsulated and isolated
Assembly language should be encapsulated and isolated
Ensuring that assembly language code is encapsulated and isolated aids portability. Where assembly language instructions are needed, they shall be encapsulated and isolated in either assembler functions or C++ functions.
Standard library function names should not be overridden
Standard library function names should not be overridden
Where the developer uses new versions of standard library functions (e.g. to enhance functionality or add checks of input values), the modified function shall have a new name. However, it is permissible to overload the name to add new parameter types if the functionality is consistent with those of the original. This ensures that the behaviour associated with the name remains consistent. So, for example, if a new version of the sqrt function is written to check that the input is not negative, the new function shall not be named sqrt, but shall be given a new name. It is permissible to add a new sqrt
function for a type not present in the library.
explicit should be used on single-parameter constructors and conversion operators
explicit should be used on single-parameter constructors and conversion operators
If you invoked a method with arguments of the wrong type, you would typically expect an error at compile time (if not in the IDE). However, when the expected parameter is a class with a single-argument constructor, the compiler will implicitly pass the method argument to that constructor to implicitly create an object of the correct type for the method invocation. Alternately, if the wrong type has a conversion operator to the correct type, the operator will be called to create an object of the needed type.
But just because you can do something, that doesn’t mean you should, and using implicit conversions makes the execution flow difficult to understand. Readers may not notice that a conversion occurs, and if they do notice, it will raise a lot of questions: Is the source type able to convert to the destination type? Is the destination type able to construct an instance from the source? Is it both? And if so, which method is called by the compiler?
Moreover, implicit promotions can lead to unexpected behavior, so they should be prevented by using the explicit
keyword on single-argument constructors and (C++11) conversion operators. Doing so will prevent the compiler from performing implicit conversions.
Braces should be used to indicate and match the structure in the non-zero initialization of arrays and structures
Braces should be used to indicate and match the structure in the non-zero initialization of arrays and structures
ISO/IEC 14882:2003 [1] requires initializer lists for arrays, structures and union types to be enclosed in a single pair of braces (though the behaviour if this is not done is undefined). The rule given here goes further in requiring the use of additional braces to indicate nested structures.
This forces the developer to explicitly consider and demonstrate the order in which elements of complex data types are initialized (e.g. multi-dimensional arrays).
A similar principle applies to structures, and nested combinations of structures, arrays and other types.
Assignments should not be made from within conditions
Assignments should not be made from within conditions
Assigning a value inside a condition (of an if statement, a for statement, a while, or a switch) can be confusing. It assigns the value and checks it at the same time, but it is easily confused with a simple equality check with == and the original intention can be unclear.
Pure virtual functions should not override non-pure virtual functions
Pure virtual functions should not override non-pure virtual functions
A `virtual function has an implementation that may be replaced in a child class. A pure virtual has no implementation, and must be implemented in child classes.
Hiding a base class implementation with a “pure implementation” (=0`) is sure to confuse extenders, who may not be aware of the base class’ implementation. Instead, they’ll see there’s no implementation in the class they’re extending and assume that none exists. When that base class implementation contains crucial functionality such as freeing resources, this could cause future users of the class to introduce bugs.
This rule raises an issue if a pure virtual function overrides a virtual function that is not pure.
time_t types should not be used in math
time_t types should not be used in math
While you can perform arithmetic on a time_t type, you may not get the results you expect from the operation because the way that a time is encoded within the type is unspecified. Therefore there is no safe way to manually perform arithmetic on time_t variables despite the fact that the ISO C standard defines time_t
as an “arithmetic type”. The relevant function calls should be used instead.
static should not be used in unnamed namespaces
static should not be used in unnamed namespaces
Since C++11, declaring a variable, class, or function in an unnamed namespace gives it internal linkage. Similarly, marking a declaration static also gives it internal linkage. Because both mechanisms have the same effect (although static
has a narrower application) using them together is clearly redundant.
Recursion should not be infinite
Recursion should not be infinite
Recursion happens when control enters a loop that has no exit. It can occur when a method invokes itself, when two methods invoke each other, or when goto statements are used to move between two code segments. Recursion can be a useful tool, but unless the method includes a provision to break out the recursion and return, the recursion will continue until the stack overflows and the program crashes.
rvalue reference members should not be copied accidentally
rvalue reference members should not be copied accidentally
C++11 introduced the concept of forwarding-reference, as a way to transfer values efficiently. In combination with std::forward, their usage allows passing values without unnecessary copies.
The expression `std::forward<T>(obj).mem, can be used to forward the value of the member, according to the type of obj: move the value of member mem if the obj is an rvalue reference and copy it otherwise. However, in the corner case, when the member mem is of rvalue reference type, the value it references will be copied even if obj itself is an rvalue, the referenced value will not be moved.
Similarly for std::move: if mem is of rvalue reference type, std::move(obj).mem will copy the value referenced by mem`.
This rule raises issues when a templates is instantiated with a type that leads to an accidental copy of members of forwarded objects.
Designated initializers should be used in their C++ compliant form
Designated initializers should be used in their C++ compliant form
C++20 introduced a restricted form of designated initializers for aggregates (i.e., arrays or classes that respect specific criteria). Designated initializers enable the initialization of aggregates by naming their fields explicitly:
constexpr functions should not be declared inline
constexpr functions should not be declared inline
Declaring a function or a static member variable `constexpr makes it implicitly inline.
In that situation, explicitly using the inline` keyword would be redundant, and might lead to confusion if it’s used in some cases but not others. It’s better to simply omit it.
Pass by reference to const should be used for large input parameters
Pass by reference to const should be used for large input parameters
To pass an input parameter to a function, there are two possibilities: pass by value, or pass by reference to const. Which one is best depends of the size of the object, which is an indicator of the cost to copy it. A small one, with cheap copy constructors, should be passed by value, while a larger one should be passed by reference to const.
This rule detects when a parameter has been passed by value, while it should have been passed by reference to const:
Because it is too large
Because it contains virtual functions and passing it by value will slice the extra members if you happen to pass an object of a derived class.
In some cases, you may want to pass by value a large object, if you modify it in the function but you don’t want the initial object to be impacted by these changes. We do not detect such a situation, which will be a false positive.
There are other ways to pass input parameters for sinks (for instance by rvalue references), but this rule is only about the choice between pass by value and pass by reference to const.
Size argument of memory functions should be consistent
Size argument of memory functions should be consistent
The memory functions memset, memcpy, memmove, and memcmp
take as last argument the number of bytes they will work on. If this size argument is badly defined (eg it is greater than the size of the destination object), it can lead to undefined behavior.
This rule raises an issue when the size argument of a memory function seems inconsistent with the other arguments of the function.
Functions should not be declared at block scope
Functions should not be declared at block scope
A function declared at block scope will refer to a member of the enclosing namespace, and so the declaration should be explicitly placed at the namespace level.
Additionally, where a declaration statement could either declare a function or an object, the compiler will choose to declare the function. To avoid potential developer confusion over the meaning of a declaration, functions should not be declared at block scope.
static should not be used for the size of an array parameter
static should not be used for the size of an array parameter
Theoretically, the use of the `static keyword on the size of an array parameter means you can assume that only arrays of at least that size will be passed as arguments to the function. I.e. a function parameter of int my_array[static 10] means that my_array will always be at least 10 elements long. If it is not, the behavior is undefined.
In practice, the use of static on the size of an array parameter means the compiler might issue a warning if a noncompliant array is passed to the function - a warning that might or might not be ignored. Therefore, in practice the use of static on an array parameter’s size merely lends a false sense of security, and static should not be used in this context.
Note that for some compiler/processor combinations, more efficient code can be generated when static` is used, but these combinations are limited, and the benefit does not outweigh the cost.
Base classes should only be declared virtual if they are used in diamond hierarchies
Base classes should only be declared virtual if they are used in diamond hierarchies
The use of virtual base classes can introduce a number of undefined and potentially confusing behaviours. Therefore, a base class shall only be declared virtual
if that base class is to be used as a common base class in a diamond hierarchy.
All partial and explicit specializations for a template should be declared in the same file as the declaration of their primary template
All partial and explicit specializations for a template should be declared in the same file as the declaration of their primary template
It is undefined behaviour if, for a set of template-arguments, an implicit instantiation is generated by the compiler, and a partial or explicit specialization is declared or defined elsewhere in the program that would match the set of template-arguments.
Objects with integer type should not be converted to objects with pointer type
Objects with integer type should not be converted to objects with pointer type
Converting an integer type to a pointer generally leads to unspecified behavior. There are several cases where it might be legitimate:
Converting the integral literal `0 to the null pointer (but you should use nullptr instead, see S4962),
Converting back to a pointer a pointer value that was converted to a large enough integer (see S1767),
On embedded devices, device drivers… converting a hard-coded address to a pointer to read some specific memory (this often goes together with the use of volatile, since such memory values can change from the outside of the program).
Since even legitimate cases are corner cases that require to be reviewed carefully, this rule simply reports all places where an integer is cast into a pointer (except the literal 0`).
Functions that throw exceptions should not be used as hash functions
Functions that throw exceptions should not be used as hash functions
You can provide your own hash function when using a standard library container based on a hash table (for instance, `std::unordered_map). One of the requirements of the hash function is that it should not throw exceptions.
If you don’t follow this requirement and your hash function throws, you may end up with corrupted data in your container.
Since this function is not supposed to throw, you should also declare it noexcept`.
Names of well-known C standard library macros and functions should not be used as identifiers
Names of well-known C standard library macros and functions should not be used as identifiers
Defining or declaring identifiers with the same names as well-known macros and functions from the C standard library has the potential to thoroughly confuse people who are unfamiliar with the code base, possibly leading them to introduce additional errors. Therefore, the names of well-known C standard library macros and functions should not be used as identifiers.
typedef should be used for function pointers
typedef should be used for function pointers
Function pointer syntax can be hard on the eyes, particularly when one function is used as a parameter to another. Providing and using a typedef instead (or a using
in C++) can make code easier to read, and should be preferred.
The name main should not be used for any function other than the global main function
The name main should not be used for any function other than the global main function
A global function named main is the entry point to the program, and is the only identifier which must be in the global namespace. The use of main
for other functions may not meet developer expectations.
Limited dependence should be placed on operator precedence
Limited dependence should be placed on operator precedence
The rules of operator precedence are complicated and can lead to errors. For this reason, parentheses should be used for clarification in complex statements. However, this does not mean that parentheses should be gratuitously added around every operation.
Parentheses are not needed:
with a unary operator, except when `! is used as left operand in comparison expressions
when all the operators in an expression are the same
when only a single operator is involved
around the right-hand side of an assignment operator unless the right-hand side itself contains an assignment
Parentheses are needed:
in the condition of a ternary operator if it uses operators
when overloaded shift operator << or >>` is used in an expression with comparison operators
Class names should comply with a naming convention
Class names should comply with a naming convention
Shared naming conventions allow teams to collaborate efficiently.
This rule raises an issue when a class name (or an Objective-C interface, protocol, or implementation name) does not match a provided regular expression.
For example, with the default provided regular expression ^[A-Z][a-zA-Z0-9]*$
, the following class and interface:
A struct should not have member functions
A struct should not have member functions
While it is possible to define a struct with member functions in C++, the general expectation is that structs only aggregate data, while classes are used for fully encapsulated abstractions, containing data and methods. Thus, including a member function in a struct
is likely to lead to confusion at best and should be avoided.
std::format should not have unused arguments
std::format should not have unused arguments
std::format takes as an argument a format string that contains replacement fields (surrounded with {}) and a set of extra arguments that will be formatted inside the replacement fields. Even if the format string is checked at compile-time, it is possible to have a mismatch between the format string and the arguments. For example, when:
The format string contains fewer replacement fields than the number of extra arguments: std::format(”{} {}”, 1, 2, 3);
The format string uses indexes for the replacement fields, but one index is missing: std::format(“{0} {0} {2}”, 1, 2, 3);
In these cases, the extra arguments are silently ignored. In the best-case scenario, it leads to dead code. Otherwise, it is a typo, and the output will not be intended.
This rule detects when the arguments of std::format are unused in a way that doesn’t trigger S6487. Therefore, you should make sure that S6487 is enabled with this rule.
Functions with noreturn attribute should not return
Functions with noreturn attribute should not return
The “noreturn” attribute should be used to indicate that a function does not return to its caller: it may terminate the program, enter an infinite loop, or throw an exception.
This attribute is typically used for functions that perform critical operations, such as exiting the program or handling an error condition. For example, the “exit” function is marked with the “noreturn” attribute because it terminates the program and does not return to its caller.
Using this attribute allows the compiler to make some assumptions that can lead to optimizations. However, functions marked with the “noreturn” attribute should not have a return statement because it leads to undefined behavior and unexpected results.
This rules equally applies to C++11 [[noreturn]] notation or C11 _Noreturn keyword notation. It raises an issue when the attribute is incorrectly used.
memset should not be used to delete sensitive data
memset should not be used to delete sensitive data
The C language specification allows the compiler to remove unnecessary code during the optimization phase. For example, when a memory buffer is about to be destroyed, any writes to that buffer may be seen as unnecessary to the operation of the program. The compiler may choose to remove these write operations.
When the `memset function is used to clear sensitive data from memory and that memory is destroyed immediately afterward, the compiler may see the memset call as unnecessary and remove it. The sensitive data will, therefore, remain in memory.
This rule raises an issue when a call to memset` is followed by the destruction of the buffer.
C-style and functional notation casts should not be used
C-style and functional notation casts should not be used
C++ allows the traditional C-style casts [E.G. `(int) f] and functional notation casts [E.G. int(f)], but adds its own forms:
static_cast<type>(expression)
const_cast<type>(expression)
dynamic_cast<type>(expression)
reinterpret_cast<type>(expression)
std::bit_cast<type>(expression) (since C++20)
C-style casts and functional notation casts are largely functionally equivalent. However, when they do not invoke a converting constructor, C-style casts are capable of performing dangerous conversions between unrelated types and of changing a variable’s const-ness. Attempt to do these things with an explicit C++-style cast, and the compiler will catch the error. Use a C-style or functional notation cast, and it cannot.
Moreover, C++20 has introduced a std::bit_cast as a way of reinterpreting a value as being of a different type of the same length preserving its binary representation. The behavior of such conversion when performed via C-style cast or reinterpret_cast` is undefined.
Additionally, C++-style casts are preferred because they are visually striking. The visual subtlety of a C-style or functional cast may mask that a cast has taken place, but a C++-style cast draws attention to itself, and makes the the programmer’s intention explicit.
This rule raises an issue when C-style cast or functional notation cast is used.
Exception specifications should be treated as part of the type
Exception specifications should be treated as part of the type
Since C++17, exception specifications have become a part of a function type. This implies that these two functions, for example, have different types:
void first() noexcept; void second();
Making exception specifications part of the type will, for the right reason, break code where a function that throws an exception is provided in a context where `noexcept function is expected.
It is important to note that, like it is not allowed to overload based on the return type, it is also not allowed to overload based on the exception specifications.
This rule will trigger on code that will stop compiling starting C++17, and on explicit casts that add noexcept` to a type.
#include paths should be portable
#include paths should be portable
The way an `#include directive finds an actual file is implementation-defined, and in practice, it slightly differs in different systems.
Therefore, a good practice is to identify the files to include in the most straightforward way possible to reduce the risk of inconsistent behaviors.
This rule raises an issue when:
The case of the file in the #include directive does not match the case of the file on the disk (the inclusion would not work on a case-sensitive OS),
The file name in the #include` directive contains trailing spaces (they would be ignored on Windows but considered on Unix).
Only standard forms of the defined directive should be used
Only standard forms of the defined directive should be used
The `defined preprocessing directive is used in the context of #if and #elif expressions to see whether a given identifier has been defined as a macro. It returns a value of 0 (false) or 1 (true), and has two valid forms, defined IDENTIFIER and defined ( IDENTIFIER ). Since it is essentially a macro existence check, it cannot take expressions as arguments.
Note that since
#if defined AN_IDENTIFIER
is equivalent to
#ifdef AN_IDENTIFIER
defined is most useful when there are multiple arguments to check, E.G.
#if defined AAA || defined BBB`
Pointers to freed memory should be set to NULL
Pointers to freed memory should be set to NULL
free or delete a block of memory and even though the memory has been released, your pointer still hold the address of the memory. Unless the pointer is immediately and explicitly reset to a null value (0 or NULL
), access to that freed memory could inadvertently be made, causing potentially serious problems at runtime.
Copy constructors should be declared for classes with template constructors with only a single generic parameter
Copy constructors should be declared for classes with template constructors with only a single generic parameter
Contrary to possible developer expectations, a template constructor will not suppress the compiler-generated copy constructor. This may lead to incorrect copy semantics for members requiring deep copies.
if statements should be preferred over switch when simpler
if statements should be preferred over switch when simpler
switch statements are useful when there are many different cases depending on the value of the same expression. For just one or two cases, however, the code will be more readable with if statements.
In particular, if statements are more suitable when the condition of the switch is boolean.
This rule detects statements that could be simplified with a simple if / else. That is when there is at most one case, not counting those that fall through to a default.
The following code:
Redundant lambda return types should be omitted
Redundant lambda return types should be omitted
It is a best practice to make lambda return types implicit whenever possible. First and foremost, doing so avoids implicit conversions, which could result in data or precision loss. Second, omitting the return type often helps future-proof the code.
The issue is raised when explicit return types are used.
Closed resources should not be accessed
Closed resources should not be accessed
Once a file has been closed, its corresponding `FILE* typed variable becomes invalid and the stream may no longer be accessed through this variable. In particular, a pointer to a FILE object may not be passed to fclose more than once.
Using the value of a pointer to a FILE` object after the associated file is closed results in undefined behavior.
std::format numeric types should be 0-padded using the numerical padding and not the character padding
std::format numeric types should be 0-padded using the numerical padding and not the character padding
std::format and the related formatting functions provide two different options to pad numerical values up to a specific width:
Custom character padding: this option can align text to the left, center, or right using any character. For example, std::format(”{:*>5}”, num) aligns num to the right (>) by inserting * characters until it reaches a width of 5.
Numeric padding with 0: this option is available for most arithmetic types and is enabled by adding 0 before the width specifier. For example, std::format(”{:05}”, num) adds enough 0 before num to align it to the right and reach a width of 5.
0 can also be used as a custom character padding, but this syntax is confusing and may produce unexpected results when aligning negative values to the right:
Curly braces should not be used on interfaces without instance variables
Curly braces should not be used on interfaces without instance variables
Shared coding conventions allow teams to collaborate efficiently. This rule checks that curly braces are omitted from interfaces with no instance variables.
Using curly braces in such a situation means that the reader of the code must pause to find the close curly brace before understanding that there are no variables. On the other hand, omitting the curly braces is a quick, clear indicator that there are no variables.
reinterpret_cast should be used carefully
reinterpret_cast should be used carefully
Because `reinterpret_cast ignores the type system, it is capable of performing dangerous conversions between unrelated types which can lead to undefined behavior.
This rule reports an issue for two problematic uses of reinterpret_cast:
when it is used to make the compiler believe that an object in memory is from a different type from its real type (for instance, casting a long* to double*, because accessing a long as if it was a double is undefined behavior (even if `++sizeof(long)
switch statements should cover all cases
switch statements should cover all cases
For completeness, a `switch over the values of an enum must either address each value in the enum or contain a default case. switch statements that are not over enum must end with a default case.
This rule is a more nuanced version of S131. Use S131 if you want to require a default case for every switch even if it already handles all enumerators of an enum`. Otherwise, use this rule.
Objects should not be created solely to be passed as arguments to functions that perform delegated object creation
Objects should not be created solely to be passed as arguments to functions that perform delegated object creation
In the standard library, several functions, instead of taking an object as an argument, take a list of arguments that will be used to construct an object in a specific place:
`std::vector::emplace_back will create the object directly inside the vector
std::make_unique will create the object and a unique_ptr that points to it
std::make_shared will create the object in a specially allocated memory area that will also contain bookkeeping information for the shared pointer, and the associated shared_ptr
std::optional has a constructor that will create an object inside the optional (this constructor is selected by using std::in_place` as its first argument)
…
These functions are said to perform delegated object creation.
Constructing an object externally and passing it to one of these functions is possible. They will then create their object by calling the copy constructor to copy the argument. But it defeats the purpose of those functions that try to create the object at the right place directly.
This rule raises an issue when a function that performs delegated object creation is passed an object of the right type explicitly created for this purpose only.
Functions that are not used in a project should be removed
Functions that are not used in a project should be removed
Unless you are in a library codebase context, functions that are declared in your program but never executed are dead code that should be removed. Cleaning out dead code decreases the size of the maintained codebase, making it easier to understand the program and preventing bugs from being introduced.
Note: S1144 is a subset of this rule; hence, it should be deactivated when this rule is activated.
Incomplete types should not be deleted
Incomplete types should not be deleted
When calling delete on an object of incomplete type, the calling code does not have enough information to do the action properly (it does not know if this object has a trivial or a nontrivial destructor, if it has overloaded the delete
operator…). Therefore, deleting a pointer to such an object can lead to undefined behavior.
auto should not be used to deduce raw pointers
auto should not be used to deduce raw pointers
Using auto when the type that would be deduced is a pointer type can cause confusion. It is much better to specify the pointer part outside of auto
.
Local variables should be initialized immediately
Local variables should be initialized immediately
Objects should be initialized as soon as they are declared. It will be implicitly the case if they have a default constructor, as this latter will be called, but otherwise the initialization must be explicit. Even when an object has a default constructor, it may be interesting to use another more relevant constructor to directly give the the object its right value.
Such direct initialization increases the readability of the code:
seeing the initial value of a variable is usually a good indicator of its purpose,
waiting until we know a good initial value before declaring a variable can lead to a reduced variable scope,
it makes reasoning on the source code simpler: we remove the burden of having to know if a variable is initialized at a specific point in the code,
it is a first step that can lead to the possibility of declaring the variable `const, which further simplifies reasoning,
it is also a first step toward declaring it auto`, which could increase readability by shifting the focus away from the exact type.
Please note that the intent of the rule is not to initialize any variable with some semi-random value, but with the value that is meaningful for this variable.
This rule raises an issue when a local variable of a built-in or pointer type is declared without an initial value.
The related rule S836 detects situations when a variable is actually read before being initialized, while this rule promotes the good practice of systematically initializing the variable.
Use symmetric transfer to switch execution between coroutines
Use symmetric transfer to switch execution between coroutines
With C++20 coroutines, the co_await/co_yield expression suspends the currently executed coroutine and resumes the execution of either the caller or the coroutine function or to some already suspended coroutine (including the current coroutine).
The resumption of the coroutine represented by the std::coroutine_handle object is usually performed by calling the .resume() on it. However, performing such an operation during the execution of await_suspend (that is part of co_await expression evaluation) will preserve the activation frame of the await_suspend function and the calling code on the stack. This may lead to stack overflows in a situation where the chain of directly resumed coroutines is deep enough.
The use of the symmetric transfer may avoid this problem. When the await_suspend function returns a std::coroutine_handle, the compiler will automatically use this handle to resume its coroutine after await_suspend returns (and its activation frame is removed from the stack). Or, when a std::noop_coroutine_handle is returned, the execution will be passed to the caller.
Symmetric transfer solution can also be used to resume the current coroutine (by returning handle passed as the parameter). However, in such cases, conditional suspension can be a more optimal solution.
This rule raises an issue on await_suspend functions that could use symmetric transfer.
Assignment operators should not be virtual
Assignment operators should not be virtual
C++ does not support polymorphic copy or move assignment operators. For example, the signature of a copy assignment operator on a “Base” class would be `Base& operator=(const Base& other).
And on a “Derived” class that extends “Base”, it would be Derived& operator=(const Derived& other).
Because these are two entirely different method signatures, the second method does not override the first, and adding virtual to the “Base” signature does not change which method is called.
It is possible to add an operator=` override in a derived class, but doing so is an indication that you may need to reexamine your application architecture.
Control characters should not be used in literals
Control characters should not be used in literals
Control characters (e.g., tabs or carriage returns) are not visible to maintainers, so they should be escaped.
Same buffer should not be used both for the output and input of s[n]printf(...)
Same buffer should not be used both for the output and input of s[n]printf(...)
As stated in the Glibc documentation:
std::format should be used instead of string concatenation and std::to_string
std::format should be used instead of string concatenation and std::to_string
`std::format, introduced by C++20, enables straightforward string construction out of values of various types.
Before C++20, one popular way to obtain the same result was the conversion of the values with std::to_string and piecewise string concatenation.
std::format is strictly superior. It is more efficient because it constructs the string in-place instead of copying substrings one by one. It is also often shorter and easier to read because the format pattern is presented in a single piece and not scattered across the concatenation expression.
This rule reports string concatenation cases that can be replaced by std::format` to improve performance and readability.
std::to_address should be used to convert iterators to raw pointers
std::to_address should be used to convert iterators to raw pointers
For the integration with the C or just older APIs, it may be useful to convert a contiguous iterator to a raw pointer to the element. In C++20 `std::to_address was introduced to perform this operation on both iterators and smart pointers, which supersedes non-portable and potentially buggy workarounds, that were required before:
The first option was to take the address of the element pointed by the iterator: &*it. However, this operation has undefined behavior if the iterator is not pointing to any element. This may happen for the iterator returned by a call to end() on the container. This may also be the case when we need the address to construct a new object (via placement new) at the location pointed to by the iterator. std::to_address(it) works in such cases.
The second option was to exploit the nature of operator-> overloading and call it explicitly on the iterator: it.operator->(). This option avoids the pitfalls of the previous one, at the cost of not being portable. It would fail on the implementations that use raw-pointers as iterators for contiguous ranges like std::vector or std::span. Moreover, it is confusing, as this functional notation syntax for operators is rarely used.
While both std::to_address and above workarounds, can be always used to get the address of the element that the iterator is pointing to (if any), incrementing or decrementing may have undefined behavior. Performing pointer arithmetic on pointer to elements is safe only in the case of contiguous iterators (e.g. iterators of std::vector, std::array, std::span, std::string or std::string_view).
This rule raises an issue when dereferencing a pointer-like object is immediately followed by taking the address of the result (&*x or std::addressof(*x)) or when operator-> is called through an explicit functional notation (x.operator->()`).
Aggregate classes should not be defined
Aggregate classes should not be defined
Aggregate classes are classes with no constructors and only public and non virtual base classes and members (the exact definition has changed with each version of the language, but the purpose is the same). Aggregates allow you to initialize a variable with lots of flexibility.
A negative aspect of this flexibility is that it is very easy for the user of an aggregate to leave some members uninitialized. Therefore, it’s usually preferable to provide a set of constructors with your class to ensure the user will have to initialize it correctly.
This rule raises an issue when an aggregate has at least one non static data member with no in-class initializer.
All code should be reachable
All code should be reachable
Some statements and expressions move the control flow out of the current code block. Additionally, some functions never return the control flow to the caller. Any unlabeled statements that come after such a jump or function call is unreachable.
For instance, within a code block, code following a statement containing any of these keywords is effectively dead code:
return
break
continue
goto
co_return
throw
Examples of functions that never return the control flow to the caller:
exit()
abort()
std::terminate()
Functions with the
[[noreturn]]
attribute.
Exceptions should not be used
Exceptions should not be used
While exceptions are a common feature of modern languages, there are several reasons to potentially avoid them:
They make the control flow of a program more difficult to understand because they introduce additional hidden exit points.
It is difficult to introduce them gradually in a codebase that was not designed with exceptions in mind.
They add to the size of each binary produced, thereby increasing both compile time and final executable size.
They may incur a small performance penalty.
The time required to handle an exception is not easy to assess, which makes them difficult to use for hard real-time applications.
If a project decides not to use exceptions, some other error-handling mechanisms must be used. One option is to immediately terminate the process when unrecoverable errors are detected. Another one is to use the return value of the functions to convey error information and explicitly check for this value at the call sites. This error information then has to be manually propagated up the call stack until reaching a point where recovery is possible.
Starting with C++23, the standard library provides the std::expected class that allows packing into a single object either the normal return value of a function when the execution succeeded or some error information when it failed. This type also simplifies checking for errors at the call site.
This rule raises an issue when:
an exception is thrown
a `try-catch block is used
an exception specification (throw(xxx)`) is present.
The rule applies both for C++ and Objective-C exception mechanisms.
C libraries should not be used
C libraries should not be used
The use of C headers and therefore C functions in a C++ program, is sometimes necessary, but should be avoided in favor of C++ headers and functions.
Bit fields should be declared with appropriate types
Bit fields should be declared with appropriate types
Some types are not very well suited for use in a bit-field, because their behavior is implementation-defined. When defining a bit-field, you should stick to the following safe and portable types:
In C: `signed short, unsigned short, signed char, unsigned char, signed int, unsigned int or _Bool
In C++ before C++14: all enumerated types, as well as signed short, unsigned short, signed char, unsigned char, signed int, unsigned int, signed long, unsigned long, signed long long, unsigned long long“ or bool`
In C++ starting at C++14: all enumerated and integral types
Results of ~ and << operations on operands of underlying types unsigned char and unsigned short should immediately be cast to the operands underlying type
Results of ~ and << operations on operands of underlying types unsigned char and unsigned short should immediately be cast to the operands underlying type
When ~ and << are applied to small integer types (unsigned char or unsigned short
), the operations are preceded by integral promotion, and the result may contain high-order bits which have not been anticipated.
String literals should not be immediately followed by macros
String literals should not be immediately followed by macros
C++ allows you to append a macro value onto the end of a string literal. Prior to C++11, it was possible to do this either with or without a space between the two. But with the introduction of user-defined literals in C++11, the preprocessing of string suffixes changed. To get the same string + macro behavior under C++ 11, you must separate the string literal and the macro with a space. Without the space, you’ll get a compile error.
For the purpose of preparing for migration to C++11, this rule raises an issue when there’s no space between a string literal and a macro.
Non-member generic functions should not be declared in associated namespaces
Non-member generic functions should not be declared in associated namespaces
Argument-dependent lookup (ADL) adds additional associated namespaces to the set of scopes searched when lookup is performed for the names of called functions. A generic function found in one of these additional namespaces would be added to the overload set and chosen by overload resolution, which is inconsistent with developer expectation.
Functions should not throw exceptions not included in their specifications
Functions should not throw exceptions not included in their specifications
When exception types are included in a method specification, only those exception types may be thrown by the method. If an attempt is made to throw anything else, then by default a std::bad_exception is thrown. If std::bad_exception is not itself listed in the method specification, then the end result is that terminate()
is called, resulting in an implementation-defined termination of the program.
Methods that don’t include exception types in their specifications can throw any exception type. However, this fact should not be taken as an argument for omitting exception types. It is far better to thoroughly specify a method, so that callers know what to expect, than to leave them in the dark.
Therefore, all exceptions that could be thrown by a method should be explicitly listed in its specification.
Uniform initialization should be used
Uniform initialization should be used
C+11 introduced “Uniform initialization”/“list initialization”. It is a way to initialize an object from a braced-init-list. This adds a third way to initialize objects in C++ on top of parentheses and equal signs.
int a\{1\}; // braces initialization int b(1); // parentheses initialization int c=1; // equal sign initialization
“Uniform initialization” was introduced to address the confusion of the many initialization syntaxes in C++ and to give a syntax that, in concept, can be used in all initialization scenarios. It helps to:
Initialize container in a way that wasn’t possible before:
// Before std::vector<int> v; v.push_back(1); v.push_back(2); // After std::vector<int>\{1,2,3\};
Avoid narrowing:
double d=2.5; int i\{d\}; // Compilation error
Avoid the most vexing parse:
class A\{\}; A a(); // Compilation error declares a function named a that returns A. A a\{\}; // Call A constructor
That is why “Uniform initialization” should be preferred.
std::string_view and std::span parameters should be directly constructed from sequences
std::string_view and std::span parameters should be directly constructed from sequences
`std::string_view (introduced in C++17) and std::span (introduced in C++20) are thin generic wrappers for a contiguous sequence of elements. These wrappers can be used to unify the interface of functions that were previously accepting references to specific container types: const std::string&, const std::vector<int>&…
One of the benefits of such modernization is that it eliminates the need to explicitly create a temporary container. This happens in situations where part of the sequence is passed as an argument: substr is called on std::string. It can also happen when the type of the container elements needs to be adjusted: converting std::vector<T*> to std::vector<const T*>. When changing the type of a function parameter to std::string_view or std::span the modification of the function call site to remove the no longer needed temporary might be missed and the code will still compile. This rule will help eliminate these temporaries.
This rule raises an issue when an unnecessary temporary is passed as an argument to a parameter of std::string_view or std::span` type.
Coroutine names should comply with a naming convention
Coroutine names should comply with a naming convention
Shared naming conventions allow teams to collaborate efficiently. This rule checks that all coroutine names match a provided regular expression.
Function parameters initial values should not be ignored
Function parameters initial values should not be ignored
While it is technically correct to assign to parameters from within function bodies, it is better to use temporary variables to store intermediate results.
Allowing parameters to be assigned to also reduces the code readability as developers will not be able to know whether the original parameter or some temporary variable is being accessed without going through the whole function.
delete should be used for special member functions
delete should be used for special member functions
Declaring a special member function with `private or protected visibility and no definition was standard practice before C++11 to prevent the function from being generated by the compiler in order to prevent copy or assignment operations, for example.
Unfortunately, this purpose is not clearly expressed by such function declarations, leaving maintainers to check that such functions are not defined or used in the private or protected scopes.
C++11 adds the ability to explicitly delete, or prevent the generation of, individual special member functions without affecting any of the others. Not only is this new syntax cleaner, but it’s also clearer to maintainers.
This rule raises an issue when any of the following is declared with less than public` visibility and not defined or not used:
default constructor
move constructor
move-assignment operator
copy constructor
copy-assignment operator
constexpr literal operators should be consteval.
constexpr literal operators should be consteval.
C++20 introduces the consteval keyword, which enforces that a function is always evaluated at compile time.
constexpr functions can produce a compile-time constant in some contexts and when called with appropriate arguments, but they can also be invoked at run-time. consteval functions must always be evaluated at compile-time. If that cannot happen, a compilation error will occur.
User-defined literal operators are designed to be called with constant operands known at compile time. Therefore, if these are intended to be evaluated at compile time with constexpr, consteval should be used instead to enforce compile time evaluation. This guarantees that no code is evaluated at run-time, and also enables error detection at compile-time.
pthread_mutex_t should not be locked when already locked, or unlocked when already unlocked
pthread_mutex_t should not be locked when already locked, or unlocked when already unlocked
Mutexes are synchronization primitives that allow to manage concurrency. This is the most fundamental building block for creating safe concurrent applications. By using a mutex, one can ensure that a block of code is executed by a single thread concurrently. Data structures are designed to maintain their invariants between member-function calls. If a data structure is accessed concurrently, and one of the accesses is a write operation, then it has a data race. Having data races is undefined behavior.
Adversaries actively exploit data races to take over systems, but data races are also a common source of data corruption in concurrent applications resulting in dormant and hard-to-find bugs.
To prevent data races, the shared resource (usually memory) must be protected by obtaining mutual access to the data during both reading and writing. Such mutual exclusion is generally achieved by using a mutex, which is frequently referred to as a lock.
A mutex has two states: released - which is the initial state, or acquired. These two states are frequently called unlocked and locked as well.
To effectively protect the shared resource from concurrent accesses, all such accesses should be guarded by the same mutex. They need to lock the mutex to gain safe exclusive access to the resource and unlock it after they are done mutating or reading it.
You can abstract away the concurrent threads sharing the mutex and think of it as owned by the current thread. It never spontaneously changes between acquired and released.
In this view, these are the possible transitions when calling `lock or unlock on a mutex in a given state:
released + lock() ⇒ acquired
acquired + unlock() ⇒ released
acquired + lock() ⇒ deadlock
released + unlock()` ⇒ undefined behavior
When a thread locks a mutex, another thread trying to acquire the same mutex will be blocked and have to wait for the first thread to release it. This waiting period can take some time. If a thread attempts to lock a mutex it has already acquired, it will deadlock because it would need to release it to lock it again.
public, protected and private sections of a class should be declared in that order
public, protected and private sections of a class should be declared in that order
Coding conventions allow teams to work efficiently together. This rule checks that the public section of a class is declared first, followed by the protected section, and ending with the private
section.
Mathematical constants should not be hardcoded
Mathematical constants should not be hardcoded
Starting from C++20, mathematical constants are defined in the header <numbers>
.
You should prefer using them instead of hardcoding your own constants.
Values returned from string find-related methods should not be treated as boolean
Values returned from string find-related methods should not be treated as boolean
Function declarations that look like variable declarations should not be used
Function declarations that look like variable declarations should not be used
Like a clever insect posing as a leaf, there are constructs in C++ which look like variable declarations, but which are actually interpreted by the compiler as function or function pointer declarations. Beyond the problem of confusing maintainers, it’s highly likely in such cases that what the coder intended is not what the compiler will do.
Function templates should not be specialized
Function templates should not be specialized
Explicit specializations of function templates are not considered in overload resolution, only the main template. As a consequence, the function that will be selected might very well be different from what seems natural to the developer, leading to hard to understand bugs. Moreover, function templates don’t allow partial specialization.
Instead of specializing a function template, you may choose to overload it with another template or non template function, since a more specialized overload will be preferred to a generic overload.
Objects should not be assigned to overlapping objects
Objects should not be assigned to overlapping objects
Assigning between objects that have an overlap in their physical storage leads to undefined behaviour.
Methods with no side effects should be declared as nodiscard
Methods with no side effects should be declared as nodiscard
The only possible interest of calling a function which does not have any side effect is the use of its return value. So if this latter is ignored, the call to this function is useless and should be removed.
By adding the nodiscard
attribute to the function, such dead code would be highlighted: indeed, if the return value of a function with this attribute is ignored, a warning is raised during compilation.
The assignment-expression of a throw statement should not itself cause an exception to be thrown
The assignment-expression of a throw statement should not itself cause an exception to be thrown
If an exception is thrown when constructing the exception object, or when evaluating the assignment expression that initializes the exception object, it is that exception that propagates in preference to the one that was about to be thrown. This may be inconsistent with developer expectations.
this should not be compared with null
this should not be compared with null
According to the C++ standard, `this can never be null, so comparisons of the two are pointless at best. At worst, because of compiler optimizations, such comparisons could lead to null pointer dereferences or obscure, difficult-to-diagnose errors in production.
This rule raises an issue when this is compared to nullptr or 0 or anything #defined as nullptr or 0, such as NULL` in most environments.
The value of a complex expression of floating type may only be cast to a narrower floating type
The value of a complex expression of floating type may only be cast to a narrower floating type
If a cast is to be used on any complex expression, the type of cast that may be applied is severely restricted. As explained in MISRA C 2004 section 6.10, conversions on complex expressions are often a source of confusion and it is therefore wise to be cautious. In order to comply with these rules, it may be necessary to use a temporary variable and introduce an extra statement.
Memory access should be explicitly bounded to prevent buffer overflows
Memory access should be explicitly bounded to prevent buffer overflows
Array overruns and buffer overflows occur when a memory access accidentally goes beyond the boundary of the allocated array or buffer.
These overreaching accesses cause some of the most damaging and hard to track defects. Not only do these faulty accesses constitute undefined behavior, but they frequently introduce security vulnerabilities, too.
Structured binding should be used
Structured binding should be used
C++17 introduced structured binding, a syntax that initializes multiple entities by elements or members of an object. It is handy to emulate several return values from a function.
Suppose you have a function that returns a pair:
The address of an automatic variable should not be returned
The address of an automatic variable should not be returned
Automatic variables are those that are allocated automatically when program flow enters the object’s scope, and deallocated automatically when program flow leaves. Therefore returning the address of an automatic variable or object is an error because by the time the calling code attempts to use the value at the returned address, it no longer exists.
Unfortunately, this type of error is not always immediately evident. While the relevant memory has been deallocated, it may not have been overwritten by the time the object is dereferenced, thus leading to unpredictable behavior because sometimes the dereference is fully successful, sometimes it is partially successful (only parts of the object have been overwritten) and other times the dereference is a complete failure.
Appropriate char types should be used for character and integer values
Appropriate char types should be used for character and integer values
There are three distinct char types, (plain) char, signed char and unsigned char. signed char and unsigned char should only be used for numeric data, and plain char should only be used for character data. Since it is implementation-defined, the signedness of the plain char
type should not be assumed.
Parameters in an overriding virtual function shall either use the same default arguments as the function they override, or else shall not specify any default arguments
Parameters in an overriding virtual function shall either use the same default arguments as the function they override, or else shall not specify any default arguments
Overriding the default parameter value inherited from a parent class will lead to unexpected results when the child class is referenced from a pointer to the parent class.
#include directives should be followed by either <filename> or filename sequences
#include directives should be followed by either <filename> or filename sequences
The `#include directive is a preprocessor directive that tells the compiler to insert the contents of a file in the source code.
However, the standard only allows the #include directive to be followed by angle brackets (<filename.h>) or double quotes (“filename.h”).
If the #include` directive contains macro names, the result of their expansion must also follow this rule:
case ranges should cover multiple values
case ranges should cover multiple values
The GNU compiler extension that allows case
s to be specified with ranges should only be used when a range is actually needed. Use it with the same number on both ends of the range, and you’ve either made a mistake because an actual range was intended, or you’ve used the syntax inappropriately in a way that is highly likely to confuse maintainers.
Functions without parameters should be declared with parameter type void
Functions without parameters should be declared with parameter type void
There is a real, functional difference between a function with an empty parameter list and one with an explicitly void parameter list: It is possible to pass parameters to a function with an empty list; the compiler won’t complain. That is not the case for a function with a void
list. Thus, it is possible, and even easy to invoke empty-list functions incorrectly without knowing it, and thereby introduce the kind of subtle bug that can be very difficult to track down.
C++ comments should be used
C++ comments should be used
C++ comments (//
) require fewer keystrokes, and take less space. Perhaps most importantly, they do not have the nesting problems that C-style comments do. Therefore C++ comments are preferred.
Redundant comparison operators should not be defined
Redundant comparison operators should not be defined
C++20 introduces rewriting rules that enable defining only a few operator overloads in a class to be able to compare class instances in many ways:
the “spaceship” `operator<=> can replace all the other comparison operators in most cases: The code a @ b (where @ is one of the following operators: <, <=, >, or >=) can be implicitly rewritten to use either a<=>b or b<=>a, and its three-way comparison semantics instead.
If operator== is defined, a!=b can be implicitly rewritten !(a==b)
If an operator<=> is defined as =default, a matching operator== is automatically generated if it does not already exist.
If you define your own version of any particular comparison operator, e.g., operator< in addition to the operator<=>, it will supersede the compiler-generated version and might result in a surprising behavior with operator< semantics inconsistent with the semantics of other operators defined through operator<=>.
In most cases, you will only have to define the following set of comparison operators in your class (possibly several of those sets, to allow for mixed-type comparison):
No comparison operator, if the class should not be compared, or
only operator== for classes that can only be compared for equality (and inequality), or
only operator<=>, defined as =default for fully comparable classes that only need to perform comparison member by member, or
both operator<=> and operator== when the comparison is more complex.
This rule will raise an issue when a class is defined:
With an operator<=> and any of the four operators <, <=, >, >= defined with the same argument type.
With both operator== and operator!= defined for the same types.
With a defaulted operator<=> and a defaulted operator== with the same argument types defined.
With two operator<=> or two operator==` that are declared with the same argument types in reverse order.
Size of bit fields should not exceed the size of their types
Size of bit fields should not exceed the size of their types
Bit fields allow the developers to declare a class member with a specific size.
However, the size of a bit field is also constrained by its type: even if the specified size is greater than the size of the type, the value of the bit field will not exceed the maximum value of this type. The extra bits will just create unused padding.
The incompatibility of the size of the type with the specified size can have two causes: either the specified size is a typo error (that is the most probable cause) or the developer did not realize the size of the type he chose was too small.
bool expressions should not be used as operands to built-in operators other than =, &&, ||, !, ==, !=, unary &, and the conditional operator
bool expressions should not be used as operands to built-in operators other than =, &&, ||, !, ==, !=, unary &, and the conditional operator
The use of `bool operands with other operators is unlikely to be meaningful (or intended). Best case it will be confusing to maintainers, worst case it will not have the intended effect. Either way, it is highly recommended to stick to boolean operators when dealing with bool operands.
This rule allows the detection of such uses, which often occur because the logical operators (&&, || and !) can be easily confused with the bitwise operators (&, | and ~`).
An identifier with external linkage should have exactly one external definition
An identifier with external linkage should have exactly one external definition
It is undefined behaviour if an identifier is used for which multiple definitions exist (in different translation units) or no definition exists at all. With the exception of templates and inline functions, multiple definitions in different translation units are not permitted, even if the definitions are the same.
Types and variables should be declared in separate statements
Types and variables should be declared in separate statements
It is possible in the same statement, to declare a user-defined type (class, struct, union or enum
) followed by variable declarations of this type. But mixing more than one concern in a single statement is confusing for maintainers.
This rule raises an issue when a variable is declared at the end of a user-defined type declaration statement.
Multiple declarations of the same object or funtion should have compatible types
Multiple declarations of the same object or funtion should have compatible types
It is undefined behaviour if the declarations of an object or function in two different translation units do not have compatible types.
The easiest way of ensuring object or function types are compatible is to make the declarations identical.
The result of make_format_args should be passed directly as an argument
The result of make_format_args should be passed directly as an argument
`std::make_format_args and std::make_wformat_args return objects containing an array of formatting arguments that can be implicitly converted to std::basic_format_args. The type of the returned object cannot be spelled; it can only be accessed through auto.
A formatting argument has reference semantics for non-built-in types and does not extend the lifetime of the passed arguments. It is your responsibility to ensure that the arguments to std::make_format_args and std::make_wformat_args outlive their return value. Specifically, be aware that:
Assigning the result of std::make_format_args to a variable of type std::basic_format_args will always dangle.
Assigning the result of std::make_format_args to a variable of type auto will dangle when the formatting arguments contain an rvalue of a non-built-in type.
While it is possible to assign std::make_format_args to a variable declared with auto if all the formatting arguments are built-in types or lvalues, it is suspicious and error-prone. That is why we recommend that the result of std::make_format_args is only used as an argument for formatting functions.
This rule detects when the result of std::make_format_args or std::make_wformat_args` isn’t used as an argument.
std::span should be used for a uniform sequence of elements contiguous in memory
std::span should be used for a uniform sequence of elements contiguous in memory
C++20 introduces `std::span, a thin generic abstraction for sequences of elements contiguous in memory represented by the beginning and length. std::span can unify the interface for such sequences, e.g., for plain arrays, std::array, std::vector, or std::string.
std::span<T const* const> can be constructed of std::vector<T*> without copying it, which makes it well suited for const-correct interfaces.
std::span can have dynamic or static extent (length). The latter is useful for compilers to optimize the handling of arrays of size known at compile time.
This rule reports:
functions that accept a span by means of a plain array or a pointer to the beginning of a sequence and its length
functions that accept begin and end iterators of a std::array or a std::vector
functions that accept std::vector<T const*> and are called with a temporary copy of std::vector<T*> created just to satisfy the type signature of the argument.
functions that accept std::vector<T*> and never modify the objects pointed to by its elements.
const member functions that return a reference or a copy of a std::vector<T*>` field.
Include guards should be used
Include guards should be used
Include guards wrap around the entire contents of a header file and ensure that no matter how many times the file is actually included, its contents are only defined once. Because multiple, potentially conflicting definitions could lead to errors, the use of include guards is a best practice.
The viable function set for a function call should either contain no function specializations, or only contain function specializations
The viable function set for a function call should either contain no function specializations, or only contain function specializations
If a function and a specialization of a function template are deemed equivalent after overload resolution, the non-specialized function will be chosen over the function specialization, which may be inconsistent with developer expectations.
Member variables should not be protected
Member variables should not be protected
Protected member variables are similar to global variables; any derived class can modify them. When protected member variables are used, invariants cannot be enforced. Also, protected member variables are hard to maintain since they can be manipulated through multiple classes in different files.
If a class is just a data store without logic, it can safely contain only `public member variables and no member functions. Otherwise, data members are tightly coupled to the class logic, and encapsulation must be used. In this case, having only private member variables enforces invariants for data and ensures that logic is defined only in the member functions of the class. Structuring it this way makes it easier to guarantee integrity and easier for maintainers to understand the code.
Using protected member variables breaks the encapsulation. The risk is that data integrity logic spreads through the class and all its derived classes, becoming a source of complexity that will be error-prone for maintainers and extenders.
That is why protected member variables should be changed to private and manipulated exclusively through public or protected member functions of the base class.
This rule raises an issue when a class or struct contains protected` member variables.
Pointers to virtual base classes should only be cast to pointers to derived classes with dynamic_cast
Pointers to virtual base classes should only be cast to pointers to derived classes with dynamic_cast
Casting from a virtual base to a derived class, using any means other than dynamic_cast has undefined behaviour. The behaviour for dynamic_cast is defined.
abort, exit, getenv and system from <stdlib.h> should not be used
abort, exit, getenv and system from <stdlib.h> should not be used
<stdlib.h>‘s abort, exit, getenv, and system
have implementation-defined behaviors, and should therefore be avoided.
[*this] should be used to capture the current object by copy
[*this] should be used to capture the current object by copy
When you are using lambdas in a member function, you can capture this implicitly through [=] or [&] or explicitly through [this]
. It will capture the current object pointer by reference or value, but the underlying object will always be captured by reference (see S5019).
This will become a problem:
When the lifetime of the lambda exceeds the one of the current object.
When you want to capture the current state of the object.
When you want to pass a copy of the object to avoid any concurrency issue.
C++14 provides a solution to this problem. You can copy the underlying object by using the following pattern:
#undef should be used with caution
#undef should be used with caution
Code that contains many macros becomes hard to understand. This is even worse when the set of defined macros is not stable, and you have to know at each point what macros are defined. Therefore, `#undef can decrease the readability of macros.
However, well-disciplined use of #undef can also improve readability, for instance when defining a macro with a limited scope: The macro is #defined, used a couple of times to reduce code duplication, then immediately #undefed.
This rule raises an issue when a #undef undefines a macro that was defined in another file. It will also raise an issue for an #undef` directive that tries to undefine a non-existing macro.
nullptr should be used to denote the null pointer
nullptr should be used to denote the null pointer
Before C++11, the only way to refer to a null pointer was by using the integer literal `0, which created ambiguity about whether a pointer or an integer was intended. Even with the NULL macro, the underlying value is still 0.
C++11 introduced the keyword nullptr`, which unambiguously refers to the null pointer. It should be used systematically.
Use conditional suspension to resume current coroutine
Use conditional suspension to resume current coroutine
One of the use cases for the coroutines is suspending execution until certain conditions are satisfied (e.g. value is produced, flag/event is triggered). In some situations, the expected result may be already available at the point of the co_await/co_yield expression, and the execution can be resumed immediately.
The C++ standard provides an efficient method to suspend the coroutine conditionally. The result of await_ready is used to determine whether a coroutine should be suspended. Returning true from this function avoids the cost of the coroutine suspension if it is not needed (e.g., the result is already available). Furthermore, the bool-returning version of await_suspend allows immediate resumption of the current coroutine in the case when false is returned (returning true indicates that the coroutine should remain suspended). Compared to symmetric transfer, this method provides better optimization opportunities, as the continuation code is known to the compiler - i.e., it is the code of the current coroutine, while in symmetric transfer the handle could point to an arbitrary coroutine.
This rule raises an issue on await_suspend that can benefit from using conditional suspension.
Threads should not be detached
Threads should not be detached
Sometimes, you might want to make a thread run indefinitely in the background by not binding it to its creation scope. Even though calling `detach() on an std::thread or std::jthread object would satisfy this need, it is not the easiest way to do it: there will be no direct way to monitor and communicate with the detached thread, the std::thread or std::jthread object is no longer associated to any thread.
An easier alternative to satisfy this need is giving the thread a global scope. This way the thread will run as long as the program does. The thread will not be bound to any scope. It is also possible to do it by giving the std::thread or std::jthread` a scope that is big enough for your use case. For example, the program’s main function.
Function names should comply with a naming convention
Function names should comply with a naming convention
For example, with the default provided regular expression: ^[a-z][a-zA-Z0-9]*$
, the function:
Integral constants should not be added to char pointers
Integral constants should not be added to char pointers
Contrary to what you might believe, using the addition operator does not append an integral constant to a string. Adding a char or an integral to a string pointer does not append it to the string. What it does instead is incrementing the string pointer by a value defined by this char
or integral.
L#macro_arg should not be used
L#macro_arg should not be used
Cumulatively, saved keystrokes can add up to a lot of saved time. Or at least it can feel that way. So the difference between using L#macro_arg and L###macro_arg
to create a wide string literal may seem perfectly justified because MSVC yields the same result for each. But if you ever need to switch to another compiler, that saved time - and then some - will be lost. So it’s best to use the cross-compiler standard from the start.
Functions should not contain too many return statements
Functions should not contain too many return statements
Having too many return statements in a function increases the function’s essential complexity because the flow of execution is broken each time a return statement is encountered. This makes it harder to read and understand the logic of the function.
The way of counting the return statements is aligned with the way we compute Cognitive Complexity.
”Under Cyclomatic Complexity, a switch is treated as an analog to an if-else if chain […] but from a maintainer’s point of view, a switch - which compares a single variable to an explicitly named set of literal values - is much easier to understand than an if-else if chain because the latter may make any number of comparisons, using any number of variables and values. “
As a consequence, all the return statements located at the top level of case statements (including default) of a same switch statement count all together as 1.
// this counts as 1 return int fun() \{ switch(variable) \{ case value1: return 1; case value2: return 2; default: return 3; \} \}
std::filesystem::path should be used to represent a file path
std::filesystem::path should be used to represent a file path
Introduced in C++17, the class `std::filesystem::path can store a file path. Compared to a regular string, it offers several advantages:
Having a dedicated type makes the intention clear
This class stores the path with an encoding that is appropriate to the OS where the program runs
It provides several functions that make it more convenient to manipulate than a string (for instance, operator/ for concatenations)
It provides a normalized way to specify the path, easing the portability of the code (on Windows and Linux, the native way is equivalent to the normalized way, which reduces overhead).
This rule raises an issue when the same string is converted several times to a path because it indicates that a single path object could have been used in all occurrences. It can also be more efficient since conversion from string to path` may require a change of encoding and memory allocation.
Digraphs should not be used
Digraphs should not be used
The use of digraphs may not meet developer expectations.
The digraphs are:
`<%
%>
<:
:>
%:
%:%:`
std::string_view should be used to pass a read-only string to a function
std::string_view should be used to pass a read-only string to a function
`std::string_view is a read-only view over a string, it doesn’t hold any data, it only holds a pointer to the first character of the string and its length. std::string_view can offer better performance than std::string in several cases:
no memory allocations are required during construction, it is cheap to pass them by value, no need to pass them by reference
no heap allocation when passing a string literal to a std::string_view function argument
substr operations over a std::string_view do not require memory allocation
When using std::string_view you shouldn’t however forget that:
it’s a non-owning range, you should keep into consideration the liveness of the pointed range
it doesn’t guarantee a null-terminated string like std::string
This rule flags const std::string& function arguments, which can be safely replaced with std::string_view ones when not relying on the null-termination character.
Note that, if you are calling substr on the parameter, you may have to modify your code to explicitly cast the result to std::string`.
Unary prefix operators should not be repeated
Unary prefix operators should not be repeated
The repetition of a unary operator is usually a typo. The second operator invalidates the first one in most cases:
Deprecated K&R syntax should not be used for function definition
Deprecated K&R syntax should not be used for function definition
In 1978, Brian Kernighan and Dennis Ritchie published the first edition of The C Programming Language. This book, known to C programmers as “K&R”, served for many years as an informal specification of the language. The version of C that it describes is commonly referred to as K&R C.
The K&R function definition syntax introduced in the book was later deprecated in the ANSI C and ISO C standards. Even though the K&R syntax is still supported in the ISO C11 standard, it’s not in ISO C++ standard versions and is not considered readable by most C/C++ developers today.
std::optional member function value_or should be used
std::optional member function value_or should be used
`C++17 introduces std::optional<T>, a template class that manages an optional contained value. By default, the container doesn’t contain any value. The contained value can be accessed through member functions like value(), operator*(), or operator->(). Before accessing the value, it is a good practice to check its presence using has_value() or operator bool().
value_or(default_value) member function returns the contained value if present or default_value otherwise. This rule flags patterns that could be simplified by a single call to value_or(default_value) instead of two steps logic:
check presence, i.e., with has_value
use value() if present, default_value` otherwise
Assignment operators should return non-const references
Assignment operators should return non-const references
Copy assignment operators and move assignment operators can return anything, including void
.
However, if you decide to declare them yourself (don’t forget the “Rule-of-Zero”, S4963), it is a recommended practice to return a non-const reference to the left-operand. It allows the developer to chain the assignment operations, increasing consistency with what other types do, and in some cases enabling writing concise code.
C++ formatting functions should be used instead of C printf-like functions
C++ formatting functions should be used instead of C printf-like functions
In contrast to C printf-like functions, C++ provides safer and more robust interfaces for performing text formatting:
The std::format interface family (C++20) allows formatting text into a string.
The std::print interface family (C++23) allows printing formatted text.
C++ formatting facilities perform validation of the format string against the type of the formatted argument. If the validation fails, it is reported as a compilation error for the calls of std::print and std::format. When the format string is not available at compile-time, std::vformat, std::vprint_unicode, and std::vprint_nonunicode can be used. They will report failures at runtime by throwing an instance of std::format_error.
Secondly, the relation between the type and format specifier is more abstract. In particular, {:d} can be used to format any integer type, regardless of its size and signedness. Similarly, {:f} works for any floating point type. Furthermore, {} can be used for any type with default format spec, which makes it usable in the generic context.
Finally, the text formatting API was designed with adaptability in mind:
Formatting of user-defined types is possible with the dedicated format specification via std::formatter specializations.
The string formatting API provides functions for:
receiving the formatted text by return - std::format.
writing the formatted text to an output iterator - std::format_to.
The std::print API provides function overloads for:
printing implicitly to the standard output.
printing to a `FILE* handle.
printing to a std::ostream& object.
This rule raises issues for calls of the printf, fprintf, sprintf and snprintf` functions that can be replaced by the C++ formatting functions.
Function pointers should not be used as function parameters
Function pointers should not be used as function parameters
When you want to receive a function as a parameter in a function definition, there are three ways to declare its parameter type:
A function pointer: `void f(void (*callback)());
A typed-erased function wrapper such as std::function: void f(std::function<void()> callback);
A template parameter: template <class Callback> void f(Callback callback);
Using a function pointer is an inferior solution for the following reasons:
Only a function pointer can be passed as an argument, while the other options offer the caller more flexibility because they can take more advanced functors, such as lambdas with some captured state
The syntax is obscure
It typically has worse performance than the template parameter solution.
See S5213 for a discussion on choosing between std::function` and a template parameter.
A single statement should not have more than one resource allocation
A single statement should not have more than one resource allocation
In a statement, the order of evaluation of sub-expressions (e.g., the arguments of a function call) is not totally specified. This means the compiler can even interleave the evaluation of these sub-expressions, especially for optimization purposes.
If you have several resource allocations in one statement, and the first succeeds while the second fails and throws an exception, the first allocated resource can leak. The classical mitigation for this issue is to use an RAII (Resource Acquisition Is Initialization) manager to wrap the raw resource. Yet, this solution may not be sufficient since the execution order is not specified.
It is possible to write code that contains several allocations and still behaves correctly. C++17 made this even easier since the evaluation order rules are more strict. However, it requires expert-level knowledge of the language. It is simpler and more future-proof to simply avoid using several allocations in a single statement.
Control flow statements if, for, while, switch and try should not be nested too deeply
Control flow statements if, for, while, switch and try should not be nested too deeply
Nested control flow statements if, for, do, while, switch and try
are often key ingredients in creating
what’s known as “Spaghetti code”. This code smell can make your program difficult to understand and maintain.
When numerous control structures are placed inside one another, the code becomes a tangled, complex web. This significantly reduces the code’s readability and maintainability, and it also complicates the testing process.
Allocation and deallocation functions should not be explicitly declared static
Allocation and deallocation functions should not be explicitly declared static
Allocation functions are always static. Explicitly declaring such a function static
needlessly clutters the code.
sizeof should not be called on pointers
sizeof should not be called on pointers
sizeof returns the size in bytes of a type. One common usage pattern, especially in C, is to use sizeof to determine the size of an array. However, arrays decay to pointers when passed as arguments to a function, and if sizeof is applied to such an argument, it will return the size of the pointer, not of the array. A similar issue happens when the array is used in an arithmetic operation.
This rule raises issues when:
sizeof is used to compute the array size of a pointer passed as a function argument.
sizeof is called on the result of an arithmetic operation involving an array.
Note: C++17 provides a std::size function that will correctly compute the number of elements of an array and fail to compile if provided with a pointer. It is simpler and safer to use this variant when available. C++20 also provides the functions std::ssize, std::ranges::size, and std::ranges::ssize with similar effects.
extern C should not be used with namespaces
extern C should not be used with namespaces
The C linkage declaration extern “C” can not be combined with a namespace. In practical terms only one function with that name can be declared as extern “C” because the namespace
is functionally ignored.
Accessible base classes should not be both virtual and non-virtual in the same hierarchy
Accessible base classes should not be both virtual and non-virtual in the same hierarchy
If a base class is both virtual
and non-virtual in a multiple inheritance hierarchy then there will be at least two copies of the base class sub-object in the derived object. This may not be consistent with developer expectations.
Multicharacter literals should not be used
Multicharacter literals should not be used
Multicharacter literals have int
type and have an implementation-defined value. This means they might be interpreted differently by different compilers. For example, they might lead to different behavior when compiled with GCC than when compiled with MSVC.
Even if they work as you expect with a specific compiler, they will make your code less portable. They are also misleading as they look like strings, hence make your code less readable.
Pointers or references obtained from aliased smart pointers should not be used as function parameters
Pointers or references obtained from aliased smart pointers should not be used as function parameters
One way to end up with a dangling pointer or a dangling reference is to pass to a function a pointer or a reference which lifecycle is not controlled. This is the case when the pointer or the reference is obtained from a smart pointer (shared_ptr or unique_ptr
) which is not locally defined or which is potentially aliased.
In this case, nothing can guarantee the developer that the pointer or the reference coming from this smart pointer will always be valid: for example, this smart pointer could be reset somewhere in the call chain.
The three expressions of a for statement should only be concerned with loop control
The three expressions of a for statement should only be concerned with loop control
for
loops are very flexible in C and C++. Because of that, they can carry a complexity that can make the code error-prone, difficult to understand, and hard to maintain.
Many for loops can be written in a way that clearly separates the iteration process from the content of the iteration. This rule makes sure that all the code relevant to the iteration is placed in the for-loop header. The compliant code is then easier to reason about.
A for loop is composed of 4 sub-parts:
for([initialization]; [condition]; [update]) [body]
We classify the variables used to control them in three categories:
A loop-counter is a variable modified in the update. It should not be modified in the body.
A loop-constant is an auxiliary variable declared in the initialization. It’s very often used to precompute some data about the end condition or the stride.
A pseudo-counter shares some properties with a loop counter, but its update conditions are more complex. It will therefore only be updated in the body, and cannot be used in the update. Using a pseudo-counter makes the loop more complex to reason about, and therefore is not permitted. They are very often declared in the initialization, for instance, to limit their scope, but in some cases reuse existing variables.
Additionally, the loop condition should refer to at least one loop-counter, and should not modify anything.
This rule is only checking for loops with a condition and an update.
Relational and subtraction operators should not be used with pointers to different arrays
Relational and subtraction operators should not be used with pointers to different arrays
Attempting to make a comparison between pointers using >, >=, < or <= will produce undefined behavior if the two pointers point to different arrays.
Additionally, directly comparing two arrays for equality or inequality has been deprecated in C++.
However, equality or inequality between an array and a pointer is still valid
Comma operator should not be used in a subscript-operator argument
Comma operator should not be used in a subscript-operator argument
The comma between two expressions that are not different function arguments is called “the comma operator” and has special evaluation semantics. The expression composed of the three expressions separated by a comma are evaluated sequentially and the results of all but the last subexpression are discarded. For example in the expression foo(), a = 1, i - 1: first foo is invoked, then a is assigned 1, and finally i - 1
is computed and becomes the result of the whole expression, ignoring the result of the previous two subexpressions.
The use of a comma operator is rarely justified, see S878. Moreover, C++20 deprecates the use of a comma operator inside “the subscript operator” argument (the square brackets []) because it might be repurposed in the later editions of the standard for multidimensional arrays. In case you still want to call the comma operator inside a subscript operator, you will have to enclose it in parentheses.
This rule reports the use of a comma operator in the argument of a subscript operator.
std::scoped_lock should be created with constructor arguments
std::scoped_lock should be created with constructor arguments
When constructing an std::scoped_lock, the constructor arguments are used to list the mutexes that the scoped_lock will lock on the creation and unlock on destruction. It is possible to construct a scoped_lock
without any parameter, but in that case, it does absolutely nothing and is just dead code, which was probably not the intent of the user.
Functions should not have more than one argument of type bool
Functions should not have more than one argument of type bool
`bool is often used to implement an enum with two values. But when used carelessly, this leads to error-prone code. While it may seem reasonable when defining a boolean function parameter, the meaning might not be so clear at the function call point.
If the function only has a single boolean argument, the potential ambiguity is mitigated: the function name can indicate its purpose, and what true and false mean. But when a function has more than one boolean argument, it becomes increasingly difficult to come up with descriptive names. Moreover, it becomes very easy for callers to inadvertently swap the argument order. In such cases, it is much clearer to use an explicit enum or, if the boolean has a logical relation to another argument, to package them together in a struct`, where the data member name can give meaning to the boolean. Another option is to split dealing with the multiple boolean arguments into multiple functions because sometimes multiple boolean parameters indicate a function that’s trying to do too much.
Format strings should comply with ISO standards
Format strings should comply with ISO standards
The possibilities of ISO C printf
format strings are limited, this is why many extensions have been added by several implementations. Even though they are widespread and similar in many implementations, they are not standard, which means that using them may create portability issues.
This rule reports an issue when format strings do not comply with ISO C standards.
Exceptions should not be raised before program start-up or after termination
Exceptions should not be raised before program start-up or after termination
Throwing an exception during startup or shutdown results in an implementation-defined termination of the program. This is because there is no where to put try/catch
blocks to catch exceptions thrown in the startup phase, when static objects are being constructed, or during the shutdown phase, when those static objects are being destroyed.
Therefore exceptions should not be thrown during the construction or destruction of static objects.
Pointer and reference parameters should be const if the corresponding object is not modified
Pointer and reference parameters should be const if the corresponding object is not modified
Const correctness is an important tool for type safety. It allows for catching coding errors at compile time and it documents the code for maintainers.
Correctly const-qualifying pointers can be tricky because the indirection they add can also be const.
For a pointer X * ptr, const can be written in three different places:
const X * ptr and X const * ptr are identical and mean that the X object ptr points to cannot be changed.
X * const ptr means that the pointer cannot be changed to point to a different X object.
In a function signature, the first const X * ptr (or its equivalent X const * ptr) is the one that will bring type-safety. It protects against changing the value pointed at.
Raw string literals should be used
Raw string literals should be used
Since C++11, raw string literals can be used to avoid the need to escape characters in a string.
This rules raises an issue when using a raw string literal would make a string easier to read. For instance, when a non-raw string contains different escaped sequences (among ’, \, ” and ?
) or more than two of the same kind.
std::has_single_bit should be used to test if an integer is a power of two
std::has_single_bit should be used to test if an integer is a power of two
Since integers are usually represented in binary form in computers, it is efficient to check if a given number is a power of two by checking if its `unsigned representation has a single-bit set.
In C++ such check could be expressed as `++x & (x-1)
The right-hand operands of && and || should not contain side effects
The right-hand operands of && and || should not contain side effects
There are some situations in C++ where certain parts of expressions may not be evaluated. If these sub-expressions contain side effects then those side effects may or may not occur, depending on the values of other sub expressions. The operators which can lead to this problem are && and ||
, where the evaluation of the right-hand operand is conditional on the value of the left-hand operand. The conditional evaluation of the right-hand operand of one of the logical operators can easily cause problems if the developer relies on a side effect occurring.
Operations that cause side effects are:
accessing a volatile object
modifying an object
modifying a file
calling a function that performs any operations that cause changes in the state of the execution environment of the calling function.
This rule raises an issue when there is assignment or the use of the increment/decrement operators in right-hand operands.
GNU extensions should not be used
GNU extensions should not be used
Proprietary compiler extensions can be handy, but they commit you to always using that compiler. This rule raises an issue when GNU extensions are used, such as:
Ternary operator with omitted second operand
Case ranges in switch statements
Expression statements, i.e. code blocks producing value
Index range in array initializers
A array initializer without `=
A structure member initializer with a colon
Decimal floating points numbers _Decimal32, _Decimal64, and _Decimal128`
Structures and union without named data members
Virtual functions should not be called from constructors or destructors
Virtual functions should not be called from constructors or destructors
When constructing an object of a derived class, the sub-object of the base class is constructed first, and only then the constructor of the derived class is called. When there are multiple levels of inheritance, the process is the same, from the most base class to the most derived class. Along this construction process, the dynamic type of the object evolves and is the type of the sub-object under construction.
The destruction of the object follows the same process in reverse order.
As a consequence, when calling a virtual function from a constructor or a destructor, the actual function being called is not necessarily the version from the most-derived type, as some developers may believe, but the version that matches the level under construction.
Bit-fields shall be either bool type or an explicitly unsigned or signed integral type
Bit-fields shall be either bool type or an explicitly unsigned or signed integral type
Using `int is implementation-defined because bit-fields of type int can be either signed or unsigned.
The use of wchar_t as a bit-field type is prohibited as ISO/IEC 14882:2003 does not explicitly define the underlying representation as signed or unsigned`.
Bitwise operators should not be applied to signed operands
Bitwise operators should not be applied to signed operands
Most built-in bitwise operators (~, >>, >>=, &, &=, ^, ^=, |, and |=) have implementation-dependent results when performed on signed operands, and bitwise left shift (<< and <<=
) has unspecified or undefined behavior when performed on negative operands.
Therefore, bitwise operations should not be performed on signed operands.
Starting with C++20, the behaviors have been defined more accurately (negative values have to be represented using two’s complement), and therefore this rule will only report an issue when the second operand of a shift operator is signed (shifting by a negative value is still undefined behavior).
User-defined types should not be passed as variadic arguments
User-defined types should not be passed as variadic arguments
Variadic arguments allow a function to accept any number of arguments (in this rule, we are not talking about variadic templates, but about functions with ellipses). But these arguments have to respect some criteria to be handled properly.
The standard imposes some requirements on the class types that can be passed as variadic arguments, and those requirements vary according to the C++ standard version in use:
Before C++11, the standard only allows POD types to be used as variadic arguments.
In C++11, the rules are relaxed such that any class type with an eligible non-trivial copy constructor, an eligible non-trivial move constructor, or a non-trivial destructor can be used in variadic arguments.
The rule detects any violations of these requirements since they can trigger undefined behavior.
Additionally, since using an incorrect type to access the passed parameter within the variadic function can lead to undefined behavior, the rule goes a step further and reports all cases when class types are passed as variadic arguments. The rationale is that, most likely, the user forgot to call a method on the object being passed (std::string_view::data()
for example) that would get a member of a built-in type.
When in need to pass class types to functions that take a variable number of arguments, consider using modern type-safe alternatives like C++11 parameter packs instead of variadic functions.
Stack allocated memory and non-owned memory should not be freed
Stack allocated memory and non-owned memory should not be freed
The free function and delete operator are used exclusively to release dynamically allocated memory. Attempting to release any other type of memory is undefined behavior.
The following non-heap memory types may not be released:
Stack allocated memory - local variables or memory allocated with the alloca,
_alloca, _malloca and __builtin_alloca
functions.Executable program code - function pointers.
Program data - global and static variables.
Read-only program data - constants and strings.
Generic exceptions should never be thrown
Generic exceptions should never be thrown
Throwing generic exceptions such as `std::exception, std::logic_error and std::runtime_error will have a negative impact on any code trying to catch these exceptions.
From a consumer perspective, it is generally a best practice to only catch exceptions you intend to handle. Other exceptions should ideally not be caught and let propagate up the stack trace so that they can be dealt with appropriately. When a generic exception is thrown, it forces consumers to catch exceptions they do not intend to handle, which they then have to re-throw.
Besides, when working with a generic type of exception, the only way to distinguish between multiple exceptions is to check their message, which is error-prone and difficult to maintain. Legitimate exceptions may be unintentionally silenced and errors may be hidden.
For instance, in the following code, the fact that checkState` throws a generic exception leads us to catch a permission error that shouldn’t have been caught:
Members should be initialized in the order they are declared
Members should be initialized in the order they are declared
Class members are initialized in the order in which they are declared in the class, not the order in which they appear in the class initializer list. To avoid errors caused by order-dependent initialization, the order of members in the initialization list should match the order in which members are declared in a class.
The initialization order, as described here, is:
If the constructor is for the most-derived class, virtual bases are initialized in the order in which they appear in depth-first left-to-right traversal of the base class declarations (left-to-right refers to the appearance in base-specifier lists)
Then, direct bases are initialized in left-to-right order as they appear in this class’s base-specifier list
Then, non-static data members are initialized in order of declaration in the class definition.
std::initializer_list constructor should not overlap with other constructors
std::initializer_list constructor should not overlap with other constructors
When a class has a constructor accepting `initializer_list of type X and another constructor that has n parameters of either type X or a type that can be converted to X, the constructor call resolution becomes complex. This makes code hard to reason about and might lead to calls resolving to unexpected constructors. What makes it even more complex, is that the constructor resolution rules are different if X is a type template parameter.
This rule flags classes that have constructors overlapping with the initializer_list constructor. It is recommended to simplify the class by:
A technical change: replace initializer_list parameter by a std::vector, a std::array, or a variadic template. This way the caller is forced to be more explicit.
A design change: make the construction of an object of type X taking object(s) of type Y as parameters equivalent to constructing it with an initializer list containing the object(s) of type Y. This way you can reduce the number of overlapping constructors to the one that takes initializer_list`.
Template should not be constrained with ad-hoc requires-expression
Template should not be constrained with ad-hoc requires-expression
Since C++20, it is possible to add a requires-clause to a template as a way to express requirements (constraints) on the template arguments. This construct is versatile and allows any expression that evaluates to either true or false at compile time to be used. One of these expressions is the requires-expression, which can be used to express required operations on types:
Memory locations should not be released more than once
Memory locations should not be released more than once
Using delete or free releases the reservation on a memory location, making it immediately available for another purpose. Releasing the exact memory location twice leads to undefined behavior and can often crash the program.
The C standard defines as undefined behavior a call to free with a pointer to a memory area that has already been released.
The C++ standard defines the first delete call as the end of the lifetime for dynamically allocated memory. Access to memory past its lifetime end, including another delete, is undefined behavior.
Requires-expression should not contain unevaluated concept checks or type predicates
Requires-expression should not contain unevaluated concept checks or type predicates
A requires-expression is a list of requirements that can be of different natures. Simple-requirements are expressions that do not start with the keyword requires and compound-requirements are expressions surrounded by curly brackets potentially followed by a noexcept specification and return type requirements.
In both cases, the expressions are not evaluated. They will only be checked for validity, and if the expression is invalid, the requires-expression evaluates to false.
When we write a concept check or a type predicate, the intent is usually to evaluate them, therefore, they don’t really belong in a simple-requirement or a compound-requirement. Instead, they should either be used directly in a concept definition (outside of a requires-expression) or, less often, as a nested-requirement (a requirement introduced by the requires keyword within the requires-expression).
This rule detects concept checks and standard type predicates (from the header <type_traits>) in single and compound requirements of requires-expressions.
Local classes should not be used
Local classes should not be used
Classes defined inside functions, called local classes, are only visible within those functions. They have advantages in some situations, but if they’re just being used as functors, lambdas are preferred.
This rule raises an issue when a class or struct
is defined inside a function.
Whitespaces should not follow @
Whitespaces should not follow @
Shared coding conventions allow teams to collaborate effectively. This rule checks that there are no whitespaces between an @
character and what it precedes.
Lambdas should not be used
Lambdas should not be used
Lambdas are a concise way of creating anonymous functions - when those functions are themselves concise. However, the use of lambdas for sizable functions can make the code difficult to read. More importantly, following variable capture in lambdas can be difficult, potentially leading to dangling pointers. Therefore lambdas should be avoided.
Variables should be initialized before use
Variables should be initialized before use
A local variable of any built-in type (such as int, float, and pointers), declared without an initial value is not initialized to any particular value. Consequently, if no value is assigned to such a variable first, the code that uses it has no defined behavior.
Initializer of a range-based for loop should be used to reduce scope of variables
Initializer of a range-based for loop should be used to reduce scope of variables
C++20 has introduced an initializer construct in the range-based `for loops. Similar to if with initializer and switch with initializer, the initializer of a range-based for loop enables you to declare and initialize a variable to make it visible only in the range condition and in the body of the loop.
Previously, variables were either declared before the statement, hence leaked into the ambient scope, or an explicit scope was used to keep the scope tight, especially when using RAII objects. This was inconvenient as it would lead to error-prone patterns.
This rule reports variables declared outside and used only inside of a range-based for` loop.
The value of an expression of floating type shall not be implicitly converted to a different type
The value of an expression of floating type shall not be implicitly converted to a different type
The value of an expression of floating type shall not be implicitly converted to a different type if:
it is not a conversion to a wider floating type, or
the expression is complex, or
the expression is a function argument, or
the expression is a return expression.
The intention when restricting implicit conversion of complex expressions is to require that in a sequence of arithmetic operations within an expression, all operations should be conducted in exactly the same
arithmetic type. Notice that this does not imply that all operands in an expression are of the same type. The expression f32a + f64b + f64c is compliant – both additions will notionally be performed in type F64.
The expression f32a + f32b + f64c is not compliant – the first addition is notionally performed in type f32 and the second in type f64. The word “notionally” is used because, in practice, the type in which arithmetic will be conducted will depend on the implemented size of a float
. By observing the principle whereby all operations are performed in a consistent (underlying) type, it is possible to avoid programmer confusion and some of the dangers associated with integral promotion.
The _t and _v version of type traits should be used instead of ::type and ::value
The _t and _v version of type traits should be used instead of ::type and ::value
When they were first introduced in the language, type traits, defined in header `<type_traits>, required to use nested types (with ::type) or nested values (with ::value) to access the result of the trait. Since then, the language introduced templated alias declaration and variable templates that allow to define traits in a more direct and readable way.
Even if the old variant still exists, the new one, which uses _t (C++14) and _v` (C++17) suffixes as discriminant, should be preferred.
Return value of setuid family of functions should always be checked
Return value of setuid family of functions should always be checked
Functions from the setuid-family including setuid and setgid
are used to change the identity of the caller process.
They are used to change privileges for the subsequent actions to be executed.
If a call to these functions returns an error that is not checked and handled appropriately, the subsequent parts of the program will execute with unexpected privileges.
This, in turn, leads to unexpected program behavior and poses a serious security risk.
Printf-style format strings should not lead to unexpected behavior at runtime
Printf-style format strings should not lead to unexpected behavior at runtime
Because printf format strings are interpreted at runtime rather than validated by the compiler, they can contain errors that lead to unexpected behavior or runtime errors. This rule statically validates the good behavior of printf formats.
The related rule S3457 is about errors that produce an unexpected string, while this rule is about errors that will create undefined behavior.
Starting with C++23, std::print should be preferred: its arguments are validated at compile-time, making it more secure.
Overriding member functions should do more than simply call the same member in the base class
Overriding member functions should do more than simply call the same member in the base class
Overriding a function just to call the overridden function from the base class without performing any other actions can be useless and misleading.
There are cases when it is justified because redeclaring the function allows some side effects:
Changing the visibility of the function in the derived class
Preventing the base class function from being hidden by an overload added in the derived class (a using-declaration could have the same effect)
To resolve ambiguities in cases of multiple inheritance
To make an inherited function final
This rule raises an issue when an override not in one of the abovementioned situations only calls the overridden function, directly forwarding its arguments.
Before preprocessing, a null statement shall only occur on a line by itself; it may be followed by a comment provided that the first character following the null statement is a white-space character
Before preprocessing, a null statement shall only occur on a line by itself; it may be followed by a comment provided that the first character following the null statement is a white-space character
Null statements should not normally be included deliberately, but where they are used, they shall appear on a line by themselves. White-space characters may precede the null statement to preserve indentation. If a comment follows the null statement, then at least one white-space character shall separate the null statement from the comment. The use of a white-space character to separate the null statement from any following comment is required on the grounds that it provides an important visual cue to reviewers. Following this rule enables a static checking tool to warn of null statements appearing on a line with other text, which would normally indicate a programming error.
Loop variables should be declared in the minimal possible scope
Loop variables should be declared in the minimal possible scope
When a loop variable is not used outside of a loop, it should be declared inside the loop declaration:
It improves readability. The scope of the variable is clearly defined.
It reduces the number of mistakes. The variable can’t be accidentally misused outside of the loop.
Resources are not retained longer than necessary.
Copy assignment operators should be protected or private in abstract classes
Copy assignment operators should be protected or private in abstract classes
An abstract class represents the interface part of a hierarchy. Invoking the copy constructor from the top of such a hierarchy bypasses the underlying implementation resulting in only the base sub-objects being copied.
Deprecated attributes should include explanations
Deprecated attributes should include explanations
The deprecated attribute can be applied with or without explanations, but marking something deprecated
without including advice as to why it’s deprecated or on what to use instead will lead maintainers to waste time trying to figure those things out - every single time the warning is encountered.
C-style array should not be used
C-style array should not be used
C-style arrays (such as `int i[10]) are not very convenient to use:
They are fixed size (even C Variable Length Arrays are not truly variable size, and they are not supported in C++)
If the number of elements in the array can vary, it will lead to manual memory allocation (or people will use fixed-size arrays that “should be large enough”, which is both a waste of memory and a limitation of the program)
It is very easy to lose the size of the array since an array passed to a function decays into a pointer
The C++ standard library proposes two types that are better than C-style arrays and together cover all the use cases of C-style arrays:
For fixed-size arrays, where the memory is on the stack, use std::array. It is like a C-style array, except that it has normal argument passing semantics, and the size is always a part of the type. You can roll your version if std::array is unavailable to you (before C++11).
For variable-size arrays, use std::vector. It can be resized and handles memory allocation transparently.
For character strings, you should use std::string instead of arrays of characters.
For arrays of characters that are not strings (e.g., alphabet, exit codes, keyboard control list), prefer std::array or std::vector as per the first two bullets.
The rule S945 is related to this rule but focuses on passing arguments of an array type. S5025 will flag the use of dynamic memory allocation that could be replaced by std::vector`.
#include_next should not be used
#include_next should not be used
`#include_next is a gcc-specific language extension that alters the search path for the specified header file by starting the search from the header file directory after the one in which the directive was encountered. It also ignores the distinction between “file” and <file>. It is typically used when you have two (probably related) header files with the same name, although there is nothing in the extension to enforce or limit the use to same-name files.
Use of this extension can be tricky to get right, and is almost never justified. Instead, you should use an absolute path in the #include` statement or rename one of the files.
Thread local variables should not be used in coroutines
Thread local variables should not be used in coroutines
In contrast to normal functions, coroutines can suspend and later resume their execution. Depending on the program, the coroutine may resume on a different thread of execution than the one it was started or run previously on.
Therefore, the access to the “same” variable with thread_local storage may produce different values, as illustrated below:
Negative memory allocations should not be made
Negative memory allocations should not be made
Passing a negative value into a memory allocation function can still result in a cleanly-allocated block of memory. However, it will likely be much, much larger than intended. This is because alloc, malloc, calloc, and realloc take a size_t parameteter, which is unsigned. Pass in a negative value, and it will be converted to SIZE_MAX - 1. According to the standard, SIZE_MAX
must be at least 65,535.
Such an allocation could result in Denial of Service as the system struggles in the wake of the too-large memory grab.
This rule logs an issue when a signed value is passed in to an allocation function.
Unions should not be used
Unions should not be used
The use of unions to access an object in different ways may result in the data being misinterpreted. Therefore, this rule prohibits the use of unions for any purpose.
Destructors should not be called explicitly
Destructors should not be called explicitly
Destructors are invoked automatically when control leaves the scope in which the object was created. Add an explicit destructor call to that, and you end up with undefined behavior because the automatic destructor invocation will be invoked on an object that no longer exists. However sometimes it is acceptable to have destructor calls for some specific use-cases, i.e. when it is desired to destroy the object but without releasing the memory.
Polymorphic base class destructor should be either public virtual or protected non-virtual
Polymorphic base class destructor should be either public virtual or protected non-virtual
When a class with no virtual destructor is used as a base class, surprises can occur if pointers to instances of this class are used. Specifically, if an instance of a derived class is `deleted through a pointer to the base type, the behavior is undefined and can lead to resource leaks, crashes or corrupted memory.
If it is not expected for base class pointers to be deleted, then the destructor should be made protected` to avoid such a misuse.
Using strcat or wcscat is security-sensitive
Using strcat or wcscat is security-sensitive
a buffer of characters, normally using the `null character as a sentinel for the end of the string. This means that the developer has to be aware of low-level details such as buffer sizes or having an extra character to store the final null character. Doing that correctly and consistently is notoriously difficult and any error can lead to a security vulnerability, for instance, giving access to sensitive data or allowing arbitrary code execution.
The function char *strcat( char *restrict dest, const char *restrict src ); appends the characters of string src at the end of dest. The wcscat does the same for wide characters and should be used with the same guidelines.
Note: the functions strncat and wcsncat might look like attractive safe replacements for strcat and wcscaty`, but they have their own set of issues (see S5815), and you should probably prefer another more adapted alternative.
Constructors and destructors should not be inline
Constructors and destructors should not be inline
Functions that invoke other `inline functions often become too complex to actually be inlined, despite the fact that they appear to be small. This problem is especially common with constructors and destructors.
A constructor always invokes the constructors of its base classes and member data before executing its own code.
Code that does not try to inline constructors and destructors, is more portable because it can be handled by compilers that get confused when generating complex nested inline` functions.
Virtual functions should not have default arguments
Virtual functions should not have default arguments
It’s best to avoid giving default argument initializers to virtual functions. While doing so is legal, the code is unlikely to be correctly maintained over time and will lead to incorrect polymorphic code and unnecessary complexity in a class hierarchy.
std::chrono components should be used to operate on time
std::chrono components should be used to operate on time
The `chrono library, introduced in C++20, provides support for calendars, time zones, and i/o formatting and parsing operations on time-related objects.
chrono is a better alternative to the C/POSIX functions that operate on time_t, tm, or timespec types. In comparison to C facilities, it provides better integration with other components of the C++ standard library (iostreams and format). Also, it supports compile-time computation and it is thread-safe.
This rule raises an issue C/POSIX functions that can be replaced with one of the std::chrono components:
querying for current time (time, timespec_get, clock_gettime)
date to time-point conversion (mktime, gmtime, localtime)
time serialization (ctime, asctime, strftime)
time parsing (strptime`)
Generic iterator-based algorithms should be constrained
Generic iterator-based algorithms should be constrained
Each template requires certain operations to be provided by the types it is instantiated with. Before C++20, the only way to describe those requirements was through documentation. Concepts, introduced in C++20, provide a way to express requirements in a way that can be checked by the compiler.
This improves the readability and maintainability of the code, most notably:
It makes it clear from the declaration what types are accepted and what operations they should support. This benefit is even higher when the concepts used to constrain the code are well-known, such as concepts defined in the standard library.
Errors from incorrect instantiations point at the call site (code produced by the programmer), and not at some obscure details in the middle of the implementation of an algorithm.
Should all template code be exhaustively constrained? Probably not, especially if that would lead to defining single-use concepts. But in the case of templates designed to work with standard-style iterators, there is no good reason not to use the standard library concepts describing them. Even adding a simple set of basic constraints, such as the required category of iterators, without covering all the operations needed for the algorithm, is already providing value.
This rule raises an issue for generic iterator-pair algorithms that are not constrained.
mktemp family of functions templates should have at least six trailing Xs
mktemp family of functions templates should have at least six trailing Xs
The standard C library provides several functions to create temporary files and directories. These functions include `mktemp, mkstemp, mkstemps and mkdtemp, and they accept as a parameter a template that defines the path where the file (or directory) should be created. The filename part of the template must contain at least six trailing “X”s to ensure the uniqueness of the filename, since these “X“‘s are replaced with a string that makes the filename unique.
Note that the template` parameter will be modified and therefore, it may not be a string constant, but should be declared as a character array, instead.
Move and swap operations should be noexcept
Move and swap operations should be noexcept
Move operations (move constructor, move assignment operator) are about efficiently transferring resource ownership. When transferring resources from the source, you don’t have to allocate any memory or perform any other operation that might fail. This is why most people will expect move operations to be non-throwing.
Additionally, if a move operation fails, the source object can have been partially altered by the move, making recovery very tricky or just impossible. Therefore, to ensure robustness, some functions (for instance, `std::move_if_noexcept, used by std::vector) will decide to copy your object if its move operations are not decorated with noexcept. This can significantly slow down your program.
If you can not implement your move operations so that they never throw, you may only provide copy operations that will be safer to use.
Swap operations are similar to move operations in that they should be equivalent to moving two objects into each other. So if you add a swap function to your type, it should be noexcept too.
Note that you should not write your move operations for most classes but rely on the “Rule-of-Zero” (S4963).
This rule raises an issue when a move or swap operation is not noexcept, which can happen in two cases:
The operation is user-defined and is not unconditionally declared as noexcept,
The operation is implicitly defined, and one of the class’s base classes or member variables does not have noexcept` move operations.
Array indices should be placed between brackets
Array indices should be placed between brackets
While C syntax considers array subscripts ([]) as symmetrical, meaning that a[i] and i[a]
are equivalent, the convention is to put the index in the brackets rather than the array name. Inverting the index and array name serves no purpose, and is very confusing.
Heterogeneous sorted containers should only be used with types that support heterogeneous comparison
Heterogeneous sorted containers should only be used with types that support heterogeneous comparison
Heterogeneous containers were introduced in C++14 to increase performance when working with `std::map, std::set, std::multimap, std::multiset, and since C++20, their unordered_ counterparts. Before their introduction, when working with a key in such a container, it was required to use an object of the exact key type. This could lead to costly object creations when we would like to work with objects of compatible but different types:
std::map<std::string, int> m; m.find("abc"); // Will convert the char const * to a std::string, maybe performing costly memory allocation
With heterogeneous containers, the previous code will not create a string, but directly compare keys in the map with the searched char const *. This is a behavior you can opt-in with providing a transparent comparison method for the map.
std::map<std::string, int, std::less<>> m; // std::less<> is transparent, std::less<anything> is not m.find("abc"); // Will directly compare the char const* with the keys, no object conversion
A transparent comparator is one that declares a nested type named is_transparent. It is supposed to be able to compare heterogeneous object types, for instance in this case compare a char const * with a string, without performing any conversion.
What happens if it cannot? Well, if the types are convertible with each other (which is usually the case when you want to work with a heterogeneous container), a conversion will happen, and a homogeneous comparison will be performed instead.
In this case, str will not be converted to a std::string when calling the function find (this function is now a member function template). However, each time during the search when a comparison will be needed between MyString and std::string, a conversion will now happen to convert str to always the same string. A unique conversion is replaced by O(ln N) conversions.
This problem appears also for the case of an unordered container for which heterogeneous lookup is enabled (available since C++20), for which equality is used to compare elements in the same bucket (having same or close hashes):
void g() \{ std::unordered_map<std::string, int, StringHash, std::equal_to<>> m; MyString str\{"abc"\}; m.find(str); \}
In this case str will not be converted to a std::string when calling the function find, and each element of the bucket that corresponds to the hash of str will be compared using homogeneous operator==, and for each such comparison, a conversion will now happen. The number of compared elements varies depending on the hash distribution from O(1) (on average) to O(N)` (in the worst case). As consequence, the performance of slow runs (when multiple hash collisions happen due to the data distribution) is made even worse.
This rule raises an issue when a transparent lookup function of a heterogeneous container is used with a type that cannot be directly compared with the container key type. Only standard associative containers with expensive to create key and straightforward comparison functions are considered.
The underlying bit representations of floating-point values should not be used
The underlying bit representations of floating-point values should not be used
The storage layout used for floating-point values may vary from one compiler to another, and therefore no floating-point comparisons or manipulations should be made which rely directly on the way the values are stored. The in-built operators and functions, which hide the storage details from the developer, should be used.
std::endl should not be used
std::endl should not be used
When injecting `std::endl into an output stream, two things happen:
An end of line character ‘\n’ is added to the stream
The stream is flushed
In many situations, you don’t need the stream to be flushed: It takes some time, and additionally, the stream is also flushed automatically in several circumstances:
When the stream is closed
In the case of std::cout, each time an input is read on std::cin or an output is written on std::cerr
In the case of std::cerr, each output is immediately written, the is no need to flush
Therefore, if your only goal is to add an end of line, ‘\n’ is usually more efficient than std::endl. If you do want to flush, you can be explicit and inject std::flush into the stream, or call the flush` member function on the stream.
Coroutine holding rare resources should not be suspended or terminated
Coroutine holding rare resources should not be suspended or terminated
Availability of rare resources, such as mutexes, often has a direct impact on the performance of an application. Holding onto rare resources for too long might lead to a significant slowdown or a deadlock.
`co_yield and co_await suspend the execution of a coroutine for an indefinite time, and co_return terminates it. At this point, all the rare resources acquired by the coroutine must be released to enable other potential consumers to use them while the coroutine is inactive.
This rule reports execution scenarios that reach co_yield, co_await, or co_return` without releasing a rare resource, such as a mutex.
When the Rule-of-Zero is not applicable, the Rule-of-Five should be followed
When the Rule-of-Zero is not applicable, the Rule-of-Five should be followed
In C++, you should not directly manipulate resources (a database transaction, a network connection, a mutex lock) but encapsulate them in RAII (Resource Acquisition Is Initialization) wrapper classes that will allow you to manipulate them safely. When defining one of those wrapper classes, you cannot rely on the compiler-generated special member functions to manage the class’ resources for you (see the Rule-of-Zero, S4963). You must define those functions yourself to make sure the class’ resources are properly copied, moved, and destroyed.
In that case, make sure you consider what should be done for all five special functions (or only the first three before C++11):
The destructor: to release the resource when the wrapper is destroyed
The copy constructor and the copy-assignment operator: to handle what should happen to the resource when the wrapper is copied (a valid option is to disable those operations with
=delete
)The move constructor and the move-assignment operator: to handle what should happen to the resource when the wrapper is moved (since C++11). If you cannot find a way to implement them more efficiently than the copy operations, as an exception to this rule, you can just leave out these operations: the compiler will not generate them and will use the copy operations as a fallback.
The operations mentioned above are interdependent. Letting the compiler generate some of these operations automatically, but not all of them, creates a situation where calling one of these functions may compromise the integrity of the resource. For example, it could result in a double-release of a resource when the wrapper is copied.
if,switch, and range-based for loop initializer should be used to reduce scope of variables
if,switch, and range-based for loop initializer should be used to reduce scope of variables
C++17 introduced a construct to create and initialize a variable within the condition of if and switch statements and C++20 added this construct to range-based for loops. Using this new feature simplifies common code patterns and helps in giving variables the right scope.
Previously, variables were either declared before the statement, hence leaked into the ambient scope, or an explicit scope was used to keep the scope tight, especially when using RAII objects. This was inconvenient as it would lead to error-prone patterns.
For example, this verbose error-prone initialization:
An objects dynamic type should not be used from its constructors or destructor
An objects dynamic type should not be used from its constructors or destructor
During construction and destruction of an object, its final type may be different from that of the completely constructed object. The result of using an object’s dynamic type in a constructor or destructor may not be consistent with developer expectations.
The dynamic type of an object is used in the following constructs:
`typeid on a class with a virtual function or a virtual function in a base class.
dynamic_cast`
A virtual call to a virtual function.
This rule also prohibits a call being made to a pure virtual function from within a constructor or destructor. Such calls lead to undefined behaviour.
Empty throws (throw;) should only be used in the compound statements of catch handlers
Empty throws (throw;) should only be used in the compound statements of catch handlers
Catch blocks define how to deal with exceptions. It is possible to partially handle an exception before passing it on to a higher level for complete handling with the empty throw statement throw;.
However, when an empty throw is called outside of a catch clause, and there is no exception object to re-throw, the program will call std::terminate. This will cause the program to end, which is unlikely to be the expected behavior.
Immediately dangling references and pointers should not be created
Immediately dangling references and pointers should not be created
You can create a temporary object as part of the evaluation of an expression.
For example:
Use std::format rather than std::vformat when the format string is known at compile time
Use std::format rather than std::vformat when the format string is known at compile time
std::format and std::vformat have the same runtime behavior for well-formed format strings. The difference is exposed when there is an error in the format string. For instance, if it contains an invalid format specifier:
std::vformat throws a std::format_error exception at runtime.
std::format makes these mistakes ill-formed, causing the compilation to fail.
std::format should be used whenever possible, allowing to catch malformed format strings early during the development process.
This rule raises an issue when std::vformat is used with a constant string known at compile time.
Macros should only expand to a braced initialiser, a constant, a parenthesised expression, a type qualifier, a storage class specifier, or a do-while-zero construct
Macros should only expand to a braced initialiser, a constant, a parenthesised expression, a type qualifier, a storage class specifier, or a do-while-zero construct
You could you use preprocessor directives to do some pretty wild things, up to and including redefining the syntax of the language. But doing so could result in unexpected behavior, and definitely would result in impossible-to-read code. Therefore the acceptable uses of macros are strictly limited.
Specifically, macros may only be used do define a:
braced initialzer
constant
parenthesized expression
type qualifier
storage class specifier
`do-while (0) construct
Macros should not be used to define statements or parts of statements except for the use of the do-while (0)` construct, or to redefine the syntax of the language. All brackets ( (), {}, and [] ) in a macro must be balanced
Objects should not be sliced
Objects should not be sliced
Slicing happens when an object from a derived type is cast to an object of one of its base classes. When this happens, the new object will not have the data member variables specific to the derived type.
The following example illustrates the unintended loss of information.
Emplacement should be preferred when insertion creates a temporary with sequence containers
Emplacement should be preferred when insertion creates a temporary with sequence containers
Sometimes, `emplace_back is more efficient and less verbose than push_back. It is expected to be faster when the object is constructed into the container instead of being constructed and assigned. This also happens when the pushed object has a different type from the one held by the container.
This rule supports standard sequence containers: std::vector, std::list, std::deque, std::forward_list, std::stack, std::queue and std::priority_queue`.
The rule raises an issue when an insertion function on a supported container leads to constructing a large temporary object that can be avoided using the provided emplacement member function.
Redundant nil checks should not be used
Redundant nil checks should not be used
Because messages sent to nil (or a pointer with a nil value) will return zero, it is often not necessary to explicitly nil
-check a pointer, and doing so simply adds unnecessary code.
Function exit paths should have appropriate return values
Function exit paths should have appropriate return values
When a function does not return an appropriate value, it causes the program to have an undefined behavior. For example, if a function is supposed to return a value indicating whether a file was successfully opened but does not return any value, the program may continue to execute as if the file was opened successfully, even though it was not. This can lead to data corruption or other issues that are difficult to diagnose.
Functions with a void return type are not expected to return any value. If they do, it may indicate a programming error.
Assembler instructions should be introduced using the asm declaration
Assembler instructions should be introduced using the asm declaration
The `asm declaration is available to all C++ implementations, allowing a consistent mechanism to be used.
However, the parameters to asm` are still implementation-defined.
Increment should not be used to set boolean variables to true
Increment should not be used to set boolean variables to true
It is possible to use the increment operator ++, to set the value of a bool(C++) or _Bool(C) variable to true
. But this feature has been deprecated in C++ since the 1998 version of the standard, removed in C++17, and even where allowed, is simply confusing.
sprintf should not be used
sprintf should not be used
+“ , it’s up to the developer to make sure the size of the buffer to be written to is large enough to avoid buffer overflows. Buffer overflows can cause the program to crash at a minimum. At worst, a carefully crafted overflow can cause malicious code to be executed.
Floating-point implementations should comply with a defined floating-point standard
Floating-point implementations should comply with a defined floating-point standard
Floating-point arithmetic has a range of problems associated with it. Some of these can be overcome by using an implementation that conforms to a recognized standard. An example of an appropriate standard is ANSI/IEEE Std 754.
The definition of the floating-point types, in accordance with Rule 3-9-2, provides an opportunity for noting the floating-point standard in use.
Appropriate memory de-allocation should be used
Appropriate memory de-allocation should be used
The same form that was used to create an object should always be used to delete it. Specifically, deallocation should correspond to allocation as per the table below.
Allocation | Deallocation |
---|---|
p = new T(); | delete p; |
p = new T[5]; | delete[] p; |
|
Multiple variables should not be declared on the same line
Multiple variables should not be declared on the same line
Declaring multiple variables or members on the same line hinders readability. Moreover, as soon as they contain references, pointers, or assignments, they become confusing for maintainers.
This rule raises an issue when a declaration declares multiple variables or members.
Expressions with underlying enum types should only have values corresponding to the enumerators of the enumeration
Expressions with underlying enum types should only have values corresponding to the enumerators of the enumeration
It is unspecified behaviour if the evaluation of an expression with `enum underlying type yields a value which does not correspond to one of the enumerators of the enumeration.
Additionally, other rules in this standard assume that objects of enum` type only contain values corresponding to the enumerators. This rule ensures the validity of these assumptions.
One way of ensuring compliance when converting to an enumeration is to use a switch statement.
Context-sensitive keywords should not be used as identifiers
Context-sensitive keywords should not be used as identifiers
The C++ standards define some identifiers as having special meaning in specific contexts. These are:
`final and override since C++11
module and import` since C++20
While it is technically possible to use them as normal identifiers, it’s clearer for the reader of the code to consider them as if they were keywords and only use them with their special meaning.
<stdio.h> should not be used in production code
<stdio.h> should not be used in production code
This includes file and I/O functions `fgetpos, fopen, ftell, gets, perror, remove, rename and ungetc.
Streams and file I/O have a large number of unspecified, undefined and implementation-defined behaviors associated with them. It is assumed within MISRA C that they will not normally be needed in production code in embedded systems.
If any of the features of stdio.h` need to be used in production code, then the issues associated with the features need to be understood.
Well-defined type-punning method should be used instead of a union-based one
Well-defined type-punning method should be used instead of a union-based one
In some performance-oriented algorithms, a solution to certain slow operations is reinterpreting a value as a different type of the same length while preserving its binary representation.
One of the superseded solutions, known as “union type-punning”, is to use a union with two members with types corresponding to the source and the target types of the cast. The operation is performed by saving the value in the member of the source type and then reading the value from the member of the target type. Despite being allowed in C, this operation has undefined behavior according to C++ standard and should be replaced by std::memcpy (or std::bit_cast in C++20).
Note: std::memcpy has no performance impact on modern compilers when used in type-punning and is optimized during compilation.
Sometimes union type-punning is used to remove const. This can create readability issues and should be replaced with const_cast.
This rule raises an issue on any use of a union that should be replaced with std::memcpy, std::bit_cast, or const_cast.
Dynamic memory should not be allocated needlessly
Dynamic memory should not be allocated needlessly
Dynamic memory allocation & deallocation (e.g. malloc / free
) is somewhat expensive. This is particularly true when it happens in a loop. It is good practice to allocate and deallocate memory only when it is needed.
This rule raises an issue when one, or more, execution path results in unused allocated memory.
Lambdas that capture this should capture everything explicitly
Lambdas that capture this should capture everything explicitly
A lambda can only capture local variables. When a lambda is defined within a member function, you may believe that you are capturing a member variable of the current class, but in fact, what you are capturing is `this. This may be very surprising, and lead to bugs if the lambda is then used after the current object has been destroyed.
Therefore, it’s better to be explicit about exactly what is captured as soon as this is captured.
If the lambda is used immediately (for instance, called or passed as an argument to std::sort), there is no such risk and no issue is raised.
In C++20, capturing this via [=] has been deprecated. An issue is raised in that case, even if the lambda is used immediately.
Note: This rule does not apply if the capture list of the lambda contains *this (possible since C++17). In that situation, what is captured is not the pointer this, but a local copy of the object pointed-to by this and any reference to this` (explicit or implicit) in the lambda body then refers to this local copy (see S6016).
String literals with different prefixes should not be concatenated
String literals with different prefixes should not be concatenated
Concatenation of wide and narrow string literals has not always been supported in C or C++, and even when supported, the meaning may be unclear to the reader. Concatenation of string literals with different encodings is only conditionally supported, and may be removed in a future version of the language.
Therefore, only string literals with the same prefix should be concatenated together.
The addresses of standard library functions should not be taken
The addresses of standard library functions should not be taken
Taking the address of a library function is not something robust: The library might make changes to a function that are compatible with a normal use of a function, but not with taking its address (for instance, adding a parameter with a default value, or adding an overload to an overload set). More specifically, the standard library has stated that there would be no barrier against such changes, and that for stability users should not take the address of standard library functions.
Starting with C++20, it’s no longer allowed to take the address of a standard library function (with some exceptions with functions for formatting streams).
Non-volatile variables should not be assigned values that are never subsequently used
Non-volatile variables should not be assigned values that are never subsequently used
Technically known as a DU dataflow anomaly, this is a process whereby a variable is given a value that is subsequently never used. At best this is inefficient, but may indicate a genuine problem. Often the presence of these constructs is due to the wrong choice of statement aggregates such as loops.
Objects or functions with external linkage shall be declared in a header file
Objects or functions with external linkage shall be declared in a header file
Placing the declarations of objects and functions with external linkage in a header file documents that they are intended to be accessible from other translation units.
If external linkage is not required, then the object or function shall either be declared in an unnamed namespace or declared static
.
This will reduce the visibility of objects and functions, which is considered to be good practice.
Only escape sequences defined in the ISO C standard should be used
Only escape sequences defined in the ISO C standard should be used
The use of an undefined escape sequence leads to undefined behavior.
Escape sequences supported in every C and C++ standard are: \n, \t, \v, \b, \r, \f, \a, \, ?, ’, ”, <Octal Number>, and \x<Hexadecimal Number>.
C99 and C++11 introduced escape sequences for Unicode characters in the form: \u<4 Hexadecimal Digits> and \U<8 Hexidecimal Digits>.
C++23 supports named escape sequences for Unicode characters in the form \N{<Name of Character>} and a delimited version of escape sequences is supported: \o{<Octal Number>}, \x{<Hexadecimal Number>}, and \u{<Hexadecimal Number>}.
empty() or is_empty() should be used to test for emptiness
empty() or is_empty() should be used to test for emptiness
When you call empty() or is_empty(), it clearly communicates the code’s intention, which is to check if the collection is empty. Using `size()
Reserved identifiers should not be defined or declared
Reserved identifiers should not be defined or declared
Defining or declaring identifiers with reserved names may lead to undefined behavior. Therefore, reserved names should not be used as identifiers.
This rule applies to:
defined
identifiers that contain two consecutive underscores
identifiers that begin with an underscore, followed by an uppercase letter
identifiers in the global namespace that start with an underscore
Empty optionals should not be accessed
Empty optionals should not be accessed
A `std::optional<T> can contain a value of type T, or can be empty. So in order to avoid crashes or misbehaviors of code, it is better to check the presence of a value in an optional before accessing it. This can be done:
By using the has_value member function
By converting the optional to a boolean
Alternatively, the member function value_or` can be used to return the contained value or a specific value if the optional is empty.
Using strlen or wcslen is security-sensitive
Using strlen or wcslen is security-sensitive
strlen(const char *s)++
measures the length of the string s (excluding the final null character). +
The function ++size_t wcslen(const wchar_t *s)++ does the same for wide characters, and should be used with the same guidelines.`Similarly to many other functions in the standard C libraries, strlen and wcslen` assume that their argument is not a null pointer.
Additionally, they expect the strings to be null-terminated. For example, the 5-letter string “abcde” must be stored in memory as “abcde\0” (i.e. using 6 characters) to be processed correctly. When a string is missing the null character at the end, these functions will iterate past the end of the buffer, which is undefined behavior.
Therefore, string parameters must end with a proper null character. The absence of this particular character can lead to security vulnerabilities that allow, for example, access to sensitive data or the execution of arbitrary code.
Generic exceptions should not be caught
Generic exceptions should not be caught
Some exception classes are designed to be used only as base classes to more specific exceptions, for instance, std::exception (the base class of all standard C++ exceptions), std::logic_error or std::runtime_error
.
Catching such generic exception types is usually a bad idea because it implies that the “catch” block is clever enough to handle any type of exception.
#else, #elif and #endif directives should reside in the same files as the #if or #ifdef directives to which they correspond
#else, #elif and #endif directives should reside in the same files as the #if or #ifdef directives to which they correspond
When the inclusion and exclusion of blocks of statements is controlled by a series of preprocessor directives, confusion can arise if all of the relevant directives do not occur within one file. This rule requires that all preprocessor directives in a sequence of the form #if / #ifdef … #elif … #else … #endif
shall reside in the same file. Observance of this rule preserves good code structure and avoids maintenance problems.
Notice that this does not preclude the possibility that such directives may exist within included files provided that all directives that relate to the same sequence are located in one file.
nodiscard attributes on functions should include explanations
nodiscard attributes on functions should include explanations
The `nodiscard attribute can be used with or without explanations, but explaining why a result should not be discarded can only improve one’s understanding of the code, and would prevent developers from wasting time figuring those things out by themselves.
This rule raises an issue when nodiscard` is used on function without any explanation.
Exceptions should not be thrown in noexcept functions
Exceptions should not be thrown in noexcept functions
`noexcept is a specifier that can be applied to a function declaration to state whether or not this function might throw an exception.
This specifier is a crucial information for the compiler as it enables it to perform automatic optimizations. It is also used by the noexcept operator, so that a developer can know whether an expression can throw, and adapt the code accordingly (for instance, to decide to move or copy an object).
When a function is specified noexcept, the compiler does not generate any code to throw exceptions and any uncaught exception will result in a call to std::terminate. This means that writing a noexcept function is an implicit agreement to the statement : “my program will terminate if any exception is thrown inside this function”.
It is a very strong commitment as there are so many ways to get an exception including any dynamic allocation.
This rule raises an issue when an exception is thrown, directly or indirectly, from a function declared noexcept`.
Assigning to an optional should directly target the optional
Assigning to an optional should directly target the optional
The class std::optional<T> either stores a value of type T or is empty.
One way to access the value of a non-empty optional is the `operator*. But using the dereference operator gives the optional appearance of a pointer when it is not: it models an object. Additionally, attempting to call the operator* on an empty optional will result in undefined behavior.
Another way to access the value of a non-empty optional is the function value(). But assigning a value to the optional object through this function will throw an exception (std::bad_optional_access) if the optional has no value, and the assignment will not happen.
For the assignment of an optional to happen correctly, whatever its state, it is better to:
assign the value directly with the operator=: e.g. myOptionalInteger = 3;
use the emplace` function (for example, when the move or copy operation is expensive or forbidden).
Logical operators should not be confused with bitwise operators
Logical operators should not be confused with bitwise operators
While working with bitwise operators & or |, it is easy to make a typo and write the equivalent logical operators && or ||. This rule raises an issue when the right operand of a logical expression && or || is a constant of integral type, as the developer probably meant to use the corresponding bitwise operator & or |
.
as_const should be used to make a value constant
as_const should be used to make a value constant
C++17 introduced `as_const: a helper function that converts a value to its corresponding const value succinctly and more explicitly.
This is usually done to force an overloaded function call on a non-const object to resolve to the const alternative. Or to instantiate a template with a const type rather than the original non-const one.
This rule detects casts that can be replaced by as_const`.
For each function parameter the type given in the declaration and definition shall be identical, and the return types shall also be identical
For each function parameter the type given in the declaration and definition shall be identical, and the return types shall also be identical
The types of the parameters and return values in the prototype and the definition must match. This requires identical types including typedef names and qualifiers, and not just identical base types.
switch case clauses should not have too many lines of code
switch case clauses should not have too many lines of code
The switch statement should be used only to clearly define some new branches in the control flow. As soon as a case clause contains too many statements this highly decreases the readability of the overall control flow statement. In such case, the content of case
clause should be extracted in a dedicated function.
Coroutines should not take const references as parameters
Coroutines should not take const references as parameters
Coroutines, introduced in C++20, are functions in which execution can be suspended and resumed. When a coroutine resumes, it takes over where it left thanks to the coroutine state.
A coroutine state is an object which contains all the information a coroutine needs to resume its execution correctly: local variables, copy of the parameters…
This means that if a coroutine has a parameter that is a reference to an object, this object must exist as long as the coroutine is not destroyed. Otherwise, the reference stored in the coroutine state will become a dangling reference and will lead to undefined behavior when the coroutine resumes.
The issue is raised for all coroutine parameters with reference-to-const semantics (such as a const reference, a std::string_view, or a std::span with const elements) that might be used after the coroutine is suspended.
To fix the issue, you can either pass the parameter by value, or not use the parameter after the first suspension point (co_await, co_yield, or initial_suspend).
Projects should not contain non-volatile POD variables having only one use
Projects should not contain non-volatile POD variables having only one use
With the exception of volatile variables, variables declared and used only once do not contribute to program computations. A use is either an assignment (explicit initialization) or a reference.
These variables are essentially noise but their presence may indicate that the wrong variable has been used elsewhere. Missing statements contribute to this problem.
The sign of an unsigned variable should not be tested
The sign of an unsigned variable should not be tested
Because the value in a variable of an unsigned type can never be less than zero, testing to see if it is negative is a useless operation which can only confuse future readers of the code.
Global variables should not be modified
Global variables should not be modified
Global variables provide a convenient way of accessing the same value or set of values from multiple places in the code without the need to pass them through the interface and possibly the entire call hierarchy. As a consequence, they can be used to avoid repetition and guarantee that the same value is used in multiple places.
However, modification of such a global variable may potentially change the behavior of any program component which makes it difficult to reason locally about the program’s correctness. Additionally, the modification of the same global variable in a multi-threaded program may lead to data races.
This rule raises an issue on the modification of publicly accessible non-atomic global variables.
Objects only accessed from within a single function should be defined at block scope
Objects only accessed from within a single function should be defined at block scope
The scope of objects shall be restricted to functions where possible. File scope shall only be used where objects need to have either internal or external linkage. Where objects are declared at file scope MISRA C 2004 Rule 8.10 applies. It is considered good practice to avoid making identifiers global except where necessary.
Whether objects are declared at the outermost or innermost block is largely a matter of style.
Function pointers should not be converted to any other type
Function pointers should not be converted to any other type
Conversion of a function pointer to a different type of pointer results in undefined behaviour. This means, for example, that a pointer to a function cannot be converted to a pointer to a different type of function.
Resources should be closed
Resources should be closed
The standard C library provides fopen and the system call open to open and possibly create files.
Each call to one of these functions must be matched with a respective call to fclose or close
.
Failing to close files that have been opened may lead to using up all of the OS’s file handles.
Standard C++ headers should be used
Standard C++ headers should be used
The `iostream.h header was provided with the first C++ compiler, CFront, and became the de facto standard. During the formal standardization process of C++, many shortcomings in iostream.h were fixed, but at the cost of introducing incompatibilities. Therefore, it was decided not to change the existing iostream.h and introduce the standard version as a new iostream header.
Modern compilers tend to remove the support of the legacy iostream.h header, and migrating to the standard version is encouraged.
This rule applies not only to iostream`, but to all standard C++ headers.
Lines starting with # should contain valid preprocessing directives
Lines starting with # should contain valid preprocessing directives
Preprocessing directives (lines that start with #
) can be used to conditionally include or exclude code from compilation. Malformed preprocessing directives could lead to the exclusion or inclusion of more code than was intended. Therefore all preprocessing directives should be syntactically meaningful.
using namespace directives should not be used in header files
using namespace directives should not be used in header files
A using directive makes names from another namespace available in the current scope. It should only be used when those names do not create an ambiguity with other names, otherwise, it is better to fully qualify the names you want to use.
When you write a header file, you don’t know from which context it will be included. Therefore, if this header contains using directives, you cannot be sure that they will not create ambiguities in that context. Those ambiguities could lead to compilation failures or, worse, to a different function being selected by overload resolution depending on the order of inclusion of headers.
A using declaration behaves in the same way but only for one name. Because of their much narrower scope, this rule does not apply to using declarations.
Exception classes should be caught by reference
Exception classes should be caught by reference
Catching an exception class by value rather than by reference can cause several problems:
Memory is allocated unnecessarily: catching by value creates a copy of the exception object, which is destroyed at the exit of the catch block.
Slicing occurs: the copy will be an instance of the exception base class rather than the potentially more specific exception class initially caught. So it may lead to a loss of precision as any additional data or functionality offered by the subclass will not be accessible.
Copying the exception class may throw an exception, leading to unexpected behavior.
Redundant pointer operator sequences should be removed
Redundant pointer operator sequences should be removed
By contract, chaining the ‘Address of’ operator & with the ‘Indirection’ operator *
results in a return to the initial value. Thus, such combinations are confusing at best, and bugs at worst.
Flexible array members should not be declared
Flexible array members should not be declared
Flexible array members are most likely to be used in conjunction with dynamic memory allocation.
The presence of flexible array members modifies the behaviour of the sizeof
operator in ways that might not be expected by a programmer. The assignment of a structure that contains a flexible array member to another structure of the same type may not behave in the expected manner as it copies only those elements up to but not including the start of the flexible array member.
sizeof should not be multiplied by another sizeof
sizeof should not be multiplied by another sizeof
Multiplying sizeof() with sizeof()
indicates a logic error.
There should not be unreachable code
There should not be unreachable code
Code is unreachable if there is no syntactic (control flow) path to it. If such code exists, it is unclear if this is intentional or simply that an appropriate path has been accidentally omitted.
Compilers may choose not to generate code for these constructs, meaning that, even if the unreachable code is intentional, it may not be present in the final executable code.
Missing statements, often caused by editing activities, are a common source of unreachable code.
The unbounded <cstring> functions should not be used
The unbounded <cstring> functions should not be used
Used correctly, the functions from the `<cstring> library are safe and reliable. Unfortunately, they are easily misused, and can read or write beyond the end of defined buffer, resulting in undefined behavior.
Therefore this rule flags all uses of the following methods, which should be avoided, in favor of functions from C++‘s new “safe string” library, <string>: strcpy, strcmp, strcat, strchr, strspn, strcspn, strpbrk, strrchr, strstr, strtok, and strlen`.
Forwarding references parameters should be used only to forward parameters
Forwarding references parameters should be used only to forward parameters
Forwarding references are a special kind of references that both ignore and preserve the value category of a function argument, making it possible to forward it by using std::forward or std::forward_like
.
Any code using such a reference for any purpose other than forwarding is actually ignoring the rvalue-ness and const-ness of the associated parameter.
Reference types should not be qualified with const or volatile
Reference types should not be qualified with const or volatile
The C++ specification forbids the qualification of reference types with `const or volatile unless it happens via a typedef, in which case it’s ignored. Most compilers treat such direct qualifications as errors, but the Microsoft compiler allows them.
This rule raises an issue on both types of const` qualification.
The order for arguments of the same type in a function call should be obvious
The order for arguments of the same type in a function call should be obvious
When a function has several consecutive parameters of the same type, there is a risk that the arguments are not provided in the right order. Moreover, it is generally the sign of code which is too low-level. Maybe
the arguments should have a stronger type
some arguments could be grouped together to form a higher level abstraction.
The use of two parameters of the same type is useful in situations like comparing arguments, combining arguments through a binary operation and swapping arguments but three or more arguments of the same type is considered bad practice.
This rule raises an issue when a function is defined with more than two consecutive parameters of the same type. For this rule, only the “raw” type of the parameter will be considered (a string const & will be considered the same type as a std::string
).
<time.h> should not be used
<time.h> should not be used
Includes time, strftime. This library is associated with clock times. Various aspects are implementation dependent or unspecified, such as the formats of times. If any of the facilities of time.h
are used, then the exact implementation for the compiler being used must be determined, and a deviation raised.
Standard namespaces should not be modified
Standard namespaces should not be modified
It may seem tidy to add your new declarations to the std or posix namespaces, but doing so results in undefined behavior. The C++14 Standard, [namespace.std] (ISO/IEC 14882-2014 §17.6.4.2.1), paragraphs 1 and 2 states:
In addition to restricting extensions to the std namespace, the C++14 Standard goes on in §17.6.4.2.2 to say:
However, the standard allows specializing standard class templates in namespace std. In that case, the specialization must respect the requirement of the original template and has to be for a “program-defined type” (a type that is specific to the program, by opposition to a type from the standard).
You may therefore think that it’s legitimate to reopen std to define a version of extension points (`std::swap, std::hash…) that work with your types, but it’s not necessary: If you call these extension points according to the correct pattern, the user-defined version will be found too.
The only extension points for which the specialization is the recommended approach are std::out_ptr and std::inout_ptr.
This rule raises an issue for any modification of the standard std and posix` namespaces that is not a template specialization.
strerror should not be used
strerror should not be used
The function `strerror returns a buffer that is only valid until the next call to strerror. In a multithreaded environment, you don’t know when this next call will happen, which makes this function dangerous to call. You should use thread-safe alternatives, such as strerror_s or strerror_r.
Note that strerror_s is defined in annex K of C11, so to have access to it, you need a standard library that supports it (this can be tested with the macro STDC_LIB_EXT1), and you need to enable it by defining the macro STDC_WANT_LIB_EXT1 before including <string.h>`.
All constructors of a class should explicitly call a constructor for all of its immediate base classes and all virtual base classes
All constructors of a class should explicitly call a constructor for all of its immediate base classes and all virtual base classes
This rule reduces confusion over which constructor will be used, and with what parameters.
Multiple mutexes should not be acquired with individual locks
Multiple mutexes should not be acquired with individual locks
Mutexes are synchronization primitives that allow you to manage concurrency. It is a common situation to have to lock more than one mutex simultaneously to get access to several resources at the same time.
If this is not done properly, it can lead to deadlocks or crashes. If one thread acquires A and then tries to acquire B, while another thread acquires B and then tries to acquire A, both threads will wait for each other forever.
In such a case, a commonly accepted good practice is to define an order on the mutexes then lock them in that order, and then unlock them in the reverse order. However, such an order is not always clearly defined or easy to ensure across a whole program.
C++ provides facilities to lock multiple mutexes in one go, with a dedicated deadlock prevention algorithm. They should be used instead. Before C++17, you should use std::lock, and since C++17 you can use a variadic constructor of std::scoped_lock
. See the examples for more details.
std::format should not be missing indexes
std::format should not be missing indexes
std::format takes as an argument a format string that contains replacement fields (surrounded with {}) and a set of extra arguments that will be formatted inside the replacement fields. Even if the format string is checked at compile-time, it is possible to have a mismatch between the format string and the arguments. For example:
The format string contains fewer replacement fields than the number of extra arguments: std::format(”{} {}”, 1, 2, 3);
The format string uses indexes for the replacement fields, but one index is missing: std::format(“{0} {0} {2}”, 1, 2, 3);
In these cases, the extra arguments are silently ignored. In the best-case scenario, it leads to dead code. Otherwise, it is a typo, and the output will not be intended.
Enums should be consistent with the bit fields they initialize
Enums should be consistent with the bit fields they initialize
Bit fields can only have integral or enumeration type. If it is quite straightforward to check if an integral type can initialize a bit field, it is however trickier with an enum type: the bit field has to be wide enough to store all the possible values of the enum.
In addition to this, the signedness of the enum should be consistent with the signedness of the bit field:
an unsigned bit field can not be initialized with a signed enum type
a signed bit field uses one bit to store the sign and this needs to be taken into account while comparing the size of the enum type with the size of the bit field.
Include guard macros should be unique
Include guard macros should be unique
Using the same macro name in the include guards of two different files means that the contents of one of the files will not be included in the compilation unit. The pernicious bugs caused by this accidental exclusion can be particularly difficult to track down.
Declarations of functions defined outside of the class should not be marked as inline
Declarations of functions defined outside of the class should not be marked as inline
It is a best practice in the public part of a class body, to describe only information relevant for reusers of this class, without implementation details like `inline specifier.
For inline member function defined outside of the class body, this rule verifies that inline` is set on the definition and not on the declaration of the function.
[[nodiscard]] attributes on types should include explanations
[[nodiscard]] attributes on types should include explanations
The `[[nodiscard]] attribute can be placed on type declarations to indicate that any value of such type should not be discarded when returned from a function. Accompanying the attribute with the message, explaining why values should not be ignored, contributes to a better understanding of code. This is especially important in the case of types, as the reason for which values of the type should not be discarded is a defining property of that type (information about failure, handle owning a scarce resource).
Moreover, marking a type as nodiscard, causes a warning to be reported for invocation of every function that returns this type. As a consequence, the cause of the warning is not directly visible from the declaration of the function and requires further investigation from the user.
This rule raises an issue when [[nodiscard]]` is used on a type without any explanation.
override or final should be used instead of virtual
override or final should be used instead of virtual
In a base class, virtual indicates that a function can be overridden. In a derived class, it indicates an override. But given the specifier’s dual meaning, it would be clearer and more sound to use derived class-specific specifiers instead: override or final
.
Track instances of the #error preprocessor directive being reached
Track instances of the #error preprocessor directive being reached
This rule creates a issue whenever an #error
preprocessor directive is reached during the project’s analysis. In most cases, this indicates that the preprocessor was badly configured. Some predefined macros or library include paths might be required to fix the configuration.
std::move shouldnt be called on an rvalue
std::move shouldnt be called on an rvalue
std::move is just a static_cast to an rvalue reference. It produces an rvalue of xvalue category. Calling std::move
on an rvalue is redundant and might lead to preventing copy elision RSPEC-5274
Semantic equivalence between binary operators and their assignment operator forms should be preserved
Semantic equivalence between binary operators and their assignment operator forms should be preserved
Where a set of operators is overloaded, it is important that the interactions between the operators meet developer expectations.
void * should not be used in typedefs, member variables, function parameters or return type
void * should not be used in typedefs, member variables, function parameters or return type
`void* is a pointer to memory of unknown type, and therefore works outside of the safety net provided by the type system. While it can be useful in a function body to interface with external code, there is no good reason to step out of the robust C++ type system when defining a function, either for the function parameters, or for the function return type. For the same reasons, having a member variable of type void* is not recommended.
If you want to work with raw memory buffer, use unsigned char * (or byte * if your compiler supports it).
If you want to work with different types of data, define a function template and use typed pointers, instead of void . If you want a single object to be able to stores objects of different types, std::any can also be a type-safe alternative to void.
If you want to provide to users of an API an opaque type, declare a type and don’t provide its definition (like with FILE*).
Note that void*` is commonly used to communicate data of unknown type with C code. This rule will nevertheless raise an issue in this case, but it can be ignored.
void functions should have external side effect(s)
void functions should have external side effect(s)
A function which does not return a value and which does not have external side effects will only consume time and will not contribute to the generation of any outputs, which may not meet developer expectations.
The following are examples of external side effects:
Reading or writing to a file, stream, etc.;
Changing the value of a non local variable;
Changing the value of an argument having reference type;
Using a volatile object;
Raising an exception.
#pragma pack should be used correctly
#pragma pack should be used correctly
`#pragma pack is a non-standard extension that specifies the packing alignment for structure, union, and class members.
It is useful to
remove padding and decrease the size of objects
align members to better fit optimal cpu alignment
However, the pragma pack directives need to be correctly defined to work properly.
This rule raises an issue if:
the specified packing value is incorrect: it can only be 1, 2, 4, 8, or 16
a parameter is ill-formed
the pop variant of this #pragma is called with both arguments identifier and value: such a call is undefined behavior
a #pragma pack(push…) is performed but there is not corresponding use of #pragma pack(pop…)
a #pragma pack(pop…) is performed but there is not corresponding use of #pragma pack(push…)
a #pragma pack` is in effect across several files: this becomes too complex and could easily lead to undefined behavior, the same structure having a different layout when seen from different translation units
Macros should not be redefined
Macros should not be redefined
A macro definition should not be redefined without marking that intent specifically by un-defining it first.
Functions should not return references or pointers to parameters that are passed by reference or const reference
Functions should not return references or pointers to parameters that are passed by reference or const reference
It is implementation-defined behaviour whether the reference parameter is a temporary object or a reference to the parameter. If the implementation uses a local copy (temporary object), this will be destroyed when the function returns. Any attempt to use such an object after its destruction will lead to undefined behaviour.
Function names should be used either as a call with a parameter list or with the & operator
Function names should be used either as a call with a parameter list or with the & operator
Using a “bald” function name is likely a bug. Rather than testing the return value of a function with a void parameter list, it implicitly retrieves the address of that function in memory. If that’s truly what’s intended, then it should be made explicit with the use of the &
(address-of) operator. If it’s not, then a parameter list (even an empty one) should be added after the function name.
Identical strings should not be compared
Identical strings should not be compared
Comparing two identical strings will always yield the same result and doesn’t achieve anything. This is likely to be made in error.
This rule raises an issue when strcmp or strncmp
is called with two identical literal strings or twice the same variable.
Pointer arithmetic should not be carried on with the result of a static_cast
Pointer arithmetic should not be carried on with the result of a static_cast
Pointer arithmetic is a way of calculating the address of objects in memory, especially in arrays.
It features operators
{plus}{plus}+, ++---++, ++—=++, {plus}{plus}= and subscript operator ++[]++.`Pointer arithmetic relies on the type of the pointer to calculate the actual address in memory.
Using the wrong type to do pointer arithmetic leads to wrong result and can corrupt memory.
++static_cast++
can be used to change the type of a pointer. As a result, doing arithmetic on its return value would result in wrong arithmetic.`[[likely]] and [[unlikely]] should be used instead of compiler built-ins
[[likely]] and [[unlikely]] should be used instead of compiler built-ins
C++20 introduces two standard attributes to indicate the likelihood of a branch: `[[likely]] and [[unlikely]].
These attributes replace the non-standard built-in __builtin_expect supported by Clang and GCC that was mostly used as part of likely() and unlikely() macros.
The standard annotations should always be preferred because they make the code portable and future-proof.
This rule reports the direct use of __builtin_expect built-in and its indirect use through likely() and unlikely()` macros.
Pointer arithmetic should not be used
Pointer arithmetic should not be used
Outside of the context of an array, explicitly calculating a pointer value will almost never yield the intended result. Even variables declared in the same statement should not be assumed to inhabit sequential memory addresses.
Using an explicitly calculated pointer will have unexpected runtime results as you either read or modify the wrong memory addresses.
Within limits, array indexes are an acceptable form of pointer math when the pointer in question is an array pointer and the array does not hold polymorphic objects/structs.
Relational operators should not be used with pointer types except where they point to the same array
Relational operators should not be used with pointer types except where they point to the same array
Attempting to make comparisons between pointers will produce undefined behaviour if the two pointers do not point to the same object.
Note: it is permissible to address the next element beyond the end of an array, but accessing this element is not allowed.
Member variables should be initialized
Member variables should be initialized
In the following example, PartInit::x is left uninitialized after the constructor finishes.
typedef names should be unique identifiers
typedef names should be unique identifiers
Reusing a `typedef name either as another typedef name or for any other purpose may lead to developer confusion.
The same typedef` shall not be duplicated anywhere in the project, even if the declarations are identical.
Note that where the type definition is made in a header file, and that header file is included in multiple source files, this rule is not violated.
Dynamic heap memory allocation should not be used
Dynamic heap memory allocation should not be used
The use of dynamic memory can lead to out-of-storage run-time failures, which are undesirable.
The built-in `new and delete operators, other than the placement versions, use dynamic heap memory. The functions calloc, malloc, realloc and free also use dynamic heap memory.
There is a range of unspecified, undefined and implementation-defined behaviour associated with dynamic memory allocation, as well as a number of other potential pitfalls. Dynamic heap memory allocation may lead to memory leaks, data inconsistency, memory exhaustion, non-deterministic behaviour, etc.
Note that some implementations may use dynamic heap memory allocation to implement other functions (for example, functions in the library cstring`). If this is the case, then these functions shall also be avoided.
Inheritance should be public
Inheritance should be public
While it is possible for inheritance to be non-public, it is rarely justified and complicates the use of the derived class. For instance, inherited member visibility is diminished and implicit and static_cast
casts from the derived class to the base class will not work.
It is sometimes used to limit the base class functionality available in the derived class. When that is the desire, composition should be used instead.
Structure and union types should be complete at the end of a translation unit
Structure and union types should be complete at the end of a translation unit
A complete declaration of the structure or union shall be included within any translation unit that refers to that structure. See section 6.1.2.5 of ISO 9899:1990 [2] for a full description of incomplete types.
A switch statement shall be a well-formed switch statement
A switch statement shall be a well-formed switch statement
A well-formed switch statement conforms to the following syntax rules, which are additional to the C++ standard syntax rules. All syntax rules not defined below are as defined in ISO/IEC 14882:2003.
The value of a complex expression should only be cast to a type that is narrower and of the same signedness as the underlying type of the expression
The value of a complex expression should only be cast to a type that is narrower and of the same signedness as the underlying type of the expression
If a cast is to be used on any complex expression, the type of cast that may be applied is severely restricted. As explained in MISRA C 2004, section 6.10, conversions on complex expressions are often a source of confusion and it is therefore wise to be cautious. In order to comply with these rules, it may be necessary to use a temporary variable and introduce an extra statement.
Keywords should be used before arguments
Keywords should be used before arguments
It may seem cleaner to omit keywords from your method declarations, but this is one time you should err on the side of verbosity. Omitting keywords in a declaration necessarily means that they’ll be omitted from calls too. What results is code that will be impenetrable to maintainers. That’s why it’s considered best practice to always use keywords. This applies both to Objective-C-style parameters without keywords, and to C-style parameter declarations, which are deprecated.
No identifier name should be reused
No identifier name should be reused
Regardless of scope, no identifier should be re-used across any files in the system.
<csignal> should not be used
<csignal> should not be used
Signal handling contains implementation-defined and undefined behaviour.
Try-catch blocks should not be nested
Try-catch blocks should not be nested
Nesting try/catch or @try/@catch
blocks severely impacts the readability of source code because it makes it too difficult to understand which block will catch which exception.
This C++ example also applies to Objective-C.
Concatenated std::format outputs should be replaced by a single invocation
Concatenated std::format outputs should be replaced by a single invocation
std::format accepts a format string composed of ordinary text and replacement fields (surrounded with {}) that are replaced with a textual representation of the remaining std::format arguments. This allows generating a complex string with a single invocation of std::format.
Since calls to std::format produce string objects, it is possible to concatenate them with other string objects or string literals. However, compared to a single std::format invocation with an adjusted format string, this concatenation is inefficient and less readable.
This rule raises an issue when the concatenation performed on the result of std::format can be replaced with a single std::format invocation.
auto should not be used as a storage class specifier
auto should not be used as a storage class specifier
Before C++11, `auto was used as a storage class specifier that indicated automatic duration. Since that’s the default, the use of auto in that context was wholly redundant.
Because the keyword was redundant and therefore rarely used, C++11 repurposes it. auto is now used to specify that the type of the variable or function should be deduced from its context.
Since it is redundant under older standards and problematic under C++11, auto’s use as a storage-class identifier should be removed.
final classes should not have virtual functions
final classes should not have virtual functions
Since final classes can’t be extended, indicating that functions in such a class are overrideable by adding the virtual
specifier is possibly misguided, and definitely confusing.
Variables must be initialized before being used
Variables must be initialized before being used
Copy/Paste from Wikipedia. [~ann.campbell.2] could you adjust ?
auto should be used to avoid repetition of types
auto should be used to avoid repetition of types
When used as a type specifier in a declaration, auto allows the compiler to deduce the type of a variable based on the type of the initialization expression.
When the spelling of the initialization expression already contains the type of the declared variable, it leaves no ambiguity and auto should be used as it makes the code easier to read and reduces duplication. This includes:
Initializations using new
Evaluation of constant unsigned integer expressions should not lead to wrap-around
Evaluation of constant unsigned integer expressions should not lead to wrap-around
Unsigned integer expressions do not strictly overflow, but instead wrap around in a modular way.
Any constant unsigned integer expressions that in effect “overflows” will not be detected by the compiler. Although there may be good reasons at run-time to rely on the modular arithmetic provided by unsigned integer types, the reasons for using it at compile-time to evaluate a constant expression are less obvious. Any instance of an unsigned integer constant expression wrapping around is therefore likely to indicate a programming error.
This rule applies equally to all phases of the translation process. Constant expressions that the compiler chooses to evaluate at compile time are evaluated in such a way that the results are identical to those that would be obtained by evaluation on the target, with the exception of those appearing in conditional preprocessing directives. For such directives, the usual rules of arithmetic apply but the int and unsigned int types behave instead as if they were long and unsigned long
respectively.
Integer literals should not be cast to bool
Integer literals should not be cast to bool
Even though C++ provides “true” and “false” as boolean literals, it allows using integer literals in places where boolean type is expected. This can be done through implicit or explicit casting.
In contexts where boolean type is expected, integral literals should be avoided. Using boolean literals instead would make your code more readable and less error-prone.
Smart pointers should not be initialized with a pointer owned by another smart pointer
Smart pointers should not be initialized with a pointer owned by another smart pointer
Multiple smart pointers should not be initialized with pointers pointing to the same memory. Smart pointers take care of calling the object deleter; this means that two smart pointers initialized with the same raw pointer value would lead to calling the delete function twice on the same address. Double deleting is a recipe for undefined behavior.
Operands of && and || should be primary (C) or postfix (C++) expressions
Operands of && and || should be primary (C) or postfix (C++) expressions
The effect of this rule is to require that operands are appropriately parenthesized. Parentheses are important in this situation both for readability of code and for ensuring that the behavior is as the developer intended.
Where an expression consists of either a sequence of only logical && or a sequence of logical ||
, extra parentheses are not required.
RAII objects should not be temporary
RAII objects should not be temporary
The RAII idiom associates the lifetime of a resource with the lifetime of an object: The resource is acquired when the object is created, and released when it is destroyed.
If the object that controls the resource lifetime is a temporary, chances are it will get destroyed while the resource should still be in use, leading to resource corruption. This rule detects temporaries that look like RAII objects.
std::format should be used instead of standard output manipulators
std::format should be used instead of standard output manipulators
C++20 introduces a new text formatting API with the `<format> header, in addition to the printf function family — inherited from C — and iostreams. std::format combines the convenience of printf, separating formatting and arguments, with the type-safety of iostreams. C++23 adds the <print> header, which provides similar features that output to a stream instead of generating a string.
Before C++20, if you wanted to format an output stream, you had to use standard manipulators that control the output streams. This approach is very verbose, is often stateful, and is not thread-safe. That is why we recommend replacing them with std::print or std::format when possible.
Some manipulators will have a temporary effect on the output. For example, std::setw. This is due to the resetting of the width property of the stream when most of the operator<< is called. Other manipulators will have a lasting effect on the output. For example, std::boolalpha. It will set the boolalpha flag of the output stream without resetting it.
This rule raises an issue when an output stream is used with standard manipulators to output a formattable type in a way that can be replaced by std::print or std::format`. You should be careful to avoid undesirable side effects when replacing a manipulator with lasting effects.
Comparison operators should not be virtual
Comparison operators should not be virtual
Making a comparison operator `virtual implies that you want to compare objects of different types by overriding operator==, for instance, in a subclass to compare instances of the base class with instances of the subclass. But polymorphic comparison operators are very difficult to get right, and are actually questionable in concept. After all, can two objects with only a few common members really be equal?
This rule raises issues on virtual` comparison operators.
STL constrained algorithms with range parameter should be used when iterating over the entire range
STL constrained algorithms with range parameter should be used when iterating over the entire range
C++20 introduces the ranges library. A range is a group of items that can be iterated over. It should provide a `begin iterator and an end sentinel. All the existing STL containers are ranges.
This new library makes working with the STL library much more powerful by introducing range adaptors and much less verbose by introducing a constrained version of most algorithms in the namespace std::ranges. Before the ranges library, you had to specify the begin and end` iterators when calling the STL algorithms, even when you want to iterate over the whole container.
This rule focuses on making your code less verbose and more readable by suggesting range-based over iterator-based algorithms when convenient.
std::move and std::forward should not be confused
std::move and std::forward should not be confused
`std::forward and std::move have different purposes:
std::move takes an object and casts it as an rvalue reference, which indicates that resources can be “stolen” from this object.
std::forward has a single use-case: to cast a templated function parameter of type forwarding reference (T&&) to the value category (lvalue or rvalue) the caller used to pass it. This allows rvalue arguments to be passed on as rvalues, and lvalues to be passed on as lvalues. This scheme is known as perfect forwarding. Note that the standard states that “a forwarding reference is an rvalue reference to a cv-unqualified template parameter that does NOT represent a template parameter of a class template”. Refer to the last noncompliant code example.
Since both rvalue references and forwarding references use the same notation (&&), an unwary developer might confuse them. If that happens, and a parameter is moved instead of forwarded, the original object can be emptied, probably crashing the software if the user tries to use the original object normally after the function call. An error in the other direction has less dire consequences and might even work as intended if the right template argument is used, but the code would be clumsy and not clearly express the intent.
This rule raises an issue when std::forward is used with a parameter not passed as a forwarding reference or when std::move` is used on a parameter passed as a forwarding reference.
Arrays should be deleted with []
Arrays should be deleted with []
Memory that is allocated with new T[n] must be freed with delete[]. Leave out the []
, and the likely result is heap corruption or, as a best-case scenario, premature program termination.
Arguments evaluation order should not be relied on
Arguments evaluation order should not be relied on
Arguments evaluation order in a function call is not specified:
Before C++17, the evaluation of each argument was unsequenced with the evaluation of other arguments, which can lead to undefined behavior if the same value is modified in several arguments,
After C++17, it is sequenced, but in an unspecified order: the behavior is not longer undefined, but the values passed to the function will be non portable.
Both cases should be avoided, because the code will probably not be what was expected.
Perfect forwarding constructors should be constrained
Perfect forwarding constructors should be constrained
Forwarding references (also known as universal references) provide the ability to write a template that can deduce and accept any kind of reference to the object (rvalue/lvalue mutable/const). This enables the creation of a perfect forwarding constructor for wrapper types: the constructor arguments are forwarded to build the underlying type:
#import should not be used
#import should not be used
#import comes from Objective-C and is a variant of #include. GCC does support it, but it requires the users of a header file to know that it should only be included once. It is much better for the header file’s implementor to write the file so that users don’t need to know this. Using a wrapper #ifndef
accomplishes this goal.
Modern literals should be used
Modern literals should be used
The modern Objective-C literal syntax should be preferred because it is clearer and easier to read. More importantly, it is easier to use correctly than the original, boxed syntax.
Pre-defined macros should not be defined, redefined or undefined
Pre-defined macros should not be defined, redefined or undefined
The standard, predefined macros, such as `FILE and LINE, are primarily intended for use by the implementation, and changing them could result in undefined behavior.
This rule checks that the following predefined macros are not defined, undefined, or redefined: assert, errno, FILE, LINE, TIME, DATE, TIMESTAMP, COUNTER, INCLUDE_LEVEL, BASE_FILE, and _Pragma`.
const qualifier should be placed consistently
const qualifier should be placed consistently
C++ language is highly elastic regarding the placement of type specifiers on type declarations. This makes it possible to place a `const qualifier before the type (“west const”) or after it (“east const”).
Mixing both const placement in a single project or, even worse, in a single file, makes the code harder to understand. It will require more effort to notice that the object cannot be modified.
This rule checks if the const` qualifier placement matches the configuration.
Operator spaceship <=> should be used to define comparable types
Operator spaceship <=> should be used to define comparable types
C++20 introduces the “spaceship” `operator<=> that replaces all the other comparison operators in most cases. When this operator is defined, the compiler can rewrite expressions using <, <=, > and >= to use this operator instead. This presents three advantages:
Less code to write (and therefore fewer bugs, too),
Guaranteed consistency between all the comparison operators (for instance, in this situation, a < b and !(a >= b) will always return the same value).
Guaranteed symmetry for comparisons: if you can write a < b, and that operation is resolved through operator<=>, you can also write b < a, and get a consistent result. Achieving the same result with classical comparison operators requires twice as many overloads if a and b have different types.
Additionally, if the operator<=> has the defaulted implementation, the compiler can implicitly generate a defaulted implementation of operator==, simplifying the class definition one step further.
Before C++20, it was common to provide only operator< for a class and ask the users of this class to write all their code only using this operator (this is what std::map requires of its key type, for instance). In this case, it is still advised to replace the operator with <=>`: the quantity of required work is similar, and users of the class will benefit from a much greater expressivity.
The value of an expression should be the same under any order of evaluation the standard permits
The value of an expression should be the same under any order of evaluation the standard permits
Apart from a few operators (notably &&, ||, ?: and ,
) the order in which sub-expressions are evaluated is unspecified and can vary. This means that no reliance can be placed on the order of evaluation of sub-expressions and, in particular, no reliance can be placed on the order in which side effects occur. Those points in the evaluation of an expression at which all previous side effects can be guaranteed to have taken place are called “sequence points”. Sequence points and side effects are described in Section 1.9(7) of ISO/IEC 14882:2003 [1].
Note that the “order of evaluation” problem is not solved by the use of parentheses, as this is not a precedence issue.
using-directives should not be used
using-directives should not be used
using directives add additional scopes to the set of scopes searched during name lookup. All identifiers in these scopes become visible, increasing the possibility that the identifier found by the compiler does not meet developer expectations.
Using-declarations or fully-qualified names restricts the set of names considered to only the name explicitly specified, and so these are safer options.
Using tmpnam, tmpnam_s or tmpnam_r is security-sensitive
Using tmpnam, tmpnam_s or tmpnam_r is security-sensitive
"tmpnam_s" and "tmpnam_r" are all used to return a file name that does not match an existing file, in order for the application to create a temporary file. However, even if the file did not exist at the time those functions were called, it might exist by the time the application tries to use the file name to create the files. This has been used by hackers to gain access to files that the application believed were trustworthy.
There are alternative functions that, in addition to creating a suitable file name, create and open the file and return the file handler. Such functions are protected from this attack vector and should be preferred. About the only reason to use these functions would be to create a temporary folder, not a temporary file.
Additionally, these functions might not be thread-safe, and if you don’t provide them buffers of sufficient size, you will have a buffer overflow.
goto should jump to labels declared later in the same function
goto should jump to labels declared later in the same function
Unconstrained use of `goto can lead to programs that are extremely difficult to comprehend and analyse. For C++, it can also lead to the program exhibiting unspecified behavior.
However, in many cases a total ban on goto requires the introduction of flags to ensure correct control flow, and it is possible that these flags may themselves be less transparent than the goto they replace.
Therefore, the restricted use of goto` is allowed where that use will not lead to semantics contrary to developer expectations. “Back” jumps are prohibited, since they can be used to create iterations without using the well-defined iteration statements supplied by the core language.
Nested blocks of code should not be left empty
Nested blocks of code should not be left empty
An empty code block is confusing. It will require some effort from maintainers to determine if it is intentional or indicates the implementation is incomplete.
Class templates, function templates, class template member functions and class template static members should be instantiated at least once
Class templates, function templates, class template member functions and class template static members should be instantiated at least once
Similar to uncalled functions, un-instantiated class and function templates are a potential source of noise and they may be symptomatic of a more serious problem such as missing paths.
Note: Even though a given class template may be instantiated many times, it is possible that some of its member functions are never instantiated.
Blocking functions should not be called inside critical sections
Blocking functions should not be called inside critical sections
Concurrent accesses to shared resources are guarded by synchronization primitives such as mutexes to prevent data races. The section of code where a mutex is held is called the critical section. Critical sections are generally designed to be as small as possible, allowing concurrent threads to progress.
It’s usually unintentional to perform blocking operations inside a critical section because the operation might block for long or even indefinitely, degrading performance or causing a deadlock.
pthread_mutex_t should be unlocked in the reverse order they were locked
pthread_mutex_t should be unlocked in the reverse order they were locked
Mutexes are synchronization primitives that allow the managing of concurrency. It is a common situation to have to use multiple mutexes to protect multiple resources with different access patterns.
In such a situation, it is crucial to define an order on the set of all mutexes:
This order should be strictly followed when locking mutexes.
The reverse order should be strictly followed when unlocking mutexes.
Failure to do so can lead to deadlocks. i.e., situations where two or more threads are blocked forever, each holding one mutex and waiting for one held by the other(s).
In C++, an easy way to make sure the unlocks are called in reverse order from the lock is to wrap the lock/unlock operations in an RAII class (since destructors of local variables are called in reverse order of their creation).
If instead of pthread_mutex_t you are using std::mutex
, there are other mechanisms that allow you to avoid deadlocks in that case, see S5524.
Implicit casts should not lower precision
Implicit casts should not lower precision
A narrowing conversion is an implicit conversion to a destination type that cannot represent all values from the source type.
It can be a floating-point type converted to an integer type, or a type with a larger range of values converted to a type with a smaller range.
Narrowing conversions can lead to a loss of information and because they are implicit, they are not always obvious.
Enumeration values should comply with a naming convention
Enumeration values should comply with a naming convention
Shared coding conventions allow teams to collaborate efficiently. This rule checks that all enumeration values match a provided regular expression.
C headers should not be used
C headers should not be used
The use of C headers and therefore C functions in a C++ program, is sometimes necessary, but should be avoided in favor of C++ headers and functions.
Special member function should not be defined unless a non standard behavior is required
Special member function should not be defined unless a non standard behavior is required
All special member functions (default constructor, copy and move constructors, copy and move assignment operators, destructor) can be automatically generated by the compiler if you don’t prevent it (for many classes, it is good practice to organize your code so that you can use these default versions, see S4963).
There are cases where it’s still useful to manually write such a function because the default implementation is not doing what you need. But when the manually written function is equivalent to the default implementation, this is an issue because:
It’s more code to write, test, and maintain for no good reason
Correctly writing the code of those functions is surprisingly difficult
Once you write one such function, you will typically have to write several (see S3624)
If you want your class to be trivial or to be an aggregate, those functions cannot be user-provided anyways
In most cases, you should just remove the code of the redundant function. In some cases, the compiler will not automatically generate the default version of the function, but you can force it to do so by using the `= default syntax.
For default constructors, you can often use the default version if you use in-class initialization instead of the initializer list. You must make it explicitly defaulted if your class has any other constructor.
For destructors, you may want to use the =default` syntax to be able to declare it as virtual (see S1235).
This rule raises an issue when any of the following is implemented in a way equivalent to the default implementation:
default constructor
destructor
move constructor
move-assignment operator
copy constructor
copy-assignment operator
enum should not be used
enum should not be used
While a C-style enum will work in Objective-C, the newer NS_ENUM and NS_OPTIONS
macros offer a simple way to define enumerations and options, specifying the type and size for each, and improving code completion in Xcode.
A loop-control-variable other than the loop-counter should not be modified within condition or expression
A loop-control-variable other than the loop-counter should not be modified within condition or expression
loop-control-variables are either the loop-counter, or flags used for early loop termination. The code is easier to understand if these are not modified within condition or expression.
Macros used in preprocessor directives should be defined before use
Macros used in preprocessor directives should be defined before use
An attempt to use an undefined identifier may elicit a warning from the preprocessor. Or it may not; the preprocessor may simply assume that the undefined token has a value of 0.
Therefore macro identifiers should not be used in preprocessor directives until after they have been defined, and this limited usage should be enforced with the use of definition tests.
Using strncpy or wcsncpy is security-sensitive
Using strncpy or wcsncpy is security-sensitive
a buffer of characters, normally using the `null character as a sentinel for the end of the string. This means that the developer has to be aware of low-level details such as buffer sizes or having an extra character to store the final null character. Doing that correctly and consistently is notoriously difficult and any error can lead to a security vulnerability, for instance, giving access to sensitive data or allowing arbitrary code execution.
The function char *strncpy(char * restrict dest, const char * restrict src, size_t count); copies the first count characters from src to dest, stopping at the first null character, and filling extra space with 0. The wcsncpy does the same for wide characters and should be used with the same guidelines.
Both of those functions are designed to work with fixed-length strings and might result in a non-null`-terminated string.
Tag names should be unique identifiers
Tag names should be unique identifiers
No tag name shall be reused either to define a different tag or for any other purpose within the program. ISO 9899:1990 [2] does not define the behaviour when an aggregate declaration uses a tag in different forms of type specifier (struct or union). Either all uses of the tag should be in structure type specifiers, or all uses should be in union type specifiers.
std::declval should not be used within requires-expression
std::declval should not be used within requires-expression
A requires-expression is used to express constraints on template arguments. A basic building block of these constraints is the capability to generate a subexpression whose type depends on a template argument.
The traditional way to write such a subexpression is by using std::declval<> (doing something more naive such as T{} is not as generic, for instance, it requires T to be default-constructible). This is, however, very verbose and can be error prone: declval<T>() yields an expression of type T&&, while referencing a variable directly produces an lvalue (T&). This, in many cases, leads to concepts incorrectly requiring only move-construction, while copies are made by the implementation.
Require-expressions introduce a more natural way to achieve that. When writing a requires-expression, it is possible to add a parameter list, similar to function parameters, and these parameters can be used later in the expression. This syntax is less verbose, more expressive, and less error-prone and should be preferred over calling std::declval in requires-expressions.
volatile should not be used to qualify objects for which the meaning is not defined
volatile should not be used to qualify objects for which the meaning is not defined
volatile
can be used to qualify many objects in C and C++, but only a few of the possible places have a well-defined meaning (global variables and local variables, for instance).
There is no well-defined meaning to the use of volatile to qualify a function return type or a function parameter.
Furthermore, for structured bindings, the volatile qualifier appertains to the decomposed object, which cannot be referred to.
Since C++20, these uses are deprecated, but even before you should not use volatile in those places.
This rule raises an issue for a volatile qualified function return type, function parameter, and structured binding (available in C++ since C++17).
std::bit_cast should be used to reinterpret binary representation instead of std::memcpy
std::bit_cast should be used to reinterpret binary representation instead of std::memcpy
`std::bit_cast is one of the standard functions working with binary representation. Together with other bit-level functions, it is defined in the <bit> header introduced by C++20.
std::bit_cast standardizes the diverse and sub-optimal approaches of reinterpreting a value as being of a different type of the same length, preserving its binary representation.
Before C++20, the correct way to reinterpret a value was a call to std::memcpy, copying the exact binary representation from a variable of one type into a variable of another. Although canonical, the use of std::memcpy might still be confusing; it is verbose, and it might introduce performance overhead if the compiler does not recognize the idiom and does not remove the function call.
In contrast, std::bit_cast clearly states the intent and is guaranteed to map to an optimal implementation.
This rule reports the uses of std::memcpy that can be replaced by std::bit_cast`.
bind should not be used
bind should not be used
Using `std::bind or boost::bind allows to create a wrapper to a function where some arguments are bound to fixed values, or the order of arguments is changed. However, this way of wrapping a function comes with some issues:
The code is not really clear, the usage of placeholders makes it difficult to follow,
The complexity of the implementation of std::bind often results in sub-optimal code,
The possibilities offered by bind are limited, for instance, it’s not possible to bind a function with variable arguments,
bind allows to silently discard some arguments, which is often not what was expected,
Fixed arguments are evaluated when bind is called, not when the bound function is called, which might be surprising in some cases,
bind requires to take the address of a function, which can be difficult (if the function is overloaded) or not recommended (for standard library functions, see S5180).
To create such a wrapper, it is usually better to:
either write a simple lambda that just calls the wrapped function with the full power of the language concerning how the arguments are going to be tinkered with.
or, if the function bind is only applied to the first arguments, call std::bind_front` that has been introduced in C++20 and that is safer to use and a simpler replacement than lambdas.
Array type function arguments should not decay to pointers
Array type function arguments should not decay to pointers
When a variable with array type decays to a pointer, its bounds are lost.
If a design requires arrays of different lengths, then a class should be used to encapsulate the array objects and so ensure that the size is maintained. Note that in C++20, the class std::span is designed for this purpose, and you can find implementation of span
that works with earlier versions of the standard.
Non-reentrant POSIX functions should be replaced with their reentrant versions
Non-reentrant POSIX functions should be replaced with their reentrant versions
A function is called reentrant if it can be interrupted in the middle of its execution and then safely called again (“re-entered”) before its previous invocations complete execution.
It is especially important that multi-threaded applications do not call the same non-reentrant function from different threads.
This rule will trigger an issue each time a function in the configurable list is invoked.
A call will be matched differently depending on the presence of the scope resolution operator `:: in the function name from the configurable list.
For example, namespace a { namespace b { void f(); } } can be matched with “f”, “b::f”, “a::b::f”, “::a::b::f” (fully qualified name yielding most precise results).
It is recommended to provide fully qualified names to the configurable list (i.e., start each name with ::`), even for the functions in the global namespace.
std::move should not inhibit optimizations
std::move should not inhibit optimizations
Usually, when copying an object, the source object is unchanged, meaning all resources owned by the source objects must be duplicated during the copy operation. If the source object is no longer used, this duplication is inefficient. Since C++11, a move semantic mechanism has been added to detect such cases and replace the expensive copy with a much cheaper move operation that will transfer resources.
The cornerstone of move semantics is detecting during a “copy” whether the source object will be reused or not. This can be done explicitly by the user, by invoking std::move (or different casts to rvalue) on the object. In such case the user promises to the compiler that they won’t care for the object’s current value any longer. In addition, the compiler will implicitly use a move operation or skip copying the object in some situations.
One case of optimization is that the copy will be elided or automatically turned into a move operation when a temporary object of type T:
is used to initialize a parameter or variable of type T or const T
is returned from the function that declares T or const T as return type
+new should not be overridden or used
+new should not be overridden or used
Shared coding conventions enable teams to collaborate more efficiently. While `new++ and a combination of alloc -init++ are functionally equivalent, the latter is preferred. In addition to being more accepted in modern code, it also better represents a separation of concerns.
If +new` is never invoked, then there is no need to override it, and any such methods become clutter in a class.
#pragma warning (default: ...) should not be used
#pragma warning (default: ...) should not be used
Using “#pragma warning (default: …)” resets the warning in question to its default settings, which may not be what the compiler was initially invoked with. Typically, this usage is seen after a warning is turned off, in preparation for code that is known to cause warnings. Instead, the warning’s current state should be saved, and then restored after the code in question.
Elements in a container should be erased with std::erase or std::erase_if
Elements in a container should be erased with std::erase or std::erase_if
Removing elements with a specific value or that follow a given predicate from a container is a common task. Before C++20, this was not straightforward. The way to do it had to depend on the type of your container.
For sequence containers, you would end up following what is called the erase-remove idiom:
Call `std::remove or std::remove_if with, as parameters, the container and the criterion to fulfill
Call the container member function erase on the result
For associative containers, you would have no other option than looping through all the elements by hand.
However, C++20 introduced two new methods: std::erase (for sequence containers only) and std::erase_if which erase all elements equal to a value or that satisfy a given predicate.
This rule raises an issue when std::erase or std::erase_if` could be used to simplify the code.
Classes should define copy constructors and operator= methods
Classes should define copy constructors and operator= methods
Any class that has memory to manage should provide all the methods necessary to properly manage that memory, including a copy constructor and an override of operator=
. Without those methods, you’re likely to end up with memory leaks and multiple class instances pointing at the same segments of memory for their members.
Preprocessor operators # and ## should not be used
Preprocessor operators # and ## should not be used
The evaluation order of both the # and ##
preprocessor operators is unspecified. Compilers have been known to implement these operators inconsistently, therefore, to avoid these problems, do not use them.
Only valid arguments should be passed to UNIX/POSIX functions
Only valid arguments should be passed to UNIX/POSIX functions
Many UNIX/POSIX functions put certain constraints on the values of their parameters. The behavior for some of those UNIX/POSIX functions is not defined but instead, their behavior is implementation-defined, if one passes incorrectly constrained parameters. This may lead to undefined behavior depending on a function’s concrete implementation. The constraints include the following:
Allocation sizes of `calloc, malloc, realloc, reallocf, alloca and valloc must be strictly positive
open and openat should be called with a flag that contains one of the access modes: O_RDONLY, O_WRONLY, or O_RDWR
open and openat with flag O_CREAT should be called with a third argument
The O_EXCL flag should be used with O_CREAT
The first argument of pthread_once should not have automatic storage duration and should be initialized by the constant PTHREAD_ONCE_INIT`
Failing to pass correctly constrained parameters can result in undefined behavior.
Fold expressions should be used instead of recursive template instantiations
Fold expressions should be used instead of recursive template instantiations
Fold expressions, introduced in C++17, are a way to expand a variadic template parameter pack with operators between each pack element. Due to the high flexibility of this construct, many variadic templates that used to be written by a recursive call can now be written in a more direct way.
In addition to a usually simpler code, fold expressions results in far less functions instantiated, which can improve compilation time.
This rule raises an issue when a recursive template instantiation that could be easily be replaced by a fold expression is detected
Variables should not be initialized to 0 or nil in an init method
Variables should not be initialized to 0 or nil in an init method
Initializing a variable to zero or `nil in an init method is completely redundant; the compiler takes care of that for you. Therefore initializing class instance variables to 0 or nil is simply wasted keystrokes.
This rule applies to methods that return an id` and have names that begin with “init”.
Macros should not be #defined or #undefd within a block
Macros should not be #defined or #undefd within a block
While it is legal to place #define and #undef
directives anywhere in a source file, placing them outside of the global namespace is misleading since their scope is not actually restricted. This may be inconsistent with developer expectations.
The original exception object should be rethrown
The original exception object should be rethrown
When throw is followed by an expression, this expression will be used to create and initialize the exception object. In other words, the exception object is copy-initialized from the expression.
struct names should comply with a naming convention
struct names should comply with a naming convention
Sharing some naming conventions enables teams to collaborate more efficiently. This rule checks that all struct
names match a provided regular expression.
Struct should explicitly specify the access level when specifying base classes
Struct should explicitly specify the access level when specifying base classes
It is not very common for a struct to have base classes. When they do, by default, they will have public inheritance. Since this is not a fact known by everybody, it’s usually better to be explicit about the visibility of base classes in a struct.
NULL should not be used as an integer value
NULL should not be used as an integer value
In C++, the literal 0 is both an integer type and the null-pointer-constant. To meet developer expectations, `NULL should be used as the null-pointer-constant, and 0 for the integer zero.
Note: as a result of this rule, NULL` is considered to have pointer type.
const member functions should not return non-const pointers or references to class-data
const member functions should not return non-const pointers or references to class-data
When an object is declared with const class type, only const member functions can be invoked on that object. The common expectation of const member functions is that the state of the object may not be modified when invoking the functions. However, returning a non-const pointer or reference to class-data from a const
function allows a modification to the conceptual state of an object.
Declarations should not be empty
Declarations should not be empty
Empty declarations are cruft; they (may) compile, but they violate the language standards, don’t contribute anything of value, and clutter up the program. Like cobwebs, they should be swept away.
Pointers should not be used
Pointers should not be used
Pointers are a powerful tool, but they can be difficult to use correctly, leading to memory leaks and double deletion. Further, they’re not usually needed in C++ because the language offers abstractions that handle the more difficult aspects of using pointers for you.
Arguments to a function-like macro should not contain tokens that look like preprocessing directives
Arguments to a function-like macro should not contain tokens that look like preprocessing directives
If any of the arguments act like preprocessor directives, the behaviour when macro substitution is made can be unpredictable.
shared_ptr should not be taken by rvalue reference
shared_ptr should not be taken by rvalue reference
Format strings should be used correctly
Format strings should be used correctly
printf format strings contain placeholders, represented by special characters such as `%s. These placeholders are interpreted at runtime rather than validated by the compiler. Using incorrect placeholders or with inappropriate arguments can result in the wrong string being created or undefined behavior.
Starting with C++20, std::format should be preferred: it is more readable and validated at compile-time, making it more secure. Rule S6494 covers that. Furthermore, C++23 provides std::print, which is similar to std::format but directly prints its output instead of generating a std::string`.
S2275 covers errors leading to undefined behavior. This rule is about errors that produce an unexpected string.
These problems are detected when the format string is a string literal:
Every argument should be used:
Handlers of a function-try-block implementation of a class constructor or destructor shall not reference non-static members from this class or its bases
Handlers of a function-try-block implementation of a class constructor or destructor shall not reference non-static members from this class or its bases
When a constructor/destructor has a function-try-block, the code inside of the catch clause will be executed after the object has been destroyed (if the object was partially constructed when the exception was thrown, this part will be destroyed before going in the catch block). Therefore, the members of the object are not available, and it is undefined behavior to access them.
Since the lifetime of a static member is greater than that of the object itself, so a static member can be accessed from the catch code.
std::cmp_* functions should be used to compare unsigned values with negative values
std::cmp_* functions should be used to compare unsigned values with negative values
Comparison between signed and unsigned integers is dangerous because it produces counter-intuitive results due to implicit conversions. When a signed integer is compared to an unsigned one, the former might be converted to unsigned. Since C++20, the conversion preserves the two’s-complement bit pattern of the signed value that always corresponds to a large unsigned result. For example, 2U < -1 is true.
This rule raises an issue when an unsigned integer is compared with a negative value.
Dynamically allocated memory should be released
Dynamically allocated memory should be released
Memory is a limited resource shared between all the applications running on the same host machine.
C and C++ do not automatically reclaim unused memory. The developer has to release the memory claimed for their application that is no longer needed. Unlike the stack that automatically allocates local variables on a function call and deallocates them on a function return, the heap offers no automatic memory management. The developer has to make sure to deallocate the memory they allocate dynamically on the heap.
This rule raises an issue when memory is allocated dynamically and not freed within the same function.
Arguments corresponding to width and precision formatting options should be integers
Arguments corresponding to width and precision formatting options should be integers
The std::format function and related formatting functions allow you to control how to display a value as text, including its width and precision.
For example, you can convert the number 3.14159 to a string with a minimum width of 10 and a precision of 2 using:
starts_with and ends_with should be used for prefix and postfix checks
starts_with and ends_with should be used for prefix and postfix checks
In C++20, std::string and std::string_view gain new member functions starts_with and ends_with
that compare their argument to the prefix and postfix of the string.
These two functions introduce a standard, concise, and efficient way of checking the prefix and postfix for strings. The ad-hoc implementations predating C++20 are often less readable, less efficient, and less reliable.
This rule raises an issue when an ad-hoc implementation checks prefixes or postfixes of a string.
Calls to std::format with a locale should use the L flag
Calls to std::format with a locale should use the L flag
std::format and other formatting functions have an overload that allows specifying a locale to format the arguments. For instance, to use a . or a , for floating point values, or to spell the months in dates. However, just passing the right locale is not enough. You have to mark each argument that is subject to internationalization by specifying the L flag in the format specification.
This rule raises an issue when a locale is passed to a formatting function, but localization is not enabled for any argument.
Namespace names should comply with a naming convention
Namespace names should comply with a naming convention
Shared coding conventions allow teams to collaborate efficiently. This rule checks that all namespace names match a provided regular expression.
contains should be used to check if a key exists in a container
contains should be used to check if a key exists in a container
C++20 introduces the member function `contains on associative containers to check if an equivalent to a specific key already exists in the container.
Calling this function can replace previous ways to check if a key is present in a container:
call find() and check that its result is not the end of the container. This was quite verbose.
call count(). This did not clearly express the intent and was not optimal in terms of performance for containers that allow a key to be present multiple times.
This rule raises an issue when contains` could be used to simplify the code.
const member function should return only const pointer/reference to a field
const member function should return only const pointer/reference to a field
`const member functions are the member functions that do not modify the object they are called on.
While returning a non-const reference or a pointer from such a function does not in itself modify the object, it creates an opportunity for modification in the future. In particular, it enables the code that uses this member function to modify a ++const++ object.
When defining a const member function, consider returning by reference/pointer to const or returning by value.
In some cases you need to be able to read the field from const objects and mutate it in non-const, as is often the case with container objects, like std::vector. Consider using const-overloading in this case.
This rule detects when a const` member function returns a reference field and has as a return type a pointer/reference to a non-const object.
Object declarations should contain no more than 2 levels of pointer indirection
Object declarations should contain no more than 2 levels of pointer indirection
While they are extraordinarily useful, pointers are not the most intuitive concept in the world. Pointers to pointers are even harder to understand and use correctly. And with each additional level of indirection, pointer variables become more difficult to use correctly. Therefore pointer declarators should be limited to no more than two levels of nesting.
dynamic_cast should be used for downcasting
dynamic_cast should be used for downcasting
Casting a base-class pointer/reference to a derived-class pointer/reference is commonly referred to as downcasting which can only be done using an explicit cast.
However, the use of `static_cast for such a cast is unsafe because it doesn’t do any runtime check. If the cast memory doesn’t contain an object of the expected derived type, your program enters the undefined behavior territory.
If your object is polymorphic, you might prefer using dynamic_cast instead, as it allows safe downcasting by performing a run-time check:
If the cast memory contains an object of the expected derived type, the check succeeds. The result of the dynamic_cast points/refers to the derived object.
If the cast memory doesn’t contain an object of the expected derived type, the check fails. If the dynamic_cast is used on a pointer, nullptr is returned. If it was used on a reference, std::bad_cast is thrown.
This rule raises an issue when static_cast` is used for downcasting.
Member data should be initialized in-class or in a constructor initialization list
Member data should be initialized in-class or in a constructor initialization list
There are three ways to initialize a non-static data member in a class:
With an in-class initializer (since C++11)
In the initialization list of a constructor
In the constructor body
You should use those methods in that order of preference. When applicable, in-class initializers are best, because they apply automatically to all constructors of the class (except for default copy/move constructors and constructors where an explicit initialization for this member is provided). But they can only be used for initialization with constant values.
If your member value depends on a parameter, you can initialize it in the constructor’s initialization list. If the initialization is complex, you can define a function to compute the value, and use this function in the initializer list.
Initialization in the constructor body has several issues. First, it’s not an initialization, but an assignment. Which means it will not work with all data types (const-qualified members, members of reference type, member of a type without default constructor…). And even if it works, the member will first be initialized, then assigned to, which means useless operations will take place. To prevent “use-before-set” errors, it’s better to immediately initialize the member with its real value.
It’s hard to find a good example where setting the value of a member in the constructor would be appropriate. One case might be when you assign to several data members in one operation. As a consequence constructor bodies are empty in many situations.
This rules raises an issue in two conditions:
When you assign a value to a member variable in the body of a constructor.
When you default-initialize in an initializer list a member variable, that would be value-initialized by default
For C++11 or later, when you initialize a member variable in the initializer list of a constructor, but could have done so directly in the class:
The variable has either no in-class initializer, or an in-class initializer with the same value as in the constructor
The initial value does not depend on a constructor parameter
Forward declarations should not be redundant
Forward declarations should not be redundant
Redundant forward declarations simply clutter the code, and like any duplications, should be removed.
Array declarations should include an explicit size specification
Array declarations should include an explicit size specification
It is possible to declare an array without explicitly specifying its size, but using an explicit size declaration is clearer, and is therefore preferred.
Integral operations should not overflow
Integral operations should not overflow
Numbers are infinite, but the types that hold them are not. Each numeric type has hard upper and lower bounds. Try to calculate or assign numbers beyond those bounds, and the result will be surprising:
For unsigned types, it will be a value that has silently wrapped around from the expected positive value to another one, following the rules of modular arithmetic (if the maximum
unsigned char is 255, adding 10 to an unsigned char
equals to 250 will yield the value 4)For signed type, this is undefined behavior.
final should not be used redundantly
final should not be used redundantly
There is no need to use the `final specifier inside a final class. Everything in it is final by default.
Similarly, there is no need to use the final specifier for union, because union`s can neither extend other classes nor be extended.
No identifier in one name space should have the same spelling as an identifier in another name space, with the exception of structure and union member names
No identifier in one name space should have the same spelling as an identifier in another name space, with the exception of structure and union member names
Name space and scope are different. This rule is not concerned with scope.
ISO C defines a number of different name spaces (see ISO 9899:1990 6.1.2.3). It is technically possible to use the same name in separate name spaces to represent completely different items. However this practice is deprecated because of the confusion it can cause, and so names should not be reused, even in separate name spaces.
Iterators arguments should define a valid range
Iterators arguments should define a valid range
Functions that deal with iterators often use the notion of range: A range is a pair or iterators, `b and e, with the following conditions:
b and e must refer to the same container
e must be reachable from b, which mean that by doing b++ enough times, we must reach e (another way to say it is that b must be before e` in the container).
An invalid range will most of the time lead to undefined behavior.
This rule detects when two iterators that do not make a valid range are passed to a function that expects a range.
Parameter values should be appropriate
Parameter values should be appropriate
The standard C library includes a variety of functions for string and general memory manipulations. Many of these functions require valid parameters and when given values outside a function’s domain, the behavior is undefined. Functions like `strlen, memset, memcpy, qsort, fread, etc., for instance, require non-NULL parameters, and passing a NULL value introduces undefined behavior. In that case, the application may crash or, even worse, silently lose data or produce incorrect results.
Consider the following code. If the pointer-typed variable buf is NULL due to a failed memory allocation, for instance, the call to memset()` will cause undefined behavior.
Null-terminated string should be passed to strlen
Null-terminated string should be passed to strlen
The use of strlen to determine the length of a string to which you are trying to append a null character is an anti-pattern. strlen requires as input an already null-terminated string; the result of passing a non-null-terminated string as an input to strlen
is undefined. It may even lead to a memory access violation.
Redundant class template arguments should not be used
Redundant class template arguments should not be used
Since C++17, class template arguments can be automatically deduced by the compiler, either by looking at the arguments of the class constructors or by using an explicitly defined deduction guide.
Using the class template argument deduction allows to:
Avoid verbose specification of all template parameter types for a class template.
Avoid writing helper function that only serves the purpose of deducing the type of a class from its arguments. For example,
std::make_pair
.Be able to instantiate a class template with hard-to-spell or unutterable names, such as the closure type of a lambda.
This rule raises an issue when explicit class template arguments that can be automatically deduced are specified.
Rvalue references should not be used
Rvalue references should not be used
Rvalue references were introduced as part of C++11. They are thus a new feature of the language, and are not yet widely understood. Using them is complicated, and code using rvalue references may be difficult to understand.
setjmp and longjmp should not be used
setjmp and longjmp should not be used
`setjmp.h functions allow the normal function mechanisms to be bypassed and should be used only with extreme caution, if at all.
Calling setjmp saves the program environment into the buffer passed into the call. Later calling longjmp returns execution to the point at which setjmp was called and restores the context that was saved into the buffer. But the values of non-volatile local variables after longjmp are indeterminate. Additionally invoking longjmp from a nested signal handler is undefined, as is longjmping back to a method that has already completed execution.
This rule flags all instances of setjmp, _setjmp, longjmp, _longjmp, sigsetjmp, siglongjmp and <setjmp.h>`.
Trigraphs should not be used
Trigraphs should not be used
Trigraphs are denoted by a sequence of 2 question marks followed by a specified third character (e.g. ??- represents a ’~’ (tilde) character and ??) represents a ’]’). They can cause accidental confusion with other uses of two question marks.
offsetof macro should not be used
offsetof macro should not be used
offsetof can lead to undefined behavior when the argument types are incompatible or when bit fields are used. Therefore offsetof
should be avoided.
Base class access specifiers should not be redundant
Base class access specifiers should not be redundant
Adding an access specifier that matches the class’ current access level needlessly clutters the code.
enum values should not be used as operands to built-in operators other than [ ], =, ==, !=, unary &, and the relational operators <, <=, >, >=
enum values should not be used as operands to built-in operators other than [ ], =, ==, !=, unary &, and the relational operators <, <=, >, >=
Enumerations are used to represent symbolic values, or sometimes bit fields. They are not supposed to be used in arithmetic contexts.
Additionally, even though comparing them with integer numbers can make sense (for instance, to test if an enum lies with a certain range), comparing them with floating point numbers does not (and is deprecated since C++20).
There are other restrictions related to the use of enums, see for instance S2753.
The sizeof a pointer type should not be divided
The sizeof a pointer type should not be divided
When sizeof
is used on a pointer, it returns the size of the pointer (4 or 8 bytes, depending on architecture and OS), not the size of the pointed-to memory area. Therefore, division of that value is not likely to yield what was intended.
Null pointers should not be dereferenced
Null pointers should not be dereferenced
A pointer to null, also known as a null pointer, is created by initializing a pointer object to 0, NULL, or in the case of C++ nullptr. A null pointer does neither point to an object nor to valid memory, and as a consequence dereferencing or accessing the memory pointed by such a pointer is undefined behavior.
swap or move operator/constructor and default constructor should be noexcept
swap or move operator/constructor and default constructor should be noexcept
Throwing `swap, move operator or default constructor make it very difficult to handle exception or can lead to undefined/inconsistent state.
If exceptions can be thrown there:
Creation and destruction of local variables allocated on the stack can throw. It makes that any scope should be protected by a try-catch block, in your own code and in all the dependencies.
If a move operator/constructor could throw, objects could be left in an inconsistent states as partially moved (just a few members moved). In what state is the source object? the target one? Copy does not have this problem as the source object is not modified. It is then much easier to recover from an exception.
For the same sort of reason as above, a throwing swap could create inconsistent states. If a swap throws, what are the states of the 2 objects. Was any state lost in the process The comparison with copy still holds.
Notes:
If a noexcept function tries to raise an exception, the program will terminate. For the above case, it might often be the best way to cope with an exception arising in a default constructor, a swap or a move.
If a move constructor/operator can throw, the STL will replace it with a copy if available.
std::swap is required to be noexcept`
Inheriting constructors should be used
Inheriting constructors should be used
When inheriting from a class, if you need to inherit its constructors without additional initialization you should prefer `using-declaration to inherit all base class’s constructors instead of writing them by hand.
using-declaration for inheriting constructor is a C++11 feature that makes all constructors of the base visible to the overload resolution when initializing the derived class.
If you need to change the accessibility of one of the inherited constructors, you can do it by keeping the using-declaration` and declaring that constructor explicitly as private:
This rule raises an issue when a constructor inherits the base class constructor without requiring any additional initialization.
Capture by reference in lambdas used locally
Capture by reference in lambdas used locally
If the lifetime of the arguments passed to a lambda function is longer than the lifetime of the lambda itself, these arguments can be passed by reference.
Doing so avoids copying potentially big objects and it should be preferred over using copy capture.
This rule reports an issue if a lambda passed into an algorithm that uses it locally (all algorithms in headers <algorithm> and <numeric>
) captures some values.
switch statements should have default clauses
switch statements should have default clauses
The requirement for a final `default clause is defensive programming. The clause should either take appropriate action, or contain a suitable comment as to why no action is taken. When the switch covers all current values of an enum - and especially when it doesn’t - a default case should still be used because there is no guarantee that the enum won’t be extended.
Note that there is a more nuanced version of this rule: S3562. Use this rule if you want to require a default case for every switch even if it already handles all enumerators of an enum`. Otherwise, use S3562.
Pointers or iterators subtracted from each other should point into the same object
Pointers or iterators subtracted from each other should point into the same object
Subtraction of iterators only gives well-defined results if the two iterators point into the same data structure in memory (same array, same object…).
This rule also applies to the particular case of pointers.
Memory should not be managed manually
Memory should not be managed manually
If you manage memory manually, it’s your responsibility to `delete all memory created with new, and to make sure it’s deleted once and only once. Ensuring this is done is error-prone, especially when your function can have early exit points.
Fortunately, the C++ language provides tools that automatically manage memory for you. Using them systematically makes the code simpler and more robust without sacrificing performance.
This rule raises an issue when you use:
new - you should prefer a factory function that returns a smart pointer, such as std::make_unique or, if shared ownership is required, std::make_shared,
new[] - you should prefer a container class, such as std::vector,
delete or delete[] - if you followed the previous advice, there is no need to manually release memory.
If your compiler does not support make_unique`, it’s easy to write your own:
template<typename T, typename... Args> std::unique_ptr<T> make_unique(Args&&... args) \{ return std::unique_ptr<T>(new T(std::forward<Args>(args)...)); \}
Tests of non-Boolean values against zero should be explicit
Tests of non-Boolean values against zero should be explicit
Where a data value is to be tested against zero then the test should be made explicit. The exception to this rule is when data represents a Boolean value, even though in C this will in practice be an integer.
This rule is in the interests of clarity, and makes clear the distinction between integers and logical values.
std::is_constant_evaluated and if consteval should only be used when necessary
std::is_constant_evaluated and if consteval should only be used when necessary
The std::is_constant_evaluated function (introduced in C++20) and the if consteval language construct (introduced in C++23) are used to determine whether the evaluation is performed at compile-time or runtime. This can be useful when, for example, two different implementations are provided for an algorithm: one that does not perform any IO operations and is compatible with compile-time evaluation, and the other one that also emits log entries at runtime.
These constructs should be used inside functions that are constexpr, and thus can be evaluated both at compile-time and at runtime.
When used inside a context that is either always evaluated at compile-time or always evaluated at runtime, a call to std::is_constant_evaluated always returns the same result, similarly if (not) consteval always evaluates the same branch, making their use redundant.
This rule raises issues for contexts where expressions are always evaluated at compile-time or always evaluated at runtime.
In contexts that are always evaluated at compile-time:
std::is_constant_evaluated() always returns true.
if consteval { /* then-branch */ } always evaluates the then-branch.
if !consteval { /* then-branch / } else { / else-branch */} always evaluates the else-branch.
These include:
The conditions of an if constexpr or a static_assert.
union names should comply with a naming convention
union names should comply with a naming convention
Sharing some naming conventions enables teams to collaborate more efficiently. This rule checks that all union names and union type alias names match a provided regular expression.
Freed memory should not be used
Freed memory should not be used
A program may allocate an additional memory block using the malloc function. When no longer needed, such memory blocks are released using the free function. After it is released, reading or writing to a heap-allocated memory block leads to undefined behavior.
Modulo by 1 operation does not make sense
Modulo by 1 operation does not make sense
Modulo by 1 operation always returns 0, it is likely a typo.
Scoped enumerations should be used
Scoped enumerations should be used
There are two kinds of enumeration:
The unscoped `enum inherited from C
The scoped enumeration enum class or enum struct added in C++ 11
Unscoped enumerations have two major drawbacks that are fixed by scoped enumerations:
enum elements are visible from their enclosing scope instead of requiring the scope resolution operator (ex: Red instead of Color::Red)
enum elements convert implicitly to int, so that heterogeneous comparisons such as `++Red
const and volatile should not be used in enum declarations
const and volatile should not be used in enum declarations
Since C++11, it’s possible to declare the underlying type of an `enum, and like any type declaration, enum declarations can contain the const or volatile specifier. But because enum values are named constants and cannot be re-assigned, those specifiers are ignored by the compiler and are therefore useless.
This rule raises an issue if const or volatile is present in the declaration of the underlying type of an enum`.
Functions and objects should not be defined in header files
Functions and objects should not be defined in header files
Because header files are likely to be included by many source files in a project, they should not contain the definitions of objects or non-inlines, i.e. they should not declare anything that actually takes up memory. Doing so could result in multiple definitions of the same symbol.
Because a struct
declaration takes up no memory, it can (some would say should) be placed in a header. Similarly, class declarations should also reside in header files, but not class implementations.
typedefs should not define pointer types
typedefs should not define pointer types
typedefs are a convenient way of encapsulating complex types in simpler names. However, typedefing a pointer type is problematic because when const
is applied to the defined type, it makes the pointer constant, not the pointed-to value. Even when that is what’s intended, maintainers may misunderstand.
std::cmp_* functions should be used to compare signed and unsigned values
std::cmp_* functions should be used to compare signed and unsigned values
Comparisons between `signed and unsigned integers are dangerous because they produce counterintuitive results outside their shared value range.
When a signed integer is compared to an unsigned one, the former might be converted to unsigned. The conversion preserves the two’s-complement bit pattern of the signed value that often corresponds to a large unsigned result. The expression 2U < -1 evaluates to true, for instance.
C++20 introduced a remedy to this common pitfall: a family of std::cmp_* functions defined in the <utility> header:
std::cmp_equal
std::cmp_not_equal
std::cmp_less
std::cmp_greater
std::cmp_less_equal
std::cmp_greater_equal
These functions correctly handle negative numbers and are safe against lossy integer conversion. For example, the comparison of 2U and -1 using std::cmp_less(2U, -1) evaluates to false` and matches common intuition.
Default capture should not be used
Default capture should not be used
Lambdas can use variables from their enclosing scope (called “capture”) either by reference or by value. Since lambdas may run asynchronously, reference capture should be used with caution because, by the time the lambda runs, the referenced variable may be out of scope, resulting in an access violation at run time.
You can specify default capture by reference (`[&]), or by value ([=]). Default reference capture can cause scope issues, but so can default value capture. Both forms of default capture implicitly also capture *this, which would automatically be used if, for example, you referenced a method from the enclosing scope.
If the lambda is used immediately (for instance, passed as an argument to std::sort`), there is no risk of dangling reference. For those lambdas, it is safe to pass everything through a default capture by reference. See also S5495.
This rule raises an issue when default capture is used unless the lambda is immediately executed.
restrict should not be used
restrict should not be used
The `restrict type qualifier is a guarantee by the programmer that there are no other pointers with access to the referenced object and that the object does not overlap with any other object in memory. Its use may allow the compiler to generate more efficient byte code.
However, this is a tricky language feature to use correctly, and there is a significant risk of unexpected program behavior if restrict is misused. Therefore, restrict` should not be used.
Exception specifications should not be used
Exception specifications should not be used
Exception specifications never really provided any advantages in C++. They have been deprecated since C++11 and removed since C++17 (specification with an exception) and C++20 (empty throw specification). If your code still contains some, you should replace empty throw() specifications with noexcept
and remove any other specifications.
A cast shall not remove any const or volatile qualification from the type of a pointer or reference
A cast shall not remove any const or volatile qualification from the type of a pointer or reference
Using const in your code improves reliability and maintenance. When passing a const value, developers assume that its value won’t be changed. But using const_cast<>() to cast away a const
qualifier, destroys developer assumptions and code reliability. It is a bad practice and reveals a flaw in the design. Furthermore, it may have an undefined behavior.
Unknown attributes should not be used
Unknown attributes should not be used
C++11 introduced “Attributes”. They provide a unified syntax to specify additional information about your code.
They can be applied to various things like types, variables, functions, names, blocks, and translation units.
\{cpp\} defines some standard attributes like \[[noreturn]], \[[nodiscard]], \[[deprecated]], \[[fallthrough]]...
Unfortunately, it is possible to use unknown attributes: attributes that are not defined. Your code will compile, but the unknown attribute will be silently ignored. This means that your code will not behave in the way you expected.
Classes should not contain both public and private data members
Classes should not contain both public and private data members
Mixing (non-const) `public and private data members is a bad practice because it confuses the intention of the class:
If the class is a collection of loosely related values, all the data members should be public.
On the other hand, if the class is trying to maintain an invariant, all the data members should be private`.
If we mix data members with different levels of accessibility, we lose clarity as to the purpose of the class.
return statements should not occur in finally blocks
return statements should not occur in finally blocks
Returning from a finally block suppresses the propagation of any unhandled exception which was thrown in the try or catch
block.
The One Definition Rule should not be violated
The One Definition Rule should not be violated
Violation of the One Definition Rule (ISO/IEC 14882:2003, §3.2) leads to undefined behaviour. In general, this means that the program shall contain exactly one definition of every non-inline function or object.
Additionally:
The definitions of a type shall consist of the same sequence of tokens, and;
The definitions of a template shall consist of the same sequence of tokens, and;
The definitions of an inline function shall consist of the same sequence of tokens.
Note that for the purposes of this rule, typedefs shall be treated as types.
Comments should not be nested
Comments should not be nested
C++ does not support the nesting of C-style comments even though some compilers support this as a non-portable language extension.
A comment beginning with `/* continues until the first */ is encountered.
Any /*` occurring inside a comment is a violation of this rule.
Return type of functions shouldnt be const qualified value
Return type of functions shouldnt be const qualified value
There is no reason to add a const qualifier to the return type of a function that returns by value.
At best, it will be superfluous. At worst, it will be a performance bug: it can prevent move operation (when copy elision doesn’t take place). When an object is const-qualified the copy constructor/assignment will be a better match than the move constructor/assignment.
One might think about adding this qualifier in order to forbid the call of unintended functions on the returned object. A common example is to avoid unintended assignments:
X x1, x2, x3; if (x1 + x2 = x3) \{ // Compiler will complain since const object cannot be assigned. Should be "x1 + x2
Functions having rvalue reference arguments should std::move those arguments
Functions having rvalue reference arguments should std::move those arguments
Rvalue reference arguments allow the efficient transfer of the ownership of objects. Therefore, it is expected that rvalue arguments or their subobjects are, conditionally or not, moved into their destination variables.
The ownership is unclear when an rvalue argument, including its subobject or elements, is never moved. This might lead to bugs.
This issue can be resolved in multiple ways:
Generally, std::move can be used to move such arguments;
For containers, C++23 std::views::as_rvalue can be used to move their elements;
It is also possible to use a range-based for loop to move elements.
This rule does not apply when the argument is a forwarding reference.
Use type-erased coroutine_handle when applicable
Use type-erased coroutine_handle when applicable
The implementation of the await_suspend method accepts the handle to the suspended coroutine as the parameter. This parameter can be defined with either specific promise type coroutine_handle<PromiseType> or type erased coroutine_handle<>. The former allows await_suspend to access the promise of the coroutine; however, it ties the implementation to a particular type. In contrast, using coroutine_handle<> increases the reusability of the code because this parameter type supports all promise types.
This rule raises an issue for the implementation of await_suspend that accepts handles to a specific promise type and yet does not use that information.
sizeof(sizeof(...)) should not be used
sizeof(sizeof(...)) should not be used
Given an expression e of type T, sizeof(e) returns the size in bytes of T. The sizeof operator results in a value of type size_t. Also, sizeof(e) has no side effects because e is not evaluated. Therefore, sizeof(sizeof(e)) is equivalent to sizeof(size_t).
In other words, sizeof(sizeof(e)) always gives the same result and does not depend on e, which is unlikely what was expected.
std::midpoint and std::lerp should be used for midpoint computation and linear interpolation
std::midpoint and std::lerp should be used for midpoint computation and linear interpolation
C++20 introduced the standard algorithms to compute the midpoint between two values and linear interpolation for a given coefficient.
`std::midpoint(a, b) computes the midpoint, or average, or arithmetic mean of two values a and b: (a+b)/2. The result is halfway from a to b, and if a and b are pointers, it points to the middle of a contiguous memory segment between the two. A naive midpoint computation might suffer from a possible overflow or be inefficient. That is why, in most cases, std::midpoint is preferable.
std::lerp(a, b, t) returns linear interpolation between values a and b with a coefficient t: a+t*(a-b), where t is between 0 and 1.
This rule reports computations that should be replaced with std::midpoint or std::lerp`.
[[nodiscard]] should be used when the return value of a function should not be ignored
[[nodiscard]] should be used when the return value of a function should not be ignored
`C++17 introduced [[nodiscard]] attribute. When you declare a function [[nodiscard]], you indicate that its return value should not be ignored. This can help prevent bugs related to:
Memory leak, in case the function returns a pointer to unmanaged memory
Performance, in case the discarded value is costly to construct
Security, in case the return value indicates an error condition that needs to be taken into account
If the return value is ignored, the compiler is encouraged to issue a warning. Also, our analyzer will raise an issue, see S5277.
Note that you can declare an enumeration or class nodiscard. In that case, the compiler will warn if the ignored value is coming from a function that returns a nodiscard enumeration or class by value.
This rule will suggest adding the [[nodiscard]]` attribute to functions with no side effects that return a value.
Template parameters should be preferred to std::function when configuring behavior at compile time
Template parameters should be preferred to std::function when configuring behavior at compile time
To parametrize an algorithm with a function, you can use one of the following techniques:
A function pointer
A typed-erased function wrapper such as std::function (C++11) or std::move_only_function (C++23)
A template parameter
S5205 explains why using function pointers is an inferior solution.
Using publicly writable directories is security-sensitive
Using publicly writable directories is security-sensitive
Operating systems have global directories where any user has write access. Those folders are mostly used as temporary storage areas like `/tmp in Linux based systems. An application manipulating files from these folders is exposed to race conditions on filenames: a malicious user can try to create a file with a predictable name before the application does. A successful attack can result in other files being accessed, modified, corrupted or deleted. This risk is even higher if the application runs with elevated permissions.
In the past, it has led to the following vulnerabilities:
This rule raises an issue whenever it detects a hard-coded path to a publicly writable directory like /tmp (see examples bellow). It also detects access to environment variables that point to publicly writable directories, e.g., TMP and TMPDIR.
/tmp
/var/tmp
/usr/tmp
/dev/shm
/dev/mqueue
/run/lock
/var/run/lock
/Library/Caches
/Users/Shared
/private/tmp
/private/var/tmp
\Windows\Temp
\Temp
\TMP`
Statements should be on separate lines
Statements should be on separate lines
Putting multiple statements on a single line lowers the code readability and makes debugging the code more complex.
Unresolved directive in <stdin> - include::{noncompliant}[]
Write one statement per line to improve readability.
Unresolved directive in <stdin> - include::{compliant}[]
A function should have a single point of exit at the end of the function
A function should have a single point of exit at the end of the function
This is required by IEC 61508, under good programming style.
Expressions should not be too complex
Expressions should not be too complex
The complexity of an expression is defined by the number of &&, || and condition ? ifTrue : ifFalse
operators it contains.
A single expression’s complexity should not become too high to keep the code readable.
Math operands should be cast before assignment
Math operands should be cast before assignment
When arithmetic is performed on integers, the result will always be an integer. You can assign that result to a `long, double, or float with automatic type conversion, but having started as an int or long, the result will likely not be what you expect.
For instance, if the result of int division is assigned to a floating-point variable, precision will have been lost before the assignment. Likewise, if the result of multiplication is assigned to a long`, it may have already overflowed before the assignment.
In either case, the result will not be what was expected. Instead, at least one operand should be cast or promoted to the final type before the operation takes place.
continue should not be used
continue should not be used
continue is an unstructured control flow statement. It makes code less testable, less readable and less maintainable. Structured control flow statements such as if
should be used instead.
Array values should not be replaced unconditionally
Array values should not be replaced unconditionally
Comparisons should only be made in the context of boolean expressions
Comparisons should only be made in the context of boolean expressions
The use of a comparison operator outside of a boolean context is an error. At best it is meaningless code, and should be eliminated. However the far more likely scenario is that it is an assignment gone wrong, and should be corrected.
General catch clauses should not be used
General catch clauses should not be used
A general catch
block seems like an efficient way to handle multiple possible exceptions. Unfortunately, it traps all exception types, casting too broad a net, and perhaps mishandling extraordinary cases. Instead, specific exception sub-types should be caught.
Standard outputs should not be used directly to log anything
Standard outputs should not be used directly to log anything
In software development, logs serve as a record of events within an application, providing crucial insights for debugging. When logging, it is essential to ensure that the logs are:
easily accessible
uniformly formatted for readability
properly recorded
securely logged when dealing with sensitive data
Those requirements are not met if a program directly writes to the standard outputs (e.g., {language_std_outputs}). That is why defining and using a dedicated logger is highly recommended.
Magic numbers should not be used
Magic numbers should not be used
Magic numbers make the code more complex to understand as it requires the reader to have knowledge about the global context to understand the number itself. Their usage may seem obvious when writing the code, but it may not be the case for another developer or later once the context faded away. -1, 0, and 1 are not considered magic numbers.
Deprecated code should be removed
Deprecated code should be removed
This rule is meant to be used as a way to track code which is marked as being deprecated. Deprecated code should eventually be removed.
Using pseudorandom number generators (PRNGs) is security-sensitive
Using pseudorandom number generators (PRNGs) is security-sensitive
Using pseudorandom number generators (PRNGs) is security-sensitive. For example, it has led in the past to the following vulnerabilities:
When software generates predictable values in a context requiring unpredictability, it may be possible for an attacker to guess the next value that will be generated, and use this guess to impersonate another user or access sensitive information.
Exceptions should not be ignored
Exceptions should not be ignored
When exceptions occur, it is usually a bad idea to simply ignore them. Instead, it is better to handle them properly, or at least to log them.
Expanding archive files without controlling resource consumption is security-sensitive
Expanding archive files without controlling resource consumption is security-sensitive
Successful Zip Bomb attacks occur when an application expands untrusted archive files without controlling the size of the expanded data, which can lead to denial of service. A Zip bomb is usually a malicious archive file of a few kilobytes of compressed data but turned into gigabytes of uncompressed data. To achieve this extreme compression ratio, attackers will compress irrelevant data (eg: a long string of repeated bytes).
Mergeable if statements should be combined
Mergeable if statements should be combined
Nested code - blocks of code inside blocks of code - is eventually necessary, but increases complexity. This is why keeping the code as flat as possible, by avoiding unnecessary nesting, is considered a good practice.
Merging if statements when possible will decrease the nesting of the code and improve its readability.
Methods should not be empty
Methods should not be empty
An empty {operationName} is generally considered bad practice and can lead to confusion, readability, and maintenance issues. Empty {operationName}s bring no functionality and are misleading to others as they might think the {operationName} implementation fulfills a specific and identified requirement.
There are several reasons for a {operationName} not to have a body:
It is an unintentional omission, and should be fixed to prevent an unexpected behavior in production.
It is not yet, or never will be, supported. In this case an exception should be thrown.
The method is an intentionally-blank override. In this case a nested comment should explain the reason for the blank override.
Literal suffix L for long integers shall be upper case
Literal suffix L for long integers shall be upper case
Using upper case literal suffixes removes the potential ambiguity between “1” (digit 1) and “l” (letter el) for declaring literals.
Using clear-text protocols is security-sensitive
Using clear-text protocols is security-sensitive
Clear-text protocols such as `ftp, telnet, or http lack encryption of transported data, as well as the capability to build an authenticated connection. It means that an attacker able to sniff traffic from the network can read, modify, or corrupt the transported content. These protocols are not secure as they expose applications to an extensive range of risks:
sensitive data exposure
traffic redirected to a malicious endpoint
malware-infected software update or installer
execution of client-side code
corruption of critical information
Even in the context of isolated networks like offline environments or segmented cloud environments, the insider threat exists. Thus, attacks involving communications being sniffed or tampered with can still happen.
For example, attackers could successfully compromise prior security layers by:
bypassing isolation mechanisms
compromising a component of the network
getting the credentials of an internal IAM account (either from a service account or an actual person)
In such cases, encrypting communications would decrease the chances of attackers to successfully leak data or steal credentials from other network components. By layering various security practices (segmentation and encryption, for example), the application will follow the defense-in-depth principle.
Note that using the http` protocol is being deprecated by major web browsers.
In the past, it has led to the following vulnerabilities:
Macro names should comply with a naming convention
Macro names should comply with a naming convention
Sharing some naming conventions is a key point to make it possible for a team to efficiently collaborate. This rule allows to check that all macro names match a provided regular expression.
Redundant pairs of parentheses should be removed
Redundant pairs of parentheses should be removed
The use of parentheses, even those not required to enforce a desired order of operations, can clarify the intent behind a piece of code. However, redundant pairs of parentheses could be misleading and should be removed.
Unused private fields should be removed
Unused private fields should be removed
If a private field is declared but not used locally, its limited visibility makes it dead code.
This is either a sign that some logic is missing or that the code should be cleaned.
Cleaning out dead code decreases the size of the maintained codebase, making it easier to understand and preventing bugs from being introduced.
Redundant casts should not be used
Redundant casts should not be used
Casting expressions are utilized to convert one data type to another, such as transforming an integer into a string. This is especially crucial in strongly typed languages like C, C++, C#, Java, Python, and others.
However, there are instances where casting expressions are not needed. These include situations like:
casting a variable to its own type
casting a subclass to a parent class (in the case of polymorphism)
the programming language is capable of automatically converting the given type to another
These scenarios are considered unnecessary casting expressions. They can complicate the code and make it more difficult to understand, without offering any advantages.
As a result, it’s generally advised to avoid unnecessary casting expressions. Instead, rely on the language’s type system to ensure type safety and code clarity.
switch statements should not contain non-case labels
switch statements should not contain non-case labels
Even if it is legal, mixing case and non-case labels in the body of a switch statement is very confusing and can even be the result of a typing error.
goto statements should not be used to jump into blocks
goto statements should not be used to jump into blocks
Use of `goto can lead to programs that are extremely difficult to comprehend and analyse, and possibly to unspecified behavior.
Unfortunately, removing goto from some code can lead to a rewritten version that is even more difficult to understand than the original. Therefore, limited use of goto is sometimes advised.
However, the use of goto to jump into or out of a sub-block of code, such as into the body of a for` loop is never acceptable, because it is extremely difficult to understand and will likely yield results other than what is intended.
Related if/else if statements should not have the same condition
Related if/else if statements should not have the same condition
Unused function parameters should be removed
Unused function parameters should be removed
A typical code smell known as unused function parameters refers to parameters declared in a function but not used anywhere within the function’s body. While this might seem harmless at first glance, it can lead to confusion and potential errors in your code. Disregarding the values passed to such parameters, the function’s behavior will be the same, but the programmer’s intention won’t be clearly expressed anymore. Therefore, removing function parameters that are not being utilized is considered best practice.
break statements should not be used except for switch cases
break statements should not be used except for switch cases
break;
is an unstructured control flow statement which makes code harder to read.
Ideally, every loop should have a single termination condition.
Variables should not be self-assigned
Variables should not be self-assigned
There is no reason to re-assign a variable to itself. Either this statement is redundant and should be removed, or the re-assignment is a mistake and some other value or variable was intended for the assignment instead.
Variable names should comply with a naming convention
Variable names should comply with a naming convention
Sharing some naming conventions is a key point to make it possible for a team to efficiently collaborate. This rule allows to check that all variable names match a provided regular expression.
if ... else if constructs should end with else clauses
if ... else if constructs should end with else clauses
This rule applies whenever an `if statement is followed by one or more else if statements; the final else if should be followed by an else statement.
The requirement for a final else statement is defensive programming.
The else statement should either take appropriate action or contain a suitable comment as to why no action is taken. This is consistent with the requirement to have a final default clause in a switch` statement.
for loop stop conditions should be invariant
for loop stop conditions should be invariant
A `for loop stop condition should test the loop counter against an invariant value (i.e. one that is true at both the beginning and ending of every loop iteration). Ideally, this means that the stop condition is set to a local variable just before the loop begins.
Stop conditions that are not invariant are slightly less efficient, as well as being difficult to understand and maintain, and likely lead to the introduction of errors in the future.
This rule tracks three types of non-invariant stop conditions:
When the loop counters are updated in the body of the for` loop
When the stop condition depend upon a method call
When the stop condition depends on an object property, since such properties could change during the execution of the loop.
Enumeration names should comply with a naming convention
Enumeration names should comply with a naming convention
Shared naming conventions allow teams to collaborate efficiently. This rule checks that all enum
names match a provided regular expression.
Unused labels should be removed
Unused labels should be removed
If a label is declared but not used in the program, it can be considered as dead code and should therefore be removed.
This will improve maintainability as developers will not wonder what this label is used for.
Conditional operators should not be nested
Conditional operators should not be nested
Nested ternaries are hard to read and can make the order of operations complex to understand.
Unresolved directive in <stdin> - include::{noncompliant}[]
Instead, use another line to express the nested operation in a separate statement.
Unresolved directive in <stdin> - include::{compliant}[]
Non-empty statements should change control flow or have at least one side-effect
Non-empty statements should change control flow or have at least one side-effect
When writing code, it is important to ensure that each statement serves a purpose and contributes to the overall functionality of the program. When they have no side effects or do not change the control flow, they can either indicate a programming error or be redundant:
The code does not behave as intended: The statements are expected to have an effect but they do not. This can be caused by mistyping, copy-and-paste errors, etc.
The statements are residual after a refactoring.
Method overloads should be grouped together in the interface
Method overloads should be grouped together in the interface
For clarity, all overloads of the same method should be grouped together. That lets both users and maintainers quickly understand all the current available options.
Changing directories improperly when using chroot is security-sensitive
Changing directories improperly when using chroot is security-sensitive
The purpose of creating a jail, the “virtual root directory” created with chroot-type functions, is to limit access to the file system by isolating the process inside this jail. However, many chroot function implementations don’t modify the current working directory, thus the process has still access to unauthorized resources outside of the “jail”.
Neither DES (Data Encryption Standard) nor DESede (3DES) should be used
Neither DES (Data Encryption Standard) nor DESede (3DES) should be used
According to the US National Institute of Standards and Technology (NIST), the Data Encryption Standard (DES) is no longer considered secure:
For similar reasons, RC2 should also be avoided.
goto statement should not be used
goto statement should not be used
goto is an unstructured control flow statement. It makes code less readable and maintainable. Structured control flow statements such as if, for, while, continue or break
should be used instead.
static base class members should not be accessed via derived types
static base class members should not be accessed via derived types
In object-oriented programming, inappropriately accessing static members of a base class via derived types is considered a code smell.
Static members are associated with the class itself, not with any specific instance of the class or its children classes. Accessing through the wrong type suggests a misunderstanding of the ownership and role of this member. This can make the maintenance of the code more complicated.
Therefore, the access should be done directly through the base class to maintain clarity and avoid potential misunderstandings.
Values should not be uselessly incremented
Values should not be uselessly incremented
A value that is incremented or decremented and then not stored is at best wasted code and at worst a bug.
Code annotated as deprecated should not be used
Code annotated as deprecated should not be used
Code is sometimes annotated as deprecated by developers maintaining libraries or APIs to indicate that the method, class, or other programming element is no longer recommended for use. This is typically due to the introduction of a newer or more effective alternative. For example, when a better solution has been identified, or when the existing code presents potential errors or security risks.
Deprecation is a good practice because it helps to phase out obsolete code in a controlled manner, without breaking existing software that may still depend on it. It is a way to warn other developers not to use the deprecated element in new code, and to replace it in existing code when possible.
Deprecated classes, interfaces, and their members should not be used, inherited or extended because they will eventually be removed. The deprecation period allows you to make a smooth transition away from the aging, soon-to-be-retired technology.
Check the documentation or the deprecation message to understand why the code was deprecated and what the recommended alternative is.
Comments should not be located at the end of lines of code
Comments should not be located at the end of lines of code
This rule verifies that single-line comments are not located at the ends of lines of code. The main idea behind this rule is that in order to be really readable, trailing comments would have to be properly written and formatted (correct alignment, no interference with the visual structure of the code, not too long to be visible) but most often, automatic code formatters would not handle this correctly: the code would end up less readable. Comments are far better placed on the previous empty line of code, where they will always be visible and properly formatted.
static members should be accessed statically
static members should be accessed statically
While it is possible to access static
members from a class instance, it’s bad form, and considered by most to be misleading because it implies to the readers of your code that there’s an instance of the member per class instance.
Methods should not have identical implementations
Methods should not have identical implementations
Two {func_name}s having the same implementation are suspicious. It might be that something else was intended. Or the duplication is intentional, which becomes a maintenance burden.
Using hardcoded IP addresses is security-sensitive
Using hardcoded IP addresses is security-sensitive
Hardcoding IP addresses is security-sensitive. It has led in the past to the following vulnerabilities:
Today’s services have an ever-changing architecture due to their scaling and redundancy needs. It is a mistake to think that a service will always have the same IP address. When it does change, the hardcoded IP will have to be modified too. This will have an impact on the product development, delivery, and deployment:
The developers will have to do a rapid fix every time this happens, instead of having an operation team change a configuration file.
It misleads to use the same address in every environment (dev, sys, qa, prod).
Last but not least it has an effect on application security. Attackers might be able to decompile the code and thereby discover a potentially sensitive address. They can perform a Denial of Service attack on the service, try to get access to the system, or try to spoof the IP address to bypass security checks. Such attacks can always be possible, but in the case of a hardcoded IP address solving the issue will take more time, which will increase an attack’s impact.
Track uses of TODO tags
Track uses of TODO tags
All branches in a conditional structure should not have exactly the same implementation
All branches in a conditional structure should not have exactly the same implementation
Having all branches of a switch or if chain with the same implementation indicates a problem.
In the following code:
Unresolved directive in <stdin> - include::{example}[]
Either there is a copy-paste error that needs fixing or an unnecessary switch or if chain that should be removed.
Nested code blocks should not be used
Nested code blocks should not be used
Nested code blocks create new scopes where variables declared within are inaccessible from the outside, and their lifespan ends with the block.
Although this may appear beneficial, their usage within a function often suggests that the function is overloaded. Thus, it may violate the Single Responsibility Principle, and the function needs to be broken down into smaller functions.
The presence of nested blocks that don’t affect the control flow might suggest possible mistakes in the code.
Field names should comply with a naming convention
Field names should comply with a naming convention
A naming convention in software development is a set of guidelines for naming code elements like variables, functions, and classes.
The goal of a naming convention is to make the code more readable and understandable, which makes it easier to maintain and debug. It also ensures consistency in the code, especially when multiple developers are working on the same project.
This rule checks that field names match a provided regular expression.
Unused functions and methods should be removed
Unused functions and methods should be removed
This rule raises an issue when a {visibility} {operationName} is never referenced in the code.
Setting loose POSIX file permissions is security-sensitive
Setting loose POSIX file permissions is security-sensitive
In Unix file system permissions, the “others
” category refers to all
users except the owner of the file system resource and the members of the group
assigned to this resource.
Granting permissions to this category can lead to unintended access to files or directories that could allow attackers to obtain sensitive information, disrupt services or elevate privileges.
Empty statements should be removed
Empty statements should be removed
Empty statements represented by a semicolon ; are statements that do not perform any operation. They are often the result of a typo or a misunderstanding of the language syntax. It is a good practice to remove empty statements since they don’t add value and lead to confusion and errors.
Child class fields should not shadow parent class fields
Child class fields should not shadow parent class fields
Having a variable with the same name in two unrelated classes is fine, but do the same thing within a class hierarchy and you’ll get confusion at best, chaos at worst.
Switch cases should end with an unconditional break statement
Switch cases should end with an unconditional break statement
When the execution is not explicitly terminated at the end of a switch case, it continues to execute the statements of the following case. While this is sometimes intentional, it often is a mistake which leads to unexpected behavior.
Parameters should be passed in the correct order
Parameters should be passed in the correct order
When the names of parameters in a method call match the names of the method arguments, it contributes to clearer, more readable code. However, when the names match, but are passed in a different order than the method arguments, it indicates a mistake in the parameter order which will likely lead to unexpected results.
Using weak hashing algorithms is security-sensitive
Using weak hashing algorithms is security-sensitive
Cryptographic hash algorithms such as MD2, MD4, MD5, MD6, HAVAL-128, HMAC-MD5, DSA (which uses SHA-1), RIPEMD, RIPEMD-128, RIPEMD-160, HMACRIPEMD160 and SHA-1 are no longer considered secure, because it is possible to have collisions
(little computational effort is enough to find two or more different inputs that produce the same hash).
The ternary operator should not be used
The ternary operator should not be used
Ternary expressions, while concise, can often lead to code that is difficult to read and understand, especially when they are nested or complex. Prioritizing readability fosters maintainability and reduces the likelihood of bugs. Therefore, they should be removed in favor of more explicit control structures, such as if/else statements, to improve the clarity and readability of the code.
Hard-coded credentials are security-sensitive
Hard-coded credentials are security-sensitive
Because it is easy to extract strings from an application source code or binary, credentials should not be hard-coded. This is particularly true for applications that are distributed or that are open-source.
In the past, it has led to the following vulnerabilities:
Credentials should be stored outside of the code in a configuration file, a database, or a management service for secrets.
This rule flags instances of hard-coded credentials used in database and LDAP connections. It looks for hard-coded credentials in connection strings, and for variable names that match any of the patterns from the provided list.
It’s recommended to customize the configuration of this rule with additional credential words such as “oauthToken”, “secret”, …
switch statements should not be nested
switch statements should not be nested
Nested `switch structures are difficult to understand because you can easily confuse the cases of an inner switch as belonging to an outer statement. Therefore nested switch statements should be avoided.
Specifically, you should structure your code to avoid the need for nested switch statements, but if you cannot, then consider moving the inner switch` to another function.
Unused local variables should be removed
Unused local variables should be removed
An unused local variable is a variable that has been declared but is not used anywhere in the block of code where it is defined. It is dead code, contributing to unnecessary complexity and leading to confusion when reading the code. Therefore, it should be removed from your code to maintain clarity and efficiency.
Exceptions should not be thrown in finally blocks
Exceptions should not be thrown in finally blocks
If an exception is already being thrown within the try block or caught in a catch block, throwing another exception in the finally block will override the original exception. This means that the original exception’s message and stack trace will be lost, potentially making it challenging to diagnose and troubleshoot the root cause of the problem.
Cognitive Complexity of functions should not be too high
Cognitive Complexity of functions should not be too high
Cognitive Complexity is a measure of how hard it is to understand the control flow of a unit of code. Code with high cognitive complexity is hard to read, understand, test, and modify.
As a rule of thumb, high cognitive complexity is a sign that the code should be refactored into smaller, easier-to-manage pieces.
Boolean expressions should not be gratuitous
Boolean expressions should not be gratuitous
Control flow constructs like if-statements allow the programmer to direct the flow of a program depending on a boolean expression. However, if the condition is always true or always false, only one of the branches will ever be executed. In that case, the control flow construct and the condition no longer serve a purpose; they become gratuitous.
Local variable and function parameter names should comply with a naming convention
Local variable and function parameter names should comply with a naming convention
A naming convention in software development is a set of guidelines for naming code elements like variables, functions, and classes. {identifier_capital_plural} hold the meaning of the written code. Their names should be meaningful and follow a consistent and easily recognizable pattern. Adhering to a consistent naming convention helps to make the code more readable and understandable, which makes it easier to maintain and debug. It also ensures consistency in the code, especially when multiple developers are working on the same project.
This rule checks that {identifier} names match a provided regular expression.
Variables should not be accessed outside of their scope
Variables should not be accessed outside of their scope
Variables should not be accessed outside of their scope
Prefer not to overload virtual functions
Prefer not to overload virtual functions
Prefer not to overload virtual functions
Variables should not be shadowed
Variables should not be shadowed
Variables should not be shadowed
Line-splicing should not be used in // comments
Line-splicing should not be used in // comments
Line-splicing should not be used in ”//” comments
Line continuation characters \ should not be followed by trailing whitespace
Line continuation characters \ should not be followed by trailing whitespace
Line continuation characters ” should not be followed by trailing whitespace
memcmp should only be called with pointers to trivially copyable types with no padding
memcmp should only be called with pointers to trivially copyable types with no padding
”memcmp” should only be called with pointers to trivially copyable types with no padding
memcpy, memmove, and memset should only be called with pointers to trivially copyable types
memcpy, memmove, and memset should only be called with pointers to trivially copyable types
”memcpy”, “memmove”, and “memset” should only be called with pointers to trivially copyable types
Use either the
// … or /* … */
comment syntax, but be consistent and do not mix them within the same file.