Skip to content

Commit ffa7080

Browse files
committed
Merge #19256: gui: change combiner for signals to optional_last_value
f1a0314 gui: change combiner for signals to optional_last_value (Cory Fields) Pull request description: [`optional_last_value`](https://www.boost.org/doc/libs/1_73_0/doc/html/boost/signals2/optional_last_value.html), which does not throw, has replaced `last_value` as Boosts default combiner. Besides being better supported, it also doesn't trigger gcc's `-Wmaybe-unitialized` warning, presumably because exceptions no longer bubble-up out of signals: ```bash In file included from ui_interface.cpp:9: /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp: In member function 'boost::signals2::detail::signal_impl<R(Args ...), Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::result_type boost::signals2::detail::signal_impl<R(Args ...), Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::operator()(Args ...) [with Combiner = boost::signals2::last_value<bool>; Group = int; GroupCompare = std::less<int>; SlotFunction = boost::function<bool(const bilingual_str&, const std::__cxx11::basic_string<char>&, unsigned int)>; ExtendedSlotFunction = boost::function<bool(const boost::signals2::connection&, const bilingual_str&, const std::__cxx11::basic_string<char>&, unsigned int)>; Mutex = boost::signals2::mutex; R = bool; Args = {const bilingual_str&, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, unsigned int}]': /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp:54:36: warning: '*((void*)& value +1)' may be used uninitialized in this function [-Wmaybe-uninitialized] if(value) return value.get(); ^ /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp:43:21: note: '*((void*)& value +1)' was declared here optional<T> value; ^~~~~ /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp: In member function 'boost::signals2::detail::signal_impl<R(Args ...), Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::result_type boost::signals2::detail::signal_impl<R(Args ...), Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::operator()(Args ...) [with Combiner = boost::signals2::last_value<bool>; Group = int; GroupCompare = std::less<int>; SlotFunction = boost::function<bool(const bilingual_str&, const std::__cxx11::basic_string<char>&, const std::__cxx11::basic_string<char>&, unsigned int)>; ExtendedSlotFunction = boost::function<bool(const boost::signals2::connection&, const bilingual_str&, const std::__cxx11::basic_string<char>&, const std::__cxx11::basic_string<char>&, unsigned int)>; Mutex = boost::signals2::mutex; R = bool; Args = {const bilingual_str&, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, unsigned int}]': /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp:54:36: warning: '*((void*)& value +1)' may be used uninitialized in this function [-Wmaybe-uninitialized] if(value) return value.get(); ^ /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp:43:21: note: '*((void*)& value +1)' was declared here optional<T> value; ^~~~~ ``` The change in default happened in [Boost 1.39.0](https://www.boost.org/users/history/version_1_39_0.html) (along with the introduction of the Signals2 library. More information is also available here https://www.boost.org/doc/libs/1_73_0/doc/html/signals2/rationale.html#id-1.3.36.9.4: > The default combiner for Boost.Signals2 has changed from the last_value combiner used by default in the original Boost.Signals library. > This is because last_value requires that at least 1 slot be connected to the signal when it is invoked (except for the last_value<void> specialization). > In a multi-threaded environment where signal invocations and slot connections and disconnections may be happening concurrently, it is difficult to fulfill this requirement. When using optional_last_value, there is no requirement for slots to be connected when a signal is invoked, since in that case the combiner may simply return an empty boost::optional. ACKs for top commit: laanwj: ACK f1a0314 Tree-SHA512: 3600f85019a3591b141dc9207f8a7e66d16d9996cf97fdf08f5133a212d55c591955ab835ffbdca20b5d62711578bc305d5525c75546fa957f180192e2a80c1e
2 parents 2629174 + f1a0314 commit ffa7080

File tree

2 files changed

+6
-6
lines changed

2 files changed

+6
-6
lines changed

src/node/ui_interface.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@
66

77
#include <util/translation.h>
88

9-
#include <boost/signals2/last_value.hpp>
9+
#include <boost/signals2/optional_last_value.hpp>
1010
#include <boost/signals2/signal.hpp>
1111

1212
CClientUIInterface uiInterface;
1313

1414
struct UISignals {
15-
boost::signals2::signal<CClientUIInterface::ThreadSafeMessageBoxSig, boost::signals2::last_value<bool>> ThreadSafeMessageBox;
16-
boost::signals2::signal<CClientUIInterface::ThreadSafeQuestionSig, boost::signals2::last_value<bool>> ThreadSafeQuestion;
15+
boost::signals2::signal<CClientUIInterface::ThreadSafeMessageBoxSig, boost::signals2::optional_last_value<bool>> ThreadSafeMessageBox;
16+
boost::signals2::signal<CClientUIInterface::ThreadSafeQuestionSig, boost::signals2::optional_last_value<bool>> ThreadSafeQuestion;
1717
boost::signals2::signal<CClientUIInterface::InitMessageSig> InitMessage;
1818
boost::signals2::signal<CClientUIInterface::NotifyNumConnectionsChangedSig> NotifyNumConnectionsChanged;
1919
boost::signals2::signal<CClientUIInterface::NotifyNetworkActiveChangedSig> NotifyNetworkActiveChanged;
@@ -42,8 +42,8 @@ ADD_SIGNALS_IMPL_WRAPPER(NotifyBlockTip);
4242
ADD_SIGNALS_IMPL_WRAPPER(NotifyHeaderTip);
4343
ADD_SIGNALS_IMPL_WRAPPER(BannedListChanged);
4444

45-
bool CClientUIInterface::ThreadSafeMessageBox(const bilingual_str& message, const std::string& caption, unsigned int style) { return g_ui_signals.ThreadSafeMessageBox(message, caption, style); }
46-
bool CClientUIInterface::ThreadSafeQuestion(const bilingual_str& message, const std::string& non_interactive_message, const std::string& caption, unsigned int style) { return g_ui_signals.ThreadSafeQuestion(message, non_interactive_message, caption, style); }
45+
bool CClientUIInterface::ThreadSafeMessageBox(const bilingual_str& message, const std::string& caption, unsigned int style) { return g_ui_signals.ThreadSafeMessageBox(message, caption, style).value_or(false);}
46+
bool CClientUIInterface::ThreadSafeQuestion(const bilingual_str& message, const std::string& non_interactive_message, const std::string& caption, unsigned int style) { return g_ui_signals.ThreadSafeQuestion(message, non_interactive_message, caption, style).value_or(false);}
4747
void CClientUIInterface::InitMessage(const std::string& message) { return g_ui_signals.InitMessage(message); }
4848
void CClientUIInterface::NotifyNumConnectionsChanged(int newNumConnections) { return g_ui_signals.NotifyNumConnectionsChanged(newNumConnections); }
4949
void CClientUIInterface::NotifyNetworkActiveChanged(bool networkActive) { return g_ui_signals.NotifyNetworkActiveChanged(networkActive); }

test/lint/lint-includes.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ EXPECTED_BOOST_INCLUDES=(
6464
boost/preprocessor/cat.hpp
6565
boost/preprocessor/stringize.hpp
6666
boost/signals2/connection.hpp
67-
boost/signals2/last_value.hpp
67+
boost/signals2/optional_last_value.hpp
6868
boost/signals2/signal.hpp
6969
boost/test/unit_test.hpp
7070
boost/thread/condition_variable.hpp

0 commit comments

Comments
 (0)