Accurately annotate Error system with [[noreturn]]

python_bindings/src/halide/halide_/PyError.cpp

@@ -16,13 +16,10 @@ class HalidePythonCompileTimeErrorReporter : public CompileTimeErrorReporter {
         halide_python_print(nullptr, msg);
     }
 
-    void error(const char *msg) override {
+    [[noreturn]] void error(const char *msg) override {
         // This method is called *only* from the Compiler -- never from jitted
         // code -- so throwing an Error here is the right thing to do.
-
         throw Error(msg);
-
-        // This method must not return!
     }
 };
 

python_bindings/stub/PyStubImpl.cpp

@@ -37,9 +37,8 @@ class HalidePythonCompileTimeErrorReporter : public CompileTimeErrorReporter {
         py::print(msg, py::arg("end") = "");
     }
 
-    void error(const char *msg) override {
+    [[noreturn]] void error(const char *msg) override {
         throw Halide::Error(msg);
-        // This method must not return!
     }
 };
 

src/Error.cpp

@@ -106,7 +106,7 @@ namespace Internal {
 
 void unhandled_exception_handler() {
     // Note that we use __cpp_exceptions (rather than HALIDE_WITH_EXCEPTIONS)
-    // to maximize the change of dealing with uncaught exceptions in weird
+    // to maximize the chance of dealing with uncaught exceptions in weird
     // build situations (i.e., exceptions enabled via C++ but HALIDE_WITH_EXCEPTIONS
     // is somehow not set).
 #ifdef __cpp_exceptions
@@ -116,7 +116,7 @@ void unhandled_exception_handler() {
         try {
             std::rethrow_exception(ce);
         } catch (Error &e) {
-            // Halide Errors are presume to be nicely formatted as-is
+            // Halide Errors are presumed to be nicely formatted as-is
             std::cerr << e.what() << std::flush;
         } catch (std::exception &e) {
             // Arbitrary C++ exceptions... who knows?
@@ -139,81 +139,53 @@ namespace {
 CompileError _compile_error("");
 RuntimeError _runtime_error("");
 InternalError _internal_error("");
-}  // namespace
 
-ErrorReport::ErrorReport(const char *file, int line, const char *condition_string, int flags)
-    : flags(flags) {
-// Note that we deliberately try to put the entire message into a single line
-// (aside from newlines inserted by user code) to make it easy to filter
-// specific warnings or messages via (e.g.) grep.... unless we are likely to be
-// outputting to a proper terminal, in which case newlines are used to improve readability.
-#if defined(HALIDE_WITH_EXCEPTIONS)
-    const bool use_newlines = false;
+template<typename T>
+[[noreturn]] void throw_error_common(const T &e) {
+    if (custom_error_reporter) {
+        custom_error_reporter->error(e.what());
+        // error() should not have returned to us, but just in case
+        // it does, make sure we don't continue.
+        abort();
+    }
+
+#ifdef HALIDE_WITH_EXCEPTIONS
+    if (std::uncaught_exceptions() > 0) {
+        // This should never happen - evaluating one of the arguments to the
+        // error message would have to throw an exception. Nonetheless, in
+        // case it does, preserve the exception already in flight and suppress
+        // this one.
+        std::rethrow_exception(std::current_exception());
+    }
+    throw e;
 #else
-    const bool use_newlines = (custom_error_reporter == nullptr) && isatty(2);
+    std::cerr << e.what() << std::flush;
+    abort();
 #endif
-    const char sep = use_newlines ? '\n' : ' ';
-
-    const char *what = (flags & Warning) ? "Warning" : "Error";
-    if (flags & User) {
-        // Only mention where inside of libHalide the error tripped if we have debug level > 0
-        debug(1) << "User error triggered at " << file << ":" << line << "\n";
-        if (condition_string) {
-            debug(1) << "Condition failed: " << condition_string << "\n";
-        }
-        msg << what << ":";
-        msg << sep;
-    } else {
-        msg << "Internal " << what << " at " << file << ":" << line;
-        msg << sep;
-        if (condition_string) {
-            msg << "Condition failed: " << condition_string << ":" << sep;
-        }
-    }
 }
 
-ErrorReport::~ErrorReport() noexcept(false) {
-    if (!msg.str().empty() && msg.str().back() != '\n') {
-        msg << "\n";
-    }
+}  // namespace
 
-    if (custom_error_reporter != nullptr) {
-        if (flags & Warning) {
-            custom_error_reporter->warning(msg.str().c_str());
-            return;
-        } else {
-            custom_error_reporter->error(msg.str().c_str());
-            // error() should not have returned to us, but just in case
-            // it does, make sure we don't continue.
-            abort();
-        }
-    }
+void throw_error(const RuntimeError &e) {
+    throw_error_common(e);
+}
 
-    // TODO: Add an option to error out on warnings too
-    if (flags & Warning) {
-        std::cerr << msg.str();
-        return;
-    }
+void throw_error(const CompileError &e) {
+    throw_error_common(e);
+}
 
-#ifdef HALIDE_WITH_EXCEPTIONS
-    if (std::uncaught_exceptions() > 0) {
-        // This should never happen - evaluating one of the arguments
-        // to the error message would have to throw an
-        // exception. Nonetheless, in case it does, preserve the
-        // exception already in flight and suppress this one.
-        return;
-    } else if (flags & Runtime) {
-        throw RuntimeError(msg.str());
-    } else if (flags & User) {
-        throw CompileError(msg.str());
+void throw_error(const InternalError &e) {
+    throw_error_common(e);
+}
+
+void issue_warning(const char *warning) {
+    if (custom_error_reporter) {
+        custom_error_reporter->warning(warning);
     } else {
-        throw InternalError(msg.str());
+        std::cerr << warning;
     }
-#else
-    std::cerr << msg.str() << std::flush;
-    abort();
-#endif
 }
+
 }  // namespace Internal
 
 }  // namespace Halide

src/Error.h

@@ -46,22 +46,28 @@ struct HALIDE_EXPORT_SYMBOL Error {
 };
 
 /** An error that occurs while running a JIT-compiled Halide pipeline. */
-struct HALIDE_EXPORT_SYMBOL RuntimeError : public Error {
+struct HALIDE_EXPORT_SYMBOL RuntimeError final : Error {
+    static constexpr auto error_name = "Runtime error";
+
     explicit RuntimeError(const char *msg);
     explicit RuntimeError(const std::string &msg);
 };
 
 /** An error that occurs while compiling a Halide pipeline that Halide
  * attributes to a user error. */
-struct HALIDE_EXPORT_SYMBOL CompileError : public Error {
+struct HALIDE_EXPORT_SYMBOL CompileError final : Error {
+    static constexpr auto error_name = "User error";
+
     explicit CompileError(const char *msg);
     explicit CompileError(const std::string &msg);
 };
 
 /** An error that occurs while compiling a Halide pipeline that Halide
  * attributes to an internal compiler bug, or to an invalid use of
  * Halide's internals. */
-struct HALIDE_EXPORT_SYMBOL InternalError : public Error {
+struct HALIDE_EXPORT_SYMBOL InternalError final : Error {
+    static constexpr auto error_name = "Internal error";
+
     explicit InternalError(const char *msg);
     explicit InternalError(const std::string &msg);
 };
@@ -77,7 +83,7 @@ class CompileTimeErrorReporter {
 public:
     virtual ~CompileTimeErrorReporter() = default;
     virtual void warning(const char *msg) = 0;
-    virtual void error(const char *msg) = 0;
+    [[noreturn]] virtual void error(const char *msg) = 0;
 };
 
 /** The default error reporter logs to stderr, then throws an exception
@@ -91,27 +97,71 @@ void set_custom_compile_time_error_reporter(CompileTimeErrorReporter *error_repo
 
 namespace Internal {
 
-struct ErrorReport {
-    enum {
-        User = 0x0001,
-        Warning = 0x0002,
-        Runtime = 0x0004
-    };
+/**
+ * If a custom error reporter is configured, notifies the reporter by calling
+ * its error() function with the value of \p e.what()
+ *
+ * Otherwise, if Halide was built with exceptions, throw \p e unless an
+ * existing exception is in flight. On the other hand, if Halide was built
+ * without exceptions, print the error message to stderr and abort().
+ *
+ * @param e The error to throw or report
+ */
+/// @{
+[[noreturn]] void throw_error(const RuntimeError &e);
+[[noreturn]] void throw_error(const CompileError &e);
+[[noreturn]] void throw_error(const InternalError &e);
+/// @}
 
+/**
+ * If a custom error reporter is configured, notifies the reporter by calling
+ * its warning() function. Otherwise, prints the warning to stderr.
+ *
+ * @param warning The warning to issue
+ */
+void issue_warning(const char *warning);
+
+template<typename T>
+struct ReportBase {
     std::ostringstream msg;
-    const int flags;
 
-    ErrorReport(const char *f, int l, const char *cs, int flags);
+    ReportBase(const char *file, int line, const char *condition_string, const char *prefix) {
+        // Only mention where inside libHalide the error tripped if we have debug level > 0
+        if (debug::debug_level() > 0) {
+            msg << prefix << " at " << file << ":" << line << ' ';
+            if (condition_string) {
+                msg << "Condition failed: " << condition_string << ' ';
+            }
+        }
+    }
 
     // Just a trick used to convert RValue into LValue
-    HALIDE_ALWAYS_INLINE ErrorReport &ref() {
-        return *this;
+    HALIDE_ALWAYS_INLINE T &ref() {
+        return *static_cast<T *>(this);
     }
 
-    template<typename T>
-    ErrorReport &operator<<(const T &x) {
+    template<typename S>
+    HALIDE_ALWAYS_INLINE T &operator<<(const S &x) {
         msg << x;
-        return *this;
+        return *static_cast<T *>(this);
+    }
+
+protected:
+    std::string finalize_message() {
+        if (!msg.str().empty() && msg.str().back() != '\n') {
+            msg << "\n";
+        }
+        return msg.str();
+    }
+};
+
+template<typename Exception>
+struct ErrorReport : ReportBase<ErrorReport<Exception>> {
+    using Base = ReportBase<ErrorReport>;
+
+    ErrorReport(const char *file, int line, const char *condition_string)
+        : Base(file, line, condition_string, Exception::error_name) {
+        this->msg << "Error: ";
     }
 
     /** When you're done using << on the object, and let it fall out of
@@ -122,19 +172,34 @@ struct ErrorReport {
      * this by only actually throwing if there isn't an exception in
      * flight already.
      */
-    ~ErrorReport() noexcept(false);
+    [[noreturn]] ~ErrorReport() noexcept(false) {
+        throw_error(Exception(this->finalize_message()));
+    }
+};
+
+struct WarningReport : ReportBase<WarningReport> {
+    WarningReport(const char *file, int line, const char *condition_string)
+        : ReportBase(file, line, condition_string, "Warning") {
+        this->msg << "Warning: ";
+    }
+
+    /** When you're done using << on the object, and let it fall out of
+     * scope, this prints the computed warning message.
+     */
+    ~WarningReport() {
+        issue_warning(this->finalize_message().c_str());
+    }
 };
 
 // This uses operator precedence as a trick to avoid argument evaluation if
 // an assertion is true: it is intended to be used as part of the
 // _halide_internal_assertion macro, to coerce the result of the stream
 // expression to void (to match the condition-is-false case).
-class Voidifier {
-public:
-    HALIDE_ALWAYS_INLINE Voidifier() = default;
+struct Voidifier {
     // This has to be an operator with a precedence lower than << but
     // higher than ?:
-    HALIDE_ALWAYS_INLINE void operator&(ErrorReport &) {
+    template<typename T>
+    HALIDE_ALWAYS_INLINE void operator&(T &) {
     }
 };
 
@@ -152,23 +217,24 @@ class Voidifier {
  * effect that all assertion parameters are totally skipped (not ever evaluated)
  * when the assertion is true.
  */
-#define _halide_internal_assertion(condition, flags) \
+#define _halide_internal_assertion(condition, type)  \
     /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \
-    (condition) ? (void)0 : ::Halide::Internal::Voidifier() & ::Halide::Internal::ErrorReport(__FILE__, __LINE__, #condition, flags).ref()
+    (condition) ? (void)0 : ::Halide::Internal::Voidifier() & ::Halide::Internal::ErrorReport<type>(__FILE__, __LINE__, #condition).ref()
 
-#define internal_error Halide::Internal::ErrorReport(__FILE__, __LINE__, nullptr, 0)
-#define user_error Halide::Internal::ErrorReport(__FILE__, __LINE__, nullptr, Halide::Internal::ErrorReport::User)
-#define user_warning Halide::Internal::ErrorReport(__FILE__, __LINE__, nullptr, Halide::Internal::ErrorReport::User | Halide::Internal::ErrorReport::Warning)
-#define halide_runtime_error Halide::Internal::ErrorReport(__FILE__, __LINE__, nullptr, Halide::Internal::ErrorReport::User | Halide::Internal::ErrorReport::Runtime)
+#define internal_error Halide::Internal::ErrorReport<Halide::InternalError>(__FILE__, __LINE__, nullptr)
+#define user_error Halide::Internal::ErrorReport<Halide::CompileError>(__FILE__, __LINE__, nullptr)
+#define user_warning Halide::Internal::WarningReport(__FILE__, __LINE__, nullptr)
+#define halide_runtime_error Halide::Internal::ErrorReport<Halide::RuntimeError>(__FILE__, __LINE__, nullptr)
 
-#define internal_assert(c) _halide_internal_assertion(c, 0)
-#define user_assert(c) _halide_internal_assertion(c, Halide::Internal::ErrorReport::User)
+#define internal_assert(c) _halide_internal_assertion(c, Halide::InternalError)
+#define user_assert(c) _halide_internal_assertion(c, Halide::CompileError)
 
 // The nicely named versions get cleaned up at the end of Halide.h,
 // but user code might want to do halide-style user_asserts (e.g. the
 // Extern macros introduce calls to user_assert), so for that purpose
 // we define an equivalent macro that can be used outside of Halide.h
-#define _halide_user_assert(c) _halide_internal_assertion(c, Halide::Internal::ErrorReport::User)
+#define _halide_internal_assert(c) _halide_internal_assertion(c, Halide::InternalError)
+#define _halide_user_assert(c) _halide_internal_assertion(c, Halide::CompileError)
 
 // N.B. Any function that might throw a user_assert or user_error may
 // not be inlined into the user's code, or the line number will be

src/autoschedulers/anderson2021/test/test.h

@@ -7,7 +7,8 @@ namespace Halide {
 namespace Internal {
 namespace Autoscheduler {
 
-#define user_assert(c) _halide_internal_assertion(c, Halide::Internal::ErrorReport::User)
+#define user_assert(c) _halide_internal_assertion(c, Halide::CompileError)
+
 #define EXPECT_EQ(expected, actual) expect_eq(__LINE__, expected, actual)
 #define APPROX_EQ(expected, actual, epsilon) approx_eq(__LINE__, expected, actual, epsilon)
 #define EXPECT(expected) expect(__LINE__, expected)