Skip to content

Commit

Permalink
Fixes CppMicroServices#852: adds a [[nodiscard]] to BundleContext::Re…
Browse files Browse the repository at this point in the history
…gisterService (CppMicroServices#863)

* noDiscard update first push

* Jeff's comments, c++14 in cmakelists for makefile test, comments to reference nodiscard

---

Adaptation for C++14:
* use [[nodiscard]] only when __has_cpp_attribute is available and
__has_cpp_attribute(nodiscard) is valid. Alternatively use
[[gnu::warn_unused_result]] if nodiscard is not available, but
gnu::warn_unused_result is.
* revert default C++ standard in doc/src/CMakeLists.txt back to C++14

Signed-off-by: Ingmar Sittl <ingmar.sittl@elektrobit.com>
  • Loading branch information
tcormackMW authored and insi-eb committed Aug 10, 2023
1 parent f8402cb commit 84522fe
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,8 @@ namespace cppmicroservices
auto iMap = std::make_shared<InterfaceMap>();
auto obj = std::make_shared<double>();
iMap->insert(std::make_pair("double", std::static_pointer_cast<void>(obj)));
fc.RegisterService(iMap);
auto reg = fc.RegisterService(iMap);
US_UNUSED(reg);

auto sRef = fc.GetServiceReference("double");
EXPECT_TRUE(static_cast<bool>(sRef));
Expand Down
19 changes: 16 additions & 3 deletions framework/include/cppmicroservices/BundleContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,9 @@ namespace cppmicroservices
* no properties.
* @return A <code>ServiceRegistration</code> object for use by the bundle
* registering the service to update the service's properties or to
* unregister the service.
* unregister the service. This object cannot be called from a discard-value
* expression as the intent is for the <code>ServiceRegistration</code>
* object is intended to be stored by the caller
*
* @throws std::runtime_error If this BundleContext is no longer valid, or if there are
case variants of the same key in the supplied properties map.
Expand All @@ -303,8 +305,15 @@ namespace cppmicroservices
* @see ServiceFactory
* @see PrototypeServiceFactory
*/
ServiceRegistrationU RegisterService(InterfaceMapConstPtr const& service,
ServiceProperties const& properties = ServiceProperties());
#ifdef __has_cpp_attribute
# if __has_cpp_attribute(nodiscard)
[[nodiscard]]
# elif __has_cpp_attribute(gnu::warn_unused_result)
[[gnu::warn_unused_result]]
# endif
#endif
ServiceRegistrationU
RegisterService(InterfaceMapConstPtr const& service, ServiceProperties const& properties = ServiceProperties());

/**
* Registers the specified service object with the specified properties
Expand Down Expand Up @@ -455,8 +464,10 @@ namespace cppmicroservices
{
auto& clazz = us_service_interface_iid<S>();
if (clazz.empty())
{
throw ServiceException("The service interface class has no "
"CPPMICROSERVICES_DECLARE_SERVICE_INTERFACE macro");
}
using BaseVectorT = std::vector<ServiceReferenceU>;
BaseVectorT serviceRefs = GetServiceReferences(clazz, filter);
std::vector<ServiceReference<S>> result;
Expand Down Expand Up @@ -521,8 +532,10 @@ namespace cppmicroservices
{
auto& clazz = us_service_interface_iid<S>();
if (clazz.empty())
{
throw ServiceException("The service interface class has no "
"CPPMICROSERVICES_DECLARE_SERVICE_INTERFACE macro");
}
return ServiceReference<S>(GetServiceReference(clazz));
}

Expand Down
15 changes: 9 additions & 6 deletions framework/test/bench/ServiceRegistryTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,9 @@ BENCHMARK_DEFINE_F(ServiceRegistryFixture, RegisterServices)
{
InterfaceMapPtr iMapCopy(std::make_shared<InterfaceMap>(*interfaceMap));
auto start = high_resolution_clock::now();
(void)fc.RegisterService(iMapCopy); // benchmark the call to RegisterService
auto reg = fc.RegisterService(iMapCopy); // benchmark the call to RegisterService
auto end = high_resolution_clock::now();
US_UNUSED(reg);
auto elapsed_seconds = duration_cast<duration<double>>(end - start);
state.SetIterationTime(elapsed_seconds.count());
}
Expand Down Expand Up @@ -118,12 +119,13 @@ BENCHMARK_DEFINE_F(ServiceRegistryFixture, RegisterServicesWithRank)
{
InterfaceMapPtr iMapCopy(std::make_shared<InterfaceMap>(*interfaceMap));
auto start = std::chrono::high_resolution_clock::now();
(void)fc.RegisterService(iMapCopy,
{
{Constants::SERVICE_RANKING,
Any(static_cast<int>(i))}
auto reg = fc.RegisterService(iMapCopy,
{
{Constants::SERVICE_RANKING,
Any(static_cast<int>(i))}
}); // benchmark the call to RegisterService
auto end = std::chrono::high_resolution_clock::now();
US_UNUSED(reg);
auto elapsed_seconds = std::chrono::duration_cast<std::chrono::duration<double>>(end - start);
state.SetIterationTime(elapsed_seconds.count());
}
Expand All @@ -147,11 +149,12 @@ BENCHMARK_DEFINE_F(ServiceRegistryFixture, FindServices)
auto regCount = state.range(0);
auto interfaceCount = state.range(1);
auto interfaceMap = MakeInterfaceMapWithNInterfaces(interfaceCount);
std::vector<ServiceRegistrationU> regs;

for (auto i = regCount; i > 0; --i)
{
InterfaceMapPtr iMapCopy(std::make_shared<InterfaceMap>(*interfaceMap));
fc.RegisterService(iMapCopy);
regs.emplace_back(fc.RegisterService(iMapCopy));
}

for (auto _ : state)
Expand Down
3 changes: 2 additions & 1 deletion framework/test/gtest/ServiceReferenceTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ TEST_F(ServiceReferenceTest, TestRegisterAndGetServiceReferenceTest)

InterfaceMap im;
im["ServiceNS::ITestServiceB"] = std::make_shared<TestServiceB>();
(void)context.RegisterService(std::make_shared<InterfaceMap const>(im));
auto reg = context.RegisterService(std::make_shared<InterfaceMap const>(im));
US_UNUSED(reg);

auto sr3 = context.GetServiceReference("ServiceNS::ITestServiceB");
ASSERT_EQ(sr3.GetInterfaceId(), "ServiceNS::ITestServiceB");
Expand Down

0 comments on commit 84522fe

Please sign in to comment.