[base] Further improve the logging facility.

src/base/logging.h

Index: src/base/logging.h
diff --git a/src/base/logging.h b/src/base/logging.h
index f54f10c1cde65e0d38d6726f48cd60ada9dfba18..27efdcecbe7eddce89adf85d4de42a59d3bdcea9 100644
--- a/src/base/logging.h
+++ b/src/base/logging.h
@@ -10,6 +10,7 @@
 #include <string>
 
 #include "src/base/build_config.h"
+#include "src/base/compiler-specific.h"
 
 extern "C" void V8_Fatal(const char* file, int line, const char* format, ...);
 
@@ -34,6 +35,46 @@ extern "C" void V8_Fatal(const char* file, int line, const char* format, ...);
 
 namespace v8 {
 namespace base {
+namespace logging {
+
+typedef int LogSeverity;
+const LogSeverity LOG_INFO = 0;
+const LogSeverity LOG_FATAL = 1;
+
+
+// A few definitions of macros that don't generate much code. These are used
+// by LOG() and LOG_IF, etc. Since these are used all over our code, it's
+// better to have compact code for these operations.
+#define COMPACT_GOOGLE_LOG_INFO                       \
+  ::v8::base::logging::LogMessage(__FILE__, __LINE__, \
+                                  ::v8::base::logging::LOG_INFO)
+#define COMPACT_GOOGLE_LOG_FATAL                      \
+  ::v8::base::logging::LogMessage(__FILE__, __LINE__, \
+                                  ::v8::base::logging::LOG_FATAL)
+
+
+// Helper macro which avoids evaluating the arguments to a stream if
+// the condition doesn't hold.
+#define LAZY_STREAM(stream, condition) \
+  V8_LIKELY(!(condition))              \
+  ? (void)0 : ::v8::base::logging::LogMessageVoidify() & (stream)
+
+
+// We use the preprocessor's merging operator, "##", so that, e.g.,
+// LOG(INFO) becomes the token COMPACT_GOOGLE_LOG_INFO.  There's some funny
+// subtle difference between ostream member streaming functions (e.g.,
+// ostream::operator<<(int) and ostream non-member streaming functions
+// (e.g., ::operator<<(ostream&, string&): it turns out that it's
+// impossible to stream something like a string directly to an unnamed
+// ostream. We employ a neat hack by calling the stream() member
+// function of LogMessage which seems to avoid the problem.
+#define LOG_STREAM(SEVERITY) COMPACT_GOOGLE_LOG_##SEVERITY.stream()
+
+
+// The actual stream used isn't important.
+#define EAT_STREAM_PARAMETERS \
+  true ? (void)0 : ::v8::base::logging::LogMessageVoidify() & LOG_STREAM(FATAL)
+
 
 // CHECK dies with a fatal error if condition is not true.  It is *not*
 // controlled by DEBUG, so the check will be executed regardless of
@@ -41,35 +82,48 @@ namespace base {
 //
 // We make sure CHECK et al. always evaluates their arguments, as
 // doing CHECK(FunctionWithSideEffect()) is a common idiom.
-#define CHECK(condition)                                             \
-  do {                                                               \
-    if (V8_UNLIKELY(!(condition))) {                                 \
-      V8_Fatal(__FILE__, __LINE__, "Check failed: %s.", #condition); \
-    }                                                                \
-  } while (0)
+#ifdef DEBUG
 
+#ifdef _PREFAST_
+// Use __analysis_assume to tell the VC++ static analysis engine that
+// assert conditions are true, to suppress warnings.  The LAZY_STREAM
+// parameter doesn't reference 'condition' in /analyze builds because
+// this evaluation confuses /analyze. The !! before condition is because
+// __analysis_assume gets confused on some conditions:
+// http://randomascii.wordpress.com/2011/09/13/analyze-for-visual-studio-the-ugly-part-5/
 
-#ifdef DEBUG
+#define CHECK(condition)                                                  \
+  __analysis_assume(!!(condition)), LAZY_STREAM(LOG_STREAM(FATAL), false) \
+                                        << "Check failed: " #condition ". "
+
+#else  // DEBUG

[Sven Panne, 2015/01/30 11:39:36]

Wrong comment

+
+#define CHECK(condition)                       \
+  LAZY_STREAM(LOG_STREAM(FATAL), !(condition)) \
+      << "Check failed: " << #condition ". "
+
+#endif  // _PREFAST_
 
 // Helper macro for binary operators.
 // Don't use this macro directly in your code, use CHECK_EQ et al below.
-#define CHECK_OP(name, op, lhs, rhs)                                    \
-  do {                                                                  \
-    if (std::string* _msg = ::v8::base::Check##name##Impl(              \
-            (lhs), (rhs), #lhs " " #op " " #rhs)) {                     \
-      V8_Fatal(__FILE__, __LINE__, "Check failed: %s.", _msg->c_str()); \
-      delete _msg;                                                      \
-    }                                                                   \
-  } while (0)
+#define CHECK_OP(name, op, lhs, rhs)                                 \
+  if (std::string* _result = ::v8::base::logging::Check##name##Impl( \
+          (lhs), (rhs), #lhs " " #op " " #rhs))                      \
+  ::v8::base::logging::LogMessage(__FILE__, __LINE__, _result).stream()
 
-#else
+#else  // DEBUG
+
+#define CHECK(condition)                                          \
+  V8_UNLIKELY(!(condition))                                       \
+  ? V8_Fatal(__FILE__, __LINE__, "Check failed: %s.", #condition) \
+  : EAT_STREAM_PARAMETERS
 
 // Make all CHECK functions discard their log strings to reduce code
 // bloat for official release builds.
 
 #define CHECK_OP(name, op, lhs, rhs) CHECK((lhs)op(rhs))
 
-#endif
+#endif  // DEBUG
 
 
 // Build the error message string.  This is separate from the "Impl"
@@ -78,9 +132,9 @@ namespace base {
 // takes ownership of the returned string.
 template <typename Lhs, typename Rhs>
 std::string* MakeCheckOpString(Lhs const& lhs, Rhs const& rhs,
-                               char const* msg) {
+                               char const* result) {
   std::ostringstream ss;
-  ss << msg << " (" << lhs << " vs. " << rhs << ")";
+  ss << result << " (" << lhs << " vs. " << rhs << ")";
   return new std::string(ss.str());
 }
 
@@ -106,21 +160,23 @@ DEFINE_MAKE_CHECK_OP_STRING(void const*)
 // unnamed enum type - see comment below.
 // The (float, float) and (double, double) instantiations are explicitly
 // externialized to ensure proper 32/64-bit comparisons on x86.
-#define DEFINE_CHECK_OP_IMPL(NAME, op)                                         \
-  template <typename Lhs, typename Rhs>                                        \
-  V8_INLINE std::string* Check##NAME##Impl(Lhs const& lhs, Rhs const& rhs,     \
-                                           char const* msg) {                  \
-    return V8_LIKELY(lhs op rhs) ? nullptr : MakeCheckOpString(lhs, rhs, msg); \
-  }                                                                            \
-  V8_INLINE std::string* Check##NAME##Impl(int lhs, int rhs,                   \
-                                           char const* msg) {                  \
-    return V8_LIKELY(lhs op rhs) ? nullptr : MakeCheckOpString(lhs, rhs, msg); \
-  }                                                                            \
-  extern template std::string* Check##NAME##Impl<float, float>(                \
-      float const& lhs, float const& rhs, char const* msg);                    \
-  extern template std::string* Check##NAME##Impl<double, double>(              \
-      double const& lhs, double const& rhs, char const* msg);
-DEFINE_CHECK_OP_IMPL(EQ, ==)
+#define DEFINE_CHECK_OP_IMPL(NAME, op)                                     \
+  template <typename Lhs, typename Rhs>                                    \
+  V8_INLINE std::string* Check##NAME##Impl(Lhs const& lhs, Rhs const& rhs, \
+                                           char const* result) {           \
+    return V8_LIKELY(lhs op rhs) ? nullptr                                 \
+                                 : MakeCheckOpString(lhs, rhs, result);    \
+  }                                                                        \
+  V8_INLINE std::string* Check##NAME##Impl(int lhs, int rhs,               \
+                                           char const* result) {           \
+    return V8_LIKELY(lhs op rhs) ? nullptr                                 \
+                                 : MakeCheckOpString(lhs, rhs, result);    \
+  }                                                                        \
+  extern template std::string* Check##NAME##Impl<float, float>(            \
+      float const& lhs, float const& rhs, char const* result);             \
+  extern template std::string* Check##NAME##Impl<double, double>(          \
+      double const& lhs, double const& rhs, char const* result);
+DEFINE_CHECK_OP_IMPL(EQ, == )
 DEFINE_CHECK_OP_IMPL(NE, !=)
 DEFINE_CHECK_OP_IMPL(LE, <=)
 DEFINE_CHECK_OP_IMPL(LT, < )
@@ -139,19 +195,64 @@ DEFINE_CHECK_OP_IMPL(GT, > )
 #define CHECK_IMPLIES(lhs, rhs) CHECK(!(lhs) || (rhs))
 
 
-// Exposed for making debugging easier (to see where your function is being
-// called, just add a call to DumpBacktrace).
-void DumpBacktrace();
+// This class more or less represents a particular log message. You create an
+// instance of LogMessage and then stream stuff to it. When you finish streaming
+// to it, the destructor is called and the full message gets streamed to the
+// appropriate destination.
+//
+// You shouldn't actually use LogMessage's constructor to log things, through.
+// You should use the macros above.
+class LogMessage FINAL {
+ public:
+  // Used for LOG(severity), CHECK() and friends.
+  LogMessage(char const* file, int line, LogSeverity severity);
+
+  // Used for CHECK_EQ() and friends. Takes ownership of the given string.
+  // Implied severity = LOG_FATAL.
+  LogMessage(char const* file, int line, std::string* result);
+
+  // Used for DCHECK_EQ() and friends. Takes ownership of the given string.
+  LogMessage(char const* file, int line, LogSeverity severity,
+             std::string* result);
+
+  ~LogMessage();
+
+  std::ostream& stream() { return stream_; }
+
+ private:
+  std::ostringstream stream_;
+  char const* const file_;  // The file information passed into the constructor.
+  int const line_;          // The line information passed into the constructor.
+  LogSeverity const severity_;
+
+  // TODO(bmeurer): Use DISALLOW_IMPLICIT_CONSTRUCTORS once macros.h no longer
+  // depends on logging.h.
+  LogMessage() V8_DELETE;
+  LogMessage(LogMessage const&) V8_DELETE;
+  void operator=(LogMessage const&) V8_DELETE;
+};
+
+
+// This class is used to explicitly ignore values in the conditional
+// logging macros. This avoids compiler warnings like "value computed
+// is not used" and "statement has no effect".
+class LogMessageVoidify FINAL {
+ public:
+  LogMessageVoidify() {}
+
+  // This has to be an operator with a precedence lower than << but
+  // higher than ?:
+  void operator&(std::ostream&) {}
+};
 
+}  // namespace logging
 }  // namespace base
 }  // namespace v8
 
 
 // The DCHECK macro is equivalent to CHECK except that it only
 // generates code in debug builds.
-// TODO(bmeurer): DCHECK_RESULT(expr) must die!
 #ifdef DEBUG
-#define DCHECK_RESULT(expr)    CHECK(expr)
 #define DCHECK(condition)      CHECK(condition)
 #define DCHECK_EQ(v1, v2)      CHECK_EQ(v1, v2)
 #define DCHECK_NE(v1, v2)      CHECK_NE(v1, v2)
@@ -162,7 +263,6 @@ void DumpBacktrace();
 #define DCHECK_NOT_NULL(val)   CHECK_NOT_NULL(val)
 #define DCHECK_IMPLIES(v1, v2) CHECK_IMPLIES(v1, v2)
 #else
-#define DCHECK_RESULT(expr)    (expr)
 #define DCHECK(condition)      ((void) 0)
 #define DCHECK_EQ(v1, v2)      ((void) 0)
 #define DCHECK_NE(v1, v2)      ((void) 0)