Skip to content

Commit 6c84a6c

Browse files
committed
fix(build): Compile found by macOS15 intel compiler
Several issues were found in test programs: - Not directly comparable enums - Ambiguous function calls involving derived function overridden implementation - Incorrectly implemented derived function that don’t use const or override correctly I expect newer compiler to pick up on these eventually so let’s fix them now. These are test fixes and don’t affect engine code.
1 parent 4f7ea08 commit 6c84a6c

File tree

6 files changed

+51
-21
lines changed

6 files changed

+51
-21
lines changed

velox/dwio/dwrf/test/ColumnWriterTest.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,10 @@ class MockStreamInformation : public StreamInformation {
6565
return streamIdentifier_.encodingKey().sequence();
6666
}
6767

68-
MOCK_CONST_METHOD0(getOffset, uint64_t());
69-
MOCK_CONST_METHOD0(getLength, uint64_t());
70-
MOCK_CONST_METHOD0(getUseVInts, bool());
71-
MOCK_CONST_METHOD0(valid, bool());
68+
MOCK_METHOD(uint64_t, getOffset, (), (const, override));
69+
MOCK_METHOD(uint64_t, getLength, (), (const, override));
70+
MOCK_METHOD(bool, getUseVInts, (), (const, override));
71+
MOCK_METHOD(bool, valid, (), (const, override));
7272

7373
private:
7474
const DwrfStreamIdentifier& streamIdentifier_;

velox/dwio/dwrf/test/OrcTest.h

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,16 +100,20 @@ class MockStripeStreams : public StripeStreams {
100100
MOCK_METHOD2(
101101
genMockDictDataSetter,
102102
std::function<void(BufferPtr&, MemoryPool*)>(uint32_t, uint32_t));
103-
MOCK_METHOD0(
103+
MOCK_METHOD(
104+
std::shared_ptr<StripeDictionaryCache>,
104105
getStripeDictionaryCache,
105-
std::shared_ptr<StripeDictionaryCache>());
106+
(),
107+
(override));
106108
MOCK_CONST_METHOD1(getEncodingProxy, proto::ColumnEncoding*(uint64_t));
107109
MOCK_CONST_METHOD1(
108110
getEncodingOrcProxy,
109111
proto::orc::ColumnEncoding*(uint64_t));
110-
MOCK_CONST_METHOD2(
112+
MOCK_METHOD(
113+
uint32_t,
111114
visitStreamsOfNode,
112-
uint32_t(uint32_t, std::function<void(const StreamInformation&)>));
115+
(uint32_t, std::function<void(const StreamInformation&)>),
116+
(const, override));
113117
MOCK_CONST_METHOD3(
114118
getStreamProxy,
115119
dwio::common::SeekableInputStream*(uint32_t, proto::Stream_Kind, bool));

velox/dwio/dwrf/test/TestStripeStream.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -840,9 +840,11 @@ class TestStripeStreams : public StripeStreamsBase {
840840
MOCK_CONST_METHOD2(
841841
getEncodingOrcProxy,
842842
proto::orc::ColumnEncoding*(uint32_t, uint32_t));
843-
MOCK_CONST_METHOD2(
843+
MOCK_METHOD(
844+
uint32_t,
844845
visitStreamsOfNode,
845-
uint32_t(uint32_t, std::function<void(const StreamInformation&)>));
846+
(uint32_t, std::function<void(const StreamInformation&)>),
847+
(const, override));
846848
MOCK_CONST_METHOD4(
847849
getStreamProxy,
848850
SeekableInputStream*(uint32_t, uint32_t, proto::Stream_Kind, bool));

velox/dwio/dwrf/test/WriterFlushTest.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,11 @@ class MockMemoryPool : public velox::memory::MemoryPool {
173173
name, kind, parent, parent->capacity());
174174
}
175175

176-
MOCK_CONST_METHOD0(peakBytes, int64_t());
176+
MOCK_METHOD(int64_t, peakBytes, (), (const, override));
177177

178178
MOCK_METHOD1(updateSubtreeMemoryUsage, int64_t(int64_t));
179179

180-
MOCK_CONST_METHOD0(alignment, uint16_t());
180+
MOCK_METHOD(uint16_t, alignment, (), (const, override));
181181

182182
uint64_t freeBytes() const override {
183183
VELOX_UNSUPPORTED("{} unsupported", __FUNCTION__);

velox/dwio/parquet/writer/arrow/tests/FileDeserializeTest.cpp

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -350,13 +350,24 @@ void CheckDataPageHeader(
350350

351351
const DataPageV1* data_page = static_cast<const DataPageV1*>(page);
352352
ASSERT_EQ(expected.num_values, data_page->num_values());
353-
ASSERT_EQ(expected.encoding, data_page->encoding());
353+
// The encoding enum for thrift::DataPageHeader isn't the same as in
354+
// DataPageV1: parquet::thrift::Encoding::type vs
355+
// parquet::arrow::Encoding::type.
354356
ASSERT_EQ(
355-
expected.definition_level_encoding,
356-
data_page->definition_level_encoding());
357+
static_cast<std::underlying_type_t<thrift::Encoding::type>>(
358+
expected.encoding),
359+
static_cast<std::underlying_type_t<Encoding::type>>(
360+
data_page->encoding()));
357361
ASSERT_EQ(
358-
expected.repetition_level_encoding,
359-
data_page->repetition_level_encoding());
362+
static_cast<std::underlying_type_t<thrift::Encoding::type>>(
363+
expected.definition_level_encoding),
364+
static_cast<std::underlying_type_t<Encoding::type>>(
365+
data_page->definition_level_encoding()));
366+
ASSERT_EQ(
367+
static_cast<std::underlying_type_t<thrift::Encoding::type>>(
368+
expected.repetition_level_encoding),
369+
static_cast<std::underlying_type_t<Encoding::type>>(
370+
data_page->repetition_level_encoding()));
360371
CheckStatistics(expected, data_page->statistics());
361372
}
362373

@@ -370,7 +381,14 @@ void CheckDataPageHeader(
370381
ASSERT_EQ(expected.num_values, data_page->num_values());
371382
ASSERT_EQ(expected.num_nulls, data_page->num_nulls());
372383
ASSERT_EQ(expected.num_rows, data_page->num_rows());
373-
ASSERT_EQ(expected.encoding, data_page->encoding());
384+
// The encoding enum for thrift::DataPageHeader isn't the same as in
385+
// DataPageV2: parquet::thrift::Encoding::type vs
386+
// parquet::arrow::Encoding::type.
387+
ASSERT_EQ(
388+
static_cast<std::underlying_type_t<thrift::Encoding::type>>(
389+
expected.encoding),
390+
static_cast<std::underlying_type_t<Encoding::type>>(
391+
data_page->encoding()));
374392
ASSERT_EQ(
375393
expected.definition_levels_byte_length,
376394
data_page->definition_levels_byte_length());

velox/type/tests/TypeTest.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -678,10 +678,16 @@ class OpaqueWithMetadataType : public OpaqueType {
678678
: OpaqueType(std::type_index(typeid(OpaqueWithMetadata))),
679679
metadata(metadata) {}
680680

681+
bool operator==(const OpaqueWithMetadataType& other) const {
682+
return OpaqueType::operator==(other) && other.metadata == metadata;
683+
}
684+
681685
bool operator==(const Type& other) const override {
682-
return OpaqueType::operator==(other) &&
683-
reinterpret_cast<const OpaqueWithMetadataType*>(&other)->metadata ==
684-
metadata;
686+
if (const auto* otherType =
687+
reinterpret_cast<const OpaqueWithMetadataType*>(&other)) {
688+
return (*this == *otherType);
689+
}
690+
return false;
685691
}
686692

687693
folly::dynamic serialize() const override {

0 commit comments

Comments
 (0)