macOS 10.12 compatibility

Add var::get/var::visit implementations of std::get/std::visit that get
used if compiling for an old macos target, and use those.

The issue is that on a <10.14 macos target Apple's libc++ is missing
std::bad_variant_access, and so any method that can throw it (such as
std::get and std::visit) can't be used.  This workaround is ugly, but
such is life when you want to support running on Apple platforms.

CMakeLists.txt

@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 3.7)
 
 # Has to be set before `project()`, and ignored on non-macos:
-set(CMAKE_OSX_DEPLOYMENT_TARGET 10.14 CACHE STRING "macOS deployment target (Apple clang only)")
+set(CMAKE_OSX_DEPLOYMENT_TARGET 10.12 CACHE STRING "macOS deployment target (Apple clang only)")
 
 project(liblokimq CXX C)
 
@@ -153,6 +153,7 @@ install(
           lokimq/lokimq.h
           lokimq/message.h
           lokimq/string_view.h
+          lokimq/variant.h
           ${CMAKE_CURRENT_BINARY_DIR}/lokimq/version.h
     DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/lokimq
 )

lokimq/bt_serialize.h

@@ -35,7 +35,7 @@
 #include <ostream>
 #include <string>
 #include <string_view>
-#include <variant>
+#include "variant.h"
 #include <cstdint>
 #include <limits>
 #include <stdexcept>
@@ -428,8 +428,8 @@ struct bt_deserialize_try_variant_impl<std::enable_if_t<!is_bt_deserializable<T>
 // Serialization of a variant; all variant types must be bt-serializable.
 template <typename... Ts>
 struct bt_serialize<std::variant<Ts...>, std::void_t<bt_serialize<Ts>...>> {
-    void operator()(std::ostream &os, const std::variant<Ts...>& val) {
-        std::visit(
+    void operator()(std::ostream& os, const std::variant<Ts...>& val) {
+        var::visit(
                 [&os] (const auto& val) {
                     using T = std::remove_cv_t<std::remove_reference_t<decltype(val)>>;
                     bt_serialize<T>{}(os, val);
@@ -556,15 +556,14 @@ inline bt_value bt_get(std::string_view s) {
 ///     auto v = get_int<uint32_t>(val); // throws if the decoded value doesn't fit in a uint32_t
 template <typename IntType, std::enable_if_t<std::is_integral_v<IntType>, int> = 0>
 IntType get_int(const bt_value &v) {
-    if (std::holds_alternative<uint64_t>(v)) {
-        uint64_t value = std::get<uint64_t>(v);
+    if (auto* value = std::get_if<uint64_t>(&v)) {
         if constexpr (!std::is_same_v<IntType, uint64_t>)
-            if (value > static_cast<uint64_t>(std::numeric_limits<IntType>::max()))
+            if (*value > static_cast<uint64_t>(std::numeric_limits<IntType>::max()))
                 throw std::overflow_error("Unable to extract integer value: stored value is too large for the requested type");
-        return static_cast<IntType>(value);
+        return static_cast<IntType>(*value);
     }
 
-    int64_t value = std::get<int64_t>(v);
+    int64_t value = var::get<int64_t>(v); // throws if no int contained
     if constexpr (!std::is_same_v<IntType, int64_t>)
         if (value > static_cast<int64_t>(std::numeric_limits<IntType>::max())
                 || value < static_cast<int64_t>(std::numeric_limits<IntType>::min()))
@@ -589,7 +588,7 @@ Tuple get_tuple(const bt_list& x) {
 }
 template <typename Tuple>
 Tuple get_tuple(const bt_value& x) {
-    return get_tuple<Tuple>(std::get<bt_list>(static_cast<const bt_variant&>(x)));
+    return get_tuple<Tuple>(var::get<bt_list>(static_cast<const bt_variant&>(x)));
 }
 
 namespace detail {
@@ -601,15 +600,15 @@ void get_tuple_impl_one(T& t, It& it) {
     } else if constexpr (is_bt_tuple<T>) {
         if (std::holds_alternative<bt_list>(v))
             throw std::invalid_argument{"Unable to convert tuple: cannot create sub-tuple from non-bt_list"};
-        t = get_tuple<T>(std::get<bt_list>(v));
+        t = get_tuple<T>(var::get<bt_list>(v));
     } else if constexpr (std::is_same_v<std::string, T> || std::is_same_v<std::string_view, T>) {
         // If we request a string/string_view, we might have the other one and need to copy/view it.
         if (std::holds_alternative<std::string_view>(v))
-            t = std::get<std::string_view>(v);
+            t = var::get<std::string_view>(v);
         else
-            t = std::get<std::string>(v);
+            t = var::get<std::string>(v);
     } else {
-        t = std::get<T>(v);
+        t = var::get<T>(v);
     }
 }
 template <typename Tuple, size_t... Is>

lokimq/lokimq.cpp

@@ -378,7 +378,7 @@ LokiMQ::run_info& LokiMQ::run_info::load(pending_command&& pending) {
 
     assert(pending.callback.index() == 0);
     return load(&pending.cat, std::move(pending.command), std::move(pending.conn), std::move(pending.access),
-            std::move(pending.remote), std::move(pending.data_parts), std::get<0>(pending.callback));
+            std::move(pending.remote), std::move(pending.data_parts), var::get<0>(pending.callback));
 }
 
 LokiMQ::run_info& LokiMQ::run_info::load(batch_job&& bj, bool reply_job, int tagged_thread) {

lokimq/variant.h

@@ -0,0 +1,87 @@
+#pragma once
+// Workarounds for macos compatibility.  On macOS we aren't allowed to touch anything in
+// std::variant that could throw if compiling with a target <10.14 because Apple fails hard at
+// properly updating their STL.  Thus, if compiling in such a mode, we have to introduce
+// workarounds.
+//
+// This header defines a `var` namespace with `var::get` and `var::visit` implementations.  On
+// everything except broken backwards macos, this is just an alias to `std`.  On broken backwards
+// macos, we provide implementations that throw std::runtime_error in failure cases since the
+// std::bad_variant_access exception can't be touched.
+//
+// You also get a BROKEN_APPLE_VARIANT macro defined if targetting a problematic mac architecture.
+
+#include <variant>
+
+#ifdef __APPLE__
+#  include <AvailabilityVersions.h>
+#  if defined(__APPLE__) && MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_14
+#    define BROKEN_APPLE_VARIANT
+#  endif
+#endif
+
+#ifndef BROKEN_APPLE_VARIANT
+
+namespace var = std; // Oh look, actual C++17 support
+
+#else
+
+// Oh look, apple.
+
+namespace var {
+
+// Apple won't let us use std::visit or std::get if targetting some version of macos earlier than
+// 10.14 because Apple is awful about not updating their STL.  So we have to provide our own, and
+// then call these without `std::` -- on crappy macos we'll come here, on everything else we'll ADL
+// to the std:: implementation.
+template <typename T, typename... Types>
+constexpr T& get(std::variant<Types...>& var) {
+    if (auto* v = std::get_if<T>(&var)) return *v;
+    throw std::runtime_error{"Bad variant access -- variant does not contain the requested type"};
+}
+template <typename T, typename... Types>
+constexpr const T& get(const std::variant<Types...>& var) {
+    if (auto* v = std::get_if<T>(&var)) return *v;
+    throw std::runtime_error{"Bad variant access -- variant does not contain the requested type"};
+}
+template <typename T, typename... Types>
+constexpr const T&& get(const std::variant<Types...>&& var) {
+    if (auto* v = std::get_if<T>(&var)) return std::move(*v);
+    throw std::runtime_error{"Bad variant access -- variant does not contain the requested type"};
+}
+template <typename T, typename... Types>
+constexpr T&& get(std::variant<Types...>&& var) {
+    if (auto* v = std::get_if<T>(&var)) return std::move(*v);
+    throw std::runtime_error{"Bad variant access -- variant does not contain the requested type"};
+}
+template <size_t I, typename Variant>
+constexpr auto get(Variant&& var) {
+    return var::get<std::variant_alternative_t<I, std::remove_cv_t<std::remove_reference_t<Variant>>>>(std::forward<Variant>(var));
+}
+
+template <size_t I, size_t... More, class Visitor, class Variant>
+constexpr auto visit_helper(Visitor&& vis, Variant&& var) {
+    if (var.index() == I)
+        return std::invoke(std::forward<Visitor>(vis), var::get<I>(std::forward<Variant>(var)));
+    else if constexpr (sizeof...(More) > 0)
+        return visit_helper<More...>(std::forward<Visitor>(vis), std::forward<Variant>(var));
+    else
+        throw std::runtime_error{"Bad visit -- variant is valueless"};
+}
+
+template <size_t... Is, class Visitor, class Variant>
+constexpr auto visit_helper(Visitor&& vis, Variant&& var, std::index_sequence<Is...>) {
+    return visit_helper<Is...>(std::forward<Visitor>(vis), std::forward<Variant>(var));
+}
+
+// Only handle a single variant here because multi-variant invocation is notably harder (and we
+// don't need it).
+template <class Visitor, class Variant>
+constexpr auto visit(Visitor&& vis, Variant&& var) {
+    return visit_helper(std::forward<Visitor>(vis), std::forward<Variant>(var),
+            std::make_index_sequence<std::variant_size_v<std::remove_reference_t<Variant>>>{});
+}
+
+} // namespace var
+
+#endif

lokimq/worker.cpp

@@ -99,7 +99,7 @@ void LokiMQ::worker_thread(unsigned int index, std::optional<std::string> tagged
 
         try {
             if (run.is_batch_job) {
-                auto* batch = std::get<detail::Batch*>(run.to_run);
+                auto* batch = var::get<detail::Batch*>(run.to_run);
                 if (run.batch_jobno >= 0) {
                     LMQ_TRACE("worker thread ", worker_id, " running batch ", batch, "#", run.batch_jobno);
                     batch->run_job(run.batch_jobno);
@@ -108,7 +108,7 @@ void LokiMQ::worker_thread(unsigned int index, std::optional<std::string> tagged
                     batch->job_completion();
                 }
             } else if (run.is_injected) {
-                auto& func = std::get<std::function<void()>>(run.to_run);
+                auto& func = var::get<std::function<void()>>(run.to_run);
                 LMQ_TRACE("worker thread ", worker_id, " invoking injected command ", run.command);
                 func();
                 func = nullptr;
@@ -120,7 +120,7 @@ void LokiMQ::worker_thread(unsigned int index, std::optional<std::string> tagged
 
                 LMQ_TRACE("Got incoming command from ", message.remote, "/", message.conn, message.conn.route.empty() ? " (outgoing)" : " (incoming)");
 
-                auto& [callback, is_request] = *std::get<const std::pair<CommandCallback, bool>*>(run.to_run);
+                auto& [callback, is_request] = *var::get<const std::pair<CommandCallback, bool>*>(run.to_run);
                 if (is_request) {
                     message.reply_tag = {run.data_parts[0].data<char>(), run.data_parts[0].size()};
                     for (auto it = run.data_parts.begin() + 1; it != run.data_parts.end(); ++it)
@@ -137,9 +137,11 @@ void LokiMQ::worker_thread(unsigned int index, std::optional<std::string> tagged
         catch (const bt_deserialize_invalid& e) {
             LMQ_LOG(warn, worker_id, " deserialization failed: ", e.what(), "; ignoring request");
         }
+#ifndef BROKEN_APPLE_VARIANT
         catch (const std::bad_variant_access& e) {
             LMQ_LOG(warn, worker_id, " deserialization failed: found unexpected serialized type (", e.what(), "); ignoring request");
         }
+#endif
         catch (const std::out_of_range& e) {
             LMQ_LOG(warn, worker_id, " deserialization failed: invalid data - required field missing (", e.what(), "); ignoring request");
         }
@@ -208,7 +210,7 @@ void LokiMQ::proxy_worker_message(std::vector<zmq::message_t>& parts) {
                 active--;
             }
             bool clear_job = false;
-            auto* batch = std::get<detail::Batch*>(run.to_run);
+            auto* batch = var::get<detail::Batch*>(run.to_run);
             if (run.batch_jobno == -1) {
                 // Returned from the completion function
                 clear_job = true;

tests/test_bt.cpp

@@ -125,10 +125,10 @@ TEST_CASE("bt_value serialization", "[bt][serialization][bt_value]") {
 TEST_CASE("bt_value deserialization", "[bt][deserialization][bt_value]") {
     auto dna1 = bt_deserialize<bt_value>("i42e");
     auto dna2 = bt_deserialize<bt_value>("i-42e");
-    REQUIRE( std::get<uint64_t>(dna1) == 42 );
-    REQUIRE( std::get<int64_t>(dna2) == -42 );
-    REQUIRE_THROWS( std::get<int64_t>(dna1) );
-    REQUIRE_THROWS( std::get<uint64_t>(dna2) );
+    REQUIRE( var::get<uint64_t>(dna1) == 42 );
+    REQUIRE( var::get<int64_t>(dna2) == -42 );
+    REQUIRE_THROWS( var::get<int64_t>(dna1) );
+    REQUIRE_THROWS( var::get<uint64_t>(dna2) );
     REQUIRE( lokimq::get_int<int>(dna1) == 42 );
     REQUIRE( lokimq::get_int<int>(dna2) == -42 );
     REQUIRE( lokimq::get_int<unsigned>(dna1) == 42 );
@@ -136,19 +136,19 @@ TEST_CASE("bt_value deserialization", "[bt][deserialization][bt_value]") {
 
     bt_value x = bt_deserialize<bt_value>("d3:barle3:foold1:ali1ei2ei3ee1:bleed1:cli-5ei4eeeee");
     REQUIRE( std::holds_alternative<bt_dict>(x) );
-    bt_dict& a = std::get<bt_dict>(x);
+    bt_dict& a = var::get<bt_dict>(x);
     REQUIRE( a.count("bar") );
     REQUIRE( a.count("foo") );
     REQUIRE( a.size() == 2 );
-    bt_list& foo = std::get<bt_list>(a["foo"]);
+    bt_list& foo = var::get<bt_list>(a["foo"]);
     REQUIRE( foo.size() == 2 );
-    bt_dict& foo1 = std::get<bt_dict>(foo.front());
-    bt_dict& foo2 = std::get<bt_dict>(foo.back());
+    bt_dict& foo1 = var::get<bt_dict>(foo.front());
+    bt_dict& foo2 = var::get<bt_dict>(foo.back());
     REQUIRE( foo1.size() == 2 );
     REQUIRE( foo2.size() == 1 );
-    bt_list& foo1a = std::get<bt_list>(foo1.at("a"));
-    bt_list& foo1b = std::get<bt_list>(foo1.at("b"));
-    bt_list& foo2c = std::get<bt_list>(foo2.at("c"));
+    bt_list& foo1a = var::get<bt_list>(foo1.at("a"));
+    bt_list& foo1b = var::get<bt_list>(foo1.at("b"));
+    bt_list& foo2c = var::get<bt_list>(foo2.at("c"));
     std::list<int> foo1a_vals, foo1b_vals, foo2c_vals;
     for (auto& v : foo1a) foo1a_vals.push_back(lokimq::get_int<int>(v));
     for (auto& v : foo1b) foo1b_vals.push_back(lokimq::get_int<int>(v));
@@ -157,7 +157,7 @@ TEST_CASE("bt_value deserialization", "[bt][deserialization][bt_value]") {
     REQUIRE( foo1b_vals == std::list<int>{} );
     REQUIRE( foo2c_vals == std::list{{-5, 4}} );
 
-    REQUIRE( std::get<bt_list>(a.at("bar")).empty() );
+    REQUIRE( var::get<bt_list>(a.at("bar")).empty() );
 }
 
 TEST_CASE("bt tuple serialization", "[bt][tuple][serialization]") {