Skip to content

Commit

Permalink
Fix fmt V11 build errors (facebookincubator#10904)
Browse files Browse the repository at this point in the history
Summary:
Recently brew updated fmt version from 10.1 to 11.0. In order to prevent build errors from happening after we upgrade Folly, the following changes are made:

1. Error "'this' argument to member function 'format' has type 'const fmt::formatter<facebook::velox::exec::Spiller::Type>', but function is not marked const".
The new fmt v11 forces the const check. To fix this error, the "const" keyword was added to the `format()` function declarations.

2. Error "error: static assertion failed due to requirement 'formattable_char': Mixing character types is disallowed." The new fmt v11 added a check if the arguments of fmt::make_format_args() have mixing different character types (e.g., char vs wchar_t or std::string vs folly::Range<const char*>). To fix this, we need to convert the folly::StringPiece to string.

These changes will be needed after we upgrade Folly, which is also dependent on fmt v11. The changes are backward compatible for fmt v10.

Fixes facebookincubator#10886

Pull Request resolved: facebookincubator#10904

Reviewed By: pedroerp

Differential Revision: D62902820

Pulled By: kgpai

fbshipit-source-id: 2dfa34515a642290290fdceab328eefc6a63ab11
  • Loading branch information
yingsu00 authored and facebook-github-bot committed Sep 21, 2024
1 parent c61a353 commit 1e736ba
Show file tree
Hide file tree
Showing 38 changed files with 89 additions and 74 deletions.
2 changes: 1 addition & 1 deletion velox/benchmarks/basic/LikeTpchBenchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ enum class TpchBenchmarkCase {

template <>
struct fmt::formatter<TpchBenchmarkCase> : fmt::formatter<int> {
auto format(const TpchBenchmarkCase& s, format_context& ctx) {
auto format(const TpchBenchmarkCase& s, format_context& ctx) const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
Expand Down
3 changes: 1 addition & 2 deletions velox/common/base/Exceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@
#include <sstream>

#include <fmt/format.h>
#include <folly/Preprocessor.h>

#include <fmt/ostream.h>
#include <folly/Preprocessor.h>

#include "velox/common/base/FmtStdFormatters.h"
#include "velox/common/base/VeloxException.h"
Expand Down
1 change: 1 addition & 0 deletions velox/common/base/Fs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#include "velox/common/base/Fs.h"

#include <fmt/format.h>
#include <glog/logging.h>

Expand Down
8 changes: 5 additions & 3 deletions velox/common/base/RuntimeMetrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@

#pragma once

#include <fmt/format.h>
#include <folly/CppAttributes.h>
#include <limits>
#include <sstream>

#include <fmt/format.h>
#include <folly/CppAttributes.h>

namespace facebook::velox {

struct RuntimeCounter {
Expand Down Expand Up @@ -113,7 +114,8 @@ class RuntimeStatWriterScopeGuard {
} // namespace facebook::velox
template <>
struct fmt::formatter<facebook::velox::RuntimeCounter::Unit> : formatter<int> {
auto format(facebook::velox::RuntimeCounter::Unit s, format_context& ctx) {
auto format(facebook::velox::RuntimeCounter::Unit s, format_context& ctx)
const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
7 changes: 4 additions & 3 deletions velox/common/base/SpillStats.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@
* limitations under the License.
*/
#pragma once

#include <stdint.h>
#include <string.h>

#include <folly/executors/CPUThreadPoolExecutor.h>

#include "velox/common/compression/Compression.h"

namespace facebook::velox::common {
Expand Down Expand Up @@ -166,9 +168,8 @@ SpillStats globalSpillStats();
template <>
struct fmt::formatter<facebook::velox::common::SpillStats>
: fmt::formatter<std::string> {
auto format(
const facebook::velox::common::SpillStats& s,
format_context& ctx) {
auto format(const facebook::velox::common::SpillStats& s, format_context& ctx)
const {
return formatter<std::string>::format(s.toString(), ctx);
}
};
9 changes: 5 additions & 4 deletions velox/common/base/Status.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@

#pragma once

#include <string>
#include <utility>

#include <fmt/format.h>
#include <fmt/ostream.h>
#include <folly/Expected.h>
#include <folly/Likely.h>
#include <string>
#include <utility>

namespace facebook::velox {

Expand Down Expand Up @@ -519,15 +520,15 @@ using Expected = folly::Expected<T, Status>;

template <>
struct fmt::formatter<facebook::velox::Status> : fmt::formatter<std::string> {
auto format(const facebook::velox::Status& s, format_context& ctx) {
auto format(const facebook::velox::Status& s, format_context& ctx) const {
return formatter<std::string>::format(s.toString(), ctx);
}
};

template <>
struct fmt::formatter<facebook::velox::StatusCode>
: fmt::formatter<std::string_view> {
auto format(facebook::velox::StatusCode code, format_context& ctx) {
auto format(facebook::velox::StatusCode code, format_context& ctx) const {
return formatter<std::string_view>::format(
facebook::velox::toString(code), ctx);
}
Expand Down
5 changes: 3 additions & 2 deletions velox/common/caching/AsyncDataCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@
#include <deque>

#include <fmt/format.h>
#include <folly/GLog.h>
#include <folly/chrono/Hardware.h>
#include <folly/container/F14Set.h>
#include <folly/futures/SharedPromise.h>
#include "folly/GLog.h"

#include "velox/common/base/BitUtil.h"
#include "velox/common/base/CoalesceIo.h"
#include "velox/common/base/Portability.h"
Expand Down Expand Up @@ -978,7 +979,7 @@ struct fmt::formatter<facebook::velox::cache::CoalescedLoad::State>
: formatter<int> {
auto format(
facebook::velox::cache::CoalescedLoad::State s,
format_context& ctx) {
format_context& ctx) const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
2 changes: 1 addition & 1 deletion velox/common/compression/Compression.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ struct fmt::formatter<facebook::velox::common::CompressionKind>
: fmt::formatter<std::string> {
auto format(
const facebook::velox::common::CompressionKind& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string>::format(
facebook::velox::common::compressionKindToString(s), ctx);
}
Expand Down
2 changes: 1 addition & 1 deletion velox/common/memory/MemoryAllocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ struct fmt::formatter<facebook::velox::memory::MemoryAllocator::InjectedFailure>
: fmt::formatter<int> {
auto format(
facebook::velox::memory::MemoryAllocator::InjectedFailure s,
format_context& ctx) {
format_context& ctx) const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
2 changes: 1 addition & 1 deletion velox/common/memory/MemoryArbitrator.h
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ struct fmt::formatter<facebook::velox::memory::MemoryArbitrator::Stats>
: formatter<std::string> {
auto format(
facebook::velox::memory::MemoryArbitrator::Stats s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string>::format(s.toString(), ctx);
}
};
Expand Down
5 changes: 2 additions & 3 deletions velox/common/memory/MemoryPool.h
Original file line number Diff line number Diff line change
Expand Up @@ -1109,9 +1109,8 @@ class StlAllocator {
template <>
struct fmt::formatter<facebook::velox::memory::MemoryPool::Kind>
: formatter<std::string> {
auto format(
facebook::velox::memory::MemoryPool::Kind s,
format_context& ctx) {
auto format(facebook::velox::memory::MemoryPool::Kind s, format_context& ctx)
const {
return formatter<std::string>::format(
facebook::velox::memory::MemoryPool::kindString(s), ctx);
}
Expand Down
4 changes: 2 additions & 2 deletions velox/connectors/fuzzer/FuzzerConnectorSplit.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ struct fmt::formatter<facebook::velox::connector::fuzzer::FuzzerConnectorSplit>
: formatter<std::string> {
auto format(
facebook::velox::connector::fuzzer::FuzzerConnectorSplit s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string>::format(s.toString(), ctx);
}
};
Expand All @@ -46,7 +46,7 @@ struct fmt::formatter<
auto format(
std::shared_ptr<facebook::velox::connector::fuzzer::FuzzerConnectorSplit>
s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string>::format(s->toString(), ctx);
}
};
4 changes: 2 additions & 2 deletions velox/connectors/hive/HiveDataSink.h
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ struct fmt::formatter<facebook::velox::connector::hive::HiveDataSink::State>
: formatter<int> {
auto format(
facebook::velox::connector::hive::HiveDataSink::State s,
format_context& ctx) {
format_context& ctx) const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
Expand All @@ -637,7 +637,7 @@ struct fmt::formatter<
: formatter<int> {
auto format(
facebook::velox::connector::hive::LocationHandle::TableType s,
format_context& ctx) {
format_context& ctx) const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
1 change: 1 addition & 0 deletions velox/connectors/hive/HiveDataSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#include "velox/connectors/hive/HiveDataSource.h"

#include <fmt/ranges.h>
#include <string>
#include <unordered_map>

Expand Down
2 changes: 1 addition & 1 deletion velox/connectors/tpch/TpchConnectorSplit.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ struct fmt::formatter<
: formatter<std::string> {
auto format(
std::shared_ptr<facebook::velox::connector::tpch::TpchConnectorSplit> s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string>::format(s->toString(), ctx);
}
};
2 changes: 1 addition & 1 deletion velox/core/PlanFragment.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ struct fmt::formatter<facebook::velox::core::ExecutionStrategy>
: formatter<int> {
auto format(
const facebook::velox::core::ExecutionStrategy& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
4 changes: 2 additions & 2 deletions velox/core/PlanNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -2402,15 +2402,15 @@ struct fmt::formatter<facebook::velox::core::PartitionedOutputNode::Kind>
: formatter<std::string> {
auto format(
facebook::velox::core::PartitionedOutputNode::Kind s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string>::format(
facebook::velox::core::PartitionedOutputNode::kindString(s), ctx);
}
};

template <>
struct fmt::formatter<facebook::velox::core::JoinType> : formatter<int> {
auto format(facebook::velox::core::JoinType s, format_context& ctx) {
auto format(facebook::velox::core::JoinType s, format_context& ctx) const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
7 changes: 5 additions & 2 deletions velox/dwio/catalog/fbhive/FileUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@
*/

#include "FileUtils.h"
#include <fmt/core.h>

#include <bitset>
#include "folly/container/Array.h"

#include <fmt/core.h>
#include <folly/container/Array.h>

#include "velox/dwio/common/exception/Exception.h"

namespace facebook {
Expand Down
5 changes: 2 additions & 3 deletions velox/dwio/common/Options.h
Original file line number Diff line number Diff line change
Expand Up @@ -644,9 +644,8 @@ template <>
struct fmt::formatter<facebook::velox::dwio::common::FileFormat>
: fmt::formatter<std::string_view> {
template <typename FormatContext>
auto format(
facebook::velox::dwio::common::FileFormat fmt,
FormatContext& ctx) {
auto format(facebook::velox::dwio::common::FileFormat fmt, FormatContext& ctx)
const {
return formatter<std::string_view>::format(
facebook::velox::dwio::common::toString(fmt), ctx);
}
Expand Down
2 changes: 1 addition & 1 deletion velox/dwio/dwrf/common/FileMetadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,7 @@ class FooterWrapper : public ProtoWrapperBase {

template <>
struct fmt::formatter<facebook::velox::dwrf::DwrfFormat> : formatter<int> {
auto format(facebook::velox::dwrf::DwrfFormat s, format_context& ctx) {
auto format(facebook::velox::dwrf::DwrfFormat s, format_context& ctx) const {
return formatter<int>::format(static_cast<int>(s), ctx);
}
};
14 changes: 7 additions & 7 deletions velox/dwio/parquet/thrift/ParquetThriftTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -3749,7 +3749,7 @@ struct fmt::formatter<facebook::velox::parquet::thrift::Type::type>
: fmt::formatter<std::string_view> {
auto format(
const facebook::velox::parquet::thrift::Type::type& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string_view>::format(
facebook::velox::parquet::thrift::to_string(s), ctx);
}
Expand All @@ -3760,7 +3760,7 @@ struct fmt::formatter<facebook::velox::parquet::thrift::CompressionCodec::type>
: fmt::formatter<std::string_view> {
auto format(
const facebook::velox::parquet::thrift::CompressionCodec::type& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string_view>::format(
facebook::velox::parquet::thrift::to_string(s), ctx);
}
Expand All @@ -3771,7 +3771,7 @@ struct fmt::formatter<facebook::velox::parquet::thrift::ConvertedType::type>
: fmt::formatter<std::string_view> {
auto format(
const facebook::velox::parquet::thrift::ConvertedType::type& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string_view>::format(
facebook::velox::parquet::thrift::to_string(s), ctx);
}
Expand All @@ -3783,7 +3783,7 @@ struct fmt::formatter<
: fmt::formatter<std::string_view> {
auto format(
const facebook::velox::parquet::thrift::FieldRepetitionType::type& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string_view>::format(
facebook::velox::parquet::thrift::to_string(s), ctx);
}
Expand All @@ -3794,7 +3794,7 @@ struct fmt::formatter<facebook::velox::parquet::thrift::Encoding::type>
: fmt::formatter<std::string_view> {
auto format(
const facebook::velox::parquet::thrift::Encoding::type& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string_view>::format(
facebook::velox::parquet::thrift::to_string(s), ctx);
}
Expand All @@ -3805,7 +3805,7 @@ struct fmt::formatter<facebook::velox::parquet::thrift::PageType::type>
: fmt::formatter<std::string_view> {
auto format(
const facebook::velox::parquet::thrift::PageType::type& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string_view>::format(
facebook::velox::parquet::thrift::to_string(s), ctx);
}
Expand All @@ -3816,7 +3816,7 @@ struct fmt::formatter<facebook::velox::parquet::thrift::BoundaryOrder::type>
: fmt::formatter<std::string_view> {
auto format(
const facebook::velox::parquet::thrift::BoundaryOrder::type& s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string_view>::format(
facebook::velox::parquet::thrift::to_string(s), ctx);
}
Expand Down
2 changes: 1 addition & 1 deletion velox/exec/Driver.h
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ DriverThreadContext* driverThreadContext();
template <>
struct fmt::formatter<facebook::velox::exec::StopReason>
: formatter<std::string> {
auto format(facebook::velox::exec::StopReason s, format_context& ctx) {
auto format(facebook::velox::exec::StopReason s, format_context& ctx) const {
return formatter<std::string>::format(
facebook::velox::exec::stopReasonString(s), ctx);
}
Expand Down
3 changes: 2 additions & 1 deletion velox/exec/HashBuild.h
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,8 @@ inline std::ostream& operator<<(std::ostream& os, HashBuild::State state) {
template <>
struct fmt::formatter<facebook::velox::exec::HashBuild::State>
: formatter<std::string> {
auto format(facebook::velox::exec::HashBuild::State s, format_context& ctx) {
auto format(facebook::velox::exec::HashBuild::State s, format_context& ctx)
const {
return formatter<std::string>::format(
facebook::velox::exec::HashBuild::stateName(s), ctx);
}
Expand Down
2 changes: 1 addition & 1 deletion velox/exec/HashTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -1098,7 +1098,7 @@ struct fmt::formatter<facebook::velox::exec::BaseHashTable::HashMode>
: formatter<std::string> {
auto format(
facebook::velox::exec::BaseHashTable::HashMode s,
format_context& ctx) {
format_context& ctx) const {
return formatter<std::string>::format(
facebook::velox::exec::BaseHashTable::modeString(s), ctx);
}
Expand Down
2 changes: 1 addition & 1 deletion velox/exec/Operator.h
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,7 @@ class SourceOperator : public Operator {

template <>
struct fmt::formatter<std::thread::id> : formatter<std::string> {
auto format(std::thread::id s, format_context& ctx) {
auto format(std::thread::id s, format_context& ctx) const {
std::ostringstream oss;
oss << s;
return formatter<std::string>::format(oss.str(), ctx);
Expand Down
Loading

0 comments on commit 1e736ba

Please sign in to comment.