Merge pull request #40739 from ClickHouse/clang-tidy-for-headers

Enable clang-tidy for headers
This commit is contained in:
Robert Schulze 2022-09-02 07:54:50 +02:00 committed by GitHub
commit c7c00f9002
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
20 changed files with 104 additions and 94 deletions

View File

@ -1,6 +1,14 @@
# To run clang-tidy from CMake, build ClickHouse with -DENABLE_CLANG_TIDY=1. To show all warnings, it is
# recommended to pass "-k0" to Ninja.
# Enable all checks + disale selected checks. Feel free to remove disabled checks from below list if
# a) the new check is not controversial (this includes many checks in readability-* and google-*) or
# b) too noisy (checks with > 100 new warnings are considered noisy, this includes e.g. cppcoreguidelines-*).
# TODO Let clang-tidy check headers in further directories
# --> HeaderFilterRegex: '^.*/(src|base|programs|utils)/.*(h|hpp)$'
HeaderFilterRegex: '^.*/(base)/.*(h|hpp)$'
Checks: '*,
-abseil-*,

View File

@ -52,15 +52,15 @@ struct Decimal
constexpr Decimal(Decimal<T> &&) noexcept = default;
constexpr Decimal(const Decimal<T> &) = default;
constexpr Decimal(const T & value_): value(value_) {}
constexpr Decimal(const T & value_): value(value_) {} // NOLINT(google-explicit-constructor)
template <typename U>
constexpr Decimal(const Decimal<U> & x): value(x.value) {}
constexpr Decimal(const Decimal<U> & x): value(x.value) {} // NOLINT(google-explicit-constructor)
constexpr Decimal<T> & operator=(Decimal<T> &&) noexcept = default;
constexpr Decimal<T> & operator = (const Decimal<T> &) = default;
constexpr operator T () const { return value; }
constexpr operator T () const { return value; } // NOLINT(google-explicit-constructor)
template <typename U>
constexpr U convertTo() const
@ -111,7 +111,7 @@ public:
using Base::Base;
using NativeType = Base::NativeType;
constexpr DateTime64(const Base & v): Base(v) {}
constexpr DateTime64(const Base & v): Base(v) {} // NOLINT(google-explicit-constructor)
};
}

View File

@ -36,14 +36,14 @@ struct DecomposedFloat
{
using Traits = FloatTraits<T>;
DecomposedFloat(T x)
explicit DecomposedFloat(T x)
{
memcpy(&x_uint, &x, sizeof(x));
}
typename Traits::UInt x_uint;
bool is_negative() const
bool isNegative() const
{
return x_uint >> (Traits::bits - 1);
}
@ -53,7 +53,7 @@ struct DecomposedFloat
{
return (exponent() == 0 && mantissa() == 0)
? 0
: (is_negative()
: (isNegative()
? -1
: 1);
}
@ -63,7 +63,7 @@ struct DecomposedFloat
return (x_uint >> (Traits::mantissa_bits)) & (((1ull << (Traits::exponent_bits + 1)) - 1) >> 1);
}
int16_t normalized_exponent() const
int16_t normalizedExponent() const
{
return int16_t(exponent()) - ((1ull << (Traits::exponent_bits - 1)) - 1);
}
@ -73,20 +73,20 @@ struct DecomposedFloat
return x_uint & ((1ull << Traits::mantissa_bits) - 1);
}
int64_t mantissa_with_sign() const
int64_t mantissaWithSign() const
{
return is_negative() ? -mantissa() : mantissa();
return isNegative() ? -mantissa() : mantissa();
}
/// NOTE Probably floating point instructions can be better.
bool is_integer_in_representable_range() const
bool isIntegerInRepresentableRange() const
{
return x_uint == 0
|| (normalized_exponent() >= 0 /// The number is not less than one
|| (normalizedExponent() >= 0 /// The number is not less than one
/// The number is inside the range where every integer has exact representation in float
&& normalized_exponent() <= static_cast<int16_t>(Traits::mantissa_bits)
&& normalizedExponent() <= static_cast<int16_t>(Traits::mantissa_bits)
/// After multiplying by 2^exp, the fractional part becomes zero, means the number is integer
&& ((mantissa() & ((1ULL << (Traits::mantissa_bits - normalized_exponent())) - 1)) == 0));
&& ((mantissa() & ((1ULL << (Traits::mantissa_bits - normalizedExponent())) - 1)) == 0));
}
@ -102,15 +102,15 @@ struct DecomposedFloat
return sign();
/// Different signs
if (is_negative() && rhs > 0)
if (isNegative() && rhs > 0)
return -1;
if (!is_negative() && rhs < 0)
if (!isNegative() && rhs < 0)
return 1;
/// Fractional number with magnitude less than one
if (normalized_exponent() < 0)
if (normalizedExponent() < 0)
{
if (!is_negative())
if (!isNegative())
return rhs > 0 ? -1 : 1;
else
return rhs >= 0 ? -1 : 1;
@ -121,11 +121,11 @@ struct DecomposedFloat
{
if (rhs == std::numeric_limits<Int>::lowest())
{
assert(is_negative());
assert(isNegative());
if (normalized_exponent() < static_cast<int16_t>(8 * sizeof(Int) - is_signed_v<Int>))
if (normalizedExponent() < static_cast<int16_t>(8 * sizeof(Int) - is_signed_v<Int>))
return 1;
if (normalized_exponent() > static_cast<int16_t>(8 * sizeof(Int) - is_signed_v<Int>))
if (normalizedExponent() > static_cast<int16_t>(8 * sizeof(Int) - is_signed_v<Int>))
return -1;
if (mantissa() == 0)
@ -136,44 +136,44 @@ struct DecomposedFloat
}
/// Too large number: abs(float) > abs(rhs). Also the case with infinities and NaN.
if (normalized_exponent() >= static_cast<int16_t>(8 * sizeof(Int) - is_signed_v<Int>))
return is_negative() ? -1 : 1;
if (normalizedExponent() >= static_cast<int16_t>(8 * sizeof(Int) - is_signed_v<Int>))
return isNegative() ? -1 : 1;
using UInt = std::conditional_t<(sizeof(Int) > sizeof(typename Traits::UInt)), make_unsigned_t<Int>, typename Traits::UInt>;
UInt uint_rhs = rhs < 0 ? -rhs : rhs;
/// Smaller octave: abs(rhs) < abs(float)
/// FYI, TIL: octave is also called "binade", https://en.wikipedia.org/wiki/Binade
if (uint_rhs < (static_cast<UInt>(1) << normalized_exponent()))
return is_negative() ? -1 : 1;
if (uint_rhs < (static_cast<UInt>(1) << normalizedExponent()))
return isNegative() ? -1 : 1;
/// Larger octave: abs(rhs) > abs(float)
if (normalized_exponent() + 1 < static_cast<int16_t>(8 * sizeof(Int) - is_signed_v<Int>)
&& uint_rhs >= (static_cast<UInt>(1) << (normalized_exponent() + 1)))
return is_negative() ? 1 : -1;
if (normalizedExponent() + 1 < static_cast<int16_t>(8 * sizeof(Int) - is_signed_v<Int>)
&& uint_rhs >= (static_cast<UInt>(1) << (normalizedExponent() + 1)))
return isNegative() ? 1 : -1;
/// The same octave
/// uint_rhs == 2 ^ normalized_exponent + mantissa * 2 ^ (normalized_exponent - mantissa_bits)
/// uint_rhs == 2 ^ normalizedExponent + mantissa * 2 ^ (normalizedExponent - mantissa_bits)
bool large_and_always_integer = normalized_exponent() >= static_cast<int16_t>(Traits::mantissa_bits);
bool large_and_always_integer = normalizedExponent() >= static_cast<int16_t>(Traits::mantissa_bits);
UInt a = large_and_always_integer
? static_cast<UInt>(mantissa()) << (normalized_exponent() - Traits::mantissa_bits)
: static_cast<UInt>(mantissa()) >> (Traits::mantissa_bits - normalized_exponent());
? static_cast<UInt>(mantissa()) << (normalizedExponent() - Traits::mantissa_bits)
: static_cast<UInt>(mantissa()) >> (Traits::mantissa_bits - normalizedExponent());
UInt b = uint_rhs - (static_cast<UInt>(1) << normalized_exponent());
UInt b = uint_rhs - (static_cast<UInt>(1) << normalizedExponent());
if (a < b)
return is_negative() ? 1 : -1;
return isNegative() ? 1 : -1;
if (a > b)
return is_negative() ? -1 : 1;
return isNegative() ? -1 : 1;
/// Float has no fractional part means that the numbers are equal.
if (large_and_always_integer || (mantissa() & ((1ULL << (Traits::mantissa_bits - normalized_exponent())) - 1)) == 0)
if (large_and_always_integer || (mantissa() & ((1ULL << (Traits::mantissa_bits - normalizedExponent())) - 1)) == 0)
return 0;
else
/// Float has fractional part means its abs value is larger.
return is_negative() ? -1 : 1;
return isNegative() ? -1 : 1;
}

View File

@ -38,6 +38,7 @@
*/
// NOLINTBEGIN(google-explicit-constructor)
#ifdef __clang__
# pragma clang diagnostic push
# pragma clang diagnostic ignored "-Wdeprecated-dynamic-exception-spec"
@ -46,6 +47,7 @@ POCO_DECLARE_EXCEPTION(Foundation_API, JSONException, Poco::Exception)
#ifdef __clang__
# pragma clang diagnostic pop
#endif
// NOLINTEND(google-explicit-constructor)
class JSON
{
@ -61,7 +63,7 @@ public:
checkInit();
}
JSON(const std::string & s) : ptr_begin(s.data()), ptr_end(s.data() + s.size()), level(0)
explicit JSON(std::string_view s) : ptr_begin(s.data()), ptr_end(s.data() + s.size()), level(0)
{
checkInit();
}
@ -71,13 +73,7 @@ public:
*this = rhs;
}
JSON & operator=(const JSON & rhs)
{
ptr_begin = rhs.ptr_begin;
ptr_end = rhs.ptr_end;
level = rhs.level;
return *this;
}
JSON & operator=(const JSON & rhs) = default;
const char * data() const { return ptr_begin; }
const char * dataEnd() const { return ptr_end; }
@ -169,7 +165,7 @@ public:
/// Перейти к следующему элементу массива или следующей name-value паре объекта.
iterator & operator++();
iterator operator++(int);
iterator operator++(int); // NOLINT(cert-dcl21-cpp)
/// Есть ли в строке escape-последовательности
bool hasEscapes() const;

View File

@ -3,6 +3,7 @@
#include <base/extended_types.h>
#include <base/defines.h>
// NOLINTBEGIN(google-runtime-int)
namespace common
{
@ -206,3 +207,5 @@ namespace common
return false;
}
}
// NOLINTEND(google-runtime-int)

View File

@ -1,6 +1,6 @@
#pragma once
#include <string.h>
#include <cstring>
#include <algorithm>
#include <type_traits>

View File

@ -143,8 +143,8 @@
/// Macros for suppressing TSA warnings for specific reads/writes (instead of suppressing it for the whole function)
/// Consider adding a comment before using these macros.
# define TSA_SUPPRESS_WARNING_FOR_READ(x) [&]() TSA_NO_THREAD_SAFETY_ANALYSIS -> const auto & { return (x); }()
# define TSA_SUPPRESS_WARNING_FOR_WRITE(x) [&]() TSA_NO_THREAD_SAFETY_ANALYSIS -> auto & { return (x); }()
# define TSA_SUPPRESS_WARNING_FOR_READ(x) ([&]() TSA_NO_THREAD_SAFETY_ANALYSIS -> const auto & { return (x); }())
# define TSA_SUPPRESS_WARNING_FOR_WRITE(x) ([&]() TSA_NO_THREAD_SAFETY_ANALYSIS -> auto & { return (x); }())
/// This macro is useful when only one thread writes to a member
/// and you want to read this member from the same thread without locking a mutex.

View File

@ -5,7 +5,6 @@
#include <base/types.h>
#include <base/wide_integer.h>
using Int128 = wide::integer<128, signed>;
using UInt128 = wide::integer<128, unsigned>;
using Int256 = wide::integer<256, signed>;
@ -18,7 +17,7 @@ static_assert(sizeof(UInt256) == 32);
/// (std::common_type), are "set in stone". Attempting to specialize them causes undefined behavior.
/// So instead of using the std type_traits, we use our own version which allows extension.
template <typename T>
struct is_signed
struct is_signed // NOLINT(readability-identifier-naming)
{
static constexpr bool value = std::is_signed_v<T>;
};
@ -30,7 +29,7 @@ template <typename T>
inline constexpr bool is_signed_v = is_signed<T>::value;
template <typename T>
struct is_unsigned
struct is_unsigned // NOLINT(readability-identifier-naming)
{
static constexpr bool value = std::is_unsigned_v<T>;
};
@ -51,7 +50,7 @@ template <class T> concept is_integer =
template <class T> concept is_floating_point = std::is_floating_point_v<T>;
template <typename T>
struct is_arithmetic
struct is_arithmetic // NOLINT(readability-identifier-naming)
{
static constexpr bool value = std::is_arithmetic_v<T>;
};
@ -66,9 +65,9 @@ template <typename T>
inline constexpr bool is_arithmetic_v = is_arithmetic<T>::value;
template <typename T>
struct make_unsigned
struct make_unsigned // NOLINT(readability-identifier-naming)
{
typedef std::make_unsigned_t<T> type;
using type = std::make_unsigned_t<T>;
};
template <> struct make_unsigned<Int128> { using type = UInt128; };
@ -79,9 +78,9 @@ template <> struct make_unsigned<UInt256> { using type = UInt256; };
template <typename T> using make_unsigned_t = typename make_unsigned<T>::type;
template <typename T>
struct make_signed
struct make_signed // NOLINT(readability-identifier-naming)
{
typedef std::make_signed_t<T> type;
using type = std::make_signed_t<T>;
};
template <> struct make_signed<Int128> { using type = Int128; };
@ -92,7 +91,7 @@ template <> struct make_signed<UInt256> { using type = Int256; };
template <typename T> using make_signed_t = typename make_signed<T>::type;
template <typename T>
struct is_big_int
struct is_big_int // NOLINT(readability-identifier-naming)
{
static constexpr bool value = false;
};
@ -104,4 +103,3 @@ template <> struct is_big_int<UInt256> { static constexpr bool value = true; };
template <typename T>
inline constexpr bool is_big_int_v = is_big_int<T>::value;

View File

@ -120,6 +120,7 @@ Out & dumpDispatchPriorities(Out & out, T && x, std::decay_t<decltype(dumpImpl<p
return dumpImpl<priority>(out, x);
}
// NOLINTNEXTLINE(google-explicit-constructor)
struct LowPriority { LowPriority(void *) {} };
template <int priority, typename Out, typename T>

View File

@ -91,10 +91,10 @@ template <size_t N>
using DivisionBy10PowN = typename SelectType
<
N,
Division<uint8_t, 0, 205U, 11>, /// divide by 10
Division<uint16_t, 1, 41943U, 22>, /// divide by 100
Division<uint32_t, 0, 3518437209U, 45>, /// divide by 10000
Division<uint64_t, 0, 12379400392853802749ULL, 90> /// divide by 100000000
Division<uint8_t, false, 205U, 11>, /// divide by 10
Division<uint16_t, true, 41943U, 22>, /// divide by 100
Division<uint32_t, false, 3518437209U, 45>, /// divide by 10000
Division<uint64_t, false, 12379400392853802749ULL, 90> /// divide by 100000000
>::Result;
template <size_t N>
@ -352,7 +352,7 @@ static inline char * writeUIntText(T x, char * p)
static_assert(is_unsigned_v<T>);
int len = digits10(x);
auto pp = p + len;
auto * pp = p + len;
while (x >= 100)
{
const auto i = x % 100;

View File

@ -5,13 +5,13 @@
#include <utility>
template <class F>
class [[nodiscard]] basic_scope_guard
class [[nodiscard]] BasicScopeGuard
{
public:
constexpr basic_scope_guard() = default;
constexpr basic_scope_guard(basic_scope_guard && src) : function{src.release()} {}
constexpr BasicScopeGuard() = default;
constexpr BasicScopeGuard(BasicScopeGuard && src) : function{src.release()} {} // NOLINT(hicpp-noexcept-move, performance-noexcept-move-constructor)
constexpr basic_scope_guard & operator=(basic_scope_guard && src)
constexpr BasicScopeGuard & operator=(BasicScopeGuard && src) // NOLINT(hicpp-noexcept-move, performance-noexcept-move-constructor)
{
if (this != &src)
{
@ -23,11 +23,11 @@ public:
template <typename G>
requires std::is_convertible_v<G, F>
constexpr basic_scope_guard(basic_scope_guard<G> && src) : function{src.release()} {}
constexpr BasicScopeGuard(BasicScopeGuard<G> && src) : function{src.release()} {} // NOLINT(google-explicit-constructor)
template <typename G>
requires std::is_convertible_v<G, F>
constexpr basic_scope_guard & operator=(basic_scope_guard<G> && src)
constexpr BasicScopeGuard & operator=(BasicScopeGuard<G> && src)
{
if (this != &src)
{
@ -39,13 +39,13 @@ public:
template <typename G>
requires std::is_convertible_v<G, F>
constexpr basic_scope_guard(const G & function_) : function{function_} {}
constexpr BasicScopeGuard(const G & function_) : function{function_} {} // NOLINT(google-explicit-constructor)
template <typename G>
requires std::is_convertible_v<G, F>
constexpr basic_scope_guard(G && function_) : function{std::move(function_)} {}
constexpr BasicScopeGuard(G && function_) : function{std::move(function_)} {} // NOLINT(google-explicit-constructor, bugprone-forwarding-reference-overload, bugprone-move-forwarding-reference)
~basic_scope_guard() { invoke(); }
~BasicScopeGuard() { invoke(); }
static constexpr bool is_nullable = std::is_constructible_v<bool, F>;
@ -70,7 +70,7 @@ public:
template <typename G>
requires std::is_convertible_v<G, F>
basic_scope_guard<F> & join(basic_scope_guard<G> && other)
BasicScopeGuard<F> & join(BasicScopeGuard<G> && other)
{
if (other.function)
{
@ -102,14 +102,13 @@ private:
F function = F{};
};
using scope_guard = basic_scope_guard<std::function<void(void)>>;
using scope_guard = BasicScopeGuard<std::function<void(void)>>;
template <class F>
inline basic_scope_guard<F> make_scope_guard(F && function_) { return std::forward<F>(function_); }
inline BasicScopeGuard<F> make_scope_guard(F && function_) { return std::forward<F>(function_); }
#define SCOPE_EXIT_CONCAT(n, ...) \
const auto scope_exit##n = make_scope_guard([&] { __VA_ARGS__; })
#define SCOPE_EXIT_FWD(n, ...) SCOPE_EXIT_CONCAT(n, __VA_ARGS__)
#define SCOPE_EXIT(...) SCOPE_EXIT_FWD(__LINE__, __VA_ARGS__)

View File

@ -14,7 +14,7 @@ template <typename Comparator>
class DebugLessComparator
{
public:
constexpr DebugLessComparator(Comparator & cmp_)
constexpr DebugLessComparator(Comparator & cmp_) // NOLINT(google-explicit-constructor)
: cmp(cmp_)
{}

View File

@ -34,8 +34,10 @@ public:
template <class Enable = typename std::is_move_assignable<T>::type>
Self & operator=(T && rhs) { t = std::move(rhs); return *this;}
// NOLINTBEGIN(google-explicit-constructor)
operator const T & () const { return t; }
operator T & () { return t; }
// NOLINTEND(google-explicit-constructor)
bool operator==(const Self & rhs) const { return t == rhs.t; }
bool operator<(const Self & rhs) const { return t < rhs.t; }
@ -58,7 +60,10 @@ namespace std
};
}
// NOLINTBEGIN(bugprone-macro-parentheses)
#define STRONG_TYPEDEF(T, D) \
struct D ## Tag {}; \
using D = StrongTypedef<T, D ## Tag>; \
// NOLINTEND(bugprone-macro-parentheses)

View File

@ -10,9 +10,11 @@ constexpr size_t GiB = 1024 * MiB;
# pragma clang diagnostic ignored "-Wreserved-identifier"
#endif
// NOLINTBEGIN(google-runtime-int)
constexpr size_t operator"" _KiB(unsigned long long val) { return val * KiB; }
constexpr size_t operator"" _MiB(unsigned long long val) { return val * MiB; }
constexpr size_t operator"" _GiB(unsigned long long val) { return val * GiB; }
// NOLINTEND(google-runtime-int)
#ifdef HAS_RESERVED_IDENTIFIER
# pragma clang diagnostic pop

View File

@ -51,8 +51,8 @@ struct fmt::formatter<wide::integer<Bits, Signed>>
{
constexpr auto parse(format_parse_context & ctx)
{
auto it = ctx.begin();
auto end = ctx.end();
const auto * it = ctx.begin();
const auto * end = ctx.end();
/// Only support {}.
if (it != end && *it != '}')

View File

@ -49,6 +49,8 @@
#include <cxxabi.h>
#endif
// NOLINTBEGIN(readability-identifier-naming, modernize-use-using, bugprone-macro-parentheses, google-explicit-constructor)
/*
* Abstractions for compiler-specific directives
*/
@ -90,8 +92,6 @@
#define PCG_EMULATED_128BIT_MATH 1
#endif
// NOLINTBEGIN(*)
namespace pcg_extras {
/*
@ -553,6 +553,6 @@ std::ostream& operator<<(std::ostream& out, printable_typename<T>) {
} // namespace pcg_extras
// NOLINTEND(*)
// NOLINTEND(readability-identifier-naming, modernize-use-using, bugprone-macro-parentheses, google-explicit-constructor)
#endif // PCG_EXTRAS_HPP_INCLUDED

View File

@ -537,7 +537,7 @@ SizeAndChecksum BackupImpl::getFileSizeAndChecksum(const String & file_name) con
if (!info)
throw Exception(
ErrorCodes::BACKUP_ENTRY_NOT_FOUND, "Backup {}: Entry {} not found in the backup", backup_name, quoteString(file_name));
return std::pair(info->size, info->checksum);
return {info->size, info->checksum};
}
BackupEntryPtr BackupImpl::readFile(const String & file_name) const

View File

@ -146,14 +146,14 @@ inline size_t writeFloatTextFastPath(T x, char * buffer)
/// The library Ryu has low performance on integers.
/// This workaround improves performance 6..10 times.
if (DecomposedFloat64(x).is_integer_in_representable_range())
if (DecomposedFloat64(x).isIntegerInRepresentableRange())
result = itoa(Int64(x), buffer) - buffer;
else
result = jkj::dragonbox::to_chars_n(x, buffer) - buffer;
}
else
{
if (DecomposedFloat32(x).is_integer_in_representable_range())
if (DecomposedFloat32(x).isIntegerInRepresentableRange())
result = itoa(Int32(x), buffer) - buffer;
else
result = jkj::dragonbox::to_chars_n(x, buffer) - buffer;

View File

@ -103,17 +103,17 @@ Graphite::RollupRule selectPatternForPath(
if (first_match->type == first_match->TypeUndef && pattern.type == pattern.TypeAll)
{
/// There is only default pattern for both retention and aggregation
return std::pair(&pattern, &pattern);
return {&pattern, &pattern};
}
if (pattern.type != first_match->type)
{
if (first_match->type == first_match->TypeRetention)
{
return std::pair(first_match, &pattern);
return {first_match, &pattern};
}
if (first_match->type == first_match->TypeAggregation)
{
return std::pair(&pattern, first_match);
return {&pattern, first_match};
}
}
}
@ -125,7 +125,7 @@ Graphite::RollupRule selectPatternForPath(
if (pattern.type == pattern.TypeAll)
{
/// Only for not default patterns with both function and retention parameters
return std::pair(&pattern, &pattern);
return {&pattern, &pattern};
}
if (first_match->type == first_match->TypeUndef)
{
@ -136,11 +136,11 @@ Graphite::RollupRule selectPatternForPath(
{
if (first_match->type == first_match->TypeRetention)
{
return std::pair(first_match, &pattern);
return {first_match, &pattern};
}
if (first_match->type == first_match->TypeAggregation)
{
return std::pair(&pattern, first_match);
return {&pattern, first_match};
}
}
}

View File

@ -1,2 +0,0 @@
# clang-tidy has been integrated into CMake:
# --> Build ClickHouse with -DENABLE_CLANG_TIDY=1 and see cmake/clang_tidy.cmake for details