Fixes #852: adds a [[nodiscard]] to BundleContext::RegisterService (#863

) * noDiscard update first push * Jeff's comments, c++14 in cmakelists for makefile test, comments to reference nodiscard
CppMicroServices · May 19, 2023 · 6ca5c26 · 6ca5c26 · github-actions · May 19, 2023
1 parent 062fd47
commit 6ca5c26
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 12 deletions.
diff --git a/compendium/DeclarativeServices/test/gtest/TestServiceComponentRuntimeImpl.cpp b/compendium/DeclarativeServices/test/gtest/TestServiceComponentRuntimeImpl.cpp
@@ -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));

diff --git a/doc/src/CMakeLists.txt b/doc/src/CMakeLists.txt
@@ -176,7 +176,7 @@ if(US_BUILD_TESTING)
   if(UNIX AND NOT APPLE AND BUILD_SHARED_LIBS)
     set(US_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${US_CXX_FLAGS}")
     # We need to supply the correct language flag to the Makefile
-    set(US_CXX_VER_FLAGS -std=c++14)
+    set(US_CXX_VER_FLAGS -std=c++17)
     usFunctionCheckCompilerFlags(${US_CXX_VER_FLAGS} _have_cxx14)
     if(NOT _have_cxx14)
       set(US_CXX_VER_FLAGS -std=c++0x)

diff --git a/framework/include/cppmicroservices/BundleContext.h b/framework/include/cppmicroservices/BundleContext.h
@@ -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.
@@ -303,8 +305,8 @@ namespace cppmicroservices
          * @see ServiceFactory
          * @see PrototypeServiceFactory
          */
-        ServiceRegistrationU RegisterService(InterfaceMapConstPtr const& service,
-                                             ServiceProperties const& properties = ServiceProperties());
+        [[nodiscard]] ServiceRegistrationU RegisterService(InterfaceMapConstPtr const& service,
+                                                           ServiceProperties const& properties = ServiceProperties());
 
         /**
          * Registers the specified service object with the specified properties
@@ -455,8 +457,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;
@@ -521,8 +525,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));
         }
 

diff --git a/framework/test/bench/ServiceRegistryTest.cpp b/framework/test/bench/ServiceRegistryTest.cpp
@@ -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());
         }
@@ -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());
         }
@@ -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)

diff --git a/framework/test/gtest/ServiceReferenceTest.cpp b/framework/test/gtest/ServiceReferenceTest.cpp
@@ -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");