Define a preprocessor flag for enabling warnings on unused ConnectionHandle (#71)

* Define a preprocessor flag for enabling warnings on unused ConnectionHandle

* add KDBINDINGS_ENABLE_WARN_UNUSED as a cmake option

* move preprocessor macros directly in a standard header file

* explicity casts the return value to void to ignore warning for CI

Author: phyBrackets

examples/01-simple-connection/main.cpp

@@ -22,7 +22,7 @@ int main()
     Signal<std::string, int> signal;
 
     // Connect a lambda
-    signal.connect([](std::string arg1, int arg2) {
+    (void)signal.connect([](std::string arg1, int arg2) {
         std::cout << arg1 << " " << arg2 << std::endl;
     });
 

examples/04-simple-property/main.cpp

@@ -35,7 +35,7 @@ int main()
 {
     Widget w("A cool widget");
 
-    w.value.valueChanged().connect([](int newValue) {
+    (void)w.value.valueChanged().connect([](int newValue) {
         std::cout << "The new value is " << newValue << std::endl;
     });
 

examples/05-property-bindings/main.cpp

@@ -31,7 +31,7 @@ int main()
     Image img;
     std::cout << "The initial size of the image = " << img.byteSize.get() << " bytes" << std::endl;
 
-    img.byteSize.valueChanged().connect([](const int &newValue) {
+    (void)img.byteSize.valueChanged().connect([](const int &newValue) {
         std::cout << "The new size of the image = " << newValue << " bytes" << std::endl;
     });
 

examples/06-lazy-property-bindings/main.cpp

@@ -33,7 +33,7 @@ int main()
     Image img;
     std::cout << "The initial size of the image = " << img.byteSize.get() << " bytes" << std::endl;
 
-    img.byteSize.valueChanged().connect([](const int &newValue) {
+    (void)img.byteSize.valueChanged().connect([](const int &newValue) {
         std::cout << "The new size of the image = " << newValue << " bytes" << std::endl;
     });
 

src/kdbindings/CMakeLists.txt

@@ -6,6 +6,8 @@
 # Contact KDAB at <[email protected]> for commercial licensing options.
 #
 
+option(KDBINDINGS_ENABLE_WARN_UNUSED "Enable warnings for unused ConnectionHandles" ON)
+
 set(HEADERS
     binding.h
     binding_evaluator.h
@@ -20,6 +22,7 @@ set(HEADERS
     connection_evaluator.h
     connection_handle.h
     utils.h
+    KDBindingsConfig.h
 )
 
 if(CMAKE_VERSION VERSION_GREATER_EQUAL "3.19.0")
@@ -33,6 +36,11 @@ add_library(KDAB::KDBindings ALIAS KDBindings)
 set_target_properties(KDBindings PROPERTIES
   INTERFACE_COMPILE_FEATURES cxx_std_17
 )
+
+if(KDBINDINGS_ENABLE_WARN_UNUSED)
+  target_compile_definitions(KDBindings INTERFACE KDBINDINGS_ENABLE_WARN_UNUSED=1)
+endif()
+
 target_include_directories(KDBindings INTERFACE
   $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/..>
   $<INSTALL_INTERFACE:include>

src/kdbindings/KDBindingsConfig.h

@@ -0,0 +1,7 @@
+#pragma once
+
+#ifdef KDBINDINGS_ENABLE_WARN_UNUSED
+#define KDBINDINGS_WARN_UNUSED [[nodiscard]]
+#else
+#define KDBINDINGS_WARN_UNUSED
+#endif

src/kdbindings/signal.h

@@ -11,6 +11,7 @@
 
 #pragma once
 
+
 #include <assert.h>
 #include <memory>
 #include <stdexcept>
@@ -21,6 +22,8 @@
 #include <kdbindings/genindex_array.h>
 #include <kdbindings/utils.h>
 
+#include <kdbindings/KDBindingsConfig.h>
+
 /**
  * @brief The main namespace of the KDBindings library.
  *
@@ -253,7 +256,7 @@ class Signal
      * @return An instance of ConnectionHandle, that can be used to disconnect
      * or temporarily block the connection.
      */
-    ConnectionHandle connect(std::function<void(Args...)> const &slot)
+    KDBINDINGS_WARN_UNUSED ConnectionHandle connect(std::function<void(Args...)> const &slot)
     {
         ensureImpl();
 
@@ -278,7 +281,7 @@ class Signal
      * The Signal class itself is not thread-safe. While the ConnectionEvaluator is inherently
      * thread-safe, ensure that any concurrent access to this Signal is protected externally to maintain thread safety.
      */
-    ConnectionHandle connectDeferred(const std::shared_ptr<ConnectionEvaluator> &evaluator, std::function<void(Args...)> const &slot)
+    KDBINDINGS_WARN_UNUSED ConnectionHandle connectDeferred(const std::shared_ptr<ConnectionEvaluator> &evaluator, std::function<void(Args...)> const &slot)
     {
         ensureImpl();
 

tests/binding/tst_binding.cpp

@@ -185,15 +185,15 @@ TEST_CASE("Binding expression updates")
                                       Private::makeNode(input)),
                     evaluator)
         };
-        prop1.valueChanged().connect([&ordering](int) { ordering.emplace_back(1); });
+        (void)prop1.valueChanged().connect([&ordering](int) { ordering.emplace_back(1); });
 
         auto prop2 = Property<int>{
             std::make_unique<Binding<int>>(
                     Private::makeNode([](auto x) { return 3 * x; },
                                       Private::makeNode(input)),
                     evaluator)
         };
-        prop2.valueChanged().connect([&ordering](int) { ordering.emplace_back(2); });
+        (void)prop2.valueChanged().connect([&ordering](int) { ordering.emplace_back(2); });
 
         input = 8;
         evaluator.evaluateAll();

tests/property/tst_property.cpp

@@ -79,7 +79,7 @@ class ClassWithProperty
 
     ClassWithProperty()
     {
-        property().valueChanged.connect(
+        (void)property().valueChanged.connect(
                 [this]() {
                     this->changed.emit();
                 });
@@ -92,7 +92,7 @@ TEST_CASE("An object with a Signal that is wrapped in a property can emit the Si
     auto handler = [&called]() { called = true; };
 
     ClassWithProperty outer;
-    outer.changed.connect(handler);
+    (void)outer.changed.connect(handler);
 
     outer.property().emitSignal();
 
@@ -159,7 +159,7 @@ TEST_CASE("Signals")
         auto handler = [&notified]() { notified = true; };
 
         auto p = new Property<int>{ 5 };
-        p->destroyed().connect(handler);
+        (void)p->destroyed().connect(handler);
 
         delete p;
         REQUIRE(notified == true);
@@ -254,7 +254,7 @@ TEST_CASE("Property Updaters")
             updatedValue = value;
             slotCalled = true;
         };
-        property.valueChanged().connect(handler);
+        (void)property.valueChanged().connect(handler);
 
         updater->set(123);
         REQUIRE(property.get() == 123);
@@ -299,7 +299,7 @@ TEST_CASE("Moving")
 
         auto property = Property<std::unique_ptr<int>>{ std::make_unique<int>(42) };
         property.valueChanged().connect(handlerVoid);
-        property.valueChanged().connect(handlerValue);
+        (void)property.valueChanged().connect(handlerValue);
 
         auto movedProperty{ std::move(property) };
         movedProperty.set(std::make_unique<int>(123));

tests/signal/tst_signal.cpp

@@ -138,13 +138,13 @@ TEST_CASE("Signal connections")
         auto evaluator = std::make_shared<ConnectionEvaluator>();
 
         std::thread thread1([&] {
-            signal1.connectDeferred(evaluator, [&val](int value) {
+            (void)signal1.connectDeferred(evaluator, [&val](int value) {
                 val += value;
             });
         });
 
         std::thread thread2([&] {
-            signal2.connectDeferred(evaluator, [&val](int value) {
+            (void)signal2.connectDeferred(evaluator, [&val](int value) {
                 val += value;
             });
         });
@@ -169,11 +169,11 @@ TEST_CASE("Signal connections")
         int val2 = 4;
         auto evaluator = std::make_shared<ConnectionEvaluator>();
 
-        signal1.connectDeferred(evaluator, [&val1](int value) {
+        (void)signal1.connectDeferred(evaluator, [&val1](int value) {
             val1 += value;
         });
 
-        signal2.connectDeferred(evaluator, [&val2](int value) {
+        (void)signal2.connectDeferred(evaluator, [&val2](int value) {
             val2 += value;
         });
 
@@ -224,7 +224,7 @@ TEST_CASE("Signal connections")
         int val = 4;
         auto evaluator = std::make_shared<ConnectionEvaluator>();
 
-        signal.connectDeferred(evaluator, [&val](int value) {
+        (void)signal.connectDeferred(evaluator, [&val](int value) {
             val += value;
         });
 
@@ -246,7 +246,7 @@ TEST_CASE("Signal connections")
         bool anotherCalled = false;
 
         auto handle = signal->connectDeferred(evaluator, [&called]() { called = true; });
-        anotherSignal.connectDeferred(evaluator, [&anotherCalled]() { anotherCalled = true; });
+        (void)anotherSignal.connectDeferred(evaluator, [&anotherCalled]() { anotherCalled = true; });
 
         signal->emit();
         anotherSignal.emit();
@@ -368,7 +368,7 @@ TEST_CASE("Signal connections")
         });
 
         int lambdaCallCount2 = 0;
-        signal.connect([&]() {
+        (void)signal.connect([&]() {
             ++lambdaCallCount2;
         });
 
@@ -396,7 +396,7 @@ TEST_CASE("Signal connections")
         handle = &result;
 
         int lambdaCallCount2 = 0;
-        signal.connect([&]() {
+        (void)signal.connect([&]() {
             ++lambdaCallCount2;
         });
 
@@ -413,12 +413,12 @@ TEST_CASE("Signal connections")
     {
         Signal<> signal;
         int lambdaCallCount = 0;
-        signal.connect([&]() {
+        (void)signal.connect([&]() {
             ++lambdaCallCount;
         });
 
         int lambdaCallCount2 = 0;
-        signal.connect([&]() {
+        (void)signal.connect([&]() {
             ++lambdaCallCount2;
         });
 
@@ -454,7 +454,7 @@ TEST_CASE("Moving")
         auto handler = [&count]() { ++count; };
 
         Signal<> signal;
-        signal.connect(handler);
+        (void)signal.connect(handler);
 
         Signal<> movedSignal{ std::move(signal) };
         movedSignal.emit();
@@ -467,7 +467,7 @@ TEST_CASE("Moving")
         auto handler = [&count]() { ++count; };
 
         Signal<> signal;
-        signal.connect(handler);
+        (void)signal.connect(handler);
 
         Signal<> movedSignal = std::move(signal);
         movedSignal.emit();