Skip to content

Latest commit

 

History

History
380 lines (378 loc) · 12.1 KB

Checks.md

File metadata and controls

380 lines (378 loc) · 12.1 KB

ICA checks

Check name Description Example

bad-rand

 Detects:
  • usage of std::rand (which is not thread-safe)
  • usage of std::random_shuffle (deprecated in C++14, removed in C++17)
  • using constants as seed
  • generation of multiple numbers by std::random_device (which may be a long operation as it's execution time isn't specified)
  • lack of std::*_distribution usage
  • one-shot usage of random engine in function
    		•  
    // good example
struct random_struct
{
    random_struct()
        : m_rand_engine(std::random_device{}()) // construct random device and call to get seed for random engine construction
    {}
    int get_rand(int from, int to)
    {
        std::uniform_int_distribution distribution(from, to); // distribution construction is quite cheap
        return distribution(m_rand_engine);
    }
private:
    mutable std::mt19937 m_rand_engine;
};

    char-in-ctype-pred

    		 Detects when char or signed char is passed to the predicate functions from <cctype> (or <ctype.h>), which may be undefined befavior. From cppreference

    "Like all other functions from <cctype>, the behavior of std::isalpha is undefined if the argument's value is neither representable as unsigned char nor equal to EOF. To use these functions safely with plain chars (or signed chars), the argument should first be converted to unsigned char" 
    char c = getchar();
bool b1 = std::isalpha(c); // warning
bool b2 = std::isalpha(static_cast<unsigned char>(c)); // OK

    const-cast-member

    		 Detects if const_cast is applied to a non-static field. Usage of mutable is preferred 
    struct Struct
{
    int get_num() const
    {
        std::uniform_int_distribution dist(0, 10);
        return dist(const_cast<std::mt19937 &>(m_rand_engine)); // warning
    }
private:
    std::mt19937 m_rand_engine; // assume it's properly initialized
};

    const-param

    		 Detects non-modified non-const function parameters and suggests to insert const. 
    void ok(const int & i) {} // OK
void param_could_be_const(int & i) {} // warning
void rvalue_is_never_moved(int && i) {} // warning

    emplace-default-value

    		 Detects, when default-constructed value is passed to map `emplace` method and suggests to use `try_emplace` method instead 
    std::unordered_map<int, std::string> mis;
mis.emplace(0, std::string{}); // warning
mis.try_emplace(0); // OK
mis.emplace(1, std::string{"one"}); // OK

    erase-in-loop

    		 Detects when `erase` is called on iterator which is incremented in a for loop. Majority of containers invalidates an iterator which is erased. Does not produce a warning if `return` or `break` statement is present at the same level as `erase`. 
    std::map<int, bool> m;
auto it = m.begin(), end = m.end();
for (; it != end; ++it) {
    if (it->first == 123) {
        m.erase(it); // warning
    }
}
auto it = m.begin(), end = m.end();
for (; it != end; ++it) {
    if (it->first == 123) {
        m.erase(it); // OK
        break;
    }
}

    find-emplace

    		 Detects `find` + `insert` pattern for STL-like containers. This is inefficient because `find` tries to find an element, traversing the container and then `insert` traverses the container one more time. In most cases this could be replaced with one `insert` or `emplace` 
    // Inefficient:
auto it = map.find(key);
if (it == map.end()) {
   map.insert({key, value});
} else {
   it->second = value;
}
 // Efficient:
auto [it, inserted] = map.emplace(key, value);
if (!inserted) {
   it->second = value;
}

    for-range-const

    		 Detects if `const` qualifier can be added to initialization stmt in for-range loop. Doesn't warn if the variable is used for template deduction 
    std::vector<int> from;
std::vector<int> to;
for (auto & el : from) { // warning
    to.push_back(el); // causes warning
}

    improper-move

    		 This check extends clang-tidy: [performance-move-const-arg](https://clang.llvm.org/extra/clang-tidy/checks/performance-move-const-arg.html).
    Detects:
  • move from non-const lvalue (may invalidate provided data)
  • if std::move argument is already rvalue
  • move to const lvalue reference
  • const rvalue parameters
    		•  
    template <class T>
auto get_rvalue(T & t) { return std::move(t); } // 'move' invalidates 't'
void foo(const std::string & s)
{
    auto substr = std::move(s.substr(1)); // warning: move of rvalue
    foo(std::move(substr)); // warning: move to const lvalue reference
}
void boo(const std::string && s) {} // warning: const rvalue parameter

    init-members

    		 Detects if member can be initialized in initialization list in constructor instead of assignment within body. 
    struct Struct
{
    Struct (const int * arr, std::size_t size)
        : m_data(arr, arr + size) // good
    {
        m_size = size; // bad, warning
    }
private:
    std::vector<int> m_data;
    std::size_t m_size;
};

    inline-methods-in-class

    		 Class method is inline by default, so an explicit keyword isn't needed.
    Detects, when an inline keyword is used for a method within class body. 
    class Foo {
public:
    inline void method();  // warning
    void another_method(); // OK
};

    move-string-stream

    		 In C++20 standard str on std::stringstream && got it's own overload, which should be cheaper then just copy if internal buffer.
    Detects calls to std::stringstream::str where std::move could be used and suggests it. 
    void foo(std::string);
...
{
    std::stringstream ss;
    ss << "Hello, world!\n";
    foo(ss.str()); // warning
}
{
    std::stringstream ss;
    ss << "Goodbye, world!\n";
    foo(std::move(ss).str()); // OK
}

    redundant-noexcept

    		 Checks noexcept specified functions for calls of potentially throwing functions which bodies aren't seen in current translation unit, as it causes insertion of std::terminate 
    void call_new() noexcept // warning
{
    const char * str = new char[4]{"lol"}; // warning: 'new' is potentially throwing
}

    release-lock

    		 Detects (unique|shared)_lock::release without unlock call on returned mutex. 
    void bad_func()
{
    std::mutex m;
    std::unique_lock l{m};
l.release(); // warning
}
void good_func()
{
    std::mutex m;
    std::unique_lock l{m};
   auto m_ptr = l.release(); // OK (unlocked afterwards)
    m_ptr->unlock();
}

    remove-c_str

    		 Detects if a class method that accepts const char * is called with calling std::string::c_str and there is an overload that accepts std::string_view. Suggests removing c_str call; 
    class Foo {
void func(const char *);
void func(std::string_view);
};
Foo f;
std::string key = "key";
f.func(key.c_str()); // warning
f.func(key); // ok

    return-value-type

    		 Detects:
  • functions that return const T, suggests returning either const T & or T instead
  • functions that do return *this; but their return type is T, suggests using const T & return type.
    		•  
    const int foo() { return 42; } // warning
struct Iterator
{
    Iterator operator ++ () // warning
    {
        inc();
        return *this;
    }
private:
    void inc();
};

    static-keyword

    		 Detects:
  • global variables in headers (may cause problems with initialization ordering)
  • declarations of functions, local for translation unit
    		•  
    // file.h
#pragma once
inline static int a; // warning
namespace { int b; } // warning
// file.cpp
static int c = 42; // OK, but will probably be judged
static void local_func() {} // warning

    temporary-in-ctor

    		 Detects if a temporary variable is used instead of direct writing into non-static field. 
    struct Struct
{
    Struct(const int * arr, std::size_t size)
    {
        std::vector<int> to_fill; // warning
        if (arr) {
            for (std::size_t i = 0; i < size; ++i) {
                to_fill.push_back(arr[i]);
            }
            data = std::move(to_fill);
        }
    }
private:
    std::vector<int> data;
};

    try_emplace-instead-emplace

    		 In some standard library implementations emplace call always constructs a map node, which is inefficient.
    Detects calls of map emplace method and suggests using try_emplace instead. NB! sometimes it implies construction of key temporary object, which may be more undesirable. 
    std::map<int, std::string> map;
map.emplace(1, "one"); // warning: will create a node first and find a place for it
map.try_emplace(1, "one"); // OK: will only construct a node if finds proper place