[rocprofiler-systems] Add result<T, E> and use it in number parsers

projects/rocprofiler-systems/source/bin/common/argument_registration.hpp

@@ -9,6 +9,7 @@
 #include "common/json_config.hpp"
 #include "common/string_utility.hpp"
 
+#include <fmt/format.h>
 #include <timemory/utility/argparse.hpp>
 
 #include <cstdlib>
@@ -183,15 +184,16 @@ register_preset_and_domain_arguments(argument_parser& parser, std::string_view t
                 if(!input.empty())
                 {
                     const auto input_integer = utility::string::safe_stoi(input);
-                    if(input_integer.has_value())
+                    if(input_integer.is_ok())
                     {
                         freq = input;
                     }
                     else
                     {
-                        std::cerr
-                            << "[rocprof-sys] WARNING: Invalid CPU sampling frequency '"
-                            << input << "', using default 100 Hz\n";
+                        std::cerr << fmt::format(
+                            "[rocprof-sys] WARNING: Invalid CPU sampling frequency "
+                            "'{}' ({}), using default 100 Hz\n",
+                            input, input_integer.error());
                     }
                 }
             }

projects/rocprofiler-systems/source/bin/common/json_config.cpp

@@ -11,6 +11,7 @@
 #include <algorithm>
 #include <array>
 #include <charconv>
+#include <concepts>
 #include <fstream>
 #include <iostream>
 #include <set>
@@ -694,30 +695,15 @@ expand_gpu_metrics(const std::string& metrics_str)
     return result;
 }
 
-void
-set_json_int(nlohmann::json& target, const std::string& value)
-{
-    if(auto number = utility::string::safe_stoi(value))
-    {
-        target = *number;
-    }
-    else
-    {
-        target = value;
-    }
-}
+template <typename Number>
+concept json_number = std::integral<Number> || std::floating_point<Number>;
 
+template <json_number Number>
 void
-set_json_double(nlohmann::json& target, const std::string& value)
+set_json_number(nlohmann::json& target, const std::string& value)
 {
-    if(auto number = utility::string::safe_stod(value))
-    {
-        target = *number;
-    }
-    else
-    {
-        target = value;
-    }
+    const auto number = utility::string::safe_from_chars<Number>(value);
+    target = number.is_ok() ? nlohmann::json(number.value()) : nlohmann::json(value);
 }
 
 bool
@@ -781,7 +767,8 @@ export_int_value(nlohmann::json&                           config,
     auto const it = env_map.find(std::string{ env_var });
     if(it != env_map.end())
     {
-        set_json_int(config[json_path_section][json_path_key]["value"], it->second);
+        set_json_number<int>(config[json_path_section][json_path_key]["value"],
+                             it->second);
     }
 }
 
@@ -794,7 +781,8 @@ export_double_value(nlohmann::json&                           config,
     auto const it = env_map.find(std::string{ env_var });
     if(it != env_map.end())
     {
-        set_json_double(config[json_path_section][json_path_key]["value"], it->second);
+        set_json_number<double>(config[json_path_section][json_path_key]["value"],
+                                it->second);
     }
 }
 
@@ -880,15 +868,15 @@ export_domain_gpu(nlohmann::json&                           config,
     }
     if(auto freq = lookup(env_map, env_vars::AMD_SMI_FREQ))
     {
-        set_json_int(gpu["sampling_rate_hz"]["value"], *freq);
+        set_json_number<int>(gpu["sampling_rate_hz"]["value"], *freq);
     }
     if(auto freq = lookup(env_map, env_vars::PROCESS_SAMPLING_FREQ))
     {
-        set_json_double(gpu["process_sampling_freq"]["value"], *freq);
+        set_json_number<double>(gpu["process_sampling_freq"]["value"], *freq);
     }
     if(auto dur = lookup(env_map, env_vars::PROCESS_SAMPLING_DURATION))
     {
-        set_json_double(gpu["process_sampling_duration"]["value"], *dur);
+        set_json_number<double>(gpu["process_sampling_duration"]["value"], *dur);
     }
     if(auto v = lookup(env_map, env_vars::USE_AINIC))
     {

projects/rocprofiler-systems/source/bin/rocprof-sys-attach/rocprof-sys-attach.cpp

@@ -184,34 +184,34 @@ void
 parse_pid(attach_options& opts, const char* arg)
 {
     auto const pid = rocprofsys::utility::string::safe_stoi(arg);
-    if(!pid)
+    if(pid.is_error())
     {
-        LOG_ERROR("Invalid PID '{}'.", arg);
+        LOG_ERROR("Invalid PID '{}': {}.", arg, pid.error());
         std::exit(EXIT_FAILURE);
     }
-    if(*pid <= 0)
+    if(pid.value() <= 0)
     {
         LOG_ERROR("PID must be a positive integer.");
         std::exit(EXIT_FAILURE);
     }
-    opts.pid = *pid;
+    opts.pid = pid.value();
 }
 
 void
 parse_detach_after(attach_options& opts, const char* arg)
 {
     auto const detach_after = rocprofsys::utility::string::safe_stoi(arg);
-    if(!detach_after)
+    if(detach_after.is_error())
     {
-        LOG_ERROR("Invalid --detach-after value '{}'.", arg);
+        LOG_ERROR("Invalid --detach-after value '{}': {}.", arg, detach_after.error());
         std::exit(EXIT_FAILURE);
     }
-    if(*detach_after < 0)
+    if(detach_after.value() < 0)
     {
         LOG_ERROR("--detach-after must be non-negative.");
         std::exit(EXIT_FAILURE);
     }
-    opts.detach_after = *detach_after;
+    opts.detach_after = detach_after.value();
 }
 
 void

projects/rocprofiler-systems/source/lib/common/result.hpp

@@ -0,0 +1,102 @@
+// Copyright (c) Advanced Micro Devices, Inc.
+// SPDX-License-Identifier: MIT
+
+#pragma once
+
+#include <stdexcept>
+#include <utility>
+#include <variant>
+
+namespace rocprofsys::inline common
+{
+/**
+ * @brief Tags a value as the error side, so `result<T, E>` stays unambiguous when `T`
+ *        and `E` are convertible to each other.
+ */
+template <typename E>
+struct failure
+{
+    E error;
+};
+
+template <typename E>
+failure(E) -> failure<E>;
+
+/**
+ * @brief Holds either a success value `T` or an error `E`, never both.
+ *
+ * Use it for operations that can fail with a reason the caller may need. Use
+ * `std::optional` when "no value" is a valid answer and not a failure.
+ *
+ * @code
+ * using parse_result = result<int, parse_error>;
+ *
+ * parse_result parse(std::string_view text)
+ * {
+ *     if(text.empty())
+ *         return parse_result{ failure{ parse_error::empty } };
+ *     return parse_result{ 42 };
+ * }
+ *
+ * auto parsed = parse(text);
+ * if(parsed.is_error())
+ *     report(parsed.error());
+ * else
+ *     use(parsed.value());
+ * @endcode
+ */
+template <typename T, typename E>
+class [[nodiscard]] result
+{
+public:
+    constexpr explicit result(T value)
+    : m_data{ std::in_place_index<0>, std::move(value) }
+    {}
+
+    constexpr explicit result(failure<E> fail)
+    : m_data{ std::in_place_index<1>, std::move(fail.error) }
+    {}
+
+    [[nodiscard]] constexpr bool is_ok() const noexcept { return m_data.index() == 0; }
+    [[nodiscard]] constexpr bool is_error() const noexcept { return m_data.index() == 1; }
+
+    /// @throws std::logic_error if this holds an error.
+    [[nodiscard]] constexpr T& value() &
+    {
+        if(is_error())
+        {
+            throw std::logic_error{ "result::value() called on a failure" };
+        }
+        return std::get<0>(m_data);
+    }
+
+    /// @throws std::logic_error if this holds an error.
+    [[nodiscard]] constexpr const T& value() const&
+    {
+        if(is_error())
+        {
+            throw std::logic_error{ "result::value() called on a failure" };
+        }
+        return std::get<0>(m_data);
+    }
+
+    /// @throws std::logic_error if this holds a value.
+    [[nodiscard]] constexpr const E& error() const&
+    {
+        if(is_ok())
+        {
+            throw std::logic_error{ "result::error() called on a success" };
+        }
+        return std::get<1>(m_data);
+    }
+
+    template <typename U>
+    [[nodiscard]] constexpr T value_or(U&& fallback) const&
+    {
+        return is_ok() ? std::get<0>(m_data) : static_cast<T>(std::forward<U>(fallback));
+    }
+
+private:
+    std::variant<T, E> m_data;
+};
+}  // namespace rocprofsys::inline common

projects/rocprofiler-systems/source/lib/common/string_utility.hpp

@@ -3,6 +3,9 @@
 
 #pragma once
 
+#include "common/result.hpp"
+
+#include <fmt/base.h>
 #include <fmt/format.h>
 
 #include <algorithm>
@@ -13,6 +16,7 @@
 #include <cstddef>
 #include <cstdint>
 #include <optional>
+#include <stdexcept>
 #include <string>
 #include <string_view>
 #include <system_error>
@@ -154,44 +158,59 @@ has_env_key(std::string_view entry, std::string_view key) noexcept
            entry[key.size()] == '=';
 }
 
-/// @brief Strictly parse a whole string as an arithmetic Tp; nullopt on empty, partial or
-///        overflow.
+/// @brief Why a string could not be parsed as a number.
+enum class parse_error
+{
+    empty,
+    not_a_number,
+    out_of_range
+};
+
+/// @brief Strictly parse a whole string as an arithmetic Tp.
+/// @return The value, or a parse_error: empty input, a partial or non-numeric string,
+///         or a value that does not fit in Tp.
 template <typename Tp>
     requires std::integral<Tp> || std::floating_point<Tp>
-[[nodiscard]] inline std::optional<Tp>
+[[nodiscard]] inline result<Tp, parse_error>
 safe_from_chars(std::string_view number_str)
 {
+    using parse_result = result<Tp, parse_error>;
+
     if(number_str.empty())
     {
-        return std::nullopt;
+        return parse_result{ failure{ parse_error::empty } };
     }
     const char* const end    = number_str.data() + number_str.size();
     Tp                value  = 0;
-    const auto        result = std::from_chars(number_str.data(), end, value);
+    const auto        parsed = std::from_chars(number_str.data(), end, value);
+    if(parsed.ec == std::errc::result_out_of_range)
+    {
+        return parse_result{ failure{ parse_error::out_of_range } };
+    }
     // Reject partial parses (e.g. "12.5" -> 12) by checking entire string was consumed
-    if(result.ec != std::errc{} || result.ptr != end)
+    if(parsed.ec != std::errc{} || parsed.ptr != end)
     {
-        return std::nullopt;
+        return parse_result{ failure{ parse_error::not_a_number } };
     }
-    return value;
+    return parse_result{ value };
 }
 
-/// @brief Strictly parse a whole string as an int; nullopt on empty, partial or overflow.
-[[nodiscard]] inline std::optional<int>
+/// @brief Strictly parse a whole string as an int. See safe_from_chars.
+[[nodiscard]] inline result<int, parse_error>
 safe_stoi(std::string_view number_str)
 {
     return safe_from_chars<int>(number_str);
 }
 
-/// @brief Strictly parse a whole string as a long; nullopt on empty, partial or overflow.
-[[nodiscard]] inline std::optional<long>
+/// @brief Strictly parse a whole string as a long. See safe_from_chars.
+[[nodiscard]] inline result<long, parse_error>
 safe_stol(std::string_view number_str)
 {
     return safe_from_chars<long>(number_str);
 }
 
-/// @brief Strictly parse a whole string as a double; nullopt on empty or partial parse.
-[[nodiscard]] inline std::optional<double>
+/// @brief Strictly parse a whole string as a double. See safe_from_chars.
+[[nodiscard]] inline result<double, parse_error>
 safe_stod(std::string_view number_str)
 {
     return safe_from_chars<double>(number_str);
@@ -319,3 +338,27 @@ hex_padded(Tp value, size_t width = k_default_hex_width)
 }
 
 }  // namespace rocprofsys::utility::string
+
+/// Formats a parse_error as a short, user-facing description.
+template <>
+struct fmt::formatter<rocprofsys::utility::string::parse_error>
+: fmt::formatter<std::string_view>
+{
+    template <typename FormatContext>
+    auto format(rocprofsys::utility::string::parse_error err, FormatContext& ctx) const
+    {
+        using rocprofsys::utility::string::parse_error;
+        switch(err)
+        {
+            case parse_error::empty:
+                return fmt::formatter<std::string_view>::format("empty value", ctx);
+            case parse_error::not_a_number:
+                return fmt::formatter<std::string_view>::format("not a number", ctx);
+            case parse_error::out_of_range:
+                return fmt::formatter<std::string_view>::format("out of range", ctx);
+            default:
+                throw std::logic_error(
+                    fmt::format("parse_error {} not handled", static_cast<int>(err)));
+        }
+    }
+};

projects/rocprofiler-systems/source/lib/common/tests/CMakeLists.txt

@@ -13,6 +13,7 @@ add_library(
     test_pci_bdf.cpp
     test_regex_utility.cpp
     test_remove_env.cpp
+    test_result.cpp
     test_static_object.cpp
     test_synchronized.cpp
     test_to_bool.cpp