Avoid Abort in release build, part 1 (#3631)

Turn "unreachable" to Status; reduce number places where it is used; remove "count" member in enum-classes.
Drive-by: straighten includes, use more forwarding

(cherry picked from commit d947eb5f98f04a403bc5e96c1f4c71e6a1b3cca4)

ci.sh

@@ -482,7 +482,7 @@ cmd_release() {
 
 cmd_opt() {
   CMAKE_BUILD_TYPE="RelWithDebInfo"
-  CMAKE_CXX_FLAGS+=" -DJXL_DEBUG_WARNING -DJXL_DEBUG_ON_ERROR"
+  CMAKE_CXX_FLAGS+=" -DJXL_DEBUG_BUILD -DJXL_DEBUG_ON_ERROR"
   cmake_configure "$@"
   cmake_build_and_test
 }

lib/base/compiler_specific.h

@@ -62,14 +62,6 @@
 #define JXL_NORETURN
 #endif
 
-#if JXL_COMPILER_MSVC
-#define JXL_UNREACHABLE_BUILTIN __assume(false)
-#elif JXL_COMPILER_CLANG || JXL_COMPILER_GCC >= 405
-#define JXL_UNREACHABLE_BUILTIN __builtin_unreachable()
-#else
-#define JXL_UNREACHABLE_BUILTIN
-#endif
-
 #if JXL_COMPILER_MSVC
 #define JXL_MAYBE_UNUSED
 #else

lib/base/status.h

@@ -9,11 +9,10 @@
 
 // Error handling: Status return type + helper macros.
 
-#include <stdarg.h>
-#include <stdint.h>
-#include <stdio.h>
-#include <stdlib.h>
-
+#include <cstdarg>
+#include <cstdint>
+#include <cstdio>
+#include <cstdlib>
 #include <type_traits>
 #include <utility>
 
@@ -136,20 +135,8 @@ inline JXL_NOINLINE bool Debug(const char* format, ...) {
 #define JXL_DEBUG_V(level, format, ...)
 #endif
 
-// Warnings (via JXL_WARNING) are enabled by default in debug builds (opt and
-// debug).
-#ifdef JXL_DEBUG_WARNING
-#undef JXL_DEBUG_WARNING
-#define JXL_DEBUG_WARNING 1
-#else  // JXL_DEBUG_WARNING
-#ifdef NDEBUG
-#define JXL_DEBUG_WARNING 0
-#else  // JXL_DEBUG_WARNING
-#define JXL_DEBUG_WARNING 1
-#endif  // NDEBUG
-#endif  // JXL_DEBUG_WARNING
 #define JXL_WARNING(format, ...) \
-  JXL_DEBUG(JXL_DEBUG_WARNING, format, ##__VA_ARGS__)
+  JXL_DEBUG(JXL_DEBUG_BUILD, format, ##__VA_ARGS__)
 
 // Exits the program after printing a stack trace when possible.
 JXL_NORETURN inline JXL_NOINLINE bool Abort() {
@@ -174,20 +161,25 @@ JXL_NORETURN inline JXL_NOINLINE bool Abort() {
                                         __FILE__, __LINE__, ##__VA_ARGS__), \
    ::jxl::Abort())
 
+#if JXL_DEBUG_BUILD
+#define JXL_DEBUG_ABORT(format, ...) JXL_ABORT(format, ##__VA_ARGS__)
+#else
+#define JXL_DEBUG_ABORT(format, ...)
+#endif
+
 // Use this for code paths that are unreachable unless the code would change
 // to make it reachable, in which case it will print a warning and abort in
 // debug builds. In release builds no code is produced for this, so only use
 // this if this path is really unreachable.
-#define JXL_UNREACHABLE(format, ...)                                   \
-  do {                                                                 \
-    if (JXL_DEBUG_WARNING) {                                           \
-      ::jxl::Debug(("%s:%d: JXL_UNREACHABLE: " format "\n"), __FILE__, \
-                   __LINE__, ##__VA_ARGS__);                           \
-      ::jxl::Abort();                                                  \
-    } else {                                                           \
-      JXL_UNREACHABLE_BUILTIN;                                         \
-    }                                                                  \
-  } while (0)
+#if JXL_DEBUG_BUILD
+#define JXL_UNREACHABLE(format, ...)                                          \
+  (::jxl::Debug(("%s:%d: JXL_UNREACHABLE: " format "\n"), __FILE__, __LINE__, \
+                ##__VA_ARGS__),                                               \
+   ::jxl::Abort(), JXL_FAILURE(format, ##__VA_ARGS__))
+#else  // JXL_DEBUG_BUILD
+#define JXL_UNREACHABLE(format, ...) \
+  JXL_FAILURE("internal: " format, ##__VA_ARGS__)
+#endif
 
 // Does not guarantee running the code, use only for debug mode checks.
 #if JXL_ENABLE_ASSERT

lib/cms/color_encoding_cms.h

@@ -353,13 +353,13 @@ struct ColorEncoding {
   PrimariesCIExy GetPrimaries() const {
     JXL_DASSERT(have_fields);
     JXL_ASSERT(HasPrimaries());
-    PrimariesCIExy xy;
+    PrimariesCIExy xy{};
     switch (primaries) {
       case Primaries::kCustom:
         xy.r = red.GetValue();
         xy.g = green.GetValue();
         xy.b = blue.GetValue();
-        return xy;
+        break;
 
       case Primaries::kSRGB:
         xy.r.x = 0.639998686;
@@ -368,7 +368,7 @@ struct ColorEncoding {
         xy.g.y = 0.600003357;
         xy.b.x = 0.150002046;
         xy.b.y = 0.059997204;
-        return xy;
+        break;
 
       case Primaries::k2100:
         xy.r.x = 0.708;
@@ -377,7 +377,7 @@ struct ColorEncoding {
         xy.g.y = 0.797;
         xy.b.x = 0.131;
         xy.b.y = 0.046;
-        return xy;
+        break;
 
       case Primaries::kP3:
         xy.r.x = 0.680;
@@ -386,9 +386,13 @@ struct ColorEncoding {
         xy.g.y = 0.690;
         xy.b.x = 0.150;
         xy.b.y = 0.060;
-        return xy;
+        break;
+
+      default:
+        JXL_DEBUG_ABORT("internal: unexpected Primaries: %d",
+                        static_cast<int>(primaries));
     }
-    JXL_UNREACHABLE("Invalid Primaries %u", static_cast<uint32_t>(primaries));
+    return xy;
   }
 
   Status SetPrimaries(const PrimariesCIExy& xy) {
@@ -429,28 +433,32 @@ struct ColorEncoding {
 
   CIExy GetWhitePoint() const {
     JXL_DASSERT(have_fields);
-    CIExy xy;
+    CIExy xy{};
     switch (white_point) {
       case WhitePoint::kCustom:
-        return white.GetValue();
+        xy = white.GetValue();
+        break;
 
       case WhitePoint::kD65:
         xy.x = 0.3127;
         xy.y = 0.3290;
-        return xy;
+        break;
 
       case WhitePoint::kDCI:
         // From https://ieeexplore.ieee.org/document/7290729 C.2 page 11
         xy.x = 0.314;
         xy.y = 0.351;
-        return xy;
+        break;
 
       case WhitePoint::kE:
         xy.x = xy.y = 1.0 / 3;
-        return xy;
+        break;
+
+      default:
+        JXL_DEBUG_ABORT("internal: unexpected WhitePoint: %d",
+                        static_cast<int>(white_point));
     }
-    JXL_UNREACHABLE("Invalid WhitePoint %u",
-                    static_cast<uint32_t>(white_point));
+    return xy;
   }
 
   Status SetWhitePoint(const CIExy& xy) {

lib/cms/jxl_cms.cc

@@ -36,6 +36,7 @@
 #else  // JPEGXL_ENABLE_SKCMS
 #include "lcms2.h"
 #include "lcms2_plugin.h"
+#include "lib/jxl/base/span.h"
 #endif  // JPEGXL_ENABLE_SKCMS
 
 #define JXL_CMS_VERBOSE 0

lib/cms/jxl_cms_internal.h

@@ -773,9 +773,12 @@ static std::string ToString(JxlColorSpace color_space) {
       return "XYB";
     case JXL_COLOR_SPACE_UNKNOWN:
       return "CS?";
+    default:
+      // Should not happen - visitor fails if enum is invalid.
+      JXL_DEBUG_ABORT("Invalid ColorSpace %u",
+                      static_cast<uint32_t>(color_space));
+      return "Invalid";
   }
-  // Should not happen - visitor fails if enum is invalid.
-  JXL_UNREACHABLE("Invalid ColorSpace %u", static_cast<uint32_t>(color_space));
 }
 
 static std::string ToString(JxlWhitePoint white_point) {
@@ -788,9 +791,12 @@ static std::string ToString(JxlWhitePoint white_point) {
       return "EER";
     case JXL_WHITE_POINT_DCI:
       return "DCI";
+    default:
+      // Should not happen - visitor fails if enum is invalid.
+      JXL_DEBUG_ABORT("Invalid WhitePoint %u",
+                      static_cast<uint32_t>(white_point));
+      return "Invalid";
   }
-  // Should not happen - visitor fails if enum is invalid.
-  JXL_UNREACHABLE("Invalid WhitePoint %u", static_cast<uint32_t>(white_point));
 }
 
 static std::string ToString(JxlPrimaries primaries) {
@@ -803,9 +809,11 @@ static std::string ToString(JxlPrimaries primaries) {
       return "DCI";
     case JXL_PRIMARIES_CUSTOM:
       return "Cst";
+    default:
+      // Should not happen - visitor fails if enum is invalid.
+      JXL_DEBUG_ABORT("Invalid Primaries %u", static_cast<uint32_t>(primaries));
+      return "Invalid";
   }
-  // Should not happen - visitor fails if enum is invalid.
-  JXL_UNREACHABLE("Invalid Primaries %u", static_cast<uint32_t>(primaries));
 }
 
 static std::string ToString(JxlTransferFunction transfer_function) {
@@ -825,11 +833,14 @@ static std::string ToString(JxlTransferFunction transfer_function) {
     case JXL_TRANSFER_FUNCTION_UNKNOWN:
       return "TF?";
     case JXL_TRANSFER_FUNCTION_GAMMA:
-      JXL_UNREACHABLE("Invalid TransferFunction: gamma");
+      JXL_DEBUG_ABORT("Invalid TransferFunction: gamma");
+      return "Invalid";
+    default:
+      // Should not happen - visitor fails if enum is invalid.
+      JXL_DEBUG_ABORT("Invalid TransferFunction %u",
+                      static_cast<uint32_t>(transfer_function));
+      return "Invalid";
   }
-  // Should not happen - visitor fails if enum is invalid.
-  JXL_UNREACHABLE("Invalid TransferFunction %u",
-                  static_cast<uint32_t>(transfer_function));
 }
 
 static std::string ToString(JxlRenderingIntent rendering_intent) {
@@ -844,8 +855,9 @@ static std::string ToString(JxlRenderingIntent rendering_intent) {
       return "Abs";
   }
   // Should not happen - visitor fails if enum is invalid.
-  JXL_UNREACHABLE("Invalid RenderingIntent %u",
+  JXL_DEBUG_ABORT("Invalid RenderingIntent %u",
                   static_cast<uint32_t>(rendering_intent));
+  return "Invalid";
 }
 
 static std::string ColorEncodingDescriptionImpl(const JxlColorEncoding& c) {
@@ -1050,7 +1062,8 @@ static Status MaybeCreateProfileImpl(const JxlColorEncoding& c,
               CreateICCCurvParaTag({2.6, 1.0, 0.0, 1.0, 0.0}, 3, &tags));
           break;
         default:
-          JXL_UNREACHABLE("Unknown TF %u", static_cast<unsigned int>(tf));
+          return JXL_UNREACHABLE("unknown TF %u",
+                                 static_cast<unsigned int>(tf));
       }
     }
     FinalizeICCTag(&tags, &tag_offset, &tag_size);

lib/extras/butteraugli.cc

@@ -45,6 +45,10 @@
 #include "lib/extras/convolve.h"
 #include "lib/extras/image_ops.h"
 
+#if BUTTERAUGLI_ENABLE_CHECKS
+#include "lib/jxl/base/printf_macros.h"
+#endif
+
 #ifndef JXL_BUTTERAUGLI_ONCE
 #define JXL_BUTTERAUGLI_ONCE
 
@@ -100,9 +104,9 @@ void ConvolveBorderColumn(const ImageF& in, const std::vector<float>& kernel,
 }
 
 // Computes a horizontal convolution and transposes the result.
-void ConvolutionWithTranspose(const ImageF& in,
-                              const std::vector<float>& kernel,
-                              ImageF* BUTTERAUGLI_RESTRICT out) {
+Status ConvolutionWithTranspose(const ImageF& in,
+                                const std::vector<float>& kernel,
+                                ImageF* BUTTERAUGLI_RESTRICT out) {
   JXL_CHECK(out->xsize() == in.ysize());
   JXL_CHECK(out->ysize() == in.xsize());
   const size_t len = kernel.size();
@@ -201,7 +205,8 @@ void ConvolutionWithTranspose(const ImageF& in,
       break;
     }
     default:
-      JXL_UNREACHABLE("Kernel size %" PRIuS " not implemented", len);
+      return JXL_UNREACHABLE("kernel size %d not implemented",
+                             static_cast<int>(len));
   }
   // left border
   for (size_t x = 0; x < border1; ++x) {
@@ -212,6 +217,7 @@ void ConvolutionWithTranspose(const ImageF& in,
   for (size_t x = border2; x < in.xsize(); ++x) {
     ConvolveBorderColumn(in, kernel, x, out->Row(x));
   }
+  return true;
 }
 
 // A blur somewhat similar to a 2D Gaussian blur.
@@ -248,8 +254,8 @@ Status Blur(const ImageF& in, float sigma, const ButteraugliParams& params,
 
   ImageF* temp_t;
   JXL_RETURN_IF_ERROR(temp->GetTransposed(in, &temp_t));
-  ConvolutionWithTranspose(in, kernel, temp_t);
-  ConvolutionWithTranspose(*temp_t, kernel, out);
+  JXL_RETURN_IF_ERROR(ConvolutionWithTranspose(in, kernel, temp_t));
+  JXL_RETURN_IF_ERROR(ConvolutionWithTranspose(*temp_t, kernel, out));
   return true;
 }
 

lib/extras/dec/jpegli.cc

@@ -115,12 +115,12 @@ void MyErrorExit(j_common_ptr cinfo) {
 }
 
 void MyOutputMessage(j_common_ptr cinfo) {
-#if JXL_DEBUG_WARNING == 1
-  char buf[JMSG_LENGTH_MAX + 1];
-  (*cinfo->err->format_message)(cinfo, buf);
-  buf[JMSG_LENGTH_MAX] = 0;
-  JXL_WARNING("%s", buf);
-#endif
+  if (JXL_DEBUG_BUILD) {
+    char buf[JMSG_LENGTH_MAX + 1];
+    (*cinfo->err->format_message)(cinfo, buf);
+    buf[JMSG_LENGTH_MAX] = 0;
+    JXL_WARNING("%s", buf);
+  }
 }
 
 void UnmapColors(uint8_t* row, size_t xsize, int components,

lib/extras/dec/jpg.cc

@@ -156,12 +156,12 @@ void MyErrorExit(j_common_ptr cinfo) {
 }
 
 void MyOutputMessage(j_common_ptr cinfo) {
-#if JXL_DEBUG_WARNING == 1
-  char buf[JMSG_LENGTH_MAX + 1];
-  (*cinfo->err->format_message)(cinfo, buf);
-  buf[JMSG_LENGTH_MAX] = 0;
-  JXL_WARNING("%s", buf);
-#endif
+  if (JXL_DEBUG_BUILD) {
+    char buf[JMSG_LENGTH_MAX + 1];
+    (*cinfo->err->format_message)(cinfo, buf);
+    buf[JMSG_LENGTH_MAX] = 0;
+    JXL_WARNING("%s", buf);
+  }
 }
 
 void UnmapColors(uint8_t* row, size_t xsize, int components,