fix: make log severity type safe (#1025)

CMakeLists.txt

@@ -793,6 +793,69 @@ if (BUILD_TESTING)
     striplog10
     PROPERTIES WILL_FAIL ON
   )
+
+  set (_glog_TEST_ENVIRONMENT
+    glog_ROOT=${glog_BINARY_DIR}
+  )
+
+  add_test (NAME log_severity_constants COMMAND ${CMAKE_CTEST_COMMAND}
+    --build-config $<CONFIG>
+    --build-and-test
+    "${glog_SOURCE_DIR}/src/log_severity_unittest"
+    "${glog_BINARY_DIR}/Tests/log_severity_constants"
+    --build-generator ${CMAKE_GENERATOR}
+    --build-makeprogram ${CMAKE_MAKE_PROGRAM}
+    --build-target glog_log_severity_constants
+    --build-options
+    -DCMAKE_BUILD_TYPE=$<CONFIG>
+    -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}
+    -DCMAKE_GENERATOR_PLATFORM=${CMAKE_GENERATOR_PLATFORM}
+    -DCMAKE_GENERATOR_TOOLSET=${CMAKE_GENERATOR_TOOLSET}
+  )
+  set_tests_properties (log_severity_constants PROPERTIES
+    ENVIRONMENT "${_glog_TEST_ENVIRONMENT}"
+    PASS_REGULAR_EXPRESSION "COMPACT_GOOGLE_LOG_[1-3]"
+  )
+
+  add_test (NAME log_severity_conversion COMMAND ${CMAKE_CTEST_COMMAND}
+    --build-config $<CONFIG>
+    --build-and-test
+    "${glog_SOURCE_DIR}/src/log_severity_unittest"
+    "${glog_BINARY_DIR}/Tests/log_severity_conversion"
+    --build-generator ${CMAKE_GENERATOR}
+    --build-makeprogram ${CMAKE_MAKE_PROGRAM}
+    --build-target glog_log_severity_conversion
+    --build-options
+    -DCMAKE_BUILD_TYPE=$<CONFIG>
+    -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}
+    -DCMAKE_GENERATOR_PLATFORM=${CMAKE_GENERATOR_PLATFORM}
+    -DCMAKE_GENERATOR_TOOLSET=${CMAKE_GENERATOR_TOOLSET}
+  )
+  set_tests_properties (log_severity_conversion PROPERTIES
+    ENVIRONMENT "${_glog_TEST_ENVIRONMENT}"
+  )
+
+  if (CMAKE_COMPILER_IS_GNUCXX)
+    set_tests_properties (log_severity_conversion PROPERTIES
+      PASS_REGULAR_EXPRESSION "error: invalid conversion from (‘|')int(’|')"
+    )
+  elseif (CMAKE_CXX_COMPILER_ID MATCHES Clang)
+    set_tests_properties (log_severity_conversion PROPERTIES
+      PASS_REGULAR_EXPRESSION "no known conversion from 'int'"
+    )
+  elseif (MSVC)
+    set_tests_properties (log_severity_conversion PROPERTIES
+      PASS_REGULAR_EXPRESSION "error C2440"
+    )
+  else (CMAKE_COMPILER_IS_GNUCXX)
+    message (AUTHOR_WARNING
+      "Unsuuported C++ compiler ${CMAKE_CXX_COMPILER_ID}: "
+      "log_severity_conversion test will be disabled"
+    )
+    set_tests_properties (log_severity_conversion DISABLED ON)
+  endif (CMAKE_COMPILER_IS_GNUCXX)
+
+  unset (_glog_TEST_ENVIRONMENT)
 endif (BUILD_TESTING)
 
 install (TARGETS glog

src/glog/log_severity.h

@@ -1,4 +1,4 @@
-// Copyright (c) 2023, Google Inc.
+// Copyright (c) 2024, Google Inc.
 // All rights reserved.
 //
 // Redistribution and use in source and binary forms, with or without
@@ -48,17 +48,31 @@
 // Variables of type LogSeverity are widely taken to lie in the range
 // [0, NUM_SEVERITIES-1].  Be careful to preserve this assumption if
 // you ever need to change their values or add a new severity.
-using LogSeverity = int;
 
-const int GLOG_INFO = 0, GLOG_WARNING = 1, GLOG_ERROR = 2, GLOG_FATAL = 3,
-          NUM_SEVERITIES = 4;
+enum LogSeverity {
+  GLOG_INFO = 0,
+  GLOG_WARNING = 1,
+  GLOG_ERROR = 2,
+  GLOG_FATAL = 3,
 #ifndef GLOG_NO_ABBREVIATED_SEVERITIES
 #  ifdef ERROR
 #  error ERROR macro is defined. Define GLOG_NO_ABBREVIATED_SEVERITIES before including logging.h. See the document for detail.
 #  endif
-const int INFO = GLOG_INFO, WARNING = GLOG_WARNING, ERROR = GLOG_ERROR,
-          FATAL = GLOG_FATAL;
+  INFO = GLOG_INFO,
+  WARNING = GLOG_WARNING,
+  ERROR = GLOG_ERROR,
+  FATAL = GLOG_FATAL
 #endif
+};
+
+#if defined(__cpp_inline_variables)
+#  if (__cpp_inline_variables >= 201606L)
+inline
+#  endif  // (__cpp_inline_variables >= 201606L)
+#endif    // defined(__cpp_inline_variables)
+          // clang-format off
+constexpr int NUM_SEVERITIES = 4;
+// clang-format on
 
 // DFATAL is FATAL in debug mode, ERROR in normal mode
 #ifdef NDEBUG

src/glog/logging.h

@@ -1526,7 +1526,7 @@ class GLOG_EXPORT LogMessageFatal : public LogMessage {
 
 // A non-macro interface to the log facility; (useful
 // when the logging level is not a compile-time constant).
-inline void LogAtLevel(int const severity, std::string const& msg) {
+inline void LogAtLevel(LogSeverity severity, std::string const& msg) {
   LogMessage(__FILE__, __LINE__, severity).stream() << msg;
 }
 

src/log_severity_unittest/CMakeLists.txt

@@ -0,0 +1,10 @@
+cmake_minimum_required (VERSION 3.16)
+project (glog_log_severity LANGUAGES CXX)
+
+find_package (glog REQUIRED NO_MODULE)
+
+add_executable (glog_log_severity_constants glog_log_severity_constants.cc)
+target_link_libraries (glog_log_severity_constants PRIVATE glog::glog)
+
+add_executable (glog_log_severity_conversion glog_log_severity_conversion.cc)
+target_link_libraries (glog_log_severity_conversion PRIVATE glog::glog)

src/log_severity_unittest/glog_log_severity_constants.cc

@@ -0,0 +1,40 @@
+// Copyright (c) 2024, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+//
+// Author: Sergiu Deitsch
+
+#include <glog/logging.h>
+
+int main() {
+  // Must not compile
+  LOG(0) << "type unsafe info";
+  LOG(1) << "type unsafe info";
+  LOG(2) << "type unsafe info";
+  LOG(3) << "type unsafe info";
+}

src/log_severity_unittest/glog_log_severity_conversion.cc

@@ -0,0 +1,42 @@
+// Copyright (c) 2024, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+//
+// Author: Sergiu Deitsch
+
+#include <glog/logging.h>
+
+int main() {
+  // Must not compile
+  google::LogMessage{__FILE__, __LINE__, -1};
+  // Cast to int to avoid implicit conversoin to nullptr
+  google::LogMessage{__FILE__, __LINE__, static_cast<int>(0)};
+  google::LogMessage{__FILE__, __LINE__, 1};
+  google::LogMessage{__FILE__, __LINE__, 2};
+  google::LogMessage{__FILE__, __LINE__, 3};
+}