From 32a97f214fdb4ec763874ce57a42a9084b6b0043 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stanislav=20Angelovi=C4=8D?= Date: Wed, 3 Apr 2024 20:40:28 +0200 Subject: [PATCH] fix: make Variant conversion operator explicit (#428) Having explicit conversion operator is a good practice according to the C++ core guidelines, as it makes the code safer and better follows the principle of least astonishment. Also, it is consistent with the standard library style, where wrappers like std::variant, std::any, std::optional... also do not provide an implicit conversion to the underlying type. Last but not least, it paves the way for the upcoming std::variant <-> sdbus::Variant implicit conversions without surprising behavior in some edge cases. --- docs/using-sdbus-c++.md | 1 + .../examplemanager-planet1-client-glue.h | 2 +- include/sdbus-c++/Types.h | 2 +- tests/integrationtests/DBusMethodsTests.cpp | 6 +++--- tests/integrationtests/integrationtests-proxy.h | 6 +++--- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/using-sdbus-c++.md b/docs/using-sdbus-c++.md index a6c127a..cf92c5c 100644 --- a/docs/using-sdbus-c++.md +++ b/docs/using-sdbus-c++.md @@ -1765,6 +1765,7 @@ sdbus-c++ v2 is a major release that comes with a number of breaking API/ABI/beh * `PollData::getRelativeTimeout()` return type was changed to `std::chrono::microseconds`. * `IConnection::processPendingRequest()` was renamed to `IConnection::processPendingEvent()`. * `Variant` constructor is now explicit. +* `Variant`'s conversion operator to the underlying type is now explicit. * `IProxy::getCurrentlyProcessedMessage()` now returns `Message` by value instead of a raw pointer to it. The caller assumes ownership of the message. * Object D-Bus API registration is now done through `IObject::addVTable()` method. The vtable gets active immediately. No `finishRegistration()` call is needed anymore. vtables can be added and removed dynamically at run time. In addition to API simplification this brings consistency with sd-bus API and increases flexibility. * Subscription to signals has been simplified. The subscription is active right after the `registerSignalHandler`/`uponSignal()` call. No need for the final call to `finishRegistration()`. diff --git a/examples/org.freedesktop.DBus.ObjectManager/examplemanager-planet1-client-glue.h b/examples/org.freedesktop.DBus.ObjectManager/examplemanager-planet1-client-glue.h index 913ad82..4b6847e 100644 --- a/examples/org.freedesktop.DBus.ObjectManager/examplemanager-planet1-client-glue.h +++ b/examples/org.freedesktop.DBus.ObjectManager/examplemanager-planet1-client-glue.h @@ -47,7 +47,7 @@ public: public: std::string Name() { - return proxy_->getProperty("Name").onInterface(INTERFACE_NAME); + return proxy_->getProperty("Name").onInterface(INTERFACE_NAME).get(); } private: diff --git a/include/sdbus-c++/Types.h b/include/sdbus-c++/Types.h index 0323e34..19bfd69 100644 --- a/include/sdbus-c++/Types.h +++ b/include/sdbus-c++/Types.h @@ -87,7 +87,7 @@ namespace sdbus { // Only allow conversion operator for true D-Bus type representations in C++ template ::is_valid>> - operator _ValueType() const + explicit operator _ValueType() const { return get<_ValueType>(); } diff --git a/tests/integrationtests/DBusMethodsTests.cpp b/tests/integrationtests/DBusMethodsTests.cpp index e8256f9..98ea768 100644 --- a/tests/integrationtests/DBusMethodsTests.cpp +++ b/tests/integrationtests/DBusMethodsTests.cpp @@ -107,9 +107,9 @@ TYPED_TEST(SdbusTestObject, CallsMethodWithStructVariantsAndGetMapSuccesfully) std::vector x{-2, 0, 2}; sdbus::Struct y{false, true}; std::map mapOfVariants = this->m_proxy->getMapOfVariants(x, y); - decltype(mapOfVariants) res{ {sdbus::Variant{-2}, sdbus::Variant{false}} - , {sdbus::Variant{0}, sdbus::Variant{false}} - , {sdbus::Variant{2}, sdbus::Variant{true}}}; + decltype(mapOfVariants) res{ {-2, sdbus::Variant{false}} + , {0, sdbus::Variant{false}} + , {2, sdbus::Variant{true}}}; ASSERT_THAT(mapOfVariants[-2].get(), Eq(res[-2].get())); ASSERT_THAT(mapOfVariants[0].get(), Eq(res[0].get())); diff --git a/tests/integrationtests/integrationtests-proxy.h b/tests/integrationtests/integrationtests-proxy.h index e161183..edae7a5 100644 --- a/tests/integrationtests/integrationtests-proxy.h +++ b/tests/integrationtests/integrationtests-proxy.h @@ -198,7 +198,7 @@ public: public: uint32_t action() { - return proxy_->getProperty("action").onInterface(INTERFACE_NAME); + return proxy_->getProperty("action").onInterface(INTERFACE_NAME).get(); } void action(const uint32_t& value) @@ -208,7 +208,7 @@ public: bool blocking() { - return proxy_->getProperty("blocking").onInterface(INTERFACE_NAME); + return proxy_->getProperty("blocking").onInterface(INTERFACE_NAME).get(); } void blocking(const bool& value) @@ -218,7 +218,7 @@ public: std::string state() { - return proxy_->getProperty("state").onInterface(INTERFACE_NAME); + return proxy_->getProperty("state").onInterface(INTERFACE_NAME).get(); } private: