chore(common): Use some c++20 #6403
Conversation
Signed-off-by: Vladislav Oleshko <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR modernizes common utilities in the Dragonfly codebase to use C++20 features. The changes replace legacy template metaprogramming with modern concepts, constraints, and the spaceship operator.
Changes:
- Replaced traditional comparison operators with C++20 spaceship operator in StringOrView class
- Converted template metaprogramming (SFINAE) to C++20 concepts and requires clauses in heap_size.h
- Renamed namespace from
heap_size_detailtodetailfor consistency
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/common/string_or_view.h | Modernized comparison operators using spaceship operator and relocated empty() method |
| src/common/heap_size.h | Converted SFINAE-based constraints to concepts and requires clauses, introduced StackOnly concept |
Comments suppressed due to low confidence (1)
src/common/heap_size.h:135
- The AccumulateContainer function is still defined in the heap_size_detail namespace (line 123), but it's forward-declared in the detail namespace (line 37) and called as detail::AccumulateContainer throughout the file. This creates a mismatch - the function definition should be in the detail namespace to match the forward declaration and usage.
namespace heap_size_detail {
template <typename Container> size_t AccumulateContainer(const Container& c) {
size_t size = 0;
if constexpr (!detail::StackOnly<typename Container::value_type>) {
for (const auto& e : c) {
size += HeapSize(e);
}
}
return size;
}
} // namespace heap_size_detail
| vector<string_view> mapped(vals.Size() + 1); | ||
| mapped[0] = key; | ||
| std::copy(vals.begin(), vals.end(), mapped.begin() + 1); | ||
| std::ranges::copy(vals.view(), mapped.begin() + 1); |
There was a problem hiding this comment.
Missing required header for C++20 ranges. The code uses std::ranges::copy which requires including the <ranges> header. Add #include <ranges> at the top of the file.
| // Overload for types that have defined UsedMemory | ||
| template <typename T> | ||
| requires requires(T t) { | ||
| { t.UsedMemory() } -> std::convertible_to<size_t>; | ||
| } | ||
| size_t HeapSize(const T& t) { | ||
| return t.UsedMemory(); | ||
| } | ||
|
|
||
| template <typename T, std::enable_if_t<heap_size_detail::StackOnlyType<T>(), bool> = true> | ||
| // Overload for types that should be explicitly excluded from calculations | ||
| template <typename T> | ||
| requires requires { | ||
| typename T::is_stackonly; | ||
| } | ||
| size_t HeapSize(const T& t) { | ||
| return 0; | ||
| } | ||
|
|
||
| template <typename T> size_t HeapSize(absl::Span<T>) { | ||
| // Overload for trivial types we don't have to account for | ||
| template <typename T> size_t HeapSize(const T& t) { | ||
| static_assert(std::is_trivial_v<T> || std::is_same_v<std::string_view, T>); | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
Missing required header for C++20 concepts. The code uses requires expressions which require including the <concepts> header (or at minimum <type_traits> which is already included, but <concepts> is more appropriate for modern C++20 code). Add #include <concepts> at the top of the file.
| auto view() const { | ||
| return std::views::iota(size_t{0}, size()) | | ||
| std::views::transform([this](size_t i) { return at(i); }); | ||
| } |
There was a problem hiding this comment.
Missing required header for C++20 ranges. The view() method uses std::views::iota and std::views::transform which require including the <ranges> header. Add #include <ranges> at the top of the file.
| auto view() const { | ||
| return std::views::iota(size_t{0}, Size()) | | ||
| std::views::transform([this](size_t i) { return (*this)[i]; }); | ||
| } |
There was a problem hiding this comment.
Missing required header for C++20 ranges. The view() method uses std::views::iota and std::views::transform which require including the <ranges> header. Add #include <ranges> at the top of the file.
| auto sub_view = args_->view() | std::views::drop(index_); | ||
| static_assert(std::ranges::sized_range<decltype(sub_view)>); | ||
|
|
||
| scratch->reserve(std::ranges::distance(sub_view)); | ||
| std::ranges::copy(sub_view, std::back_inserter(*scratch)); | ||
| return ArgSlice{scratch->data(), scratch->size()}; |
There was a problem hiding this comment.
Missing required header for C++20 ranges. The code uses std::views::drop, std::ranges::sized_range, std::ranges::distance, and std::ranges::copy which require including the <ranges> header. Add #include <ranges> at the top of the file.
| vector<string_view> mapped(vals.Size() + 1); | ||
| mapped[0] = key; | ||
| std::copy(vals.begin(), vals.end(), mapped.begin() + 1); | ||
| std::ranges::copy(vals.view(), mapped.begin() + 1); |
There was a problem hiding this comment.
Missing required header for C++20 ranges. The code uses std::ranges::copy which requires including the <ranges> header. Add #include <ranges> at the top of the file.
1 participant
Just a PR to play with c++20 features:
-> remove 100 loc from a total of 500 loc