Move RTC_CHECK_OP error message construction out of header file.
This simplifies the logic, prevents emitting code for every pair of
argument types to RTC_CHECK_OP and partially unblocks removing streams from
the check code altogether.
Bug: webrtc:8982
Change-Id: Ib6652ac9a342e4471c12574a79872833cc943407
Reviewed-on: https://webrtc-review.googlesource.com/86544
Commit-Queue: Jonas Olsson <jonasolsson@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23821}diff --git a/rtc_base/checks.cc b/rtc_base/checks.cc
index 462bef8..32ac8d6 100644
--- a/rtc_base/checks.cc
+++ b/rtc_base/checks.cc
@@ -36,22 +36,58 @@
#include "rtc_base/checks.h"
namespace rtc {
-
-// MSVC doesn't like complex extern templates and DLLs.
-#if !defined(COMPILER_MSVC)
-// Explicit instantiations for commonly used comparisons.
-template std::string* MakeCheckOpString<int, int>(
- const int&, const int&, const char* names);
-template std::string* MakeCheckOpString<unsigned long, unsigned long>(
- const unsigned long&, const unsigned long&, const char* names);
-template std::string* MakeCheckOpString<unsigned long, unsigned int>(
- const unsigned long&, const unsigned int&, const char* names);
-template std::string* MakeCheckOpString<unsigned int, unsigned long>(
- const unsigned int&, const unsigned long&, const char* names);
-template std::string* MakeCheckOpString<std::string, std::string>(
- const std::string&, const std::string&, const char* name);
-#endif
namespace webrtc_checks_impl {
+
+// Reads one argument from args, appends it to s and advances fmt.
+// Returns true iff an argument was sucessfully parsed.
+bool ParseArg(va_list* args,
+ const CheckArgType** fmt,
+ std::ostream& s) { // no-presubmit-check TODO(webrtc:8982)
+ if (**fmt == CheckArgType::kEnd)
+ return false;
+
+ switch (**fmt) {
+ case CheckArgType::kInt:
+ s << va_arg(*args, int);
+ break;
+ case CheckArgType::kLong:
+ s << va_arg(*args, long);
+ break;
+ case CheckArgType::kLongLong:
+ s << va_arg(*args, long long);
+ break;
+ case CheckArgType::kUInt:
+ s << va_arg(*args, unsigned);
+ break;
+ case CheckArgType::kULong:
+ s << va_arg(*args, unsigned long);
+ break;
+ case CheckArgType::kULongLong:
+ s << va_arg(*args, unsigned long long);
+ break;
+ case CheckArgType::kDouble:
+ s << va_arg(*args, double);
+ break;
+ case CheckArgType::kLongDouble:
+ s << va_arg(*args, long double);
+ break;
+ case CheckArgType::kCharP:
+ s << va_arg(*args, const char*);
+ break;
+ case CheckArgType::kStdString:
+ s << *va_arg(*args, const std::string*);
+ break;
+ case CheckArgType::kVoidP:
+ s << reinterpret_cast<std::uintptr_t>(va_arg(*args, const void*));
+ break;
+ default:
+ s << "[Invalid CheckArgType:" << static_cast<int8_t>(**fmt) << "]";
+ return false;
+ }
+ (*fmt)++;
+ return true;
+}
+
RTC_NORETURN void FatalLog(const char* file,
int line,
const char* message,
@@ -62,50 +98,25 @@
std::ostringstream ss; // no-presubmit-check TODO(webrtc:8982)
ss << "\n\n#\n# Fatal error in: " << file << ", line " << line
- << "\n# last system error: " << LAST_SYSTEM_ERROR
- << "\n# Check failed: " << message << "\n# ";
+ << "\n# last system error: " << LAST_SYSTEM_ERROR << "\n# Check failed: ";
- for (; *fmt != CheckArgType::kEnd; ++fmt) {
- switch (*fmt) {
- case CheckArgType::kInt:
- ss << va_arg(args, int);
- break;
- case CheckArgType::kLong:
- ss << va_arg(args, long);
- break;
- case CheckArgType::kLongLong:
- ss << va_arg(args, long long);
- break;
- case CheckArgType::kUInt:
- ss << va_arg(args, unsigned);
- break;
- case CheckArgType::kULong:
- ss << va_arg(args, unsigned long);
- break;
- case CheckArgType::kULongLong:
- ss << va_arg(args, unsigned long long);
- break;
- case CheckArgType::kDouble:
- ss << va_arg(args, double);
- break;
- case CheckArgType::kLongDouble:
- ss << va_arg(args, long double);
- break;
- case CheckArgType::kCharP:
- ss << va_arg(args, const char*);
- break;
- case CheckArgType::kStdString:
- ss << *va_arg(args, const std::string*);
- break;
- case CheckArgType::kVoidP:
- ss << va_arg(args, const void*);
- break;
- default:
- ss << "[Invalid CheckArgType:" << static_cast<int8_t>(*fmt) << "]";
- goto processing_loop_end;
- }
+ if (*fmt == CheckArgType::kCheckOp) {
+ // This log message was generated by RTC_CHECK_OP, so we have to complete
+ // the error message using the operands that have been passed as the first
+ // two arguments.
+ fmt++;
+
+ std::ostringstream s1, s2; // no-presubmit-check TODO(webrtc:8982)
+ if (ParseArg(&args, &fmt, s1) && ParseArg(&args, &fmt, s2))
+ ss << message << " (" << s1.str() << " vs. " << s2.str() << ")\n# ";
+ } else {
+ ss << message << "\n# ";
}
-processing_loop_end:
+
+ // Append all the user-supplied arguments to the message.
+ while (ParseArg(&args, &fmt, ss))
+ ;
+
va_end(args);
std::string s = ss.str();
diff --git a/rtc_base/checks.h b/rtc_base/checks.h
index b37df0d..a65088d 100644
--- a/rtc_base/checks.h
+++ b/rtc_base/checks.h
@@ -40,7 +40,7 @@
#ifdef __cplusplus
// C++ version.
-#include <sstream>
+#include <sstream> // no-presubmit-check TODO(webrtc:8982)
#include <string>
#include "rtc_base/numerics/safe_compare.h"
@@ -101,6 +101,12 @@
kCharP,
kStdString,
kVoidP,
+
+ // kCheckOp doesn't represent an argument type. Instead, it is sent as the
+ // first argument from RTC_CHECK_OP to make FatalLog use the next two
+ // arguments to build the special CHECK_OP error message
+ // (the "a == b (1 vs. 2)" bit).
+ kCheckOp,
};
RTC_NORETURN void FatalLog(const char* file,
@@ -191,6 +197,16 @@
static constexpr CheckArgType t[] = {Us::Type()..., CheckArgType::kEnd};
FatalLog(file, line, message, t, args.GetVal()...);
}
+
+ template <typename... Us>
+ RTC_NORETURN RTC_FORCE_INLINE static void CallCheckOp(const char* file,
+ const int line,
+ const char* message,
+ const Us&... args) {
+ static constexpr CheckArgType t[] = {CheckArgType::kCheckOp, Us::Type()...,
+ CheckArgType::kEnd};
+ FatalLog(file, line, message, t, args.GetVal()...);
+ }
};
// Inductive case: We've already seen at least one << argument. The most recent
@@ -227,6 +243,14 @@
prior_->Call(file, line, message, arg_, args...);
}
+ template <typename... Us>
+ RTC_NORETURN RTC_FORCE_INLINE void CallCheckOp(const char* file,
+ const int line,
+ const char* message,
+ const Us&... args) const {
+ prior_->CallCheckOp(file, line, message, arg_, args...);
+ }
+
private:
// The most recent argument.
T arg_;
@@ -235,6 +259,7 @@
const LogStreamer<Ts...>* prior_;
};
+template <bool isCheckOp>
class FatalLogCall final {
public:
FatalLogCall(const char* file, int line, const char* message)
@@ -244,7 +269,8 @@
template <typename... Ts>
RTC_NORETURN RTC_FORCE_INLINE void operator&(
const LogStreamer<Ts...>& streamer) {
- streamer.Call(file_, line_, message_);
+ isCheckOp ? streamer.CallCheckOp(file_, line_, message_)
+ : streamer.Call(file_, line_, message_);
}
private:
@@ -259,10 +285,10 @@
// in a particularly convoluted way with an extra ?: because that appears to be
// the simplest construct that keeps Visual Studio from complaining about
// condition being unused).
-#define RTC_EAT_STREAM_PARAMETERS(ignored) \
- (true ? true : ((void)(ignored), true)) \
- ? static_cast<void>(0) \
- : rtc::webrtc_checks_impl::FatalLogCall("", 0, "") & \
+#define RTC_EAT_STREAM_PARAMETERS(ignored) \
+ (true ? true : ((void)(ignored), true)) \
+ ? static_cast<void>(0) \
+ : rtc::webrtc_checks_impl::FatalLogCall<false>("", 0, "") & \
rtc::webrtc_checks_impl::LogStreamer<>()
// Call RTC_EAT_STREAM_PARAMETERS with an argument that fails to compile if
@@ -277,80 +303,19 @@
//
// We make sure RTC_CHECK et al. always evaluates |condition|, as
// doing RTC_CHECK(FunctionWithSideEffect()) is a common idiom.
-#define RTC_CHECK(condition) \
- while (!(condition)) \
- rtc::webrtc_checks_impl::FatalLogCall(__FILE__, __LINE__, #condition) & \
+#define RTC_CHECK(condition) \
+ while (!(condition)) \
+ rtc::webrtc_checks_impl::FatalLogCall<false>(__FILE__, __LINE__, \
+ #condition) & \
rtc::webrtc_checks_impl::LogStreamer<>()
// Helper macro for binary operators.
// Don't use this macro directly in your code, use RTC_CHECK_EQ et al below.
-// RTC_CHECK_EQ(...) else { ... } work properly.
-#define RTC_CHECK_OP(name, op, val1, val2) \
- while (std::string* _result = \
- rtc::Check##name##Impl((val1), (val2), #val1 " " #op " " #val2)) \
- rtc::webrtc_checks_impl::FatalLogCall(__FILE__, __LINE__, \
- _result->c_str()) & \
- rtc::webrtc_checks_impl::LogStreamer<>()
-
-// Build the error message string. This is separate from the "Impl"
-// function template because it is not performance critical and so can
-// be out of line, while the "Impl" code should be inline. Caller
-// takes ownership of the returned string.
-template<class t1, class t2>
-std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) {
- // TODO(jonasolsson): Use absl::StrCat() instead.
- std::ostringstream ss;
- ss << names << " (" << v1 << " vs. " << v2 << ")";
- std::string* msg = new std::string(ss.str());
- return msg;
-}
-
-// MSVC doesn't like complex extern templates and DLLs.
-#if !defined(COMPILER_MSVC)
-// Commonly used instantiations of MakeCheckOpString<>. Explicitly instantiated
-// in logging.cc.
-extern template std::string* MakeCheckOpString<int, int>(
- const int&, const int&, const char* names);
-extern template
-std::string* MakeCheckOpString<unsigned long, unsigned long>(
- const unsigned long&, const unsigned long&, const char* names);
-extern template
-std::string* MakeCheckOpString<unsigned long, unsigned int>(
- const unsigned long&, const unsigned int&, const char* names);
-extern template
-std::string* MakeCheckOpString<unsigned int, unsigned long>(
- const unsigned int&, const unsigned long&, const char* names);
-extern template
-std::string* MakeCheckOpString<std::string, std::string>(
- const std::string&, const std::string&, const char* name);
-#endif
-
-// Helper functions for RTC_CHECK_OP macro.
-// The (int, int) specialization works around the issue that the compiler
-// will not instantiate the template version of the function on values of
-// unnamed enum type - see comment below.
-#define DEFINE_RTC_CHECK_OP_IMPL(name) \
- template <class t1, class t2> \
- inline std::string* Check##name##Impl(const t1& v1, const t2& v2, \
- const char* names) { \
- if (rtc::Safe##name(v1, v2)) \
- return nullptr; \
- else \
- return rtc::MakeCheckOpString(v1, v2, names); \
- } \
- inline std::string* Check##name##Impl(int v1, int v2, const char* names) { \
- if (rtc::Safe##name(v1, v2)) \
- return nullptr; \
- else \
- return rtc::MakeCheckOpString(v1, v2, names); \
- }
-DEFINE_RTC_CHECK_OP_IMPL(Eq)
-DEFINE_RTC_CHECK_OP_IMPL(Ne)
-DEFINE_RTC_CHECK_OP_IMPL(Le)
-DEFINE_RTC_CHECK_OP_IMPL(Lt)
-DEFINE_RTC_CHECK_OP_IMPL(Ge)
-DEFINE_RTC_CHECK_OP_IMPL(Gt)
-#undef DEFINE_RTC_CHECK_OP_IMPL
+#define RTC_CHECK_OP(name, op, val1, val2) \
+ while (!rtc::Safe##name((val1), (val2))) \
+ rtc::webrtc_checks_impl::FatalLogCall<true>(__FILE__, __LINE__, \
+ #val1 " " #op " " #val2) & \
+ rtc::webrtc_checks_impl::LogStreamer<>() << (val1) << (val2)
#define RTC_CHECK_EQ(val1, val2) RTC_CHECK_OP(Eq, ==, val1, val2)
#define RTC_CHECK_NE(val1, val2) RTC_CHECK_OP(Ne, !=, val1, val2)
@@ -384,8 +349,9 @@
#define RTC_NOTREACHED() RTC_DCHECK(RTC_UNREACHABLE_CODE_HIT)
// TODO(bugs.webrtc.org/8454): Add an RTC_ prefix or rename differently.
-#define FATAL() \
- rtc::webrtc_checks_impl::FatalLogCall(__FILE__, __LINE__, "FATAL()") & \
+#define FATAL() \
+ rtc::webrtc_checks_impl::FatalLogCall<false>(__FILE__, __LINE__, \
+ "FATAL()") & \
rtc::webrtc_checks_impl::LogStreamer<>()
// Performs the integer division a/b and returns the result. CHECKs that the
diff --git a/rtc_base/logging_unittest.cc b/rtc_base/logging_unittest.cc
index 158f204..af51212 100644
--- a/rtc_base/logging_unittest.cc
+++ b/rtc_base/logging_unittest.cc
@@ -194,38 +194,34 @@
EXPECT_EQ(sev, LogMessage::GetLogToStream(nullptr));
}
-/*
#if GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
TEST(LogTest, Checks) {
EXPECT_DEATH(FATAL() << "message",
"\n\n#\n"
- "# Fatal error in: \\S+, line \\d+\n"
- "# last system error: \\d+\n"
+ "# Fatal error in: \\S+, line \\w+\n"
+ "# last system error: \\w+\n"
"# Check failed: FATAL\\(\\)\n"
- "# message"
- );
+ "# message");
int a = 1, b = 2;
EXPECT_DEATH(RTC_CHECK_EQ(a, b) << 1 << 2u,
"\n\n#\n"
- "# Fatal error in: \\S+, line \\d+\n"
- "# last system error: \\d+\n"
+ "# Fatal error in: \\S+, line \\w+\n"
+ "# last system error: \\w+\n"
"# Check failed: a == b \\(1 vs. 2\\)\n"
- "# 12"
- );
+ "# 12");
RTC_CHECK_EQ(5, 5);
RTC_CHECK(true) << "Shouldn't crash" << 1;
EXPECT_DEATH(RTC_CHECK(false) << "Hi there!",
"\n\n#\n"
- "# Fatal error in: \\S+, line \\d+\n"
- "# last system error: \\d+\n"
+ "# Fatal error in: \\S+, line \\w+\n"
+ "# last system error: \\w+\n"
"# Check failed: false\n"
- "# Hi there!"
- );
+ "# Hi there!");
}
#endif
-*/
+
// Test using multiple log streams. The INFO stream should get the INFO message,
// the VERBOSE stream should get the INFO and the VERBOSE.
// We should restore the correct global state at the end.
