Halide::Error should not extend std::runtime_error (halide#6927)

* Halide::Error should not extend std::runtime_error

Unfortunately, the std error/exception classes aren't marked for DLLEXPORT under MSVC; we need our Error classes to be DLLEXPORT for libHalide (and python bindings). The current situation basically causes MSVC to generator another version of `std::runtime_error` marked for DLLEXPORT, which can lead to ODR violations, which are bad. AFAICT we don't really rely on this inheritance anywhere, so this just eliminates the inheritance entirely.

(Note that I can't point to a specific malfunction resulting from this, but casual googling based on the many warnings MSVC emits about the current situation has me convinced that it needs addressing.)

* noexcept

Author: steven-johnson

src/Error.cpp

@@ -33,8 +33,48 @@ bool exceptions_enabled() {
 #endif
 }
 
+Error::Error(const char *msg)
+    : what_(new char[strlen(msg) + 1]) {
+    strcpy(what_, msg);
+}
+
 Error::Error(const std::string &msg)
-    : std::runtime_error(msg) {
+    : Error(msg.c_str()) {
+}
+
+Error::Error(const Error &that)
+    : Error(that.what_) {
+}
+
+Error &Error::operator=(const Error &that) {
+    if (this != &that) {
+        delete[] this->what_;
+        this->what_ = new char[strlen(that.what_) + 1];
+        strcpy(this->what_, that.what_);
+    }
+    return *this;
+}
+
+Error::Error(Error &&that) noexcept {
+    this->what_ = that.what_;
+    that.what_ = nullptr;
+}
+
+Error &Error::operator=(Error &&that) noexcept {
+    if (this != &that) {
+        delete[] this->what_;
+        this->what_ = that.what_;
+        that.what_ = nullptr;
+    }
+    return *this;
+}
+
+Error::~Error() {
+    delete[] what_;
+}
+
+const char *Error::what() const noexcept {
+    return what_;
 }
 
 CompileError::CompileError(const std::string &msg)
@@ -49,6 +89,18 @@ InternalError::InternalError(const std::string &msg)
     : Error(msg) {
 }
 
+CompileError::CompileError(const char *msg)
+    : Error(msg) {
+}
+
+RuntimeError::RuntimeError(const char *msg)
+    : Error(msg) {
+}
+
+InternalError::InternalError(const char *msg)
+    : Error(msg) {
+}
+
 namespace Internal {
 
 // Force the classes to exist, even if exceptions are off
@@ -96,11 +148,7 @@ ErrorReport::ErrorReport(const char *file, int line, const char *condition_strin
     }
 }
 
-ErrorReport::~ErrorReport()
-#if __cplusplus >= 201100 || _MSC_VER >= 1900
-    noexcept(false)
-#endif
-{
+ErrorReport::~ErrorReport() noexcept(false) {
     if (!msg.str().empty() && msg.str().back() != '\n') {
         msg << "\n";
     }

src/Error.h

@@ -12,29 +12,58 @@ namespace Halide {
 /** Query whether Halide was compiled with exceptions. */
 bool exceptions_enabled();
 
-/** A base class for Halide errors. */
-struct HALIDE_EXPORT_SYMBOL Error : public std::runtime_error {
+/** A base class for Halide errors.
+ *
+ * Note that this deliberately does *not* descend from std::runtime_error, or
+ * even std::exception; unfortunately, std::runtime_error is not marked as
+ * DLLEXPORT on Windows, but Error needs to be marked as such, and mismatching
+ * DLLEXPORT annotations in a class inheritance hierarchy in this way can lead
+ * to ODR violations. Instead, we just attempt to replicate the API of
+ * runtime_error here. */
+struct HALIDE_EXPORT_SYMBOL Error {
+    Error() = delete;
+
     // Give each class a non-inlined constructor so that the type
     // doesn't get separately instantiated in each compilation unit.
-    Error(const std::string &msg);
+    explicit Error(const char *msg);
+    explicit Error(const std::string &msg);
+
+    Error(const Error &);
+    Error &operator=(const Error &);
+    Error(Error &&) noexcept;
+    Error &operator=(Error &&) noexcept;
+
+    virtual ~Error();
+
+    virtual const char *what() const noexcept;
+
+private:
+    // Using a std::string here will cause MSVC to complain about the fact
+    // that class std::string isn't declared DLLEXPORT, even though the
+    // field is private; rather than suppress the warning, we'll just use
+    // an old-fashioned new-and-delete to keep it nice and clean.
+    char *what_;
 };
 
 /** An error that occurs while running a JIT-compiled Halide pipeline. */
 struct HALIDE_EXPORT_SYMBOL RuntimeError : public Error {
-    RuntimeError(const std::string &msg);
+    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 {
-    CompileError(const std::string &msg);
+    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 {
-    InternalError(const std::string &msg);
+    explicit InternalError(const char *msg);
+    explicit InternalError(const std::string &msg);
 };
 
 /** CompileTimeErrorReporter is used at compile time (*not* runtime) when

src/Generator.cpp

@@ -1044,9 +1044,15 @@ namespace Internal {
 int generate_filter_main(int argc, char **argv, const GeneratorFactoryProvider &generator_factory_provider) {
     try {
         return generate_filter_main_inner(argc, argv, generator_factory_provider);
-    } catch (std::runtime_error &err) {
+    } catch (::Halide::Error &err) {
         user_error << "Unhandled exception: " << err.what() << "\n";
         return -1;
+    } catch (std::exception &err) {
+        user_error << "Unhandled exception: " << err.what() << "\n";
+        return -1;
+    } catch (...) {
+        user_error << "Unhandled exception: (unknown)\n";
+        return -1;
     }
 }
 #else