Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/01-simple-connection/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});

Expand Down
2 changes: 1 addition & 1 deletion examples/04-simple-property/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});

Expand Down
2 changes: 1 addition & 1 deletion examples/05-property-bindings/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});

Expand Down
2 changes: 1 addition & 1 deletion examples/06-lazy-property-bindings/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});

Expand Down
8 changes: 8 additions & 0 deletions src/kdbindings/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
# Contact KDAB at <info@kdab.com> for commercial licensing options.
#

option(KDBINDINGS_ENABLE_WARN_UNUSED "Enable warnings for unused ConnectionHandles" ON)

set(HEADERS
binding.h
binding_evaluator.h
Expand All @@ -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")
Expand All @@ -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>
Expand Down
7 changes: 7 additions & 0 deletions src/kdbindings/KDBindingsConfig.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#pragma once

#ifdef KDBINDINGS_ENABLE_WARN_UNUSED
#define KDBINDINGS_WARN_UNUSED [[nodiscard]]
#else
#define KDBINDINGS_WARN_UNUSED
#endif
7 changes: 5 additions & 2 deletions src/kdbindings/signal.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

#pragma once


#include <assert.h>
#include <memory>
#include <stdexcept>
Expand All @@ -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.
*
Expand Down Expand Up @@ -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();

Expand All @@ -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();

Expand Down
4 changes: 2 additions & 2 deletions tests/binding/tst_binding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
10 changes: 5 additions & 5 deletions tests/property/tst_property.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class ClassWithProperty

ClassWithProperty()
{
property().valueChanged.connect(
(void)property().valueChanged.connect(
[this]() {
this->changed.emit();
});
Expand All @@ -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();

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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));
Expand Down
24 changes: 12 additions & 12 deletions tests/signal/tst_signal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
});
Expand All @@ -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;
});

Expand Down Expand Up @@ -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;
});

Expand All @@ -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();
Expand Down Expand Up @@ -368,7 +368,7 @@ TEST_CASE("Signal connections")
});

int lambdaCallCount2 = 0;
signal.connect([&]() {
(void)signal.connect([&]() {
++lambdaCallCount2;
});

Expand Down Expand Up @@ -396,7 +396,7 @@ TEST_CASE("Signal connections")
handle = &result;

int lambdaCallCount2 = 0;
signal.connect([&]() {
(void)signal.connect([&]() {
++lambdaCallCount2;
});

Expand All @@ -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;
});

Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down