Skip to content

Commit

Permalink
Update clang-tidy. (#10730)
Browse files Browse the repository at this point in the history
- Install cmake using pip.
- Fix compile command generation.
- Clean up the tidy script and remove the need to load the yaml file.
- Fix modernized type traits.
- Fix span class. Polymorphism support is dropped
  • Loading branch information
trivialfis authored Aug 21, 2024
1 parent 03bd118 commit cb54374
Show file tree
Hide file tree
Showing 34 changed files with 362 additions and 388 deletions.
5 changes: 4 additions & 1 deletion include/xgboost/collective/socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -686,8 +686,11 @@ class TCPSocket {
* \return size of data actually received return -1 if error occurs
*/
auto Recv(void *buf, std::size_t len, std::int32_t flags = 0) {
char *_buf = reinterpret_cast<char *>(buf);
char *_buf = static_cast<char *>(buf);
// See https://github.com/llvm/llvm-project/issues/104241 for skipped tidy analysis
// NOLINTBEGIN(clang-analyzer-unix.BlockInCriticalSection)
return recv(handle_, _buf, len, flags);
// NOLINTEND(clang-analyzer-unix.BlockInCriticalSection)
}
/**
* \brief Send string, format is matched with the Python socket wrapper in RABIT.
Expand Down
2 changes: 1 addition & 1 deletion include/xgboost/host_device_vector.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ enum GPUAccess {

template <typename T>
class HostDeviceVector {
static_assert(std::is_standard_layout<T>::value, "HostDeviceVector admits only POD types");
static_assert(std::is_standard_layout_v<T>, "HostDeviceVector admits only POD types");

public:
explicit HostDeviceVector(size_t size = 0, T v = T(), DeviceOrd device = DeviceOrd::CPU());
Expand Down
110 changes: 45 additions & 65 deletions include/xgboost/json.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@

#include <functional>
#include <map>
#include <memory>
#include <string>
#include <type_traits> // std::enable_if,std::enable_if_t
#include <type_traits> // std::enable_if_t
#include <utility>
#include <vector>

Expand Down Expand Up @@ -223,6 +222,14 @@ class JsonObject : public Value {
~JsonObject() override = default;
};

namespace detail {
template <typename T, typename U>
using IsSameT = std::enable_if_t<std::is_same_v<std::remove_cv_t<T>, std::remove_cv_t<U>>>;

template <typename T>
using IsF64T = std::enable_if_t<std::is_same_v<T, double>>;
} // namespace detail

class JsonNumber : public Value {
public:
using Float = float;
Expand All @@ -232,15 +239,11 @@ class JsonNumber : public Value {

public:
JsonNumber() : Value(ValueKind::kNumber) {}
template <typename FloatT,
typename std::enable_if<std::is_same<FloatT, Float>::value>::type* = nullptr>
JsonNumber(FloatT value) : Value(ValueKind::kNumber) { // NOLINT
number_ = value;
}
template <typename FloatT,
typename std::enable_if<std::is_same<FloatT, double>::value>::type* = nullptr>
JsonNumber(FloatT value) : Value{ValueKind::kNumber}, // NOLINT
number_{static_cast<Float>(value)} {}
template <typename FloatT, typename detail::IsSameT<FloatT, Float>* = nullptr>
JsonNumber(FloatT value) : Value(ValueKind::kNumber), number_{value} {} // NOLINT
template <typename FloatT, typename detail::IsF64T<FloatT>* = nullptr>
JsonNumber(FloatT value) // NOLINT
: Value{ValueKind::kNumber}, number_{static_cast<Float>(value)} {}
JsonNumber(JsonNumber const& that) = delete;
JsonNumber(JsonNumber&& that) noexcept : Value{ValueKind::kNumber}, number_{that.number_} {}

Expand All @@ -258,6 +261,13 @@ class JsonNumber : public Value {
}
};

namespace detail {
template <typename IntT>
using Not32SizeT = std::enable_if_t<std::is_same_v<IntT, std::uint32_t> &&
!std::is_same_v<std::size_t, std::uint32_t>>;
}


class JsonInteger : public Value {
public:
using Int = int64_t;
Expand All @@ -267,24 +277,18 @@ class JsonInteger : public Value {

public:
JsonInteger() : Value(ValueKind::kInteger) {} // NOLINT
template <typename IntT, typename detail::IsSameT<IntT, Int>* = nullptr>
JsonInteger(IntT value) : Value(ValueKind::kInteger), integer_{value} {} // NOLINT
template <typename IntT, typename detail::IsSameT<IntT, std::size_t>* = nullptr>
JsonInteger(IntT value) // NOLINT
: Value(ValueKind::kInteger), integer_{static_cast<Int>(value)} {}
template <typename IntT, typename detail::IsSameT<IntT, std::int32_t>* = nullptr>
JsonInteger(IntT value) // NOLINT
: Value(ValueKind::kInteger), integer_{static_cast<Int>(value)} {}
template <typename IntT,
typename std::enable_if<std::is_same<IntT, Int>::value>::type* = nullptr>
JsonInteger(IntT value) : Value(ValueKind::kInteger), integer_{value} {} // NOLINT
template <typename IntT,
typename std::enable_if<std::is_same<IntT, size_t>::value>::type* = nullptr>
JsonInteger(IntT value) : Value(ValueKind::kInteger), // NOLINT
integer_{static_cast<Int>(value)} {}
template <typename IntT,
typename std::enable_if<std::is_same<IntT, int32_t>::value>::type* = nullptr>
JsonInteger(IntT value) : Value(ValueKind::kInteger), // NOLINT
integer_{static_cast<Int>(value)} {}
template <typename IntT,
typename std::enable_if<
std::is_same<IntT, uint32_t>::value &&
!std::is_same<std::size_t, uint32_t>::value>::type * = nullptr>
typename detail::Not32SizeT<IntT>* = nullptr>
JsonInteger(IntT value) // NOLINT
: Value(ValueKind::kInteger),
integer_{static_cast<Int>(value)} {}
: Value(ValueKind::kInteger), integer_{static_cast<Int>(value)} {}

JsonInteger(JsonInteger &&that) noexcept
: Value{ValueKind::kInteger}, integer_{that.integer_} {}
Expand Down Expand Up @@ -325,12 +329,8 @@ class JsonBoolean : public Value {
public:
JsonBoolean() : Value(ValueKind::kBoolean) {} // NOLINT
// Ambigious with JsonNumber.
template <typename Bool,
typename std::enable_if<
std::is_same<Bool, bool>::value ||
std::is_same<Bool, bool const>::value>::type* = nullptr>
JsonBoolean(Bool value) : // NOLINT
Value(ValueKind::kBoolean), boolean_{value} {}
template <typename Bool, typename detail::IsSameT<std::remove_cv_t<Bool>, bool>* = nullptr>
JsonBoolean(Bool value) : Value(ValueKind::kBoolean), boolean_{value} {} // NOLINT
JsonBoolean(JsonBoolean&& value) noexcept: // NOLINT
Value(ValueKind::kBoolean), boolean_{value.boolean_} {}

Expand Down Expand Up @@ -506,71 +506,52 @@ bool IsA(Json const& j) {

namespace detail {
// Number
template <typename T,
typename std::enable_if<
std::is_same<T, JsonNumber>::value>::type* = nullptr>
template <typename T, typename std::enable_if_t<std::is_same_v<T, JsonNumber>>* = nullptr>
JsonNumber::Float& GetImpl(T& val) { // NOLINT
return val.GetNumber();
}
template <typename T,
typename std::enable_if<
std::is_same<T, JsonNumber const>::value>::type* = nullptr>
template <typename T, typename std::enable_if_t<std::is_same_v<T, JsonNumber const>>* = nullptr>
JsonNumber::Float const& GetImpl(T& val) { // NOLINT
return val.GetNumber();
}

// Integer
template <typename T,
typename std::enable_if<
std::is_same<T, JsonInteger>::value>::type* = nullptr>
template <typename T, typename std::enable_if_t<std::is_same_v<T, JsonInteger>>* = nullptr>
JsonInteger::Int& GetImpl(T& val) { // NOLINT
return val.GetInteger();
}
template <typename T,
typename std::enable_if<
std::is_same<T, JsonInteger const>::value>::type* = nullptr>
template <typename T, typename std::enable_if_t<std::is_same_v<T, JsonInteger const>>* = nullptr>
JsonInteger::Int const& GetImpl(T& val) { // NOLINT
return val.GetInteger();
}

// String
template <typename T,
typename std::enable_if<
std::is_same<T, JsonString>::value>::type* = nullptr>
template <typename T, typename std::enable_if_t<std::is_same_v<T, JsonString>>* = nullptr>
std::string& GetImpl(T& val) { // NOLINT
return val.GetString();
}
template <typename T,
typename std::enable_if<
std::is_same<T, JsonString const>::value>::type* = nullptr>
template <typename T, typename std::enable_if_t<std::is_same_v<T, JsonString const>>* = nullptr>
std::string const& GetImpl(T& val) { // NOLINT
return val.GetString();
}

// Boolean
template <typename T,
typename std::enable_if<
std::is_same<T, JsonBoolean>::value>::type* = nullptr>
template <typename T, typename std::enable_if_t<std::is_same_v<T, JsonBoolean>>* = nullptr>
bool& GetImpl(T& val) { // NOLINT
return val.GetBoolean();
}
template <typename T,
typename std::enable_if<
std::is_same<T, JsonBoolean const>::value>::type* = nullptr>
typename std::enable_if_t<std::is_same_v<T, JsonBoolean const>>* = nullptr>
bool const& GetImpl(T& val) { // NOLINT
return val.GetBoolean();
}

// Array
template <typename T,
typename std::enable_if<
std::is_same<T, JsonArray>::value>::type* = nullptr>
template <typename T, typename std::enable_if_t<std::is_same_v<T, JsonArray>>* = nullptr>
std::vector<Json>& GetImpl(T& val) { // NOLINT
return val.GetArray();
}
template <typename T,
typename std::enable_if<
std::is_same<T, JsonArray const>::value>::type* = nullptr>
template <typename T, typename std::enable_if_t<std::is_same_v<T, JsonArray const>>* = nullptr>
std::vector<Json> const& GetImpl(T& val) { // NOLINT
return val.GetArray();
}
Expand All @@ -586,12 +567,11 @@ std::vector<T> const& GetImpl(JsonTypedArray<T, kind> const& val) {
}

// Object
template <typename T, typename std::enable_if<std::is_same<T, JsonObject>::value>::type* = nullptr>
template <typename T, typename std::enable_if_t<std::is_same_v<T, JsonObject>>* = nullptr>
JsonObject::Map& GetImpl(T& val) { // NOLINT
return val.GetObject();
}
template <typename T,
typename std::enable_if<std::is_same<T, JsonObject const>::value>::type* = nullptr>
template <typename T, typename std::enable_if_t<std::is_same_v<T, JsonObject const>>* = nullptr>
JsonObject::Map const& GetImpl(T& val) { // NOLINT
return val.GetObject();
}
Expand Down
9 changes: 3 additions & 6 deletions include/xgboost/json_io.h
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
/**
* Copyright 2019-2023, XGBoost Contributors
* Copyright 2019-2024, XGBoost Contributors
*/
#ifndef XGBOOST_JSON_IO_H_
#define XGBOOST_JSON_IO_H_
#include <dmlc/endian.h>
#include <xgboost/base.h>
#include <xgboost/json.h>

#include <cinttypes>
#include <cstdint> // for int8_t
#include <limits>
#include <map>
#include <memory>
#include <sstream>
#include <string>
#include <utility>
#include <vector>
Expand Down Expand Up @@ -111,7 +108,7 @@ class JsonReader {
};

class JsonWriter {
template <typename T, std::enable_if_t<!std::is_same<Json, T>::value>* = nullptr>
template <typename T, std::enable_if_t<!std::is_same_v<Json, T>>* = nullptr>
void Save(T const& v) {
this->Save(Json{v});
}
Expand Down
20 changes: 10 additions & 10 deletions include/xgboost/linalg.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ namespace detail {
struct ArrayInterfaceHandler {
template <typename T>
static constexpr char TypeChar() {
return (std::is_floating_point<T>::value
return (std::is_floating_point_v<T>
? 'f'
: (std::is_integral<T>::value ? (std::is_signed<T>::value ? 'i' : 'u') : '\0'));
: (std::is_integral_v<T> ? (std::is_signed_v<T> ? 'i' : 'u') : '\0'));
}
};

Expand Down Expand Up @@ -93,7 +93,7 @@ struct RangeTag {
*/
template <typename T>
constexpr int32_t CalcSliceDim() {
return std::is_same<T, IntTag>::value ? 0 : 1;
return std::is_same_v<T, IntTag> ? 0 : 1;
}

template <typename T, typename... S>
Expand All @@ -114,7 +114,7 @@ template <typename S>
using RemoveCRType = std::remove_const_t<std::remove_reference_t<S>>;

template <typename S>
using IndexToTag = std::conditional_t<std::is_integral<RemoveCRType<S>>::value, IntTag, S>;
using IndexToTag = std::conditional_t<std::is_integral_v<RemoveCRType<S>>, IntTag, S>;

template <int32_t n, typename Fn>
LINALG_HD constexpr auto UnrollLoop(Fn fn) {
Expand Down Expand Up @@ -159,7 +159,7 @@ inline LINALG_HD int Popc(uint64_t v) {

template <std::size_t D, typename Head>
LINALG_HD void IndexToArr(std::size_t (&arr)[D], Head head) {
static_assert(std::is_integral<std::remove_reference_t<Head>>::value, "Invalid index type.");
static_assert(std::is_integral_v<std::remove_reference_t<Head>>, "Invalid index type.");
arr[D - 1] = head;
}

Expand All @@ -169,7 +169,7 @@ LINALG_HD void IndexToArr(std::size_t (&arr)[D], Head head) {
template <std::size_t D, typename Head, typename... Rest>
LINALG_HD void IndexToArr(std::size_t (&arr)[D], Head head, Rest &&...index) {
static_assert(sizeof...(Rest) < D, "Index overflow.");
static_assert(std::is_integral<std::remove_reference_t<Head>>::value, "Invalid index type.");
static_assert(std::is_integral_v<std::remove_reference_t<Head>>, "Invalid index type.");
arr[D - sizeof...(Rest) - 1] = head;
IndexToArr(arr, std::forward<Rest>(index)...);
}
Expand All @@ -193,7 +193,7 @@ constexpr auto ArrToTuple(T (&arr)[N]) {
template <typename I, std::int32_t D>
LINALG_HD auto UnravelImpl(I idx, common::Span<size_t const, D> shape) {
std::size_t index[D]{0};
static_assert(std::is_signed<decltype(D)>::value,
static_assert(std::is_signed_v<decltype(D)>,
"Don't change the type without changing the for loop.");
auto const sptr = shape.data();
for (int32_t dim = D; --dim > 0;) {
Expand Down Expand Up @@ -379,7 +379,7 @@ class TensorView {
* \brief Slice dimension for Index tag.
*/
template <size_t old_dim, size_t new_dim, int32_t D, typename Index, typename... S>
LINALG_HD std::enable_if_t<std::is_integral<Index>::value, size_t> MakeSliceDim(
LINALG_HD std::enable_if_t<std::is_integral_v<Index>, size_t> MakeSliceDim(
size_t new_shape[D], size_t new_stride[D], Index i, S &&...slices) const {
static_assert(old_dim < kDim);
auto offset = stride_[old_dim] * i;
Expand Down Expand Up @@ -547,7 +547,7 @@ class TensorView {
*/
[[nodiscard]] LINALG_HD bool CContiguous() const {
StrideT stride;
static_assert(std::is_same<decltype(stride), decltype(stride_)>::value);
static_assert(std::is_same_v<decltype(stride), decltype(stride_)>);
// It's contiguous if the stride can be calculated from shape.
detail::CalcStride(shape_, stride);
return common::Span<size_t const, kDim>{stride_} == common::Span<size_t const, kDim>{stride};
Expand All @@ -557,7 +557,7 @@ class TensorView {
*/
[[nodiscard]] LINALG_HD bool FContiguous() const {
StrideT stride;
static_assert(std::is_same<decltype(stride), decltype(stride_)>::value);
static_assert(std::is_same_v<decltype(stride), decltype(stride_)>);
// It's contiguous if the stride can be calculated from shape.
detail::CalcStride<kDim, true>(shape_, stride);
return common::Span<size_t const, kDim>{stride_} == common::Span<size_t const, kDim>{stride};
Expand Down
2 changes: 1 addition & 1 deletion include/xgboost/parameter.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class FieldEntry<EnumClass> : public FieldEntry<int> { \
public: \
FieldEntry() { \
static_assert( \
std::is_same<int, typename std::underlying_type<EnumClass>::type>::value, \
std::is_same_v<int, typename std::underlying_type_t<EnumClass>>, \
"enum class must be backed by int"); \
is_enum_ = true; \
} \
Expand Down
Loading

0 comments on commit cb54374

Please sign in to comment.