Если регулярно использовать статический анализатор кода, то можно сократить время на гадание, почему новый код работает как-то не так, как задумывалось. Рассмотрим очередную интересную ошибку, когда в процессе рефакторинга сломалась функция и это осталось не замеченным человеком.
Думал сделать перерыв в серии маленьких заметок про то, как анализатор PVS-Studio оперативно находит ошибки в новом коде. Но, увидев в почте отчёт от системы автоматической проверки проекта Blender, не выдержал. Предлагаю вашему вниманию очередную простую, но именно из-за этого красивую ошибку.
Итак, жил-был вот такой код для обработки вектора значений. Суть — не дать значениям выходить за определённый диапазон.
#define CLAMP(a, b, c) \
{ \
if ((a) < (b)) { \
(a) = (b); \
} \
else if ((a) > (c)) { \
(a) = (c); \
} \
} \
(void)0
template <typename T> inline T
clamp(const T &a, const bT &min_v, const bT &max_v)
{
T result = a;
for (int i = 0; i < T::type_length; i++) {
CLAMP(result[i], min_v, max_v);
}
return result;
}И было всё хорошо. А потом программист решил, что нет смысла использовать самодельный макрос CLAMP, а лучше воспользоваться стандартной функцией std::clamp. И в коммите, призванном сделать мир лучше, код стал таким:
template <typename T, int Size>
inline vec_base<T, Size>
clamp(const vec_base<T, Size> &a, const T &min, const T &max)
{
vec_base<T, Size> result = a;
for (int i = 0; i < Size; i++) {
std::clamp(result[i], min, max);
}
return result;
}Вот только поспешил. Видите ошибку? Возможно, видите, возможно, нет. В любом случае, программист, написавший этот код, явно не заметил, что код сломался.
Зато дотошный статический анализатор кода PVS-Studio тут как тут с сообщением:
[CWE-252] V530: The return value of function 'clamp' is required to be utilized. BLI_math_vector.hh 88
Дело в том, что функция std::clamp не меняет значение элемента в контейнере:
template <class T>
constexpr const T&
clamp( const T& v, const T& lo, const T& hi );Макрос CLAMP раньше изменял значение, а стандартная функция нет. Поэтому теперь код сломан и ждёт, когда кто-то заметит проявление ошибки и пойдёт искать её причину. Такую ошибку можно было бы заметить и исправить ещё на этапе написания кода, если бы использовался PVS-Studio. Не будьте как разработчики Blender :). Используйте статический анализ кода на регулярной основе. И вы сэкономите свои силы и время.
Примечание. Кстати, рядом в коде есть ещё одно неправильное использование std::clamp.
Правильный вариант кода:
template <typename T, int Size>
inline vec_base<T, Size>
clamp(const vec_base<T, Size> &a, const T &min, const T &max)
{
vec_base<T, Size> result = a;
for (int i = 0; i < Size; i++) {
result[i] = std::clamp(result[i], min, max);
}
return result;
}Спасибо за внимание. И, если ещё не видели, заглядывайте почитать про топ-10 ошибок в C++ открытых проектах, которые мы нашли за 2021 год.
Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. How PVS-Studio prevents rash code changes, example N4.
