feat: support failure functions that throw (#1074)
diff --git a/src/glog/logging.h b/src/glog/logging.h
index 6e3ea26..a0759df 100644
--- a/src/glog/logging.h
+++ b/src/glog/logging.h
@@ -521,8 +521,10 @@
GLOG_EXPORT void InstallPrefixFormatter(PrefixFormatterCallback callback,
void* data = nullptr);
-// Install a function which will be called after LOG(FATAL).
-GLOG_EXPORT void InstallFailureFunction(logging_fail_func_t fail_func);
+// Install a function which will be called after LOG(FATAL). Returns the
+// previously set function.
+GLOG_EXPORT logging_fail_func_t
+InstallFailureFunction(logging_fail_func_t fail_func);
[[deprecated(
"Use the type-safe std::chrono::minutes EnableLogCleaner overload "
@@ -606,13 +608,11 @@
// A container for a string pointer which can be evaluated to a bool -
// true iff the pointer is nullptr.
struct CheckOpString {
- CheckOpString(std::string* str) : str_(str) {}
- // No destructor: if str_ is non-nullptr, we're about to LOG(FATAL),
- // so there's no point in cleaning up str_.
+ CheckOpString(std::unique_ptr<std::string> str) : str_(std::move(str)) {}
explicit operator bool() const noexcept {
return GOOGLE_PREDICT_BRANCH_NOT_TAKEN(str_ != nullptr);
}
- std::string* str_;
+ std::unique_ptr<std::string> str_;
};
// Function is overloaded for integral types to allow static const
@@ -671,7 +671,8 @@
// Build the error message string. Specify no inlining for code size.
template <typename T1, typename T2>
-std::string* MakeCheckOpString(const T1& v1, const T2& v2, const char* exprtext)
+std::unique_ptr<std::string> MakeCheckOpString(const T1& v1, const T2& v2,
+ const char* exprtext)
#if defined(__has_attribute)
# if __has_attribute(used)
__attribute__((noinline))
@@ -696,15 +697,15 @@
// For inserting the second variable (adds an intermediate " vs. ").
std::ostream* ForVar2();
// Get the result (inserts the closing ")").
- std::string* NewString();
+ std::unique_ptr<std::string> NewString();
private:
std::ostringstream* stream_;
};
template <typename T1, typename T2>
-std::string* MakeCheckOpString(const T1& v1, const T2& v2,
- const char* exprtext) {
+std::unique_ptr<std::string> MakeCheckOpString(const T1& v1, const T2& v2,
+ const char* exprtext) {
CheckOpMessageBuilder comb(exprtext);
MakeCheckOpValueString(comb.ForVar1(), v1);
MakeCheckOpValueString(comb.ForVar2(), v2);
@@ -715,17 +716,18 @@
// 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_CHECK_OP_IMPL(name, op) \
- template <typename T1, typename T2> \
- inline std::string* name##Impl(const T1& v1, const T2& v2, \
- const char* exprtext) { \
- if (GOOGLE_PREDICT_TRUE(v1 op v2)) \
- return nullptr; \
- else \
- return MakeCheckOpString(v1, v2, exprtext); \
- } \
- inline std::string* name##Impl(int v1, int v2, const char* exprtext) { \
- return name##Impl<int, int>(v1, v2, exprtext); \
+#define DEFINE_CHECK_OP_IMPL(name, op) \
+ template <typename T1, typename T2> \
+ inline std::unique_ptr<std::string> name##Impl(const T1& v1, const T2& v2, \
+ const char* exprtext) { \
+ if (GOOGLE_PREDICT_TRUE(v1 op v2)) { \
+ return nullptr; \
+ } \
+ return MakeCheckOpString(v1, v2, exprtext); \
+ } \
+ inline std::unique_ptr<std::string> name##Impl(int v1, int v2, \
+ const char* exprtext) { \
+ return name##Impl<int, int>(v1, v2, exprtext); \
}
// We use the full name Check_EQ, Check_NE, etc. in case the file including
@@ -757,14 +759,15 @@
// with other string implementations that get defined after this
// file is included). Save the current meaning now and use it
// in the macro.
-typedef std::string _Check_string;
+using _Check_string = std::string;
# define CHECK_OP_LOG(name, op, val1, val2, log) \
- while (google::logging::internal::_Check_string* _result = \
+ while (std::unique_ptr<google::logging::internal::_Check_string> _result = \
google::logging::internal::Check##name##Impl( \
google::logging::internal::GetReferenceableValue(val1), \
google::logging::internal::GetReferenceableValue(val2), \
#val1 " " #op " " #val2)) \
- log(__FILE__, __LINE__, google::logging::internal::CheckOpString(_result)) \
+ log(__FILE__, __LINE__, \
+ google::logging::internal::CheckOpString(std::move(_result))) \
.stream()
#else
// In optimized mode, use CheckOpString to hint to compiler that
@@ -820,8 +823,8 @@
// Helper functions for string comparisons.
// To avoid bloat, the definitions are in logging.cc.
-#define DECLARE_CHECK_STROP_IMPL(func, expected) \
- GLOG_EXPORT std::string* Check##func##expected##Impl( \
+#define DECLARE_CHECK_STROP_IMPL(func, expected) \
+ GLOG_EXPORT std::unique_ptr<std::string> Check##func##expected##Impl( \
const char* s1, const char* s2, const char* names);
DECLARE_CHECK_STROP_IMPL(strcmp, true)
@@ -840,7 +843,7 @@
while (google::logging::internal::CheckOpString _result = \
google::logging::internal::Check##func##expected##Impl( \
(s1), (s2), #s1 " " #op " " #s2)) \
- LOG(FATAL) << *_result.str_
+ LOG(FATAL) << (*_result.str_)
// String (char*) equality/inequality checks.
// CASE versions are case-insensitive.
@@ -1332,7 +1335,7 @@
LogMessage(const char* file, int line,
const logging::internal::CheckOpString& result);
- ~LogMessage();
+ ~LogMessage() noexcept(false);
// Flush a buffered message to the sink set in the constructor. Always
// called by the destructor, it may also be called from elsewhere if
@@ -1409,7 +1412,7 @@
LogMessageFatal(const char* file, int line);
LogMessageFatal(const char* file, int line,
const logging::internal::CheckOpString& result);
- [[noreturn]] ~LogMessageFatal();
+ [[noreturn]] ~LogMessageFatal() noexcept(false);
};
// A non-macro interface to the log facility; (useful
@@ -1462,7 +1465,7 @@
template <typename T>
T CheckNotNull(const char* file, int line, const char* names, T&& t) {
if (t == nullptr) {
- LogMessageFatal(file, line, new std::string(names));
+ LogMessageFatal(file, line, std::make_unique<std::string>(names));
}
return std::forward<T>(t);
}
diff --git a/src/googletest.h b/src/googletest.h
index 971c5b4..33af3fe 100644
--- a/src/googletest.h
+++ b/src/googletest.h
@@ -196,8 +196,19 @@
} \
} while (0)
-vector<void (*)()> g_testlist; // the tests to run
+# define EXPECT_THROW(statement, exception) \
+ do { \
+ try { \
+ statement; \
+ } catch (const exception&) { \
+ printf("ok\n"); \
+ } catch (...) { \
+ fprintf(stderr, "%s\n", "Unexpected exception thrown"); \
+ exit(EXIT_FAILURE); \
+ } \
+ } while (0)
+vector<void (*)()> g_testlist; // the tests to run
# define TEST(a, b) \
struct Test_##a##_##b { \
Test_##a##_##b() { g_testlist.push_back(&Run); } \
diff --git a/src/logging.cc b/src/logging.cc
index f95fb61..fb283fd 100644
--- a/src/logging.cc
+++ b/src/logging.cc
@@ -1723,8 +1723,9 @@
const char* LogMessage::basename() const noexcept { return data_->basename_; }
const LogMessageTime& LogMessage::time() const noexcept { return time_; }
-LogMessage::~LogMessage() {
+LogMessage::~LogMessage() noexcept(false) {
Flush();
+ bool fail = data_->severity_ == GLOG_FATAL && exit_on_dfatal;
#ifdef GLOG_THREAD_LOCAL_STORAGE
if (data_ == static_cast<void*>(&thread_msg_data)) {
data_->~LogMessageData();
@@ -1735,6 +1736,26 @@
#else // !defined(GLOG_THREAD_LOCAL_STORAGE)
delete allocated_;
#endif // defined(GLOG_THREAD_LOCAL_STORAGE)
+ //
+
+ if (fail) {
+ const char* message = "*** Check failure stack trace: ***\n";
+ if (write(fileno(stderr), message, strlen(message)) < 0) {
+ // Ignore errors.
+ }
+ AlsoErrorWrite(GLOG_FATAL,
+ glog_internal_namespace_::ProgramInvocationShortName(),
+ message);
+#if defined(__cpp_lib_uncaught_exceptions) && \
+ (__cpp_lib_uncaught_exceptions >= 201411L)
+ if (std::uncaught_exceptions() == 0)
+#else
+ if (!std::uncaught_exception())
+#endif
+ {
+ Fail();
+ }
+ }
}
int LogMessage::preserved_errno() const { return data_->preserved_errno_; }
@@ -1894,22 +1915,7 @@
}
}
- // release the lock that our caller (directly or indirectly)
- // LogMessage::~LogMessage() grabbed so that signal handlers
- // can use the logging facility. Alternately, we could add
- // an entire unsafe logging interface to bypass locking
- // for signal handlers but this seems simpler.
- log_mutex.unlock();
LogDestination::WaitForSinks(data_);
-
- const char* message = "*** Check failure stack trace: ***\n";
- if (write(fileno(stderr), message, strlen(message)) < 0) {
- // Ignore errors.
- }
- AlsoErrorWrite(GLOG_FATAL,
- glog_internal_namespace_::ProgramInvocationShortName(),
- message);
- Fail();
}
}
@@ -1941,8 +1947,8 @@
std::abort();
}
-void InstallFailureFunction(logging_fail_func_t fail_func) {
- g_logging_fail_func = fail_func;
+logging_fail_func_t InstallFailureFunction(logging_fail_func_t fail_func) {
+ return std::exchange(g_logging_fail_func, fail_func);
}
void LogMessage::Fail() { g_logging_fail_func(); }
@@ -2533,17 +2539,17 @@
namespace internal {
// Helper functions for string comparisons.
#define DEFINE_CHECK_STROP_IMPL(name, func, expected) \
- string* Check##func##expected##Impl(const char* s1, const char* s2, \
- const char* names) { \
+ std::unique_ptr<string> Check##func##expected##Impl( \
+ const char* s1, const char* s2, const char* names) { \
bool equal = s1 == s2 || (s1 && s2 && !func(s1, s2)); \
- if (equal == expected) \
+ if (equal == (expected)) \
return nullptr; \
else { \
ostringstream ss; \
if (!s1) s1 = ""; \
if (!s2) s2 = ""; \
ss << #name " failed: " << names << " (" << s1 << " vs. " << s2 << ")"; \
- return new string(ss.str()); \
+ return std::make_unique<std::string>(ss.str()); \
} \
}
DEFINE_CHECK_STROP_IMPL(CHECK_STREQ, strcmp, true)
@@ -2635,7 +2641,7 @@
const logging::internal::CheckOpString& result)
: LogMessage(file, line, result) {}
-LogMessageFatal::~LogMessageFatal() {
+LogMessageFatal::~LogMessageFatal() noexcept(false) {
Flush();
LogMessage::Fail();
}
@@ -2655,9 +2661,9 @@
return stream_;
}
-string* CheckOpMessageBuilder::NewString() {
+std::unique_ptr<string> CheckOpMessageBuilder::NewString() {
*stream_ << ")";
- return new string(stream_->str());
+ return std::make_unique<std::string>(stream_->str());
}
template <>
diff --git a/src/logging_unittest.cc b/src/logging_unittest.cc
index 1c5153b..321f38f 100644
--- a/src/logging_unittest.cc
+++ b/src/logging_unittest.cc
@@ -41,6 +41,7 @@
#include <mutex>
#include <queue>
#include <sstream>
+#include <stdexcept>
#include <string>
#include <thread>
#include <vector>
@@ -1571,3 +1572,17 @@
EXPECT_FALSE(
SendEmail("!/bin/true@example.com", "Example subject", "Example body"));
}
+
+TEST(Logging, FatalThrow) {
+ auto const fail_func =
+ InstallFailureFunction(+[]()
+#if defined(__has_attribute)
+# if __has_attribute(noreturn)
+ __attribute__((noreturn))
+# endif // __has_attribute(noreturn)
+#endif // defined(__has_attribute)
+ { throw std::logic_error{"fail"}; });
+ auto restore_fail = [fail_func] { InstallFailureFunction(fail_func); };
+ ScopedExit<decltype(restore_fail)> restore{restore_fail};
+ EXPECT_THROW({ LOG(FATAL) << "must throw to fail"; }, std::logic_error);
+}