Skip to content

Commit

Permalink
fix: Some minor fixes for RawVector (#11693)
Browse files Browse the repository at this point in the history
Summary:
The patch addresses a couple of issues with `RawVector` class:
1. Const version of `operator[]` returns `T` by value, which seems to be an unfortunate implementation mistake
2. Added an additional requirement for `T` to be `std::trivially_copyable_v<T>`. It makes little sense to use `raw_vector` for non-trivially copyable T:s, so explicitly forbid it.

Pull Request resolved: #11693

Reviewed By: yuandagits

Differential Revision: D66683059

Pulled By: xiaoxmeng

fbshipit-source-id: e5016a3bce54c4580c52946e8a0f1c8a3e7c828f
  • Loading branch information
ManManson authored and facebook-github-bot committed Dec 3, 2024
1 parent 2872a16 commit 0a685b1
Showing 1 changed file with 9 additions and 4 deletions.
13 changes: 9 additions & 4 deletions velox/common/base/RawVector.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,22 @@
#include "velox/common/base/BitUtil.h"
#include "velox/common/base/SimdUtil.h"

#include <type_traits>

namespace facebook::velox {

/// Class template similar to std::vector with no default construction and a
/// SIMD load worth of padding below and above the data. The idea is that one
/// can access the data at full SIMD width at both ends.
///
/// `T` should name a trivially copyable and trivially destructible type.
template <typename T>
class raw_vector {
public:
raw_vector() {
static_assert(std::is_trivially_destructible<T>::value);
}
static_assert(
std::is_trivially_destructible_v<T> && std::is_trivially_copyable_v<T>);

raw_vector() = default;

explicit raw_vector(int32_t size) {
resize(size);
Expand Down Expand Up @@ -92,7 +97,7 @@ class raw_vector {
return data_[index];
}

T operator[](int32_t index) const {
const T& operator[](int32_t index) const {
return data_[index];
}

Expand Down

0 comments on commit 0a685b1

Please sign in to comment.