Skip to content

Commit

Permalink
tidy: add const checks
Browse files Browse the repository at this point in the history
This change implements checking for when methods can be made const, when
pointers can be made const, and when returning const objects. Many
functions are explicitly excluded from this, since they do not perserve
logical constness, since they mutate a pointer member.
  • Loading branch information
Riolku committed Nov 30, 2023
1 parent 0482d9d commit 3ab6bf1
Show file tree
Hide file tree
Showing 43 changed files with 82 additions and 66 deletions.
4 changes: 4 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ Checks:

modernize-use-override,

readability-const-return-type,
readability-make-member-function-const,
readability-non-const-parameter,

HeaderFilterRegex:
src/include|tools/benchmark/include|tools/nodejs_api/src_cpp/include|tools/python_api/src_cpp/include|tools/rust_api/include|tools/shell/include
WarningsAsErrors: "*"
1 change: 1 addition & 0 deletions src/function/cast_from_string_functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ struct SplitStringMapOperation {
uint64_t& offset;
ValueVector* resultVector;

// NOLINTNEXTLINE(readability-make-member-function-const): Semantically non-const.
inline bool handleKey(const char* start, const char* end, const CSVOption* option) {
trimRightWhitespace(start, end);
CastString::copyStringToVector(StructVector::getFieldVector(resultVector, 0).get(), offset,
Expand Down
2 changes: 1 addition & 1 deletion src/include/common/data_chunk/data_chunk_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class DataChunkState {
selVector->selectedSize = size;
}
inline void setOriginalSize(uint64_t size) { originalSize = size; }
inline uint64_t getOriginalSize() { return originalSize; }
inline uint64_t getOriginalSize() const { return originalSize; }
inline bool isFlat() const { return fStateType == FStateType::FLAT; }
inline void setToFlat() { fStateType = FStateType::FLAT; }
inline void setToUnflat() { fStateType = FStateType::UNFLAT; }
Expand Down
4 changes: 2 additions & 2 deletions src/include/common/serializer/buffered_serializer.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ class BufferedSerializer : public Writer {
// Retrieves the data after the writing has been completed.
inline BinaryData getData() { return std::move(blob); }

inline uint64_t getSize() { return blob.size; }
inline uint64_t getSize() const { return blob.size; }

inline uint8_t* getBlobData() { return blob.data.get(); }
inline uint8_t* getBlobData() const { return blob.data.get(); }

inline void reset() { blob.size = 0; }

Expand Down
1 change: 1 addition & 0 deletions src/include/common/types/blob.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ struct Blob {
return *reinterpret_cast<const T*>(data.value.getData());
}
template<typename T>
// NOLINTNEXTLINE(readability-non-const-parameter): Would cast away qualifiers.
static inline T getValue(char* data) {
return *reinterpret_cast<T*>(data);
}
Expand Down
8 changes: 4 additions & 4 deletions src/include/function/aggregate/avg.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ struct AvgFunction {

static void updateAll(uint8_t* state_, common::ValueVector* input, uint64_t multiplicity,
storage::MemoryManager* /*memoryManager*/) {
auto state = reinterpret_cast<AvgState*>(state_);
auto* state = reinterpret_cast<AvgState*>(state_);
KU_ASSERT(!input->state->isFlat());
if (input->hasNoNullsGuarantee()) {
for (auto i = 0u; i < input->state->selVector->selectedSize; ++i) {
Expand Down Expand Up @@ -64,11 +64,11 @@ struct AvgFunction {

static void combine(
uint8_t* state_, uint8_t* otherState_, storage::MemoryManager* /*memoryManager*/) {
auto otherState = reinterpret_cast<AvgState*>(otherState_);
auto* otherState = reinterpret_cast<AvgState*>(otherState_);
if (otherState->isNull) {
return;
}
auto state = reinterpret_cast<AvgState*>(state_);
auto* state = reinterpret_cast<AvgState*>(state_);
if (state->isNull) {
state->sum = otherState->sum;
state->isNull = false;
Expand All @@ -79,7 +79,7 @@ struct AvgFunction {
}

static void finalize(uint8_t* state_) {
auto state = reinterpret_cast<AvgState*>(state_);
auto* state = reinterpret_cast<AvgState*>(state_);
if (!state->isNull) {
state->avg = state->sum / (double_t)state->count;
}
Expand Down
1 change: 1 addition & 0 deletions src/include/function/aggregate/count.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ struct CountFunction : public BaseCountFunction {
static void updateAll(uint8_t* state_, common::ValueVector* input, uint64_t multiplicity,
storage::MemoryManager* memoryManager);

// NOLINTNEXTLINE(readability-non-const-parameter): Would cast away qualifiers.
static inline void updatePos(uint8_t* state_, common::ValueVector* /*input*/,
uint64_t multiplicity, uint32_t /*pos*/, storage::MemoryManager* /*memoryManager*/) {
reinterpret_cast<CountState*>(state_)->count += multiplicity;
Expand Down
6 changes: 3 additions & 3 deletions src/include/function/aggregate/min_max.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ struct MinMaxFunction {
static void updateAll(uint8_t* state_, common::ValueVector* input, uint64_t /*multiplicity*/,
storage::MemoryManager* memoryManager) {
KU_ASSERT(!input->state->isFlat());
auto state = reinterpret_cast<MinMaxState*>(state_);
auto* state = reinterpret_cast<MinMaxState*>(state_);
if (input->hasNoNullsGuarantee()) {
for (auto i = 0u; i < input->state->selVector->selectedSize; ++i) {
auto pos = input->state->selVector->selectedPositions[i];
Expand Down Expand Up @@ -68,11 +68,11 @@ struct MinMaxFunction {
template<class OP>
static void combine(
uint8_t* state_, uint8_t* otherState_, storage::MemoryManager* memoryManager) {
auto otherState = reinterpret_cast<MinMaxState*>(otherState_);
auto* otherState = reinterpret_cast<MinMaxState*>(otherState_);
if (otherState->isNull) {
return;
}
auto state = reinterpret_cast<MinMaxState*>(state_);
auto* state = reinterpret_cast<MinMaxState*>(state_);
if (state->isNull) {
state->setVal(otherState->val, memoryManager);
state->isNull = false;
Expand Down
8 changes: 4 additions & 4 deletions src/include/function/aggregate/sum.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ struct SumFunction {
static void updateAll(uint8_t* state_, common::ValueVector* input, uint64_t multiplicity,
storage::MemoryManager* /*memoryManager*/) {
KU_ASSERT(!input->state->isFlat());
auto state = reinterpret_cast<SumState*>(state_);
auto* state = reinterpret_cast<SumState*>(state_);
if (input->hasNoNullsGuarantee()) {
for (auto i = 0u; i < input->state->selVector->selectedSize; ++i) {
auto pos = input->state->selVector->selectedPositions[i];
Expand All @@ -42,7 +42,7 @@ struct SumFunction {

static inline void updatePos(uint8_t* state_, common::ValueVector* input, uint64_t multiplicity,
uint32_t pos, storage::MemoryManager* /*memoryManager*/) {
auto state = reinterpret_cast<SumState*>(state_);
auto* state = reinterpret_cast<SumState*>(state_);
updateSingleValue(state, input, pos, multiplicity);
}

Expand All @@ -61,11 +61,11 @@ struct SumFunction {

static void combine(
uint8_t* state_, uint8_t* otherState_, storage::MemoryManager* /*memoryManager*/) {
auto otherState = reinterpret_cast<SumState*>(otherState_);
auto* otherState = reinterpret_cast<SumState*>(otherState_);
if (otherState->isNull) {
return;
}
auto state = reinterpret_cast<SumState*>(state_);
auto* state = reinterpret_cast<SumState*>(state_);
if (state->isNull) {
state->sum = otherState->sum;
state->isNull = false;
Expand Down
15 changes: 9 additions & 6 deletions src/include/function/aggregate_function.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,32 +54,35 @@ struct AggregateFunction final : public BaseScalarFunction {
std::move(combineFunc), std::move(finalizeFunc), isDistinct, nullptr /* bindFunc */,
std::move(paramRewriteFunc)} {}

inline uint32_t getAggregateStateSize() { return initialNullAggregateState->getStateSize(); }
inline uint32_t getAggregateStateSize() const {
return initialNullAggregateState->getStateSize();
}

// NOLINTNEXTLINE(readability-make-member-function-const): Returns a non-const pointer.
inline AggregateState* getInitialNullAggregateState() {
return initialNullAggregateState.get();
}

inline std::unique_ptr<AggregateState> createInitialNullAggregateState() {
inline std::unique_ptr<AggregateState> createInitialNullAggregateState() const {
return initializeFunc();
}

inline void updateAllState(uint8_t* state, common::ValueVector* input, uint64_t multiplicity,
storage::MemoryManager* memoryManager) {
storage::MemoryManager* memoryManager) const {
return updateAllFunc(state, input, multiplicity, memoryManager);
}

inline void updatePosState(uint8_t* state, common::ValueVector* input, uint64_t multiplicity,
uint32_t pos, storage::MemoryManager* memoryManager) {
uint32_t pos, storage::MemoryManager* memoryManager) const {
return updatePosFunc(state, input, multiplicity, pos, memoryManager);
}

inline void combineState(
uint8_t* state, uint8_t* otherState, storage::MemoryManager* memoryManager) {
uint8_t* state, uint8_t* otherState, storage::MemoryManager* memoryManager) const {
return combineFunc(state, otherState, memoryManager);
}

inline void finalizeState(uint8_t* state) { return finalizeFunc(state); }
inline void finalizeState(uint8_t* state) const { return finalizeFunc(state); }

inline bool isFunctionDistinct() const { return isDistinct; }

Expand Down
2 changes: 1 addition & 1 deletion src/include/processor/operator/order_by/key_block_merger.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ class KeyBlockMergeTask {
}

private:
uint64_t findRightKeyBlockIdx(uint8_t* leftEndTuplePtr);
uint64_t findRightKeyBlockIdx(uint8_t* leftEndTuplePtr) const;

public:
static const uint32_t batch_size = 10000;
Expand Down
1 change: 1 addition & 0 deletions src/include/processor/operator/order_by/order_by_scan.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ struct OrderByScanLocalState {
void init(
std::vector<DataPos>& outVectorPos, SortSharedState& sharedState, ResultSet& resultSet);

// NOLINTNEXTLINE(readability-make-member-function-const): Updates vectorsToRead.
inline uint64_t scan() { return payloadScanner->scan(vectorsToRead); }
};

Expand Down
2 changes: 1 addition & 1 deletion src/include/processor/operator/order_by/radix_sort.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class RadixSort {
uint32_t numBytesToSort);

std::vector<TieRange> findTies(uint8_t* keyBlockPtr, uint32_t numTuplesToFindTies,
uint32_t numBytesToSort, uint32_t baseTupleIdx);
uint32_t numBytesToSort, uint32_t baseTupleIdx) const;

void fillTmpTuplePtrSortingBlock(TieRange& keyBlockTie, uint8_t* keyBlockPtr);

Expand Down
9 changes: 6 additions & 3 deletions src/include/processor/operator/order_by/top_k.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ class TopKSortState {

void finalize();

inline uint64_t getNumTuples() { return numTuples; }
inline uint64_t getNumTuples() const { return numTuples; }

inline SortSharedState* getSharedState() { return orderBySharedState.get(); }

std::unique_ptr<PayloadScanner> getScanner(uint64_t skip, uint64_t limit) {
std::unique_ptr<PayloadScanner> getScanner(uint64_t skip, uint64_t limit) const {
return std::make_unique<PayloadScanner>(orderBySharedState->getMergedKeyBlock(),
orderBySharedState->getPayloadTables(), skip, limit);
}
Expand Down Expand Up @@ -51,11 +51,12 @@ class TopKBuffer {

void reduce();

// NOLINTNEXTLINE(readability-make-member-function-const): Semantically non-const.
inline void finalize() { sortState->finalize(); }

void merge(TopKBuffer* other);

inline std::unique_ptr<PayloadScanner> getScanner() {
inline std::unique_ptr<PayloadScanner> getScanner() const {
return sortState->getScanner(skip, limit);
}

Expand Down Expand Up @@ -110,6 +111,7 @@ class TopKLocalState {
void append(const std::vector<common::ValueVector*>& keyVectors,
const std::vector<common::ValueVector*>& payloadVectors);

// NOLINTNEXTLINE(readability-make-member-function-const): Semantically non-const.
inline void finalize() { buffer->finalize(); }

std::unique_ptr<TopKBuffer> buffer;
Expand All @@ -128,6 +130,7 @@ class TopKSharedState {
buffer->merge(localState->buffer.get());
}

// NOLINTNEXTLINE(readability-make-member-function-const): Semantically non-const.
inline void finalize() { buffer->finalize(); }

std::unique_ptr<TopKBuffer> buffer;
Expand Down
1 change: 1 addition & 0 deletions src/include/processor/operator/order_by/top_k_scanner.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ struct TopKLocalScanState {
void init(
std::vector<DataPos>& outVectorPos, TopKSharedState& sharedState, ResultSet& resultSet);

// NOLINTNEXTLINE(readability-make-member-function-const): Semantically non-const.
inline uint64_t scan() { return payloadScanner->scan(vectorsToScan); }
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ struct SniffCSVColumnCountDriver {
bool emptyRow = true;
uint64_t numColumns = 0;

bool done(uint64_t rowNum);
bool done(uint64_t rowNum) const;
void addValue(uint64_t rowNum, common::column_id_t columnIdx, std::string_view value);
bool addRow(uint64_t rowNum, common::column_id_t columntCount);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ class ColumnReader {
uint64_t maxRepeat);
virtual ~ColumnReader() = default;
inline common::LogicalType* getDataType() const { return type.get(); }
inline bool hasDefines() { return maxDefine > 0; }
inline bool hasRepeats() { return maxRepeat > 0; }
inline bool hasDefines() const { return maxDefine > 0; }
inline bool hasRepeats() const { return maxRepeat > 0; }
virtual inline void skip(uint64_t numValues) { pendingSkips += numValues; }
virtual inline void dictionary(
const std::shared_ptr<ResizeableBuffer>& /*data*/, uint64_t /*num_entries*/) {
Expand Down Expand Up @@ -64,7 +64,7 @@ class ColumnReader {
void preparePage(kuzu_parquet::format::PageHeader& pageHdr);
void prepareDataPage(kuzu_parquet::format::PageHeader& pageHdr);
template<class VALUE_TYPE, class CONVERSION>
void plainTemplated(const std::shared_ptr<ByteBuffer>& plainData, uint8_t* defines,
void plainTemplated(const std::shared_ptr<ByteBuffer>& plainData, const uint8_t* defines,
uint64_t numValues, parquet_filter_t& filter, uint64_t resultOffset,
common::ValueVector* result) {
for (auto i = 0u; i < numValues; i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class DbpDecoder {

template<typename T>
void GetBatch(uint8_t* values_target_ptr, uint32_t batch_size) {
auto values = reinterpret_cast<T*>(values_target_ptr);
auto* values = reinterpret_cast<T*>(values_target_ptr);

if (batch_size == 0) {
return;
Expand Down Expand Up @@ -106,7 +106,7 @@ class DbpDecoder {
GetBatch<uint32_t>(reinterpret_cast<uint8_t*>(data.get()), values_left_in_miniblock);
}

uint64_t TotalValues() { return total_value_count; }
uint64_t TotalValues() const { return total_value_count; }

private:
ByteBuffer buffer_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class RleBpDecoder {

template<typename T>
void GetBatch(uint8_t* values_target_ptr, uint32_t batch_size) {
auto values = reinterpret_cast<T*>(values_target_ptr);
auto* values = reinterpret_cast<T*>(values_target_ptr);
uint32_t values_read = 0;

while (values_read < batch_size) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,15 @@ class ByteBuffer { // on to the 10 thousandth impl
return val;
}

void copyTo(char* dest, uint64_t len) {
void copyTo(char* dest, uint64_t len) const {
available(len);
std::memcpy(dest, ptr, len);
}

// NOLINTNEXTLINE(readability-make-member-function-const): Semantically non-const.
void zero() { std::memset(ptr, 0, len); }

void available(uint64_t req_len) {
void available(uint64_t req_len) const {
if (req_len > len) {
throw std::runtime_error("Out of buffer");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ class ThriftFileTransport

void SetLocation(uint64_t location_p) { location = location_p; }

uint64_t GetLocation() { return location; }
uint64_t GetLocation() const { return location; }
uint64_t GetSize() { return handle->getFileSize(); }

private:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class BooleanStatisticsState : public ColumnWriterStatistics {
bool max;

public:
bool hasStats() { return !(min && !max); }
bool hasStats() const { return !(min && !max); }

std::string getMin() override { return getMinValue(); }
std::string getMax() override { return getMaxValue(); }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class RleBpEncoder {
void writeValue(common::Serializer& writer, uint32_t value);
void finishWrite(common::Serializer& writer);

uint64_t getByteCount();
uint64_t getByteCount() const;

static uint8_t getVarintSize(uint32_t val);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class StringStatisticsState : public ColumnWriterStatistics {
std::string max;

public:
bool hasValidStats() { return hasStats; }
bool hasValidStats() const { return hasStats; }

void update(const common::ku_string_t& val);

Expand Down Expand Up @@ -54,7 +54,7 @@ class StringColumnWriterState : public BasicColumnWriterState {
// key_bit_width== 0 signifies the chunk is written in plain encoding
uint32_t keyBitWidth;

bool isDictionaryEncoded() { return keyBitWidth != 0; }
bool isDictionaryEncoded() const { return keyBitWidth != 0; }
};

class StringWriterPageState : public ColumnWriterPageState {
Expand All @@ -64,7 +64,7 @@ class StringWriterPageState : public ColumnWriterPageState {
KU_ASSERT(isDictionaryEncoded() || (bitWidth == 0 && dictionary.empty()));
}

inline bool isDictionaryEncoded() { return bitWidth != 0; }
inline bool isDictionaryEncoded() const { return bitWidth != 0; }
// If 0, we're writing a plain page.
uint32_t bitWidth;
const string_map_t<uint32_t>& dictionary;
Expand Down
Loading

0 comments on commit 3ab6bf1

Please sign in to comment.