Skip to content

Commit

Permalink
enable more clang-tidy checks & fix the warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
KRM7 committed Dec 29, 2023
1 parent 8af4f76 commit 0bd126d
Show file tree
Hide file tree
Showing 18 changed files with 80 additions and 35 deletions.
11 changes: 3 additions & 8 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,17 @@
-readability-named-parameter,
-readability-function-cognitive-complexity,
-readability-isolate-declaration,
-readability-container-data-pointer,
-readability-suspicious-call-argument,
-readability-implicit-bool-conversion,
-readability-uppercase-literal-suffix,
-readability-braces-around-statements,
-bugprone-easily-swappable-parameters,
-bugprone-exception-escape,
-cppcoreguidelines-special-member-functions,
-cppcoreguidelines-avoid-magic-numbers,
-cppcoreguidelines-non-private-member-variables-in-classes,
-cppcoreguidelines-prefer-member-initializer,
-cppcoreguidelines-narrowing-conversions,
-cppcoreguidelines-c-copy-assignment-signature,
-cppcoreguidelines-avoid-non-const-global-variables,
-cppcoreguidelines-owning-memory,
-misc-non-private-member-variables-in-classes,
-misc-unconventional-assign-operator,
-misc-no-recursion,
-misc-const-correctness,
-misc-use-anonymous-namespace
Expand Down Expand Up @@ -87,9 +81,10 @@ WarningsAsErrors: >
CheckOptions:
- { key: hicpp-signed-bitwise.IgnorePositiveIntegerLiterals, value: true }
- { key: readability-implicit-bool-conversion.AllowIntegerConditions, value: true }
- { key: bugprone-narrowing-conversions.IgnoreConversionFromTypes, value: difference_type;ptrdiff_t;size_type;size_t;bool }
- { key: readability-implicit-bool-conversion.AllowPointerConditions, value: true }
- { key: readability-simplify-boolean-expr.SimplifyDeMorgan, value: false }
- { key: readability-braces-around-statements.ShortStatementLines, value: 2 }
- { key: bugprone-narrowing-conversions.IgnoreConversionFromTypes, value: difference_type;ptrdiff_t;size_type;size_t;bool }
- { key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: false }
- { key: performance-unnecessary-value-param.AllowedTypes, value: iterator* }
- { key: readability-simplify-boolean-expr.SimplifyDeMorgan, value: false }
- { key: cppcoreguidelines-macro-usage.AllowedRegexp, value: GAPP_* }
4 changes: 4 additions & 0 deletions src/algorithm/algorithm_base.decl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@ namespace gapp::algorithm
template<typename T>
Candidates<T> optimalSolutions(const GA<T>& ga, const Population<T>& pop) const;


/** Destructor. */
~Algorithm() override = default;

protected:

Algorithm() = default;
Expand Down
2 changes: 1 addition & 1 deletion src/algorithm/nd_sort.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ namespace gapp::algorithm::dtl
GAPP_ASSERT(std::ranges::all_of(elements_, detail::between(0_sz, elements_.size()), &FrontElement::idx));
}

void ParetoFronts::resize(size_type new_size) noexcept
void ParetoFronts::resize(size_type new_size)
{
GAPP_ASSERT(new_size <= elements_.size());

Expand Down
6 changes: 3 additions & 3 deletions src/algorithm/nd_sort.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ namespace gapp::algorithm::dtl

constexpr ParetoFrontsRange() = default;

constexpr ParetoFrontsRange(iterator first, iterator last) noexcept :
constexpr ParetoFrontsRange(const iterator& first, const iterator& last) noexcept :
first_(first), last_(last)
{}

template<typename Range>
constexpr explicit ParetoFrontsRange(Range&& range) noexcept :
constexpr explicit ParetoFrontsRange(Range& range) noexcept :
first_(range.begin()), last_(range.end())
{}

Expand Down Expand Up @@ -84,7 +84,7 @@ namespace gapp::algorithm::dtl
iterator end() noexcept { return elements_.end(); }
const_iterator end() const noexcept { return elements_.end(); }

void resize(size_type new_size) noexcept;
void resize(size_type new_size);

std::vector<size_t> ranks() const;
std::vector<ParetoFrontsRange> fronts();
Expand Down
8 changes: 4 additions & 4 deletions src/algorithm/nsga2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,10 @@ namespace gapp::algorithm
const size_t idx2 = rng::randomIndex(fmat);

// lower ranks and higher crowding distances are better
if (ranks_[idx1] < ranks_[idx2]) return idx1;
else if (ranks_[idx1] > ranks_[idx2]) return idx2;
else if (dists_[idx1] > dists_[idx2]) return idx1;
else return idx2;
if (ranks_[idx1] < ranks_[idx2]) return idx1;
if (ranks_[idx1] > ranks_[idx2]) return idx2;
if (dists_[idx1] > dists_[idx2]) return idx1;
return idx2;
}

std::vector<size_t> NSGA2::nextPopulationImpl(const GaInfo& ga, const FitnessMatrix& fmat)
Expand Down
10 changes: 8 additions & 2 deletions src/algorithm/nsga3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,15 @@ namespace gapp::algorithm
pimpl_(std::make_unique<Impl>(*rhs.pimpl_))
{}

NSGA3& NSGA3::operator=(NSGA3 rhs) noexcept
NSGA3& NSGA3::operator=(const NSGA3& other)
{
this->pimpl_.swap(rhs.pimpl_);
this->pimpl_ = std::make_unique<Impl>(*other.pimpl_);
return *this;
}

NSGA3& NSGA3::operator=(NSGA3&& other) noexcept
{
this->pimpl_ = std::move(other.pimpl_);
return *this;
}

Expand Down
3 changes: 2 additions & 1 deletion src/algorithm/nsga3.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ namespace gapp::algorithm

NSGA3(const NSGA3&);
NSGA3(NSGA3&&) noexcept;
NSGA3& operator=(NSGA3) noexcept;
NSGA3& operator=(const NSGA3&);
NSGA3& operator=(NSGA3&&) noexcept;
~NSGA3() override;

private:
Expand Down
5 changes: 2 additions & 3 deletions src/core/ga_info.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ namespace gapp
GaInfo::~GaInfo() noexcept = default;


GaInfo::GaInfo(Positive<size_t> population_size, std::unique_ptr<algorithm::Algorithm> algorithm, std::unique_ptr<stopping::StopCondition> stop_condition) noexcept :
algorithm_(std::move(algorithm)), stop_condition_(std::move(stop_condition)), population_size_(population_size)
GaInfo::GaInfo(Positive<size_t> population_size, std::unique_ptr<algorithm::Algorithm> algorithm, std::unique_ptr<stopping::StopCondition> stop_condition) :
algorithm_(std::move(algorithm)), stop_condition_(std::move(stop_condition)), population_size_(population_size), use_default_algorithm_(!algorithm_)
{
use_default_algorithm_ = !algorithm;
if (!algorithm_) algorithm_ = std::make_unique<algorithm::SingleObjective>();
if (!stop_condition_) stop_condition_ = std::make_unique<stopping::NoEarlyStop>();
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/ga_info.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ namespace gapp
* @param stop_condition The early-stop condition to use for the algorithm. The algorithm will stop when
* reaching the maximum number of generations set regardless of the stop condition specified here.
*/
GaInfo(Positive<size_t> population_size, std::unique_ptr<algorithm::Algorithm> algorithm, std::unique_ptr<stopping::StopCondition> stop_condition) noexcept;
GaInfo(Positive<size_t> population_size, std::unique_ptr<algorithm::Algorithm> algorithm, std::unique_ptr<stopping::StopCondition> stop_condition);


/**
Expand Down
4 changes: 2 additions & 2 deletions src/stop_condition/stop_condition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ namespace gapp::stopping
const FitnessVector current_mean = detail::fitnessMean(fitness_matrix.begin(), fitness_matrix.end());

const bool improved = metricImproved(best_fitness_mean_, current_mean, delta_);
improved ? reset() : (void) (cntr_ -= bool(cntr_));
improved ? reset() : (void) (cntr_ -= !!cntr_); // NOLINT(*conversion)

return cntr_ == 0;
}
Expand All @@ -83,7 +83,7 @@ namespace gapp::stopping
const FitnessVector current_max = detail::maxFitness(fitness_matrix.begin(), fitness_matrix.end());

const bool improved = metricImproved(fitness_max_, current_max, delta_);
improved ? reset() : (void) (cntr_ -= bool(cntr_));
improved ? reset() : (void) (cntr_ -= !!cntr_); // NOLINT(*conversion)

return cntr_ == 0;
}
Expand Down
12 changes: 12 additions & 0 deletions src/utility/bit.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ namespace gapp::detail
return value & (T{ 1 } << n);
}

template<typename T>
constexpr bool first_bit(T value) noexcept
{
return value & detail::msb_mask<T>;
}

template<typename T>
constexpr bool last_bit(T value) noexcept
{
return value & detail::lsb_mask<T>;
}

} // namespace gapp::detail

#endif // !GA_UTILITY_BIT_HPP
2 changes: 1 addition & 1 deletion src/utility/cone_tree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ namespace gapp::detail

thread_local std::vector<const Node*> node_stack(nodes_.size() / 2);
node_stack.clear();
node_stack.push_back(&nodes_[0]);
node_stack.push_back(&nodes_.front());

FindResult best{ {}, -math::inf<double> };

Expand Down
4 changes: 2 additions & 2 deletions src/utility/functional.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,8 @@ namespace gapp::detail
return *this;
}

move_only_function(move_only_function&&) = default;
move_only_function& operator=(move_only_function&& other) = default;
move_only_function(move_only_function&&) = default;
move_only_function& operator=(move_only_function&&) = default;

R operator()(Args... args)
{
Expand Down
14 changes: 10 additions & 4 deletions src/utility/matrix.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ namespace gapp::detail
/* Modifiers (for rows) */

constexpr void append_row(std::span<const T> row);
constexpr void pop_back() noexcept { resize(nrows() - 1, ncols()); }
constexpr void pop_back() noexcept { resize(nrows() - 1, ncols()); } // NOLINT(*exception-escape)

constexpr iterator erase(const_iterator row);
constexpr iterator erase(const_iterator first, const_iterator last);
Expand All @@ -177,7 +177,7 @@ namespace gapp::detail
constexpr size_type size() const noexcept { return nrows_; } /* For the bounds checking in stable_iterator */
constexpr size_type nrows() const noexcept { return nrows_; }
constexpr size_type ncols() const noexcept { return ncols_; }
constexpr size_type empty() const noexcept { return data_.empty(); }
constexpr bool empty() const noexcept { return data_.empty(); }

constexpr void reserve(size_type nrows, size_type ncols) { data_.reserve(nrows * ncols); }

Expand Down Expand Up @@ -303,7 +303,6 @@ namespace gapp::detail
size_type row_;
};


template<typename T, typename A>
class MatrixRowRef : public MatrixRowBase<MatrixRowRef<T, A>, Matrix<T, A>>
{
Expand All @@ -318,6 +317,8 @@ namespace gapp::detail
MatrixRowRef(const MatrixRowRef&) = default;
MatrixRowRef(MatrixRowRef&&) = default;

// NOLINTBEGIN(*unconventional-assign-operator, *assignment-signature)

constexpr const MatrixRowRef& operator=(MatrixRowRef<T, A> rhs) const
{
return *this = std::span{ rhs.begin(), rhs.end() };
Expand Down Expand Up @@ -345,7 +346,9 @@ namespace gapp::detail
return *this;
}

constexpr void swap(std::span<T> other) const
// NOLINTEND(*unconventional-assign-operator, *assignment-signature)

constexpr void swap(std::span<T> other) const noexcept(std::is_nothrow_swappable_v<T>)
{
GAPP_ASSERT(other.size() == this->size(), "Rows must be the same size to swap them.");

Expand All @@ -359,18 +362,21 @@ namespace gapp::detail

template<typename T, typename A>
constexpr void swap(MatrixRowRef<T, A> lhs, MatrixRowRef<T, A> rhs)
noexcept(std::is_nothrow_swappable_v<T>)
{
lhs.swap(rhs);
}

template<typename T, typename A>
constexpr void swap(MatrixRowRef<T, A> lhs, std::span<std::type_identity_t<T>> rhs)
noexcept(std::is_nothrow_swappable_v<T>)
{
lhs.swap(rhs);
}

template<typename T, typename A>
constexpr void swap(std::span<std::type_identity_t<T>> lhs, MatrixRowRef<T, A> rhs)
noexcept(std::is_nothrow_swappable_v<T>)
{
rhs.swap(lhs);
}
Expand Down
11 changes: 10 additions & 1 deletion src/utility/rcu.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ namespace gapp::detail
private:
struct registered_reader
{
registered_reader() noexcept
registered_reader() noexcept // NOLINT(*exception-escape)
{
std::unique_lock _{ tls_readers->lock };
tls_readers->list.push_back(this);
Expand Down Expand Up @@ -75,6 +75,7 @@ namespace gapp::detail
alignas(128) inline static thread_local registered_reader reader;
};

// NOLINTBEGIN(*owning-memory)

template<typename T>
class rcu_obj
Expand All @@ -101,6 +102,12 @@ namespace gapp::detail
return *this;
}

rcu_obj(const rcu_obj&) = delete;
rcu_obj(rcu_obj&&) = delete;

rcu_obj& operator=(const rcu_obj&) = delete;
rcu_obj& operator=(rcu_obj&&) = delete;

T& get() noexcept { return *data_.load(std::memory_order_consume); }
const T& get() const noexcept { return *data_.load(std::memory_order_consume); }

Expand All @@ -120,6 +127,8 @@ namespace gapp::detail
std::atomic<T*> data_;
};

// NOLINTEND(*owning-memory)

} // namespace gapp::detail

#endif // !GA_UTILITY_RCU_HPP
4 changes: 2 additions & 2 deletions src/utility/rng.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ namespace gapp::rng

struct RegisteredGenerator
{
RegisteredGenerator() noexcept
RegisteredGenerator() noexcept // NOLINT(*exception-escape)
{
std::scoped_lock _{ tls_generators_->lock };
instance.get() = global_generator_.jump();
Expand Down Expand Up @@ -311,7 +311,7 @@ namespace gapp::rng
bit_pool = prng() | detail::msb_mask<uint64_t>;
}

const bool bit = bit_pool & detail::lsb_mask<uint64_t>;
const bool bit = detail::last_bit(bit_pool);
bit_pool >>= 1;

return bit;
Expand Down
5 changes: 5 additions & 0 deletions src/utility/scope_exit.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ namespace gapp::detail
vars_(vars...), old_values_(std::forward<Us>(vars)...)
{}

restore_on_exit(const restore_on_exit&) = delete;
restore_on_exit(restore_on_exit&&) = delete;
restore_on_exit& operator=(const restore_on_exit&) = delete;
restore_on_exit& operator=(restore_on_exit&&) = delete;

constexpr ~restore_on_exit() noexcept { vars_ = std::move(old_values_); }

private:
Expand Down
8 changes: 8 additions & 0 deletions src/utility/thread_pool.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ namespace gapp::detail

~thread_pool() noexcept { stop(); }

thread_pool() = default;

thread_pool(const thread_pool&) = delete;
thread_pool(thread_pool&&) = delete;

thread_pool& operator=(const thread_pool&) = delete;
thread_pool& operator=(thread_pool&&) = delete;

private:
using task_t = detail::move_only_function<void()>;

Expand Down

0 comments on commit 0bd126d

Please sign in to comment.