pw_log: Explicitly pass verbosity to PW_LOG

Updates the PW_LOG contract to ensure that PW_LOG's default
implementation does not rely on any global preprocessor defines,
instead it ensures that all arguments are explicitly passed. This
is done by passing the verbosity explicitly as an argument by
selecting them in the PW_LOG_* macros instead and updating the
PW_LOG_ENABLE_IF to also take in verbosity.

Lastly PW_LOG_ENABLE_IF_DEFAULT is removed and PW_LOG_ENABLE_IF is
changed to be a module configuration option, meaning that there
should be a single definition for a toolchain (this is not permitted
to vary between compile units).

Change-Id: Idef3483e1aa173295f85b18f4971b2891abb9b25
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/239035
Reviewed-by: Wyatt Hepler <[email protected]>
Reviewed-by: Ben Lawson <[email protected]>
Reviewed-by: Keir Mierle <[email protected]>
Lint: Lint 🤖 <[email protected]>

pw_bluetooth_sapphire/public/pw_bluetooth_sapphire/internal/host/common/log.h

@@ -139,6 +139,7 @@ constexpr void CheckFormat([[maybe_unused]] const char* fmt, ...) {}
 // specify any additional args.
 #define bt_log(level, tag, /*fmt*/...)             \
   PW_LOG(static_cast<int>(bt::LogSeverity::level), \
+         PW_LOG_LEVEL,                             \
          tag,                                      \
          GetPwLogFlags(bt::LogSeverity::level),    \
          __VA_ARGS__);                             \

pw_chre/chre_api_re.cc

@@ -44,5 +44,6 @@ DLL_EXPORT void chreLog(enum chreLogLevel level,
   PW_ASSERT(status.ok());
   va_end(args);
 
-  PW_LOG(ToPigweedLogLevel(level), "CHRE", PW_LOG_FLAGS, "%s", log);
+  PW_LOG(
+      ToPigweedLogLevel(level), PW_LOG_LEVEL, "CHRE", PW_LOG_FLAGS, "%s", log);
 }

pw_hex_dump/public/pw_hex_dump/log_bytes.h

@@ -96,7 +96,12 @@ inline void LogBytes(int log_level, pw::ConstByteSpan bytes) {
   pw::dump::FormattedHexDumper hex_dumper(temp, flags);
   if (hex_dumper.BeginDump(bytes).ok()) {
     while (hex_dumper.DumpLine().ok()) {
-      PW_LOG(log_level, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "%s", temp.data());
+      PW_LOG(log_level,
+             PW_LOG_LEVEL,
+             PW_LOG_MODULE_NAME,
+             PW_LOG_FLAGS,
+             "%s",
+             temp.data());
     }
   }
 }

pw_log/basic_log_test.cc

@@ -75,42 +75,56 @@ TEST(BasicLog, CriticalLevel) {
 
 TEST(BasicLog, ManualLevel) {
   PW_LOG(PW_LOG_LEVEL_DEBUG,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          0,
          "A manual DEBUG-level message");
   PW_LOG(PW_LOG_LEVEL_DEBUG,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          1,
          "A manual DEBUG-level message; with a flag");
 
-  PW_LOG(
-      PW_LOG_LEVEL_INFO, PW_LOG_MODULE_NAME, 0, "A manual INFO-level message");
   PW_LOG(PW_LOG_LEVEL_INFO,
+         PW_LOG_LEVEL,
+         PW_LOG_MODULE_NAME,
+         0,
+         "A manual INFO-level message");
+  PW_LOG(PW_LOG_LEVEL_INFO,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          1,
          "A manual INFO-level message; with a flag");
 
-  PW_LOG(
-      PW_LOG_LEVEL_WARN, PW_LOG_MODULE_NAME, 0, "A manual WARN-level message");
   PW_LOG(PW_LOG_LEVEL_WARN,
+         PW_LOG_LEVEL,
+         PW_LOG_MODULE_NAME,
+         0,
+         "A manual WARN-level message");
+  PW_LOG(PW_LOG_LEVEL_WARN,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          1,
          "A manual WARN-level message; with a flag");
 
   PW_LOG(PW_LOG_LEVEL_ERROR,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          0,
          "A manual ERROR-level message");
   PW_LOG(PW_LOG_LEVEL_ERROR,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          1,
          "A manual ERROR-level message; with a flag");
 
   PW_LOG(PW_LOG_LEVEL_CRITICAL,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          0,
          "A manual CRITICAL-level message");
   PW_LOG(PW_LOG_LEVEL_CRITICAL,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          1,
          "A manual CRITICAL-level message; with a flag");
@@ -121,11 +135,11 @@ TEST(BasicLog, FromAFunction) { LoggingFromFunction(); }
 TEST(BasicLog, CustomLogLevels) {
   // Log levels other than the standard ones work; what each backend does is
   // implementation defined.
-  PW_LOG(0, "", 0, "Custom log level: 0");
-  PW_LOG(1, "", 0, "Custom log level: 1");
-  PW_LOG(2, "", 0, "Custom log level: 2");
-  PW_LOG(3, "", 0, "Custom log level: 3");
-  PW_LOG(100, "", 0, "Custom log level: 100");
+  PW_LOG(0, PW_LOG_LEVEL, "", 0, "Custom log level: 0");
+  PW_LOG(1, PW_LOG_LEVEL, "", 0, "Custom log level: 1");
+  PW_LOG(2, PW_LOG_LEVEL, "", 0, "Custom log level: 2");
+  PW_LOG(3, PW_LOG_LEVEL, "", 0, "Custom log level: 3");
+  PW_LOG(100, PW_LOG_LEVEL, "", 0, "Custom log level: 100");
 }
 
 #define TEST_FAILED_LOG "IF THIS MESSAGE WAS LOGGED, THE TEST FAILED"
@@ -149,15 +163,19 @@ TEST(BasicLog, FilteringByFlags) {
 #define PW_LOG_SKIP_LOGS_WITH_FLAGS 1
 
   // Flag is set so these should all get zapped.
-  PW_LOG(PW_LOG_LEVEL_INFO, PW_LOG_MODULE_NAME, 1, TEST_FAILED_LOG);
-  PW_LOG(PW_LOG_LEVEL_ERROR, PW_LOG_MODULE_NAME, 1, TEST_FAILED_LOG);
+  PW_LOG(
+      PW_LOG_LEVEL_INFO, PW_LOG_LEVEL, PW_LOG_MODULE_NAME, 1, TEST_FAILED_LOG);
+  PW_LOG(
+      PW_LOG_LEVEL_ERROR, PW_LOG_LEVEL, PW_LOG_MODULE_NAME, 1, TEST_FAILED_LOG);
 
   // However, a different flag bit should still log.
   PW_LOG(PW_LOG_LEVEL_INFO,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          1 << 1,
          "This flagged log is intended to appear");
   PW_LOG(PW_LOG_LEVEL_ERROR,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          1 << 1,
          "This flagged log is intended to appear");
@@ -174,7 +192,7 @@ TEST(BasicLog, ChangingTheModuleName) {
 }
 
 TEST(BasicLog, ShortNames) {
-  LOG(PW_LOG_LEVEL_INFO, PW_LOG_MODULE_NAME, 0, "Shrt lg");
+  LOG(PW_LOG_LEVEL_INFO, PW_LOG_LEVEL, PW_LOG_MODULE_NAME, 0, "Shrt lg");
   LOG_DEBUG("A debug log: %d", 1);
   LOG_INFO("An info log: %d", 2);
   LOG_WARN("A warning log: %d", 3);
@@ -183,7 +201,7 @@ TEST(BasicLog, ShortNames) {
 }
 
 TEST(BasicLog, UltraShortNames) {
-  LOG(PW_LOG_LEVEL_INFO, PW_LOG_MODULE_NAME, 0, "Shrt lg");
+  LOG(PW_LOG_LEVEL_INFO, PW_LOG_LEVEL, PW_LOG_MODULE_NAME, 0, "Shrt lg");
   DBG("A debug log: %d", 1);
   INF("An info log: %d", 2);
   WRN("A warning log: %d", 3);
@@ -200,7 +218,7 @@ TEST(BasicLog, FromPlainC) { BasicLogTestPlainC(); }
 // functions tests fail to compile, because the arguments end up out-of-order.
 
 #undef PW_LOG
-#define PW_LOG(level, module, flags, message, ...)                       \
+#define PW_LOG(level, verbosity, module, flags, message, ...)            \
   DoNothingFakeFunction(module,                                          \
                         "%d/%d/%d: incoming transmission [" message "]", \
                         level,                                           \

pw_log/basic_log_test_plain_c.c

@@ -68,53 +68,75 @@ void BasicLogTestPlainC(void) {
 
   // Core log macro, with manually specified level and flags.
   PW_LOG(PW_LOG_LEVEL_DEBUG,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          0,
          "A manual DEBUG-level message");
   PW_LOG(PW_LOG_LEVEL_DEBUG,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          1,
          "A manual DEBUG-level message; with a flag");
 
-  PW_LOG(
-      PW_LOG_LEVEL_INFO, PW_LOG_MODULE_NAME, 0, "A manual INFO-level message");
   PW_LOG(PW_LOG_LEVEL_INFO,
+         PW_LOG_LEVEL,
+         PW_LOG_MODULE_NAME,
+         0,
+         "A manual INFO-level message");
+  PW_LOG(PW_LOG_LEVEL_INFO,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          1,
          "A manual INFO-level message; with a flag");
 
-  PW_LOG(
-      PW_LOG_LEVEL_WARN, PW_LOG_MODULE_NAME, 0, "A manual WARN-level message");
   PW_LOG(PW_LOG_LEVEL_WARN,
+         PW_LOG_LEVEL,
+         PW_LOG_MODULE_NAME,
+         0,
+         "A manual WARN-level message");
+  PW_LOG(PW_LOG_LEVEL_WARN,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          1,
          "A manual WARN-level message; with a flag");
 
   PW_LOG(PW_LOG_LEVEL_ERROR,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          0,
          "A manual ERROR-level message");
   PW_LOG(PW_LOG_LEVEL_ERROR,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          1,
          "A manual ERROR-level message; with a flag");
 
   PW_LOG(PW_LOG_LEVEL_CRITICAL,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          0,
          "A manual CRITICAL-level message");
   PW_LOG(PW_LOG_LEVEL_CRITICAL,
+         PW_LOG_LEVEL,
          PW_LOG_MODULE_NAME,
          1,
          "A manual CRITICAL-level message; with a flag");
 
   // Log levels other than the standard ones work; what each backend does is
   // implementation defined.
-  PW_LOG(0, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Custom log level: 0");
-  PW_LOG(1, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Custom log level: 1");
-  PW_LOG(2, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Custom log level: 2");
-  PW_LOG(3, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Custom log level: 3");
-  PW_LOG(100, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Custom log level: 100");
+  PW_LOG(
+      0, PW_LOG_LEVEL, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Custom log level: 0");
+  PW_LOG(
+      1, PW_LOG_LEVEL, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Custom log level: 1");
+  PW_LOG(
+      2, PW_LOG_LEVEL, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Custom log level: 2");
+  PW_LOG(
+      3, PW_LOG_LEVEL, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Custom log level: 3");
+  PW_LOG(100,
+         PW_LOG_LEVEL,
+         PW_LOG_MODULE_NAME,
+         PW_LOG_FLAGS,
+         "Custom log level: 100");
 
   // Logging from a function.
   LoggingFromFunctionPlainC();
@@ -129,7 +151,7 @@ void BasicLogTestPlainC(void) {
 }
 
 #undef PW_LOG
-#define PW_LOG(level, module, flags, message, ...)                       \
+#define PW_LOG(level, verbosity, module, flags, message, ...)            \
   DoNothingFakeFunction(module,                                          \
                         "%d/%d/%d: incoming transmission [" message "]", \
                         level,                                           \

pw_log/docs.rst

@@ -85,14 +85,16 @@ Logging macros
 These are the primary macros for logging information about the functioning of a
 system, intended to be used directly.
 
-.. c:macro:: PW_LOG(level, module, flags, fmt, ...)
+.. c:macro:: PW_LOG(level, verbosity, module, flags, fmt, ...)
 
    This is the primary mechanism for logging.
 
-   *level* - An integer level as defined by ``pw_log/levels.h``.
+   *level* - An integer level as defined by ``pw_log/levels.h`` for this log.
+
+   *verbosity* - An integer level as defined by ``pw_log/levels.h`` which is the
+   minimum level which is enabled.
 
-   *module* - A string literal for the module name. Defaults to
-   :c:macro:`PW_LOG_MODULE_NAME`.
+   *module* - A string literal for the module name.
 
    *flags* - Arbitrary flags the backend can leverage. The semantics of these
    flags are not defined in the facade, but are instead meant as a general
@@ -116,8 +118,8 @@ system, intended to be used directly.
 
    .. code-block:: cpp
 
-      PW_LOG(PW_LOG_LEVEL_INFO, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Temp is %d degrees", temp);
-      PW_LOG(PW_LOG_LEVEL_ERROR, PW_LOG_MODULE_NAME, UNRELIABLE_DELIVERY, "It didn't work!");
+      PW_LOG(PW_LOG_LEVEL_INFO, PW_LOG_LEVEL_DEBUG, PW_LOG_MODULE_NAME, PW_LOG_FLAGS, "Temp is %d degrees", temp);
+      PW_LOG(PW_LOG_LEVEL_ERROR, PW_LOG_LEVEL_DEBUG, PW_LOG_MODULE_NAME, UNRELIABLE_DELIVERY, "It didn't work!");
 
    .. note::
 
@@ -208,12 +210,18 @@ more details.
    ``PW_LOG_FLAGS_DEFAULT`` will change the behavior of all source files that
    have not explicitly set ``PW_LOG_FLAGS``. Defaults to ``0``.
 
-.. c:macro:: PW_LOG_ENABLE_IF_DEFAULT
+.. c:macro:: PW_LOG_ENABLE_IF(level, verbosity, flags)
 
-   Controls the default value of ``PW_LOG_ENABLE_IF``. Setting
-   ``PW_LOG_ENABLE_IF_DEFAULT`` will change the behavior of all source files that
-   have not explicitly set ``PW_LOG_ENABLE_IF``. Defaults to
-   ``((level) >= PW_LOG_LEVEL)``.
+   Filters logs by an arbitrary expression based on ``level``, ``verbosity``,
+   and ``flags``. Source files that define
+   ``PW_LOG_ENABLE_IF(level, verbosity, flags)`` will display if the given
+   expression evaluates true. Defaults to
+   ``((int32_t)(level) >= (int32_t)(verbosity))``.
+
+.. attention::
+
+   At this time, only compile time filtering is supported. In the future, we
+   plan to add support for runtime filtering.
 
 
 Per-source file configuration
@@ -278,49 +286,6 @@ source files, not headers. For example:
         PW_LOG_WARN("This is above INFO level, and will display");
       }
 
-.. c:macro:: PW_LOG_ENABLE_IF(level, flags)
-
-   Filters logs by an arbitrary expression based on ``level`` and ``flags``.
-   Source files that define ``PW_LOG_ENABLE_IF(level, flags)`` will display if
-   the given expression evaluates true. Defaults to
-   ``PW_LOG_ENABLE_IF_DEFAULT``.
-
-   Example:
-
-   .. code-block:: cpp
-
-      // Pigweed's log facade will call this macro to decide to log or not. In
-      // this case, it will drop logs with the PII flag set if display of PII is
-      // not enabled for the application.
-      #define PW_LOG_ENABLE_IF(level, flags) \
-          (level >= PW_LOG_LEVEL_INFO && \
-           !((flags & MY_PRODUCT_PII_MASK) && MY_PRODUCT_LOG_PII_ENABLED)
-
-      #include "pw_log/log.h"
-
-      // This define might be supplied by the build system.
-      #define MY_PRODUCT_LOG_PII_ENABLED false
-
-      // This is the PII mask bit selected by the application.
-      #define MY_PRODUCT_PII_MASK (1 << 5)
-
-      void DoSomethingWithSensitiveInfo() {
-        PW_LOG_DEBUG("This won't be logged at all");
-        PW_LOG_INFO("This is INFO level, and will display");
-
-        // In this example, this will not be logged since logging with PII
-        // is disabled by the above macros.
-        PW_LOG(PW_LOG_LEVEL_INFO,
-               MY_PRODUCT_PII_MASK,
-               "Sensitive: %d",
-               sensitive_info);
-      }
-
-.. attention::
-
-   At this time, only compile time filtering is supported. In the future, we
-   plan to add support for runtime filtering.
-
 .. _module-pw_log-logging_attributes:
 
 ------------------

pw_log/public/pw_log/config.h

@@ -34,11 +34,11 @@
 #define PW_LOG_FLAGS_DEFAULT 0
 #endif  // PW_LOG_FLAGS_DEFAULT
 
-// PW_LOG_ENABLE_IF_DEFAULT controls the default value of PW_LOG_ENABLE_IF.
+// PW_LOG_ENABLE_IF controls what logs are enabled
 //
 // This expression determines whether or not the statement is enabled and
 // should be passed to the backend.
-#ifndef PW_LOG_ENABLE_IF_DEFAULT
-#define PW_LOG_ENABLE_IF_DEFAULT(level, module, flags) \
-  ((int32_t)(level) >= (int32_t)(PW_LOG_LEVEL))
-#endif  // PW_LOG_ENABLE_IF_DEFAULT
+#ifndef PW_LOG_ENABLE_IF
+#define PW_LOG_ENABLE_IF(level, verbosity, module, flags) \
+  ((int32_t)(level) >= (int32_t)(verbosity))
+#endif  // PW_LOG_ENABLE_IF