sdbus++: handle duplicate generated variant types
4ac7e56e8e18202fad3b2734346c6d4c7a6957fc added support for 'size_t'
and 'ssize_t' types to sdbus++. On some architectures these collide
with '[u]int32_t' or '[u]int64_t' types and so may not be added to a
combined variant (ie. variant<uint32_t, size_t> will fail to compile
on some architectures). The generated bindings in sdbus++ need a
variant from the set of all properties for some of the constructor
forms.
Create a template type which will formulate a non-duplicative type set
from which a variant can be constructed and modify the sdbus++ generator
to use this variant type instead of std::variant directly. Added test
cases here to cover this condition for 'size' and 'ssize' sdbus++ types.
It is possible this could have been resolved in the generator itself,
but then the generator would have needed to know which architecture the
generated bindings were going to be compiled for. This would have made
the header files incompatible between x86-64 and arm32. I chose instead
to keep the header files consistent across all architectures and let the
compiler make the type decision.
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I7db5ad2c32d86d37d920a1801cdabfcaeda89489
diff --git a/include/sdbusplus/utility/dedup_variant.hpp b/include/sdbusplus/utility/dedup_variant.hpp
new file mode 100644
index 0000000..e3352d7
--- /dev/null
+++ b/include/sdbusplus/utility/dedup_variant.hpp
@@ -0,0 +1,64 @@
+#pragma once
+#include <type_traits>
+#include <variant>
+
+/** See `sdbusplus::utility::dedup_variant` below. */
+
+namespace sdbusplus
+{
+namespace utility
+{
+
+namespace details
+{
+
+/** Find the deduplicated variant type.
+ *
+ * @tparam T - A type of the form 'variant<...>'.
+ * @tparam Unused - An empty type set (non-empty sets are matched in
+ * specialization).
+ *
+ * This template is only matched when Unused is empty, which means all
+ * types have been processed (deduplicated).
+ */
+template <typename T, typename... Unused>
+struct dedup_variant
+{
+ using type = T;
+};
+
+/** Find the deduplicated variant type.
+ *
+ * @tparam Done - The types which have already been deduplicated.
+ * @tparam First - The first type to be deduplicated.
+ * @tparam Rest - The remaining types to be deduplicated.
+ *
+ * This template specialization is matched when there are remaining
+ * items to be analyzed, since the 'First' is a stronger match than the
+ * (empty) 'Types' in the previous template.
+ *
+ * Check for First in Done and recursively create the variant type as
+ * appropriate (add First if First not in Done, skip otherwise).
+ */
+template <typename... Done, typename First, typename... Rest>
+struct dedup_variant<std::variant<Done...>, First, Rest...> :
+ public std::conditional_t<
+ (std::is_same_v<First, Done> || ...),
+ dedup_variant<std::variant<Done...>, Rest...>,
+ dedup_variant<std::variant<Done..., First>, Rest...>>
+{};
+
+} // namespace details
+
+/** This type is useful for generated code which may inadvertantly contain
+ * duplicate types if specified simply as `std::variant<A, B, C>`. Some
+ * types, such as `uint32_t` and `size_t` are the same on some architectures
+ * and different on others. `dedup_variant_t<uint32_t, size_t>` will evalute
+ * to `std::variant<uint32_t>` on architectures where there is a collision.
+ */
+template <typename T, typename... Types>
+using dedup_variant =
+ typename details::dedup_variant<std::variant<T>, Types...>::type;
+
+} // namespace utility
+} // namespace sdbusplus
diff --git a/test/server/Test.interface.yaml b/test/server/Test.interface.yaml
index ac36b84..30df1f9 100644
--- a/test/server/Test.interface.yaml
+++ b/test/server/Test.interface.yaml
@@ -14,5 +14,11 @@
- const
- name: Countable
type: size
+ - name: SignedCountable
+ type: ssize
+ - name: UnsignedInt32
+ type: uint32
+ - name: UnsignedInt64
+ type: uint64
- name: ObjectPath
type: object_path
diff --git a/test/utility/type_traits.cpp b/test/utility/type_traits.cpp
index 33bf8c1..0a61648 100644
--- a/test/utility/type_traits.cpp
+++ b/test/utility/type_traits.cpp
@@ -1,3 +1,4 @@
+#include <sdbusplus/utility/dedup_variant.hpp>
#include <sdbusplus/utility/type_traits.hpp>
#include <type_traits>
@@ -82,4 +83,13 @@
ASSERT_THAT(has_member_contains_v<Bar>, Eq(false));
}
+// Tests for dedup_variant.
+static_assert(std::is_same_v<std::variant<size_t>,
+ sdbusplus::utility::dedup_variant<size_t>>);
+static_assert(std::is_same_v<std::variant<char, size_t>,
+ sdbusplus::utility::dedup_variant<char, size_t>>);
+static_assert(std::is_same_v<
+ std::variant<uint32_t, uint64_t>,
+ sdbusplus::utility::dedup_variant<uint32_t, uint64_t, size_t>>);
+
} // namespace
diff --git a/tools/sdbusplus/templates/interface.server.cpp.mako b/tools/sdbusplus/templates/interface.server.cpp.mako
index c936151..6a12968 100644
--- a/tools/sdbusplus/templates/interface.server.cpp.mako
+++ b/tools/sdbusplus/templates/interface.server.cpp.mako
@@ -1,11 +1,10 @@
#include <algorithm>
#include <map>
+#include <sdbusplus/exception.hpp>
#include <sdbusplus/sdbus.hpp>
#include <sdbusplus/server.hpp>
-#include <sdbusplus/exception.hpp>
#include <string>
#include <tuple>
-#include <variant>
#include <${"/".join(interface.name.split('.') + [ 'server.hpp' ])}>
% for m in interface.methods + interface.properties + interface.signals:
diff --git a/tools/sdbusplus/templates/interface.server.hpp.mako b/tools/sdbusplus/templates/interface.server.hpp.mako
index aa30ebb..90c8418 100644
--- a/tools/sdbusplus/templates/interface.server.hpp.mako
+++ b/tools/sdbusplus/templates/interface.server.hpp.mako
@@ -1,11 +1,11 @@
#pragma once
#include <map>
-#include <string>
#include <sdbusplus/sdbus.hpp>
#include <sdbusplus/server.hpp>
+#include <sdbusplus/utility/dedup_variant.hpp>
+#include <string>
#include <systemd/sd-bus.h>
#include <tuple>
-#include <variant>
% for m in interface.methods + interface.properties + interface.signals:
${ m.cpp_prototype(loader, interface=interface, ptype='callback-hpp-includes') }
% endfor
@@ -64,7 +64,7 @@
% endfor
% if interface.properties:
- using PropertiesVariant = std::variant<
+ using PropertiesVariant = sdbusplus::utility::dedup_variant<
${",\n ".join(sorted(setOfPropertyTypes()))}>;
/** @brief Constructor to initialize the object from a map of