Skip to content

Commit

Permalink
Switch to use C++20 standard
Browse files Browse the repository at this point in the history
  • Loading branch information
czentgr committed Sep 12, 2024
1 parent d1ac079 commit af0b130
Show file tree
Hide file tree
Showing 12 changed files with 45 additions and 14 deletions.
23 changes: 22 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,21 @@ endif()
# Required so velox code can be used in a dynamic library
set(CMAKE_POSITION_INDEPENDENT_CODE TRUE)

set(CMAKE_CXX_STANDARD 17)
# For C++20 support we need GNU GCC11 (or later versions) or Clang/AppleClang 15
# (or later versions) to get support for the used features.
if(NOT
(("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU" AND ${CMAKE_CXX_COMPILER_VERSION}
VERSION_GREATER_EQUAL 11)
OR (("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang"
OR "${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang")
AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 15)))
message(
FATAL_ERROR
"Unsupported compiler ${CMAKE_CXX_COMPILER_ID} with version ${CMAKE_CXX_COMPILER_VERSION} found."
)
endif()

set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED True)

execute_process(
Expand Down Expand Up @@ -338,6 +352,13 @@ if("${ENABLE_ALL_WARNINGS}")
-Wno-unused-result \
-Wno-format-overflow \
-Wno-strict-aliasing")
# Avoid compiler bug for GCC 12.2.1
# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105329
if (CMAKE_CXX_COMPILER_VERSION VERSION_EQUAL "12.2.1")
set(KNOWN_COMPILER_SPECIFIC_WARNINGS
"${KNOWN_COMPILER_SPECIFIC_WARNINGS} \
-Wno-restrict")
endif()
endif()

set(KNOWN_WARNINGS
Expand Down
2 changes: 1 addition & 1 deletion pyvelox/pyvelox.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ static void addExpressionBindings(
std::vector<VectorPtr> inputs;
names.reserve(name_input_map.size());
inputs.reserve(name_input_map.size());
for (const std::pair<std::string, VectorPtr>& pair :
for (const std::pair<const std::string, VectorPtr>& pair :
name_input_map) {
names.push_back(pair.first);
inputs.push_back(pair.second);
Expand Down
4 changes: 2 additions & 2 deletions velox/common/base/Semaphore.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ class Semaphore {
private:
std::mutex mutex_;
std::condition_variable cv_;
volatile int32_t count_;
volatile int32_t numWaiting_{0};
std::atomic<int32_t> count_;
std::atomic<int32_t> numWaiting_{0};
};

} // namespace facebook::velox
2 changes: 1 addition & 1 deletion velox/common/base/tests/Memcpy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ int main(int argc, char** argv) {
Semaphore sem(0);
std::vector<CopyCallable> ops;
ops.resize(FLAGS_threads);
volatile uint64_t totalSum = 0;
std::atomic<uint64_t> totalSum = 0;
uint64_t totalUsec = 0;
for (auto repeat = 0; repeat < FLAGS_repeats; ++repeat) {
// Read once through 'other' to clear cache effects.
Expand Down
3 changes: 1 addition & 2 deletions velox/connectors/hive/iceberg/IcebergSplit.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@
#include <string>

#include "velox/connectors/hive/HiveConnectorSplit.h"
#include "velox/connectors/hive/iceberg/IcebergDeleteFile.h"

namespace facebook::velox::connector::hive::iceberg {

struct IcebergDeleteFile;

struct HiveIcebergSplit : public connector::hive::HiveConnectorSplit {
std::vector<IcebergDeleteFile> deleteFiles;

Expand Down
6 changes: 5 additions & 1 deletion velox/core/Expressions.h
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,11 @@ class LambdaTypedExpr : public ITypedExpr {
if (*casted->type() != *this->type()) {
return false;
}
return *signature_ == *casted->signature_ && *body_ == *casted->body_;
return operator==(*casted);
}

bool operator==(const LambdaTypedExpr& other) const {
return signature_ == other.signature_ && body_ == other.body_;
}

folly::dynamic serialize() const override;
Expand Down
2 changes: 1 addition & 1 deletion velox/dwio/common/SelectiveColumnReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ class SelectiveColumnReader {
template <typename T>
inline void addValue(T value) {
static_assert(
std::is_pod_v<T>,
std::is_standard_layout_v<T>,
"General case of addValue is only for primitive types");
VELOX_DCHECK_NOT_NULL(rawValues_);
VELOX_DCHECK_LE((numValues_ + 1) * sizeof(T), values_->capacity());
Expand Down
2 changes: 1 addition & 1 deletion velox/expression/tests/ExprEncodingsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ class ExprEncodingsTest
execCtx_->pool(),
vector->type(),
vector->size(),
std::make_unique<SimpleVectorLoader>([=](RowSet /*rows*/) {
std::make_unique<SimpleVectorLoader>([=, this](RowSet /*rows*/) {
auto indices =
makeIndices(vector->size(), [](auto row) { return row; });
return wrapInDictionary(indices, vector->size(), vector);
Expand Down
2 changes: 1 addition & 1 deletion velox/expression/tests/ExprTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3958,7 +3958,7 @@ TEST_P(ParameterizedExprTest, cseOverLazyDictionary) {
pool(),
BIGINT(),
5,
std::make_unique<SimpleVectorLoader>([=](RowSet /*rows*/) {
std::make_unique<SimpleVectorLoader>([=, this](RowSet /*rows*/) {
return wrapInDictionary(
makeIndicesInReverse(5),
makeFlatVector<int64_t>({8, 9, 10, 11, 12}));
Expand Down
7 changes: 7 additions & 0 deletions velox/external/date/date.h
Original file line number Diff line number Diff line change
Expand Up @@ -3970,6 +3970,12 @@ make_time(const std::chrono::duration<Rep, Period>& d)
return hh_mm_ss<std::chrono::duration<Rep, Period>>(d);
}

/// Building on macOS using Apple Clang 15.0.0.15000309 (Xcode_15.4) results in
/// ambiguous symbols (related to std::chrono::duration<long long> type parameter) because
/// the newer SDK includes a definitoon for this overloaded operator when C++20 standard is used.
/// AppleClang 15.0.0.15000309 (Xcode_15.4) or newer does not need the overloaded definition but
/// AppleClang 15.0.0.15000100 (Xcode_15.2), for example, does.
#if !defined(__APPLE__) || (defined(__APPLE__) && (__apple_build_version__ < 15000309))
template <class CharT, class Traits, class Duration>
inline
typename std::enable_if
Expand All @@ -3983,6 +3989,7 @@ operator<<(std::basic_ostream<CharT, Traits>& os, const sys_time<Duration>& tp)
auto const dp = date::floor<days>(tp);
return os << year_month_day(dp) << ' ' << make_time(tp-dp);
}
#endif // !defined(__APPLE__) || (defined(__APPLE__) && (__apple_build_version__ < 15000309))

template <class CharT, class Traits>
inline
Expand Down
2 changes: 1 addition & 1 deletion velox/vector/tests/utils/VectorMaker.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class VectorMaker {
pool_,
CppToType<T>::create(),
size,
std::make_unique<SimpleVectorLoader>([=](RowSet rowSet) {
std::make_unique<SimpleVectorLoader>([=, this](RowSet rowSet) {
// Populate requested rows with correct data and fill in gaps with
// "garbage".
SelectivityVector rows(rowSet.back() + 1, false);
Expand Down
4 changes: 2 additions & 2 deletions velox/vector/tests/utils/VectorTestBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -781,7 +781,7 @@ class VectorTestBase {
pool(),
CppToType<T>::create(),
size,
std::make_unique<SimpleVectorLoader>([=](RowSet rows) {
std::make_unique<SimpleVectorLoader>([=, this](RowSet rows) {
if (expectedSize.has_value()) {
VELOX_CHECK_EQ(rows.size(), *expectedSize);
}
Expand All @@ -799,7 +799,7 @@ class VectorTestBase {
pool(),
vector->type(),
vector->size(),
std::make_unique<SimpleVectorLoader>([=](RowSet /*rows*/) {
std::make_unique<SimpleVectorLoader>([=, this](RowSet /*rows*/) {
auto indices =
makeIndices(vector->size(), [](auto row) { return row; });
return wrapInDictionary(indices, vector->size(), vector);
Expand Down

0 comments on commit af0b130

Please sign in to comment.