Merge branch 'dev' into jr/more-thorough-error-handling

src/binary/mach-o.hpp

@@ -165,21 +165,21 @@ namespace detail {
         static inline NODISCARD Result<mach_o, internal_error> open_mach_o(const std::string& object_path) {
             auto file = raii_wrap(std::fopen(object_path.c_str(), "rb"), file_deleter);
             if(file == nullptr) {
-                return internal_error("Unable to read object file " + object_path);
+                return internal_error("Unable to read object file {}", object_path);
             }
             auto magic = load_bytes<std::uint32_t>(file, 0);
             if(!magic) {
                 return magic.unwrap_error();
             }
             if(!is_mach_o(magic.unwrap_value())) {
-                return internal_error("File is not mach-o " + object_path);
+                return internal_error("File is not mach-o {}", object_path);
             }
             mach_o obj(std::move(file), object_path, magic.unwrap_value());
             auto result = obj.load();
             if(result.is_error()) {
                 return result.unwrap_error();
             } else {
-                return std::move(obj);
+                return obj;
             }
         }
 
@@ -670,7 +670,7 @@ namespace detail {
     inline Result<bool, internal_error> macho_is_fat(const std::string& object_path) {
         auto file = raii_wrap(std::fopen(object_path.c_str(), "rb"), file_deleter);
         if(file == nullptr) {
-            return internal_error("Unable to read object file " + object_path);
+            return internal_error("Unable to read object file {}", object_path);
         }
         auto magic = load_bytes<std::uint32_t>(file, 0);
         if(!magic) {

src/binary/pe.hpp

@@ -34,14 +34,14 @@ namespace detail {
         errno_t ret = fopen_s(&file_ptr, object_path.c_str(), "rb");
         auto file = raii_wrap(std::move(file_ptr), file_deleter);
         if(ret != 0 || file == nullptr) {
-            throw file_error("Unable to read object file " + object_path);
+            throw internal_error("Unable to read object file {}", object_path);
         }
         auto magic = load_bytes<std::array<char, 2>>(file, 0);
         if(!magic) {
             return magic.unwrap_error();
         }
         if(std::memcmp(magic.unwrap_value().data(), "MZ", 2) != 0) {
-            return internal_error("File is not a PE file " + object_path);
+            return internal_error("File is not a PE file {}", object_path);
         }
         auto e_lfanew = load_bytes<DWORD>(file, 0x3c); // dos header + 0x3c
         if(!e_lfanew) {
@@ -53,7 +53,7 @@ namespace detail {
             return signature.unwrap_error();
         }
         if(std::memcmp(signature.unwrap_value().data(), "PE\0\0", 4) != 0) {
-            return internal_error("File is not a PE file " + object_path);
+            return internal_error("File is not a PE file {}", object_path);
         }
         auto size_of_optional_header_raw = load_bytes<WORD>(file, nt_header_offset + 4 + 0x10); // file header + 0x10
         if(!size_of_optional_header_raw) {

src/symbols/symbols_with_dbghelp.cpp

@@ -71,8 +71,7 @@ namespace dbghelp {
                 return (T)-1;
             } else {
                 throw internal_error(
-                    std::string("SymGetTypeInfo failed: ")
+                    "SymGetTypeInfo failed: {}", std::system_error(GetLastError(), std::system_category()).what()
-                    + std::system_error(GetLastError(), std::system_category()).what()
                 );
             }
         }
@@ -86,8 +85,7 @@ namespace dbghelp {
             !SymGetTypeInfo(proc, modbase, type_index, static_cast<::IMAGEHLP_SYMBOL_TYPE_INFO>(SymType), &info)
         ) {
             throw internal_error(
-                std::string("SymGetTypeInfo failed: ")
+                "SymGetTypeInfo failed: {}", std::system_error(GetLastError(), std::system_category()).what()
-                + std::system_error(GetLastError(), std::system_category()).what()
             );
         }
         // special case to properly free a buffer and convert string to narrow chars, only used for
@@ -248,8 +246,8 @@ namespace dbghelp {
                         )
                     ) {
                         throw internal_error(
-                            std::string("SymGetTypeInfo failed: ")
+                            "SymGetTypeInfo failed: {}",
-                            + std::system_error(GetLastError(), std::system_category()).what()
+                            std::system_error(GetLastError(), std::system_category()).what()
                         );
                     }
                     // get children type

src/symbols/symbols_with_dl.cpp

@@ -15,9 +15,12 @@ namespace libdl {
     stacktrace_frame resolve_frame(const frame_ptr addr) {
         Dl_info info;
         if(dladdr(reinterpret_cast<void*>(addr), &info)) { // thread-safe
+            auto base = get_module_image_base(info.dli_fname);
             return {
                 addr,
-                addr - reinterpret_cast<std::uintptr_t>(info.dli_fbase) + get_module_image_base(info.dli_fname),
+                base.has_value()
+                    ? addr - reinterpret_cast<std::uintptr_t>(info.dli_fbase) + base.unwrap_value()
+                    : 0,
                 nullable<std::uint32_t>::null(),
                 nullable<std::uint32_t>::null(),
                 info.dli_fname ? info.dli_fname : "",

src/symbols/symbols_with_libbacktrace.cpp

@@ -38,7 +38,7 @@ namespace libbacktrace {
     }
 
     void error_callback(void*, const char* msg, int errnum) {
-        throw internal_error(microfmt::format("Libbacktrace error: {}, code {}\n", msg, errnum));
+        throw internal_error("Libbacktrace error: {}, code {}", msg, errnum);
     }
 
     backtrace_state* get_backtrace_state() {

src/utils/common.hpp

@@ -58,6 +58,14 @@
 #define MAGENTA ESC "35m"
 #define CYAN    ESC "36m"
 
+#if IS_GCC || IS_CLANG
+ #define NODISCARD __attribute__((warn_unused_result))
+// #elif IS_MSVC && _MSC_VER >= 1700
+//  #define NODISCARD _Check_return_
+#else
+ #define NODISCARD
+#endif
+
 namespace cpptrace {
 namespace detail {
     static const stacktrace_frame null_frame {

src/utils/dbghelp_syminit_manager.hpp

@@ -25,7 +25,7 @@ namespace detail {
         void init(HANDLE proc) {
             if(set.count(proc) == 0) {
                 if(!SymInitialize(proc, NULL, TRUE)) {
-                    throw internal_error(microfmt::format("SymInitialize failed {}", GetLastError()));
+                    throw internal_error("SymInitialize failed {}", GetLastError());
                 }
                 set.insert(proc);
             }

src/utils/dwarf.hpp

@@ -28,7 +28,7 @@ namespace libdwarf {
         char* msg = dwarf_errmsg(error);
         (void)dbg;
         // dwarf_dealloc_error(dbg, error);
-        throw internal_error(microfmt::format("Cpptrace dwarf error {} {}\n", ev, msg));
+        throw internal_error("Cpptrace dwarf error {} {}", ev, msg);
     }
 
     struct die_object {

src/utils/error.hpp

@@ -21,6 +21,8 @@ namespace detail {
         std::string msg;
     public:
         internal_error(std::string message) : msg(std::move(message)) {}
+        template<typename... Args>
+        internal_error(const char* format, Args&&... args) : msg(microfmt::format(format, args...)) {}
         const char* what() const noexcept override {
             return msg.c_str();
         }
@@ -63,21 +65,17 @@ namespace detail {
         const char* name   = assert_names[static_cast<std::underlying_type<assert_type>::type>(type)];
         if(message == "") {
             throw internal_error(
-                microfmt::format(
                 "Cpptrace {} failed at {}:{}: {}\n"
                 "    %s(%s);\n",
                 action, location.file, location.line, signature,
                 name, expression
-                )
             );
         } else {
             throw internal_error(
-                microfmt::format(
                 "Cpptrace {} failed at {}:{}: {}: {}\n"
                 "    %s(%s);\n",
                 action, location.file, location.line, signature, message.c_str(),
                 name, expression
-                )
             );
         }
     }
@@ -89,17 +87,13 @@ namespace detail {
     ) {
         if(message == "") {
             throw internal_error(
-                microfmt::format(
                 "Cpptrace panic {}:{}: {}\n",
                 location.file, location.line, signature
-                )
             );
         } else {
             throw internal_error(
-                microfmt::format(
                 "Cpptrace panic {}:{}: {}: {}\n",
                 location.file, location.line, signature, message.c_str()
-                )
             );
         }
     }

src/utils/microfmt.hpp

@@ -254,7 +254,7 @@ namespace microfmt {
                         } else if(*it == '{') { // try to parse variable width
                             MICROFMT_ASSERT(peek() == '}');
                             it += 2;
-                            MICROFMT_ASSERT(arg_i < N);
+                            MICROFMT_ASSERT(arg_i < args.size());
                             options.width = args[arg_i++].unwrap_int();
                         }
                         // try to parse fill/base
@@ -277,7 +277,7 @@ namespace microfmt {
                             }
                         }
                         MICROFMT_ASSERT(*it == '}');
-                        MICROFMT_ASSERT(arg_i < N);
+                        MICROFMT_ASSERT(arg_i < args.size());
                         args[arg_i++].write(str, options);
                     }
                 } else if(*it == '}') {

src/utils/utils.hpp

@@ -301,32 +301,24 @@ namespace detail {
         }
 
         NODISCARD T& unwrap() & {
-            if(!holds_value) {
+            ASSERT(holds_value, "Optional does not contain a value");
-                ASSERT(false, "Illegal unwrap: Optional does not contain a value");
-            }
             return uvalue;
         }
 
         NODISCARD const T& unwrap() const & {
-            if(!holds_value) {
+            ASSERT(holds_value, "Optional does not contain a value");
-                ASSERT(false, "Illegal unwrap: Optional does not contain a value");
-            }
             return uvalue;
         }
 
-        NODISCARD T unwrap() && {
+        NODISCARD T&& unwrap() && {
-            if(!holds_value) {
+            ASSERT(holds_value, "Optional does not contain a value");
-                ASSERT(false, "Illegal unwrap: Optional does not contain a value");
-            }
             return std::move(uvalue);
         }
 
-        // NODISCARD const T unwrap() const && {
+        NODISCARD const T&& unwrap() const && {
-        //     if(!holds_value) {
+            ASSERT(holds_value, "Optional does not contain a value");
-        //         ASSERT(false, "Illegal unwrap: Optional does not contain a value");
+            return std::move(uvalue);
-        //     }
+        }
-        //     return std::move(uvalue);
-        // }
 
         template<typename U>
         NODISCARD T value_or(U&& default_value) const & {
@@ -339,26 +331,41 @@ namespace detail {
         }
     };
 
-    // TODO: Less stupid implementation with optionals
     // TODO: Better dump error
     // TODO: Explicit constructors for value, then add Ok()/Error() helpers
     template<typename T, typename E, typename std::enable_if<!std::is_same<T, E>::value, int>::type = 0>
     class Result {
         // Not using a union because I don't want to have to deal with that
-        optional<T> value_;
+        union {
-        optional<E> error_;
+            T value_;
+            E error_;
+        };
+        enum class member { value, error };
+        member active;
     public:
-        Result(T value) : value_(std::move(value)) {}
+        Result(T value) : value_(std::move(value)), active(member::value) {}
-        Result(E error) : error_(std::move(error)) {}
+        Result(E error) : error_(std::move(error)), active(member::error) {}
+        Result(Result&& other) : active(other.active) {
+            if(other.active == member::value) {
+                new (&value_) T(std::move(other.value_));
+            } else {
+                new (&error_) E(std::move(other.error_));
+            }
+        }
+        ~Result() {
+            if(active == member::value) {
+                value_.~T();
+            } else {
+                error_.~E();
+            }
+        }
 
         bool has_value() const {
-            ASSERT(value_.has_value() != error_.has_value(), "Illegal state for Result");
+            return active == member::value;
-            return value_.has_value();
         }
 
         bool is_error() const {
-            ASSERT(value_.has_value() != error_.has_value(), "Illegal state for Result");
+            return active == member::error;
-            return error_.has_value();
         }
 
         explicit operator bool() const {
@@ -366,119 +373,64 @@ namespace detail {
         }
 
         NODISCARD optional<T> value() const & {
-            return value_;
+            return has_value() ? value_ : nullopt;
         }
 
         NODISCARD optional<E> error() const & {
-            return error_;
+            return is_error() ? error_ : nullopt;
         }
 
         NODISCARD optional<T> value() && {
-            return std::move(value_);
+            return has_value() ? std::move(value_) : nullopt;
         }
 
         NODISCARD optional<E> error() && {
-            return std::move(error_);
+            return is_error() ? std::move(error_) : nullopt;
         }
 
         NODISCARD T& unwrap_value() & {
-            return value_.unwrap();
+            ASSERT(has_value(), "Result does not contain a value");
+            return value_;
         }
 
         NODISCARD const T& unwrap_value() const & {
-            return value_.unwrap();
+            ASSERT(has_value(), "Result does not contain a value");
+            return value_;
         }
 
         NODISCARD T unwrap_value() && {
-            return std::move(value_).unwrap();
+            ASSERT(has_value(), "Result does not contain a value");
+            return std::move(value_);
         }
 
-        // NODISCARD const T unwrap() const && {
-        //     return std::move(value_).unwrap();
-        // }
-
         NODISCARD E& unwrap_error() & {
-            return error_.unwrap();
+            ASSERT(is_error(), "Result does not contain an error");
+            return error_;
         }
 
         NODISCARD const E& unwrap_error() const & {
-            return error_.unwrap();
+            ASSERT(is_error(), "Result does not contain an error");
+            return error_;
         }
 
         NODISCARD E unwrap_error() && {
-            return std::move(error_).unwrap();
+            ASSERT(is_error(), "Result does not contain an error");
+            return std::move(error_);
         }
 
-        // NODISCARD const E unwrap_error() const && {
-        //     return std::move(error_).unwrap();
-        // }
-
-
-        // NODISCARD const T& value() const & {
-        //     ASSERT(has_value());
-        //     return value_.unwrap();
-        // }
-
-        // NODISCARD const E& error() const & {
-        //     ASSERT(is_error());
-        //     return error_.unwrap();
-        // }
-
-        // NODISCARD T value() && {
-        //     ASSERT(has_value());
-        //     return std::move(value_).unwrap();
-        // }
-
-        // NODISCARD E error() && {
-        //     ASSERT(is_error());
-        //     return std::move(error_).unwrap();
-        // }
-
-        // NODISCARD T& unwrap() & {
-        //     return value_.unwrap();
-        // }
-
-        // NODISCARD const T& unwrap() const & {
-        //     return value_.unwrap();
-        // }
-
-        // NODISCARD T unwrap() && {
-        //     return std::move(value_).unwrap();
-        // }
-
-        // NODISCARD const T unwrap() const && {
-        //     return std::move(value_).unwrap();
-        // }
-
-        // NODISCARD E& unwrap_error() & {
-        //     return error_.unwrap();
-        // }
-
-        // NODISCARD const E& unwrap_error() const & {
-        //     return error_.unwrap();
-        // }
-
-        // NODISCARD E unwrap_error() && {
-        //     return std::move(error_).unwrap();
-        // }
-
-        // NODISCARD const E unwrap_error() const && {
-        //     return std::move(error_).unwrap();
-        // }
-
         template<typename U>
         NODISCARD T value_or(U&& default_value) const & {
-            return value_.value_or(std::forward<U>(default_value));
+            return has_value() ? value_ : static_cast<T>(std::forward<U>(default_value));
         }
 
         template<typename U>
         NODISCARD T value_or(U&& default_value) && {
-            return std::move(value_).value_or(std::forward<U>(default_value));
+            return has_value() ? std::move(value_) : static_cast<T>(std::forward<U>(default_value));
         }
 
         void drop_error() const {
             if(is_error()) {
-                std::fprintf(stderr, "%s", unwrap_error().what());
+                std::fprintf(stderr, "%s\n", unwrap_error().what());
             }
         }
     };