From fc507b26548882a8492de57abf0a8923ecf7709a Mon Sep 17 00:00:00 2001 From: Adam Simpkins Date: Fri, 30 Jun 2017 12:36:56 -0700 Subject: [PATCH] Don't try to access XlogFileScopeInfo->category if not supported Summary: This was breaking Windows builds. Reviewed By: Orvid Differential Revision: D5351468 fbshipit-source-id: 3385931b82b11d71d98861fb45efc71a632d470b --- .../logging/LogStreamProcessor.cpp | 52 ++++--- .../experimental/logging/LogStreamProcessor.h | 143 +++++++++--------- 2 files changed, 104 insertions(+), 91 deletions(-) diff --git a/folly/experimental/logging/LogStreamProcessor.cpp b/folly/experimental/logging/LogStreamProcessor.cpp index 2415ed4a..6fe08881 100644 --- a/folly/experimental/logging/LogStreamProcessor.cpp +++ b/folly/experimental/logging/LogStreamProcessor.cpp @@ -52,20 +52,6 @@ LogStreamProcessor::LogStreamProcessor( INTERNAL, std::string()) {} -LogStreamProcessor::LogStreamProcessor( - XlogFileScopeInfo* fileScopeInfo, - LogLevel level, - folly::StringPiece filename, - unsigned int lineNumber, - AppendType) noexcept - : LogStreamProcessor( - fileScopeInfo, - level, - filename, - lineNumber, - INTERNAL, - std::string()) {} - LogStreamProcessor::LogStreamProcessor( const LogCategory* category, LogLevel level, @@ -90,13 +76,6 @@ LogCategory* getXlogCategory( } return categoryInfo->getCategory(&xlog_detail::xlogFileScopeInfo); } - -LogCategory* getXlogCategory(XlogFileScopeInfo* fileScopeInfo) { - // By the time a LogStreamProcessor is created, the XlogFileScopeInfo object - // should have already been initialized to perform the log level check. - // Therefore we never need to check if it is initialized here. - return fileScopeInfo->category; -} } /** @@ -125,12 +104,28 @@ LogStreamProcessor::LogStreamProcessor( message_{std::move(msg)}, stream_{this} {} +#ifdef __INCLUDE_LEVEL__ +namespace { +LogCategory* getXlogCategory(XlogFileScopeInfo* fileScopeInfo) { + // By the time a LogStreamProcessor is created, the XlogFileScopeInfo object + // should have already been initialized to perform the log level check. + // Therefore we never need to check if it is initialized here. + return fileScopeInfo->category; +} +} + /** * Construct a LogStreamProcessor from an XlogFileScopeInfo. * * We intentionally define this in LogStreamProcessor.cpp instead of * LogStreamProcessor.h to avoid having it inlined at every XLOG() call site, * to reduce the emitted code size. + * + * This is only defined if __INCLUDE_LEVEL__ is available. The + * XlogFileScopeInfo APIs are only invoked if we can use __INCLUDE_LEVEL__ to + * tell that an XLOG() statement occurs in a non-header file. For compilers + * that do not support __INCLUDE_LEVEL__, the category information is always + * passed in as XlogCategoryInfo rather than as XlogFileScopeInfo. */ LogStreamProcessor::LogStreamProcessor( XlogFileScopeInfo* fileScopeInfo, @@ -146,6 +141,21 @@ LogStreamProcessor::LogStreamProcessor( message_{std::move(msg)}, stream_{this} {} +LogStreamProcessor::LogStreamProcessor( + XlogFileScopeInfo* fileScopeInfo, + LogLevel level, + folly::StringPiece filename, + unsigned int lineNumber, + AppendType) noexcept + : LogStreamProcessor( + fileScopeInfo, + level, + filename, + lineNumber, + INTERNAL, + std::string()) {} +#endif + /* * We intentionally define the LogStreamProcessor destructor in * LogStreamProcessor.cpp instead of LogStreamProcessor.h to avoid having it diff --git a/folly/experimental/logging/LogStreamProcessor.h b/folly/experimental/logging/LogStreamProcessor.h index f5fec5e2..66db917c 100644 --- a/folly/experimental/logging/LogStreamProcessor.h +++ b/folly/experimental/logging/LogStreamProcessor.h @@ -108,39 +108,29 @@ class LogStreamProcessor { AppendType) noexcept; /** - * LogStreamProcessor constructors for use with XLOG() macros with no extra - * arguments. + * LogStreamProcessor constructor for use with a LOG() macro with arguments + * to be concatenated with folly::to() * - * These are defined separately from the above constructor so that the work - * of initializing the XLOG LogCategory data is done in a separate function - * body defined in LogStreamProcessor.cpp. We intentionally want to avoid - * inlining this work at every XLOG() statement, to reduce the emitted code - * size. + * Note that the filename argument is not copied. The caller should ensure + * that it points to storage that will remain valid for the lifetime of the + * LogStreamProcessor. (This is always the case for the __FILE__ + * preprocessor macro.) */ + template LogStreamProcessor( - XlogCategoryInfo* categoryInfo, - LogLevel level, - folly::StringPiece categoryName, - bool isCategoryNameOverridden, - folly::StringPiece filename, - unsigned int lineNumber, - AppendType) noexcept; - LogStreamProcessor( - XlogFileScopeInfo* fileScopeInfo, - LogLevel level, - folly::StringPiece filename, - unsigned int lineNumber, - AppendType) noexcept; - LogStreamProcessor( - XlogFileScopeInfo* fileScopeInfo, + const LogCategory* category, LogLevel level, - folly::StringPiece /* categoryName */, - bool /* isCategoryNameOverridden */, folly::StringPiece filename, unsigned int lineNumber, - AppendType) noexcept - : LogStreamProcessor(fileScopeInfo, level, filename, lineNumber, APPEND) { - } + AppendType, + Args&&... args) noexcept + : LogStreamProcessor( + category, + level, + filename, + lineNumber, + INTERNAL, + createLogString(std::forward(args)...)) {} /** * LogStreamProcessor constructor for use with a LOG() macro with arguments @@ -157,7 +147,8 @@ class LogStreamProcessor { LogLevel level, folly::StringPiece filename, unsigned int lineNumber, - AppendType, + FormatType, + folly::StringPiece fmt, Args&&... args) noexcept : LogStreamProcessor( category, @@ -165,11 +156,25 @@ class LogStreamProcessor { filename, lineNumber, INTERNAL, - createLogString(std::forward(args)...)) {} + formatLogString(fmt, std::forward(args)...)) {} - /** - * Versions of the above constructor for use in XLOG() statements. + /* + * Versions of the above constructors for use in XLOG() statements. + * + * These are defined separately from the above constructor so that the work + * of initializing the XLOG LogCategory data is done in a separate function + * body defined in LogStreamProcessor.cpp. We intentionally want to avoid + * inlining this work at every XLOG() statement, to reduce the emitted code + * size. */ + LogStreamProcessor( + XlogCategoryInfo* categoryInfo, + LogLevel level, + folly::StringPiece categoryName, + bool isCategoryNameOverridden, + folly::StringPiece filename, + unsigned int lineNumber, + AppendType) noexcept; template LogStreamProcessor( XlogCategoryInfo* categoryInfo, @@ -191,72 +196,69 @@ class LogStreamProcessor { createLogString(std::forward(args)...)) {} template LogStreamProcessor( - XlogFileScopeInfo* fileScopeInfo, + XlogCategoryInfo* categoryInfo, LogLevel level, - folly::StringPiece /* categoryName */, - bool /* isCategoryNameOverridden */, + folly::StringPiece categoryName, + bool isCategoryNameOverridden, folly::StringPiece filename, unsigned int lineNumber, - AppendType, + FormatType, + folly::StringPiece fmt, Args&&... args) noexcept : LogStreamProcessor( - fileScopeInfo, + categoryInfo, level, + categoryName, + isCategoryNameOverridden, filename, lineNumber, INTERNAL, - createLogString(std::forward(args)...)) {} + formatLogString(fmt, std::forward(args)...)) {} - /** - * LogStreamProcessor constructor for use with a LOG() macro with arguments - * to be concatenated with folly::to() +#ifdef __INCLUDE_LEVEL__ + /* + * Versions of the above constructors to use in XLOG() macros that appear in + * .cpp files. These are only used if the compiler supports the + * __INCLUDE_LEVEL__ macro, which we need to determine that the XLOG() + * statement is not in a header file. * - * Note that the filename argument is not copied. The caller should ensure - * that it points to storage that will remain valid for the lifetime of the - * LogStreamProcessor. (This is always the case for the __FILE__ - * preprocessor macro.) + * These behave identically to the XlogCategoryInfo versions of the + * APIs, but slightly more optimized, and allow the XLOG() code to avoid + * storing category information at each XLOG() call site. */ - template LogStreamProcessor( - const LogCategory* category, + XlogFileScopeInfo* fileScopeInfo, LogLevel level, folly::StringPiece filename, unsigned int lineNumber, - FormatType, - folly::StringPiece fmt, - Args&&... args) noexcept - : LogStreamProcessor( - category, - level, - filename, - lineNumber, - INTERNAL, - formatLogString(fmt, std::forward(args)...)) {} - - /** - * Versions of the above constructor for use in XLOG() statements. - */ + AppendType) noexcept; + LogStreamProcessor( + XlogFileScopeInfo* fileScopeInfo, + LogLevel level, + folly::StringPiece /* categoryName */, + bool /* isCategoryNameOverridden */, + folly::StringPiece filename, + unsigned int lineNumber, + AppendType) noexcept + : LogStreamProcessor(fileScopeInfo, level, filename, lineNumber, APPEND) { + } template LogStreamProcessor( - XlogCategoryInfo* categoryInfo, + XlogFileScopeInfo* fileScopeInfo, LogLevel level, - folly::StringPiece categoryName, - bool isCategoryNameOverridden, + folly::StringPiece /* categoryName */, + bool /* isCategoryNameOverridden */, folly::StringPiece filename, unsigned int lineNumber, - FormatType, - folly::StringPiece fmt, + AppendType, Args&&... args) noexcept : LogStreamProcessor( - categoryInfo, + fileScopeInfo, level, - categoryName, - isCategoryNameOverridden, filename, lineNumber, INTERNAL, - formatLogString(fmt, std::forward(args)...)) {} - + createLogString(std::forward(args)...)) {} template LogStreamProcessor( XlogFileScopeInfo* fileScopeInfo, @@ -275,6 +277,7 @@ class LogStreamProcessor { lineNumber, INTERNAL, formatLogString(fmt, std::forward(args)...)) {} +#endif ~LogStreamProcessor() noexcept; -- 2.34.1