Fix OSS logging primitives macros (facebookresearch#140)

Summary: The logging macros were not quite working as expected, and a little more black magic fu was required to make them compile and work as expected on all platforms. Reviewed By: SeaOtocinclus Differential Revision: D52315578
georges-berenger · Dec 20, 2023 · a18f28e · a18f28e
1 parent 0f78024
commit a18f28e
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 28 deletions.
diff --git a/tools/vrs/VrsCommand.cpp b/tools/vrs/VrsCommand.cpp
@@ -18,9 +18,7 @@
 
 #include <cstdlib>
 #include <cstring>
-#include <iomanip>
 #include <iostream>
-#include <sstream>
 #include <string>
 
 #define DEFAULT_LOG_CHANNEL "VrsCommand"

diff --git a/tools/vrs/vrs.cpp b/tools/vrs/vrs.cpp
@@ -16,8 +16,6 @@
 
 #include "VrsCommand.h"
 
-#include <iostream>
-
 #include <vrs/os/Utils.h>
 
 using namespace std;

diff --git a/vrs/oss/logging/Checks.cpp b/vrs/oss/logging/Checks.cpp
@@ -17,11 +17,12 @@
 #include "Checks.h"
 
 #include <cstdlib>
-#include <string>
 
 #include <fmt/color.h>
 #include <fmt/core.h>
 
+#include <vrs/os/Platform.h>
+
 // Abort Macro.
 #if IS_ANDROID_PLATFORM()
 #include <android/log.h>
@@ -41,8 +42,8 @@
 namespace vrs {
 namespace logging {
 
-void logAndAbort(const std::string& condition, const std::string& message) {
-  fmt::print(stderr, fg(fmt::color::red), "{} {}", condition, message);
+void logAndAbort(const char* condition, const std::string& message) {
+  fmt::print(stderr, fg(fmt::color::red), "Check '{}' failed. {}\n", condition, message);
   XR_ABORT_IMPL(message);
 }
 

diff --git a/vrs/oss/logging/Checks.h b/vrs/oss/logging/Checks.h
@@ -20,14 +20,10 @@
 
 #include <fmt/core.h>
 
-#include <vrs/os/CompilerAttributes.h>
-
-#include "LogLevel.h"
-
 namespace vrs {
 namespace logging {
 
-void logAndAbort(const std::string& condition, const std::string& message = {});
+void logAndAbort(const char* condition, const std::string& message = {});
 
 } // namespace logging
 } // namespace vrs
@@ -36,10 +32,10 @@ void logAndAbort(const std::string& condition, const std::string& message = {});
 // Check Macros.
 //
 
-#define XR_CHECK_FORMAT(condition, ...) \
-  (condition ? 0 : ((vrs::logging::logAndAbort(#condition, fmt::format(__VA_ARGS__))), 0))
+#define XR_CHECK_FORMAT(condition, fmtstr, ...) \
+  (condition ? 0 : ((vrs::logging::logAndAbort(#condition, fmt::format(fmtstr, ##__VA_ARGS__))), 0))
 
-#define XR_CHECK(condition, ...) XR_CHECK_FORMAT(condition, ##__VA_ARGS__, "")
+#define XR_CHECK(condition, ...) XR_CHECK_FORMAT(condition, "" __VA_ARGS__)
 
 #define XR_CHECK_EQ(val1, val2, ...) XR_CHECK((val1) == (val2), ##__VA_ARGS__)
 

diff --git a/vrs/oss/logging/Verify.h b/vrs/oss/logging/Verify.h
@@ -19,10 +19,6 @@
 #include <fmt/color.h>
 #include <fmt/core.h>
 
-#include <vrs/os/CompilerAttributes.h>
-
-#include "LogLevel.h"
-
 #ifndef DEFAULT_LOG_CHANNEL
 #error "DEFAULT_LOG_CHANNEL must be defined before including <logging/Verify.h>"
 #endif // DEFAULT_LOG_CHANNEL
@@ -33,14 +29,15 @@
 // condition.
 //
 
-#define XR_VERIFY_C(channel_, condition_, ...)  \
-  (condition_ ? 1                               \
-              : (fmt::print(                    \
-                     stderr,                    \
-                     fg(fmt::color::red),       \
-                     "Verify {} failed: {}",    \
-                     #condition_,               \
-                     fmt::format(__VA_ARGS__)), \
+#define XR_VERIFY_C(channel_, condition_, fmtstr, ...)      \
+  (condition_ ? 1                                           \
+              : (fmt::print(                                \
+                     stderr,                                \
+                     fg(fmt::color::red),                   \
+                     "{}: Verify '{}' failed. {}\x1b[0m\n", \
+                     channel_,                              \
+                     #condition_,                           \
+                     fmt::format(fmtstr, ##__VA_ARGS__)),   \
                  0))
 
-#define XR_VERIFY(cond, ...) XR_VERIFY_C(DEFAULT_LOG_CHANNEL, cond, ##__VA_ARGS__, "")
+#define XR_VERIFY(cond, ...) XR_VERIFY_C(DEFAULT_LOG_CHANNEL, cond, "" __VA_ARGS__)