Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix warnings from GCC 13 #3387

Merged
merged 1 commit into from
Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/common/arrow/arrow_array_scan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -510,9 +510,11 @@ void ArrowConverter::fromArrowArray(const ArrowSchema* schema, const ArrowArray*
dstOffset, count);
case 'w': {
// ARRAY
auto arrowNumElements = std::stoul(arrowType + 3);
auto outputNumElements = ArrayType::getNumElements(&outputVector.dataType);
KU_ASSERT(arrowNumElements == outputNumElements);
RUNTIME_CHECK({
auto arrowNumElements = std::stoul(arrowType + 3);
auto outputNumElements = ArrayType::getNumElements(&outputVector.dataType);
KU_ASSERT(arrowNumElements == outputNumElements);
});
return scanArrowArrayFixedList(schema, array, outputVector, mask, srcOffset, dstOffset,
count);
}
Expand Down
7 changes: 1 addition & 6 deletions src/common/types/blob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,8 @@ uint64_t Blob::fromString(const char* str, uint64_t length, uint8_t* resultBuffe
resultBuffer[resultPos++] =
(firstByte << HexFormatConstants::NUM_BYTES_TO_SHIFT_FOR_FIRST_BYTE) + secondByte;
i += HexFormatConstants::LENGTH - 1;
} else if (str[i] <= 127) {
resultBuffer[resultPos++] = str[i];
} else {
// LCOV_EXCL_START
throw InternalException("Invalid byte encountered in STRING -> BLOB conversion that "
"should have been caught during getBlobSize");
// LCOV_EXCL_STOP
resultBuffer[resultPos++] = str[i];
}
}
return resultPos;
Expand Down
8 changes: 4 additions & 4 deletions src/function/cast/cast_array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,18 @@ bool CastArrayHelper::checkCompatibleNestedTypes(LogicalTypeID sourceTypeID,
if (targetTypeID == LogicalTypeID::ARRAY || targetTypeID == LogicalTypeID::LIST) {
return true;
}
}
} break;
case LogicalTypeID::MAP:
case LogicalTypeID::STRUCT: {
if (sourceTypeID == targetTypeID) {
return true;
}
}
} break;
case LogicalTypeID::ARRAY: {
if (targetTypeID == LogicalTypeID::LIST || targetTypeID == LogicalTypeID::ARRAY) {
return true;
}
}
} break;
default:
return false;
}
Expand Down Expand Up @@ -62,7 +62,7 @@ bool CastArrayHelper::containsListToArray(const LogicalType* srcType, const Logi
return true;
}
}
}
} break;
default:
return false;
}
Expand Down
1 change: 1 addition & 0 deletions src/function/vector_cast_functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,7 @@ static std::unique_ptr<ScalarFunction> bindCastBetweenNested(const std::string&
std::vector<LogicalTypeID>{sourceTypeID}, targetTypeID,
nestedTypesCastExecFunction);
}
[[fallthrough]];
}
default:
throw ConversionException{stringFormat("Unsupported casting function from {} to {}.",
Expand Down
3 changes: 3 additions & 0 deletions src/include/common/assert.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,19 @@ namespace common {
}

#if defined(KUZU_RUNTIME_CHECKS) || !defined(NDEBUG)
#define RUNTIME_CHECK(code) code
#define KU_ASSERT(condition) \
static_cast<bool>(condition) ? \
void(0) : \
kuzu::common::kuAssertFailureInternal(#condition, __FILE__, __LINE__)
#else
#define KU_ASSERT(condition) void(0)
#define RUNTIME_CHECK(code) void(0)
#endif

#define KU_UNREACHABLE \
[[unlikely]] kuzu::common::kuAssertFailureInternal("KU_UNREACHABLE", __FILE__, __LINE__)
#define KU_UNUSED(expr) (void)(expr)

} // namespace common
} // namespace kuzu
2 changes: 1 addition & 1 deletion src/include/common/types/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ concept HashableTypes = (std::integral<T> || std::floating_point<T> ||
std::is_same_v<T, list_entry_t> || std::is_same_v<T, internalID_t> ||
std::is_same_v<T, interval_t> || std::is_same_v<T, ku_string_t>);

enum class KUZU_API LogicalTypeID : uint8_t {
enum class LogicalTypeID : uint8_t {
ANY = 0,
NODE = 10,
REL = 11,
Expand Down
4 changes: 1 addition & 3 deletions src/include/function/function.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ struct Function {
Function(std::string name, std::vector<common::LogicalTypeID> parameterTypeIDs)
: name{std::move(name)}, parameterTypeIDs{std::move(parameterTypeIDs)}, isVarLength{false} {
}
Function(const Function& other)
: name{other.name}, parameterTypeIDs{other.parameterTypeIDs},
isVarLength{other.isVarLength} {}
Function(const Function&) = default;

virtual ~Function() = default;

Expand Down
7 changes: 3 additions & 4 deletions src/include/main/plan_printer.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@ class OpProfileTree {
static std::string genHorizLine(uint32_t len);

inline void validateRowIdxAndColIdx(uint32_t rowIdx, uint32_t colIdx) const {
KU_ASSERT(0 <= rowIdx && rowIdx < opProfileBoxes.size() && 0 <= colIdx &&
colIdx < opProfileBoxes[rowIdx].size());
KU_ASSERT(rowIdx < opProfileBoxes.size() && colIdx < opProfileBoxes[rowIdx].size());
(void)rowIdx;
(void)colIdx;
}
Expand All @@ -70,8 +69,8 @@ class OpProfileTree {
OpProfileBox* getOpProfileBox(uint32_t rowIdx, uint32_t colIdx) const;

bool hasOpProfileBox(uint32_t rowIdx, uint32_t colIdx) const {
return 0 <= rowIdx && rowIdx < opProfileBoxes.size() && 0 <= colIdx &&
colIdx < opProfileBoxes[rowIdx].size() && getOpProfileBox(rowIdx, colIdx);
return rowIdx < opProfileBoxes.size() && colIdx < opProfileBoxes[rowIdx].size() &&
getOpProfileBox(rowIdx, colIdx);
}

//! Returns true if there is a valid OpProfileBox on the upper left side of the OpProfileBox
Expand Down
21 changes: 11 additions & 10 deletions src/include/optimizer/remove_unnecessary_join_optimizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,17 @@
namespace kuzu {
namespace optimizer {

// Due to the nature of graph pattern, a (node)-[rel]-(node) is always interpreted as two joins.
// However, in many cases, a single join is sufficient.
// E.g. MATCH (a)-[e]->(b) RETURN e.date
// Our planner will generate a plan where the HJ is redundant.
// HJ
// / \
// E(e) S(b)
// |
// S(a)
// This optimizer prunes such redundant joins.
/* Due to the nature of graph pattern, a (node)-[rel]-(node) is always interpreted as two joins.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, was this causing a warning?

Copy link
Collaborator Author

@benjaminwinger benjaminwinger Apr 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was giving the warning multi-line comment [-Werror=comment specifically on the line with the backslash

From the gcc docs:

Warn whenever a comment-start sequence ‘/*’ appears in a ‘/*’ comment, or whenever a backslash-newline appears in a ‘//’ comment. This warning is enabled by -Wall.

Basically it thinks you're trying to use a single line comment as a multi-line by including a backslash+newline:

//      HJ
//     /  \
     E(e) S(b)
//    |
//   S(a)

Because technically the above would be valid.

* However, in many cases, a single join is sufficient.
* E.g. MATCH (a)-[e]->(b) RETURN e.date
* Our planner will generate a plan where the HJ is redundant.
* HJ
* / \
* E(e) S(b)
* |
* S(a)
* This optimizer prunes such redundant joins.
*/
class RemoveUnnecessaryJoinOptimizer : public LogicalOperatorVisitor {
public:
void rewrite(planner::LogicalPlan* plan);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ class ParquetReader {
inline const kuzu_parquet::format::RowGroup& getGroup(ParquetReaderScanState& state) {
KU_ASSERT(
state.currentGroup >= 0 && (uint64_t)state.currentGroup < state.groupIdxList.size());
KU_ASSERT(state.groupIdxList[state.currentGroup] >= 0 &&
state.groupIdxList[state.currentGroup] < metadata->row_groups.size());
KU_ASSERT(state.groupIdxList[state.currentGroup] < metadata->row_groups.size());
return metadata->row_groups[state.groupIdxList[state.currentGroup]];
}
static std::unique_ptr<common::LogicalType> deriveLogicalType(
Expand Down
6 changes: 4 additions & 2 deletions src/planner/operator/extend/logical_extend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ f_group_pos_set LogicalExtend::getGroupsPosToFlatten() {

void LogicalExtend::computeFactorizedSchema() {
copyChildSchema(0);
auto boundGroupPos = schema->getGroupPos(*boundNode->getInternalID());
RUNTIME_CHECK({
auto boundGroupPos = schema->getGroupPos(*boundNode->getInternalID());
KU_ASSERT(schema->getGroup(boundGroupPos)->isFlat());
});
uint32_t nbrGroupPos = 0u;
KU_ASSERT(schema->getGroup(boundGroupPos)->isFlat());
nbrGroupPos = schema->createGroup();
schema->insertToGroupAndScope(nbrNode->getInternalID(), nbrGroupPos);
for (auto& property : properties) {
Expand Down
8 changes: 5 additions & 3 deletions src/processor/operator/aggregate/simple_aggregate_scan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ void SimpleAggregateScan::initLocalStateInternal(ResultSet* resultSet, Execution
BaseAggregateScan::initLocalStateInternal(resultSet, context);
KU_ASSERT(!aggregatesPos.empty());
auto outDataChunkPos = aggregatesPos[0].dataChunkPos;
for (auto& dataPos : aggregatesPos) {
KU_ASSERT(dataPos.dataChunkPos == outDataChunkPos);
}
RUNTIME_CHECK({
for (auto& dataPos : aggregatesPos) {
KU_ASSERT(dataPos.dataChunkPos == outDataChunkPos);
}
});
outDataChunk = resultSet->dataChunks[outDataChunkPos].get();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ static std::unique_ptr<function::TableFuncBindData> bindFunc(main::ClientContext
resultColumnNames, scanInput->expectedColumnTypes, detectedColumnTypes, resultColumnTypes);
auto config = scanInput->config.copy();
KU_ASSERT(!config.filePaths.empty() && config.getNumFiles() == resultColumnNames.size());
row_idx_t numRows;
row_idx_t numRows = 0;
for (auto i = 0u; i < config.getNumFiles(); i++) {
auto reader = make_unique<NpyReader>(config.filePaths[i]);
if (i == 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <sstream>

#include "brotli/decode.h"
#include "common/assert.h"
#include "common/exception/not_implemented.h"
#include "common/exception/runtime.h"
#include "common/types/date_t.h"
Expand Down Expand Up @@ -539,6 +540,7 @@ std::unique_ptr<ColumnReader> ColumnReader::createTimestampReader(ParquetReader&
}
// LCOV_EXCL_STOP
}
KU_UNREACHABLE;
}
default: {
KU_UNREACHABLE;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "processor/operator/persistent/reader/parquet/parquet_timestamp.h"

#include <cstring>

namespace kuzu {
namespace processor {

Expand All @@ -22,7 +24,8 @@ common::timestamp_t ParquetTimeStampUtils::parquetTimestampNsToTimestamp(const i

int64_t ParquetTimeStampUtils::impalaTimestampToMicroseconds(const Int96& impalaTimestamp) {
int64_t daysSinceEpoch = impalaTimestamp.value[2] - JULIAN_TO_UNIX_EPOCH_DAYS;
auto nanoSeconds = *reinterpret_cast<const int64_t*>(impalaTimestamp.value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acquamarin should take a look here and add a test if needed.

int64_t nanoSeconds;
memcpy(&nanoSeconds, &impalaTimestamp.value, sizeof(nanoSeconds));
auto microseconds = nanoSeconds / NANOSECONDS_PER_MICRO;
return daysSinceEpoch * MICROSECONDS_PER_DAY + microseconds;
}
Expand Down
1 change: 1 addition & 0 deletions src/storage/compression/compression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ template<typename T>
void IntegerBitpacking<T>::setValuesFromUncompressed(const uint8_t* srcBuffer, offset_t posInSrc,
uint8_t* dstBuffer, offset_t posInDst, offset_t numValues, const CompressionMetadata& metadata,
const NullMask* nullMask) const {
KU_UNUSED(nullMask);
auto header = BitpackHeader::readHeader(metadata.data);
// This is a fairly naive implementation which uses fastunpack/fastpack
// to modify the data by decompressing/compressing a single chunk of values.
Expand Down
2 changes: 1 addition & 1 deletion src/storage/index/hash_index.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ void HashIndex<T>::mergeBulkInserts() {
// TODO: one pass would also reduce locking when frames are unpinned,
// which is useful if this can be paralellized
reserve(bulkInsertLocalStorage.size());
auto originalNumEntries = this->indexHeaderForWriteTrx->numEntries;
RUNTIME_CHECK(auto originalNumEntries = this->indexHeaderForWriteTrx->numEntries;);

// Storing as many slots in-memory as on-disk shouldn't be necessary (for one it makes memory
// usage an issue as we may need significantly more memory to store the slots that we would
Expand Down
3 changes: 1 addition & 2 deletions src/storage/store/list_column.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ void ListColumn::scan(Transaction* transaction, ChunkState& readState, offset_t
auto offsetToWriteListData = listOffsetInVector;
auto numValues = endOffsetInGroup - startOffsetInGroup;
numValues = std::min(numValues, listOffsetInfoInStorage.numTotal);
KU_ASSERT(numValues >= 0);
KU_ASSERT(endOffsetInGroup >= startOffsetInGroup);
for (auto i = 0u; i < numValues; i++) {
list_size_t size = listOffsetInfoInStorage.getListSize(i);
resultVector->setValue(i + offsetInVector, list_entry_t{listOffsetInVector, size});
Expand All @@ -105,7 +105,6 @@ void ListColumn::scan(Transaction* transaction, ChunkState& readState, offset_t
for (auto i = 0u; i < numValues; i++) {
offset_t startOffset = listOffsetInfoInStorage.getListStartOffset(i);
offset_t appendSize = listOffsetInfoInStorage.getListSize(i);
KU_ASSERT(appendSize >= 0);
dataColumn->scan(transaction,
readState.childrenStates[DATA_COLUMN_CHILD_READ_STATE_IDX], startOffset,
startOffset + appendSize, dataVector, offsetToWriteListData);
Expand Down
8 changes: 4 additions & 4 deletions test/main/system_config_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,21 @@ TEST_F(SystemConfigTest, testMaxDBSize) {
systemConfig->maxDBSize = 1024;
try {
auto db = std::make_unique<Database>(databasePath, *systemConfig);
} catch (BufferManagerException e) {
} catch (const BufferManagerException& e) {
ASSERT_EQ(std::string(e.what()),
"Buffer manager exception: The given max db size should be at least 4194304 bytes.");
}
systemConfig->maxDBSize = 4194305;
try {
auto db = std::make_unique<Database>(databasePath, *systemConfig);
} catch (BufferManagerException e) {
} catch (const BufferManagerException& e) {
ASSERT_EQ(std::string(e.what()),
"Buffer manager exception: The given max db size should be a power of 2.");
}
systemConfig->maxDBSize = 4194304;
try {
auto db = std::make_unique<Database>(databasePath, *systemConfig);
} catch (BufferManagerException e) {
} catch (const BufferManagerException& e) {
ASSERT_EQ(std::string(e.what()),
"Buffer manager exception: No more frame groups can be added to the allocator.");
}
Expand All @@ -60,7 +60,7 @@ TEST_F(SystemConfigTest, testBufferPoolSize) {
systemConfig->bufferPoolSize = 1024;
try {
auto db = std::make_unique<Database>(databasePath, *systemConfig);
} catch (BufferManagerException e) {
} catch (const BufferManagerException& e) {
ASSERT_EQ(std::string(e.what()),
"Buffer manager exception: The given buffer pool size should be at least 4KB.");
}
Expand Down
2 changes: 1 addition & 1 deletion test/test_helper/test_helper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ std::vector<std::unique_ptr<TestQueryConfig>> TestHelper::parseTestFile(const st
}
std::ifstream ifs(path);
std::string line;
TestQueryConfig* currentConfig;
TestQueryConfig* currentConfig = nullptr;
while (getline(ifs, line)) {
if (line.starts_with("-NAME")) {
auto config = std::make_unique<TestQueryConfig>();
Expand Down
2 changes: 1 addition & 1 deletion third_party/antlr4_runtime/src/tree/ParseTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ namespace tree {
/// based upon the parser.
virtual std::string toStringTree(Parser *parser, bool pretty = false) = 0;

virtual bool operator == (const ParseTree &other) const;
bool operator == (const ParseTree &other) const;

/// The <seealso cref="ParseTreeVisitor"/> needs a double dispatch method.
// ml: This has been changed to use Any instead of a template parameter, to avoid the need of a virtual template function.
Expand Down
6 changes: 3 additions & 3 deletions third_party/mbedtls/library/constant_time.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ int mbedtls_ct_memcmp(const void* a, const void* b, size_t n) {
* This avoids IAR compiler warning:
* 'the order of volatile accesses is undefined ..' */
unsigned char x = A[i], y = B[i];
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-volatile"
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-volatile"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And now GCC 13 complains that pragma clang is an unknown pragma. I'm not sure we can win this one without globally opting out.

diff |= x ^ y;
#pragma GCC diagnostic pop
#pragma clang diagnostic pop
}

return ((int)diff);
Expand Down
9 changes: 7 additions & 2 deletions third_party/miniparquet/src/thrift/Thrift.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* under the License.
*/

#include <cstddef>
#ifndef _THRIFT_THRIFT_H_
#define _THRIFT_THRIFT_H_ 1

Expand Down Expand Up @@ -50,9 +51,13 @@
namespace kuzu_apache {
namespace thrift {

class TEnumIterator
: public std::iterator<std::forward_iterator_tag, std::pair<int, const char*> > {
class TEnumIterator {
public:
using iterator_category = std::forward_iterator_tag;
using value_type = std::pair<int, const char*>;
using pointer = std::pair<int, const char*>*;
using reference = std::pair<int, const char*>&;
using difference_type = std::ptrdiff_t;
TEnumIterator(int n, int* enums, const char** names)
: ii_(0), n_(n), enums_(enums), names_(names) {}

Expand Down
2 changes: 1 addition & 1 deletion tools/shell/embedded_shell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ void highlight(char* buffer, char* resultBuf, uint32_t renderWidth, uint32_t cur
tokenList.emplace_back(word);
for (std::string& token : tokenList) {
if (token.find(' ') == std::string::npos) {
for (const std::string& keyword : keywordList) {
for (const std::string keyword : keywordList) {
if (regex_search(token,
std::regex("^" + keyword + "$", std::regex_constants::icase)) ||
regex_search(token,
Expand Down
Loading