Skip to content

Commit

Permalink
*: Upgrade fmtlib and print enum value explicitly (#8332)
Browse files Browse the repository at this point in the history
close #8331
  • Loading branch information
JaySon-Huang authored Nov 8, 2023
1 parent 1bdd6ad commit b44a3f9
Show file tree
Hide file tree
Showing 34 changed files with 185 additions and 76 deletions.
2 changes: 1 addition & 1 deletion contrib/fmtlib
Submodule fmtlib updated 86 files
+8 −0 .github/dependabot.yml
+6 −0 .github/issue_template.md
+3 −2 .github/pull_request_template.md
+30 −0 .github/workflows/cifuzz.yml
+12 −1 .github/workflows/doc.yml
+42 −10 .github/workflows/linux.yml
+20 −2 .github/workflows/macos.yml
+65 −0 .github/workflows/scorecard.yml
+61 −21 .github/workflows/windows.yml
+99 −70 CMakeLists.txt
+1,187 −3 ChangeLog.rst
+1 −1 LICENSE.rst
+55 −38 README.rst
+11 −2 doc/CMakeLists.txt
+270 −155 doc/api.rst
+9 −6 doc/build.py
+2 −2 doc/index.rst
+163 −10 doc/syntax.rst
+5 −5 include/fmt/args.h
+482 −341 include/fmt/chrono.h
+116 −122 include/fmt/color.h
+62 −170 include/fmt/compile.h
+976 −1,290 include/fmt/core.h
+820 −1,801 include/fmt/format-inl.h
+2,253 −847 include/fmt/format.h
+0 −2 include/fmt/locale.h
+70 −146 include/fmt/os.h
+129 −55 include/fmt/ostream.h
+202 −192 include/fmt/printf.h
+367 −425 include/fmt/ranges.h
+465 −0 include/fmt/std.h
+86 −64 include/fmt/xchar.h
+40 −29 src/fmt.cc
+15 −96 src/format.cc
+117 −80 src/os.cc
+3 −3 support/Vagrantfile
+0 −43 support/appveyor-build.py
+0 −31 support/appveyor.yml
+0 −1 support/bazel/.bazelrc
+1 −1 support/bazel/.bazelversion
+1 −2 support/bazel/BUILD.bazel
+5 −4 support/bazel/README.md
+1 −1 support/build.gradle
+0 −70 support/cmake/cxx14.cmake
+4 −1 support/cmake/fmt-config.cmake.in
+6 −0 support/manage.py
+1 −1 support/printable.py
+7 −0 support/rst2md.py
+30 −17 test/CMakeLists.txt
+1 −1 test/add-subdirectory-test/CMakeLists.txt
+1 −1 test/args-test.cc
+396 −18 test/chrono-test.cc
+6 −0 test/color-test.cc
+42 −4 test/compile-error-test/CMakeLists.txt
+2 −1 test/compile-fp-test.cc
+37 −39 test/compile-test.cc
+167 −232 test/core-test.cc
+18 −0 test/detect-stdfs.cc
+2 −0 test/enforce-checks-test.cc
+1 −1 test/find-package-test/CMakeLists.txt
+257 −151 test/format-impl-test.cc
+465 −388 test/format-test.cc
+1 −1 test/fuzzing/CMakeLists.txt
+2 −2 test/fuzzing/one-arg.cc
+2 −2 test/fuzzing/two-args.cc
+3 −1 test/gtest-extra-test.cc
+1 −1 test/gtest-extra.cc
+2 −7 test/gtest-extra.h
+1 −7 test/gtest/CMakeLists.txt
+2 −2 test/gtest/gmock-gtest-all.cc
+2 −2 test/mock-allocator.h
+36 −96 test/module-test.cc
+24 −69 test/os-test.cc
+69 −79 test/ostream-test.cc
+3 −90 test/posix-mock-test.cc
+0 −2 test/posix-mock.h
+14 −42 test/printf-test.cc
+198 −34 test/ranges-test.cc
+1 −1 test/scan-test.cc
+17 −14 test/scan.h
+1 −1 test/static-export-test/CMakeLists.txt
+257 −0 test/std-test.cc
+2 −3 test/test-main.cc
+4 −4 test/unicode-test.cc
+2 −6 test/util.h
+177 −95 test/xchar-test.cc
2 changes: 1 addition & 1 deletion contrib/fmtlib-cmake/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ set (SRCS
../fmtlib/include/fmt/core.h
../fmtlib/include/fmt/format.h
../fmtlib/include/fmt/format-inl.h
../fmtlib/include/fmt/locale.h
../fmtlib/include/fmt/os.h
../fmtlib/include/fmt/ostream.h
../fmtlib/include/fmt/printf.h
../fmtlib/include/fmt/ranges.h
../fmtlib/include/fmt/std.h
../fmtlib/include/fmt/xchar.h
)

Expand Down
2 changes: 1 addition & 1 deletion dbms/src/DataStreams/RuntimeFilter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ void RuntimeFilter::build()
throw TiFlashException(
Errors::Coprocessor::BadRequest,
"The source expr {} of rf {} should be column ref",
source_expr.tp(),
tipb::ExprType_Name(source_expr.tp()),
id);
}
}
Expand Down
1 change: 1 addition & 0 deletions dbms/src/Encryption/RateLimiter.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <Common/nocopyable.h>
#include <Server/StorageConfigParser.h>
#include <fmt/core.h>
#include <fmt/std.h>

#include <atomic>
#include <chrono>
Expand Down
25 changes: 13 additions & 12 deletions dbms/src/Flash/Coprocessor/DAGUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -753,11 +753,11 @@ const String & getAggFunctionName(const tipb::Expr & expr)
return it->second;
}

const auto errmsg = fmt::format(
throw TiFlashException(
Errors::Coprocessor::Unimplemented,
"{}(distinct={}) is not supported.",
tipb::ExprType_Name(expr.tp()),
expr.has_distinct() ? "true" : "false");
throw TiFlashException(errmsg, Errors::Coprocessor::Unimplemented);
expr.has_distinct());
}

const String & getWindowFunctionName(const tipb::Expr & expr)
Expand All @@ -766,8 +766,7 @@ const String & getWindowFunctionName(const tipb::Expr & expr)
if (it != window_func_map.end())
return it->second;

const auto errmsg = fmt::format("{} is not supported.", tipb::ExprType_Name(expr.tp()));
throw TiFlashException(errmsg, Errors::Coprocessor::Unimplemented);
throw TiFlashException(Errors::Coprocessor::Unimplemented, "{} is not supported.", tipb::ExprType_Name(expr.tp()));
}


Expand All @@ -786,8 +785,9 @@ const String & getFunctionName(const tipb::Expr & expr)
auto it = scalar_func_map.find(expr.sig());
if (it == scalar_func_map.end())
throw TiFlashException(
tipb::ScalarFuncSig_Name(expr.sig()) + " is not supported.",
Errors::Coprocessor::Unimplemented);
Errors::Coprocessor::Unimplemented,
"{} is not supported.",
tipb::ScalarFuncSig_Name(expr.sig()));
return it->second;
}
}
Expand All @@ -803,7 +803,7 @@ String getExchangeTypeName(const tipb::ExchangeType & tp)
case tipb::ExchangeType::Hash:
return "Hash";
default:
throw TiFlashException(fmt::format("Not supported Exchange type: {}", tp), Errors::Coprocessor::Internal);
throw TiFlashException(Errors::Coprocessor::Internal, "Not supported Exchange type: {}", fmt::underlying(tp));
}
}

Expand All @@ -826,7 +826,7 @@ String getJoinTypeName(const tipb::JoinType & tp)
case tipb::JoinType::TypeSemiJoin:
return "SemiJoin";
default:
throw TiFlashException(fmt::format("Not supported Join type: {}", tp), Errors::Coprocessor::Internal);
throw TiFlashException(Errors::Coprocessor::Internal, "Not supported Join type: {}", fmt::underlying(tp));
}
}

Expand All @@ -838,8 +838,9 @@ String getJoinExecTypeName(const tipb::JoinExecType & tp)
return "HashJoin";
default:
throw TiFlashException(
fmt::format("Not supported Join exectution type: {}", tp),
Errors::Coprocessor::Internal);
Errors::Coprocessor::Internal,
"Not supported Join exectution type: {}",
fmt::underlying(tp));
}
}

Expand Down Expand Up @@ -882,7 +883,7 @@ String getFieldTypeName(Int32 tp)
case TiDB::TypeString:
return "String";
default:
throw TiFlashException(fmt::format("Not supported field type: {}", tp), Errors::Coprocessor::Internal);
throw TiFlashException(Errors::Coprocessor::Internal, "Not supported field type: {}", tp);
}
}

Expand Down
6 changes: 4 additions & 2 deletions dbms/src/Flash/Coprocessor/JoinInterpreterHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,10 @@ std::pair<ASTTableJoin::Kind, size_t> getJoinKindAndBuildSideIndex(
auto join_type_it = join_type_map.find(std::make_pair(tipb_join_type, inner_index));
if (unlikely(join_type_it == join_type_map.end()))
throw TiFlashException(
fmt::format("Unknown join type in dag request {} {}", tipb_join_type, inner_index),
Errors::Coprocessor::BadRequest);
Errors::Coprocessor::BadRequest,
"Unknown join type in dag request {} {}",
fmt::underlying(tipb_join_type),
inner_index);
return join_type_it->second;
}

Expand Down
6 changes: 5 additions & 1 deletion dbms/src/Flash/Disaggregated/S3LockClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,11 @@ std::pair<bool, String> S3LockClient::makeCall(
throw Exception(ErrorCodes::TIMEOUT_EXCEEDED, "deadline exceed, " + tracing_log->identifier());
}
// retry
LOG_ERROR(tracing_log, "meets error, code={} msg={}", status.error_code(), status.error_message());
LOG_ERROR(
tracing_log,
"meets error, code={} msg={}",
magic_enum::enum_name(status.error_code()),
status.error_message());
address = updateOwnerAddr(deadline, tracing_log);
}

Expand Down
20 changes: 17 additions & 3 deletions dbms/src/Flash/FlashService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,12 @@ grpc::Status FlashService::Coprocessor(
};
grpc::Status ret = cop_limiter->executeFor(std::move(exec_func), max_queued_duration_ms, std::move(timeout_func));

LOG_IMPL(log, log_level, "Handle coprocessor request done: {}, {}", ret.error_code(), ret.error_message());
LOG_IMPL(
log,
log_level,
"Handle coprocessor request done: {}, {}",
magic_enum::enum_name(ret.error_code()),
ret.error_message());
return ret;
}

Expand Down Expand Up @@ -342,7 +347,11 @@ grpc::Status FlashService::BatchCoprocessor(
return cop_handler.execute();
});

LOG_INFO(log, "Handle batch coprocessor request done: {}, {}", ret.error_code(), ret.error_message());
LOG_INFO(
log,
"Handle batch coprocessor request done: {}, {}",
magic_enum::enum_name(ret.error_code()),
ret.error_message());
return ret;
}

Expand Down Expand Up @@ -459,7 +468,12 @@ grpc::Status FlashService::CoprocessorStream(
grpc::Status ret
= cop_stream_limiter->executeFor(std::move(exec_func), max_queued_duration_ms, std::move(timeout_func));

LOG_IMPL(log, log_level, "Handle coprocessor stream request done: {}, {}", ret.error_code(), ret.error_message());
LOG_IMPL(
log,
log_level,
"Handle coprocessor stream request done: {}, {}",
magic_enum::enum_name(ret.error_code()),
ret.error_message());
return ret;
}

Expand Down
7 changes: 6 additions & 1 deletion dbms/src/Flash/LogSearch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <boost_wrapper/string.h>

#include <boost/algorithm/string/predicate.hpp>
#include <magic_enum.hpp>

namespace DB
{
Expand Down Expand Up @@ -365,7 +366,11 @@ LogIterator::~LogIterator()
{
if (err_info.has_value())
{
LOG_DEBUG(log, "LogIterator search end with error {} at line {}.", err_info->second, err_info->first);
LOG_DEBUG(
log,
"LogIterator search end with error {} at line {}.",
magic_enum::enum_name(err_info->second),
err_info->first);
}
}

Expand Down
2 changes: 1 addition & 1 deletion dbms/src/Flash/Mpp/AsyncRequestHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ class AsyncRequestHandler final : public GRPCKickTag
String done_msg = fmt::format("Exchange receiver meet error : {}", finish_status.error_message());
String log_msg = fmt::format(
"Finish fail. err code: {}, err msg: {}, retry time {}",
finish_status.error_code(),
magic_enum::enum_name(finish_status.error_code()),
finish_status.error_message(),
retry_times);
retryOrDone(std::move(done_msg), log_msg);
Expand Down
4 changes: 2 additions & 2 deletions dbms/src/Flash/Mpp/ExchangeReceiver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ class AsyncRequestHandlerv1 : public GRPCKickTag
LOG_WARNING(
log,
"Finish fail. err code: {}, err msg: {}, retry time {}",
finish_status.error_code(),
magic_enum::enum_name(finish_status.error_code()),
finish_status.error_message(),
retry_times);
retryOrDone(fmt::format("Exchange receiver meet error : {}", finish_status.error_message()));
Expand Down Expand Up @@ -681,7 +681,7 @@ void ExchangeReceiverBase<RPCContext>::readLoop(const Request & req)
LOG_WARNING(
log,
"EstablishMPPConnectionRequest meets rpc fail. Err code = {}, err msg = {}, retriable = {}",
status.error_code(),
magic_enum::enum_name(status.error_code()),
status.error_message(),
retriable);
// if we have received some data, we should not retry.
Expand Down
15 changes: 14 additions & 1 deletion dbms/src/Flash/Mpp/MppVersion.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@

#pragma once

#include <common/defines.h>
#include <fmt/format.h>

#include <string>

namespace DB
Expand All @@ -26,6 +29,11 @@ enum MppVersion : int64_t
//
MppVersionMAX,
};
// Make MppVersion formatable by fmtlib
ALWAYS_INLINE inline auto format_as(MppVersion v)
{
return fmt::underlying(v);
}

enum MPPDataPacketVersion : int64_t
{
Expand All @@ -34,11 +42,16 @@ enum MPPDataPacketVersion : int64_t
//
MPPDataPacketMAX,
};
// Make MPPDataPacketVersion formatable by fmtlib
ALWAYS_INLINE inline auto format_as(MPPDataPacketVersion v)
{
return fmt::underlying(v);
}

bool ReportStatusToCoordinator(int64_t mpp_version, const std::string & coordinator_address);
bool ReportExecutionSummaryToCoordinator(int64_t mpp_version, bool report_execution_summary);
bool CheckMppVersion(int64_t mpp_version);
std::string GenMppVersionErrorMessage(int64_t mpp_version);
int64_t GetMppVersion();

} // namespace DB
} // namespace DB
2 changes: 1 addition & 1 deletion dbms/src/Flash/Planner/PhysicalPlan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ void PhysicalPlan::build(const tipb::Executor * executor)
}
default:
throw TiFlashException(
fmt::format("{} executor is not supported", executor->tp()),
fmt::format("{} executor is not supported", fmt::underlying(executor->tp())),
Errors::Planner::Unimplemented);
}
}
Expand Down
4 changes: 2 additions & 2 deletions dbms/src/Flash/tests/gtest_collation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ std::queue<tipb::ExecType> ExecutorCollation::checkExecutorCollation(
break; /// Do nothing
default:
{
auto exception_str = fmt::format("Unhandled executor {}", type);
auto exception_str = fmt::format("Unhandled executor {}", fmt::underlying(type));
throw Exception(exception_str);
}
}
Expand Down Expand Up @@ -313,7 +313,7 @@ void ExecutorCollation::checkScalarFunctionCollation(std::shared_ptr<tipb::DAGRe
break; /// Do nothing
default:
{
auto exception_str = fmt::format("Unhandled executor {}", type);
auto exception_str = fmt::format("Unhandled executor {}", fmt::underlying(type));
throw Exception(exception_str);
}
}
Expand Down
4 changes: 2 additions & 2 deletions dbms/src/Flash/tests/gtest_join_executor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2291,7 +2291,7 @@ ColumnsWithTypeAndName genSemiJoinResult(
r.column = r.column->filter(filter, -1);
}
else
throw Exception(ErrorCodes::LOGICAL_ERROR, "Semi join Type {} is not supported", type);
throw Exception(ErrorCodes::LOGICAL_ERROR, "Semi join Type {} is not supported", magic_enum::enum_name(type));
return res;
}
} // namespace
Expand Down Expand Up @@ -3133,4 +3133,4 @@ CATCH
#undef WRAP_FOR_JOIN_TEST_END

} // namespace tests
} // namespace DB
} // namespace DB
18 changes: 18 additions & 0 deletions dbms/src/Interpreters/SettingsCommon.h
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ struct SettingInt
std::atomic<IntType> value;
};

// Make SettingInt formatable by fmtlib
template <typename IntType>
ALWAYS_INLINE inline auto format_as(SettingInt<IntType> s)
{
return s.get();
}

using SettingUInt64 = SettingInt<UInt64>;
using SettingInt64 = SettingInt<Int64>;
Expand Down Expand Up @@ -338,6 +344,12 @@ struct SettingFloat
std::atomic<float> value;
};

// Make SettingFloat formatable by fmtlib
ALWAYS_INLINE inline auto format_as(SettingFloat s)
{
return s.get();
}

/// MemoryLimit can either be an UInt64 (means memory limit in bytes),
/// or be a float-point number (means memory limit ratio of total RAM, from 0.0 to 1.0).
/// 0 or 0.0 means unlimited.
Expand Down Expand Up @@ -439,6 +451,12 @@ struct SettingDouble
std::atomic<double> value;
};

// Make SettingDouble formatable by fmtlib
ALWAYS_INLINE inline auto format_as(SettingDouble s)
{
return s.get();
}

enum class LoadBalancing
{
/// among replicas with a minimum number of errors selected randomly
Expand Down
2 changes: 1 addition & 1 deletion dbms/src/Interpreters/WindowDescription.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ WindowFrame::FrameType getFrameTypeFromTipb(const tipb::WindowFrameType & type)
case tipb::WindowFrameType::Groups:
return WindowFrame::FrameType::Groups;
default:
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unknown frame type {}", type);
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Unknown frame type {}", fmt::underlying(type));
}
}

Expand Down
6 changes: 3 additions & 3 deletions dbms/src/Server/MockComputeClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class MockComputeClient
{
throw Exception(fmt::format(
"Meet error while dispatch mpp task, error code = {}, message = {}",
status.error_code(),
magic_enum::enum_name(status.error_code()),
status.error_message()));
}
if (response.has_error())
Expand All @@ -63,7 +63,7 @@ class MockComputeClient
{
throw Exception(fmt::format(
"Meet error while run coprocessor task, error code = {}, message = {}",
status.error_code(),
magic_enum::enum_name(status.error_code()),
status.error_message()));
}

Expand All @@ -73,4 +73,4 @@ class MockComputeClient
private:
std::unique_ptr<tikvpb::Tikv::Stub> stub{};
};
} // namespace DB
} // namespace DB
2 changes: 1 addition & 1 deletion dbms/src/Server/Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ pingcap::ClusterConfig getClusterConfig(
ca_path,
cert_path,
key_path,
config.api_version);
fmt::underlying(config.api_version));
return config;
}

Expand Down
2 changes: 1 addition & 1 deletion dbms/src/Storages/DeltaMerge/DeltaMergeStore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1589,7 +1589,7 @@ bool DeltaMergeStore::checkSegmentUpdate(
LOG_DEBUG(
log,
"Foreground flush cache in checkSegmentUpdate, thread={} segment={} input_type={}",
thread_type,
magic_enum::enum_name(thread_type),
segment->info(),
magic_enum::enum_name(input_type));
segment->flushCache(*dm_context);
Expand Down
Loading

0 comments on commit b44a3f9

Please sign in to comment.