From e4217a39a0ead7c7a92663e76646f7a8a08d41c1 Mon Sep 17 00:00:00 2001 From: dragonly Date: Mon, 18 Oct 2021 16:56:13 +0800 Subject: [PATCH 1/7] fix clang-tidy warnings for a batch of files --- dbms/src/Common/Decimal.h | 36 ++++++++++---------- dbms/src/Common/ErrorExporter.h | 1 - dbms/src/Common/EventRecorder.h | 1 - dbms/src/Common/Exception.cpp | 6 ++-- dbms/src/Common/Exception.h | 8 ++--- dbms/src/Functions/FunctionsRound.h | 2 +- dbms/src/Functions/FunctionsTiDBConversion.h | 10 +++--- 7 files changed, 31 insertions(+), 33 deletions(-) diff --git a/dbms/src/Common/Decimal.h b/dbms/src/Common/Decimal.h index d99d260ba32..5ece7529f85 100644 --- a/dbms/src/Common/Decimal.h +++ b/dbms/src/Common/Decimal.h @@ -146,7 +146,7 @@ struct Decimal Decimal(const Decimal & d) = default; Decimal() = default; - Decimal(T v_) + Decimal(T v_) // NOLINT(google-explicit-constructor) : value(v_) {} @@ -156,18 +156,18 @@ struct Decimal String toString(ScaleType) const; template || std::is_same_v || std::is_integral_v || std::is_same_v> * = nullptr> - operator U() const + explicit operator U() const { return static_cast(value); } template = sizeof(T)> * = nullptr> - operator Decimal() const + explicit operator Decimal() const { return static_cast(value); } - operator T() const + explicit operator T() const { return value; } @@ -364,33 +364,33 @@ class DecimalMaxValue final : public ext::singleton } } - Int256 get(PrecType idx) const + Int256 _get(PrecType idx) const { return number[idx]; } - static Int256 Get(PrecType idx) + static Int256 get(PrecType idx) { - return instance().get(idx); + return instance()._get(idx); } - static Int256 MaxValue() + static Int256 maxValue() { - return Get(maxDecimalPrecision()); + return get(maxDecimalPrecision()); } }; template inline typename T::NativeType getScaleMultiplier(ScaleType scale) { - return static_cast(DecimalMaxValue::Get(scale) + 1); + return static_cast(DecimalMaxValue::get(scale) + 1); } template inline void checkDecimalOverflow(Decimal v, PrecType prec) { - auto maxValue = DecimalMaxValue::Get(prec); - if (v.value > maxValue || v.value < -maxValue) + auto max_value = DecimalMaxValue::get(prec); + if (v.value > max_value || v.value < -max_value) { throw TiFlashException("Decimal value overflow", Errors::Decimal::Overflow); } @@ -460,15 +460,15 @@ std::enable_if_t, U> ToDecimal(T value, ScaleType sc { value *= 10; } - if (std::abs(value) > static_cast(DecimalMaxValue::Get(decimal_max_prec))) + if (std::abs(value) > static_cast(DecimalMaxValue::get(decimal_max_prec))) { throw TiFlashException("Decimal value overflow", Errors::Decimal::Overflow); } // rounding - T tenTimesValue = value * 10; + T ten_times_value = value * 10; using UType = typename U::NativeType; UType v(value); - if (Int256(tenTimesValue) % 10 >= 5) + if (Int256(ten_times_value) % 10 >= 5) { v++; } @@ -496,13 +496,13 @@ std::enable_if_t, U> ToDecimal(const T & v, ScaleType v_scale, Scal } else { - bool need2Round = false; + bool need_to_round = false; for (ScaleType i = scale; i < v_scale; i++) { - need2Round = (value < 0 ? -value : value) % 10 >= 5; + need_to_round = (value < 0 ? -value : value) % 10 >= 5; value /= 10; } - if (need2Round) + if (need_to_round) { if (value < 0) value--; diff --git a/dbms/src/Common/ErrorExporter.h b/dbms/src/Common/ErrorExporter.h index a9025d35dae..f9b4b28c3e2 100644 --- a/dbms/src/Common/ErrorExporter.h +++ b/dbms/src/Common/ErrorExporter.h @@ -40,7 +40,6 @@ void ErrorExporter::writeError(const TiFlashError & error) error.description.data(), error.workaround.data()); DB::writeString(buffer, wb); - return; } void ErrorExporter::flush() diff --git a/dbms/src/Common/EventRecorder.h b/dbms/src/Common/EventRecorder.h index f8ed545da16..95d5f881d40 100644 --- a/dbms/src/Common/EventRecorder.h +++ b/dbms/src/Common/EventRecorder.h @@ -10,7 +10,6 @@ class EventRecorder EventRecorder(ProfileEvents::Event event_, ProfileEvents::Event event_elapsed_) : event(event_) , event_elapsed(event_elapsed_) - , watch() { watch.start(); } diff --git a/dbms/src/Common/Exception.cpp b/dbms/src/Common/Exception.cpp index f0190262410..4c21065d072 100644 --- a/dbms/src/Common/Exception.cpp +++ b/dbms/src/Common/Exception.cpp @@ -149,9 +149,9 @@ int getCurrentExceptionCode() void rethrowFirstException(const Exceptions & exceptions) { - for (size_t i = 0, size = exceptions.size(); i < size; ++i) - if (exceptions[i]) - std::rethrow_exception(exceptions[i]); + for (const auto & exception : exceptions) + if (exception) + std::rethrow_exception(exception); } diff --git a/dbms/src/Common/Exception.h b/dbms/src/Common/Exception.h index efcdce4cb20..5b78de34309 100644 --- a/dbms/src/Common/Exception.h +++ b/dbms/src/Common/Exception.h @@ -18,8 +18,8 @@ namespace DB class Exception : public Poco::Exception { public: - Exception() {} /// For deferred initialization. - Exception(const std::string & msg, int code = 0) + Exception() = default; /// For deferred initialization. + explicit Exception(const std::string & msg, int code = 0) : Poco::Exception(msg, code) {} Exception(const std::string & msg, const std::string & arg, int code = 0) @@ -52,7 +52,7 @@ class Exception : public Poco::Exception class ErrnoException : public Exception { public: - ErrnoException(const std::string & msg, int code = 0, int saved_errno_ = 0) + explicit ErrnoException(const std::string & msg, int code = 0, int saved_errno_ = 0) : Exception(msg, code) , saved_errno(saved_errno_) {} @@ -75,7 +75,7 @@ class ErrnoException : public Exception using Exceptions = std::vector; -[[noreturn]] void throwFromErrno(const std::string & s, int code = 0, int the_errno = errno); +[[noreturn]] void throwFromErrno(const std::string & s, int code = 0, int e = errno); /** Try to write an exception to the log (and forget about it). diff --git a/dbms/src/Functions/FunctionsRound.h b/dbms/src/Functions/FunctionsRound.h index 651311e3064..644f6c0f3db 100644 --- a/dbms/src/Functions/FunctionsRound.h +++ b/dbms/src/Functions/FunctionsRound.h @@ -1176,7 +1176,7 @@ struct TiDBDecimalRound scaled_value *= PowForOutput::result[-difference]; // check overflow and construct result. - if (scaled_value > DecimalMaxValue::Get(info.output_prec)) + if (scaled_value > DecimalMaxValue::get(info.output_prec)) throw TiFlashException("Data truncated", Errors::Decimal::Overflow); auto result = static_cast(scaled_value); diff --git a/dbms/src/Functions/FunctionsTiDBConversion.h b/dbms/src/Functions/FunctionsTiDBConversion.h index 246ca26ae5c..4567548a5f6 100644 --- a/dbms/src/Functions/FunctionsTiDBConversion.h +++ b/dbms/src/Functions/FunctionsTiDBConversion.h @@ -813,7 +813,7 @@ struct TiDBConvertToDecimal static U toTiDBDecimalInternal(T value, PrecType prec, ScaleType scale, const Context & context) { using UType = typename U::NativeType; - auto maxValue = DecimalMaxValue::Get(prec); + auto maxValue = DecimalMaxValue::get(prec); if (value > maxValue || value < -maxValue) { context.getDAGContext()->handleOverflowError("cast to decimal", Errors::Types::Truncated); @@ -874,7 +874,7 @@ struct TiDBConvertToDecimal { value *= 10; } - auto max_value = DecimalMaxValue::Get(prec); + auto max_value = DecimalMaxValue::get(prec); if (value > static_cast(max_value)) { context.getDAGContext()->handleOverflowError("cast real to decimal", Errors::Types::Truncated); @@ -934,7 +934,7 @@ struct TiDBConvertToDecimal } } - auto max_value = DecimalMaxValue::Get(prec); + auto max_value = DecimalMaxValue::get(prec); if (value > max_value || value < -max_value) { context.getDAGContext()->handleOverflowError("cast decimal as decimal", Errors::Types::Truncated); @@ -1021,7 +1021,7 @@ struct TiDBConvertToDecimal if (decimal_parts.exp_part.data[0] == '-') return static_cast(0); else - return static_cast(DecimalMaxValue::Get(prec)); + return static_cast(DecimalMaxValue::get(prec)); } } Int256 v = 0; @@ -1033,7 +1033,7 @@ struct TiDBConvertToDecimal if (decimal_parts.int_part.data[pos] == '-') is_negative = true; } - Int256 max_value = DecimalMaxValue::Get(prec); + Int256 max_value = DecimalMaxValue::get(prec); Int64 current_scale = frac_offset_by_exponent >= 0 ? -(decimal_parts.int_part.size - pos + frac_offset_by_exponent) From 6c0960b61b324ebc539f6fcf8a8dc7d770e6b235 Mon Sep 17 00:00:00 2001 From: Yilong Li Date: Tue, 19 Oct 2021 11:20:46 +0800 Subject: [PATCH 2/7] Update dbms/src/Common/Decimal.h Co-authored-by: Fu Zhe --- dbms/src/Common/Decimal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Common/Decimal.h b/dbms/src/Common/Decimal.h index 5ece7529f85..e478dc6711d 100644 --- a/dbms/src/Common/Decimal.h +++ b/dbms/src/Common/Decimal.h @@ -468,7 +468,7 @@ std::enable_if_t, U> ToDecimal(T value, ScaleType sc T ten_times_value = value * 10; using UType = typename U::NativeType; UType v(value); - if (Int256(ten_times_value) % 10 >= 5) + if (static_cast(ten_times_value) % 10 >= 5) { v++; } From f12feacccd953c40a6227f6150558d7342bbfd11 Mon Sep 17 00:00:00 2001 From: dragonly Date: Tue, 19 Oct 2021 11:24:19 +0800 Subject: [PATCH 3/7] address comments --- dbms/src/Common/Decimal.h | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/dbms/src/Common/Decimal.h b/dbms/src/Common/Decimal.h index e478dc6711d..e32fb2c935d 100644 --- a/dbms/src/Common/Decimal.h +++ b/dbms/src/Common/Decimal.h @@ -356,27 +356,28 @@ class DecimalMaxValue final : public ext::singleton Int256 number[decimal_max_prec + 1]; public: - DecimalMaxValue() + static Int256 get(PrecType idx) { - for (PrecType i = 1; i <= decimal_max_prec; i++) - { - number[i] = number[i - 1] * 10 + 9; - } + return instance().getInternal(idx); } - Int256 _get(PrecType idx) const + static Int256 maxValue() { - return number[idx]; + return get(maxDecimalPrecision()); } - static Int256 get(PrecType idx) +private: + DecimalMaxValue() { - return instance()._get(idx); + for (PrecType i = 1; i <= decimal_max_prec; i++) + { + number[i] = number[i - 1] * 10 + 9; + } } - static Int256 maxValue() + Int256 getInternal(PrecType idx) const { - return get(maxDecimalPrecision()); + return number[idx]; } }; From d499f94e1c0eee6b7cfb015cbf1dcf48e95761d1 Mon Sep 17 00:00:00 2001 From: dragonly Date: Tue, 19 Oct 2021 12:08:10 +0800 Subject: [PATCH 4/7] fix compile --- dbms/src/Common/Decimal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Common/Decimal.h b/dbms/src/Common/Decimal.h index e32fb2c935d..ef6b34d3588 100644 --- a/dbms/src/Common/Decimal.h +++ b/dbms/src/Common/Decimal.h @@ -162,7 +162,7 @@ struct Decimal } template = sizeof(T)> * = nullptr> - explicit operator Decimal() const + operator Decimal() const // NOLINT(google-explicit-constructor) { return static_cast(value); } From f75ec760fd8ec4af346abba3f1e11769ce09850e Mon Sep 17 00:00:00 2001 From: dragonly Date: Tue, 19 Oct 2021 13:25:47 +0800 Subject: [PATCH 5/7] fix compile --- dbms/src/Common/Decimal.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbms/src/Common/Decimal.h b/dbms/src/Common/Decimal.h index ef6b34d3588..fe59a33d85f 100644 --- a/dbms/src/Common/Decimal.h +++ b/dbms/src/Common/Decimal.h @@ -156,7 +156,7 @@ struct Decimal String toString(ScaleType) const; template || std::is_same_v || std::is_integral_v || std::is_same_v> * = nullptr> - explicit operator U() const + operator U() const // NOLINT(google-explicit-constructor) { return static_cast(value); } @@ -167,7 +167,7 @@ struct Decimal return static_cast(value); } - explicit operator T() const + operator T() const // NOLINT(google-explicit-constructor) { return value; } From e7d8c4425449923721ee0c48359e332a0a36ea3e Mon Sep 17 00:00:00 2001 From: dragonly Date: Tue, 19 Oct 2021 13:58:50 +0800 Subject: [PATCH 6/7] fix compile --- dbms/src/Functions/FunctionBinaryArithmetic.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dbms/src/Functions/FunctionBinaryArithmetic.h b/dbms/src/Functions/FunctionBinaryArithmetic.h index a33fe40d116..9db33f47d13 100644 --- a/dbms/src/Functions/FunctionBinaryArithmetic.h +++ b/dbms/src/Functions/FunctionBinaryArithmetic.h @@ -479,7 +479,7 @@ struct DecimalBinaryOperation res = res / scale; if constexpr (check_overflow) { - if (res > DecimalMaxValue::MaxValue()) + if (res > DecimalMaxValue::maxValue()) { throw Exception("Decimal math overflow", ErrorCodes::DECIMAL_OVERFLOW); } @@ -503,7 +503,7 @@ struct DecimalBinaryOperation auto res = Op::template apply(a, b); if constexpr (check_overflow) { - if (res > DecimalMaxValue::MaxValue()) + if (res > DecimalMaxValue::maxValue()) { throw Exception("Decimal math overflow", ErrorCodes::DECIMAL_OVERFLOW); } @@ -532,7 +532,7 @@ struct DecimalBinaryOperation if constexpr (check_overflow) { - if (res > DecimalMaxValue::MaxValue()) + if (res > DecimalMaxValue::maxValue()) { throw Exception("Decimal math overflow", ErrorCodes::DECIMAL_OVERFLOW); } @@ -558,7 +558,7 @@ struct DecimalBinaryOperation if constexpr (check_overflow) { - if (res > DecimalMaxValue::MaxValue()) + if (res > DecimalMaxValue::maxValue()) { throw Exception("Decimal math overflow", ErrorCodes::DECIMAL_OVERFLOW); } From ba6c8c42f5aa09877044faaad893900dd43663bd Mon Sep 17 00:00:00 2001 From: dragonly Date: Tue, 19 Oct 2021 15:17:56 +0800 Subject: [PATCH 7/7] works on my ubuntu --- dbms/src/Functions/tests/gtest_arithmetic_functions.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dbms/src/Functions/tests/gtest_arithmetic_functions.cpp b/dbms/src/Functions/tests/gtest_arithmetic_functions.cpp index 5084839d435..80aa231f237 100644 --- a/dbms/src/Functions/tests/gtest_arithmetic_functions.cpp +++ b/dbms/src/Functions/tests/gtest_arithmetic_functions.cpp @@ -762,9 +762,8 @@ try using NativeType = Decimal::NativeType; \ using FieldType = DecimalField; \ auto prec = (precision); \ - auto & builder = DecimalMaxValue::instance(); \ auto max_scale = std::min(decimal_max_scale, static_cast(prec) - 1); \ - auto exp10_x = static_cast(builder.Get(max_scale)) + 1; /* exp10_x: 10^x */ \ + auto exp10_x = static_cast(DecimalMaxValue::get(max_scale)) + 1; /* exp10_x: 10^x */ \ auto decimal_max = exp10_x * 10 - 1; \ auto zero = static_cast(0); /* for Int256 */ \ ASSERT_COLUMN_EQ( \