From 5c7da1b3578f8afca624c5a514648bc455ce0d87 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 16 Nov 2020 21:03:30 +0100 Subject: [PATCH 1/3] Get rid of CppUnit boiler plate in wxEvtHandler unit test No changes, just remove the useless test case class and use individual test cases instead of its methods. --- tests/events/evthandler.cpp | 149 +++++++++++++++--------------------- 1 file changed, 62 insertions(+), 87 deletions(-) diff --git a/tests/events/evthandler.cpp b/tests/events/evthandler.cpp index 931cdee8ba..2a1856d7f7 100644 --- a/tests/events/evthandler.cpp +++ b/tests/events/evthandler.cpp @@ -152,63 +152,10 @@ wxGCC_WARNING_RESTORE(unused-function) } // anonymous namespace -// -------------------------------------------------------------------------- -// test class -// -------------------------------------------------------------------------- - -class EvtHandlerTestCase : public CppUnit::TestCase +TEST_CASE("Event::BuiltinConnect", "[event][connect]") { -public: - EvtHandlerTestCase() {} - -private: - CPPUNIT_TEST_SUITE( EvtHandlerTestCase ); - CPPUNIT_TEST( BuiltinConnect ); - CPPUNIT_TEST( LegacyConnect ); - CPPUNIT_TEST( DisconnectWildcard ); - CPPUNIT_TEST( AutoDisconnect ); - CPPUNIT_TEST( BindFunction ); - CPPUNIT_TEST( BindStaticMethod ); - CPPUNIT_TEST( BindFunctor ); - CPPUNIT_TEST( BindMethod ); - CPPUNIT_TEST( BindMethodUsingBaseEvent ); - CPPUNIT_TEST( BindFunctionUsingBaseEvent ); - CPPUNIT_TEST( BindNonHandler ); - CPPUNIT_TEST( InvalidBind ); - CPPUNIT_TEST( UnbindFromHandler ); - CPPUNIT_TEST_SUITE_END(); - - void BuiltinConnect(); - void LegacyConnect(); - void DisconnectWildcard(); - void AutoDisconnect(); - void BindFunction(); - void BindStaticMethod(); - void BindFunctor(); - void BindMethod(); - void BindMethodUsingBaseEvent(); - void BindFunctionUsingBaseEvent(); - void BindNonHandler(); - void InvalidBind(); - void UnbindFromHandler(); - - - // these member variables exceptionally don't use "m_" prefix because - // they're used so many times MyHandler handler; - MyEvent e; - wxDECLARE_NO_COPY_CLASS(EvtHandlerTestCase); -}; - -// register in the unnamed registry so that these tests are run by default -CPPUNIT_TEST_SUITE_REGISTRATION( EvtHandlerTestCase ); - -// also include in its own registry so that these tests can be run alone -CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( EvtHandlerTestCase, "EvtHandlerTestCase" ); - -void EvtHandlerTestCase::BuiltinConnect() -{ handler.Connect(wxEVT_IDLE, wxIdleEventHandler(MyHandler::OnIdle)); handler.Disconnect(wxEVT_IDLE, wxIdleEventHandler(MyHandler::OnIdle)); @@ -234,8 +181,10 @@ void EvtHandlerTestCase::BuiltinConnect() handler.Unbind(wxEVT_IDLE, &MyHandler::StaticOnIdle); } -void EvtHandlerTestCase::LegacyConnect() +TEST_CASE("Event::LegacyConnect", "[event][connect]") { + MyHandler handler; + handler.Connect( LegacyEventType, (wxObjectEventFunction)&MyHandler::OnEvent ); handler.Connect( 0, LegacyEventType, (wxObjectEventFunction)&MyHandler::OnEvent ); handler.Connect( 0, 0, LegacyEventType, (wxObjectEventFunction)&MyHandler::OnEvent ); @@ -254,42 +203,47 @@ void EvtHandlerTestCase::LegacyConnect() handler.Disconnect( 0, 0, LegacyEventType, (wxObjectEventFunction)&MyHandler::OnEvent, NULL, &handler ); } -void EvtHandlerTestCase::DisconnectWildcard() +TEST_CASE("Event::DisconnectWildcard", "[event][connect][disconnect]") { + MyHandler handler; + // should be able to disconnect a different handler using "wildcard search" MyHandler sink; wxEvtHandler source; source.Connect(wxEVT_IDLE, wxIdleEventHandler(MyHandler::OnIdle), NULL, &sink); - CPPUNIT_ASSERT(source.Disconnect(wxID_ANY, wxEVT_IDLE)); + CHECK(source.Disconnect(wxID_ANY, wxEVT_IDLE)); // destruction of source and sink here should properly clean up the // wxEventConnectionRef without crashing } -void EvtHandlerTestCase::AutoDisconnect() +TEST_CASE("Event::AutoDisconnect", "[event][connect][disconnect]") { wxEvtHandler source; { MyHandler sink; source.Connect(wxEVT_IDLE, wxIdleEventHandler(MyHandler::OnIdle), NULL, &sink); // mismatched event type, so nothing should be disconnected - CPPUNIT_ASSERT(!source.Disconnect(wxEVT_THREAD, wxIdleEventHandler(MyHandler::OnIdle), NULL, &sink)); + CHECK(!source.Disconnect(wxEVT_THREAD, wxIdleEventHandler(MyHandler::OnIdle), NULL, &sink)); } // destruction of sink should have automatically disconnected it, so // there should be nothing to disconnect anymore - CPPUNIT_ASSERT(!source.Disconnect(wxID_ANY, wxEVT_IDLE)); + CHECK(!source.Disconnect(wxID_ANY, wxEVT_IDLE)); } -void EvtHandlerTestCase::BindFunction() +TEST_CASE("Event::BindFunction", "[event][bind]") { + MyHandler handler; + MyEvent e; + // function tests handler.Bind( MyEventType, GlobalOnMyEvent ); g_called.Reset(); handler.ProcessEvent(e); - CPPUNIT_ASSERT( g_called.function ); + CHECK( g_called.function ); handler.Unbind( MyEventType, GlobalOnMyEvent ); g_called.Reset(); handler.ProcessEvent(e); - CPPUNIT_ASSERT( !g_called.function ); // check that it was disconnected + CHECK( !g_called.function ); // check that it was disconnected handler.Bind( MyEventType, GlobalOnMyEvent, 0 ); handler.Unbind( MyEventType, GlobalOnMyEvent, 0 ); @@ -298,18 +252,21 @@ void EvtHandlerTestCase::BindFunction() handler.Unbind( MyEventType, GlobalOnMyEvent, 0, 0 ); } -void EvtHandlerTestCase::BindStaticMethod() +TEST_CASE("Event::BindStaticMethod", "[event][bind]") { + MyHandler handler; + MyEvent e; + // static method tests (this is same as functions but still test it just in // case we hit some strange compiler bugs) handler.Bind( MyEventType, &MyHandler::StaticOnMyEvent ); g_called.Reset(); handler.ProcessEvent(e); - CPPUNIT_ASSERT( g_called.smethod ); + CHECK( g_called.smethod ); handler.Unbind( MyEventType, &MyHandler::StaticOnMyEvent ); g_called.Reset(); handler.ProcessEvent(e); - CPPUNIT_ASSERT( !g_called.smethod ); + CHECK( !g_called.smethod ); handler.Bind( MyEventType, &MyHandler::StaticOnMyEvent, 0 ); handler.Unbind( MyEventType, &MyHandler::StaticOnMyEvent, 0 ); @@ -318,19 +275,22 @@ void EvtHandlerTestCase::BindStaticMethod() handler.Unbind( MyEventType, &MyHandler::StaticOnMyEvent, 0, 0 ); } -void EvtHandlerTestCase::BindFunctor() +TEST_CASE("Event::BindFunctor", "[event][bind]") { + MyHandler handler; + MyEvent e; + // generalized functor tests MyFunctor functor; handler.Bind( MyEventType, functor ); g_called.Reset(); handler.ProcessEvent(e); - CPPUNIT_ASSERT( g_called.functor ); + CHECK( g_called.functor ); handler.Unbind( MyEventType, functor ); g_called.Reset(); handler.ProcessEvent(e); - CPPUNIT_ASSERT( !g_called.functor ); + CHECK( !g_called.functor ); handler.Bind( MyEventType, functor, 0 ); handler.Unbind( MyEventType, functor, 0 ); @@ -343,26 +303,29 @@ void EvtHandlerTestCase::BindFunctor() // not work MyFunctor func; handler.Bind( MyEventType, MyFunctor() ); - CPPUNIT_ASSERT( !handler.Unbind( MyEventType, func )); + CHECK( !handler.Unbind( MyEventType, func )); handler.Bind( MyEventType, MyFunctor(), 0 ); - CPPUNIT_ASSERT( !handler.Unbind( MyEventType, func, 0 )); + CHECK( !handler.Unbind( MyEventType, func, 0 )); handler.Bind( MyEventType, MyFunctor(), 0, 0 ); - CPPUNIT_ASSERT( !handler.Unbind( MyEventType, func, 0, 0 )); + CHECK( !handler.Unbind( MyEventType, func, 0, 0 )); } -void EvtHandlerTestCase::BindMethod() +TEST_CASE("Event::BindMethod", "[event][bind]") { + MyHandler handler; + MyEvent e; + // class method tests handler.Bind( MyEventType, &MyHandler::OnMyEvent, &handler ); g_called.Reset(); handler.ProcessEvent(e); - CPPUNIT_ASSERT( g_called.method ); + CHECK( g_called.method ); handler.Unbind( MyEventType, &MyHandler::OnMyEvent, &handler ); g_called.Reset(); handler.ProcessEvent(e); - CPPUNIT_ASSERT( !g_called.method ); + CHECK( !g_called.method ); handler.Bind( MyEventType, &MyHandler::OnMyEvent, &handler, 0 ); handler.Unbind( MyEventType, &MyHandler::OnMyEvent, &handler, 0 ); @@ -371,19 +334,22 @@ void EvtHandlerTestCase::BindMethod() handler.Unbind( MyEventType, &MyHandler::OnMyEvent, &handler, 0, 0 ); } -void EvtHandlerTestCase::BindMethodUsingBaseEvent() +TEST_CASE("Event::BindMethodUsingBaseEvent", "[event][bind]") { + MyHandler handler; + MyEvent e; + // test connecting a method taking just wxEvent and not MyEvent: this // should work too if we don't need any MyEvent-specific information in the // handler handler.Bind( MyEventType, &MyHandler::OnEvent, &handler ); g_called.Reset(); handler.ProcessEvent(e); - CPPUNIT_ASSERT( g_called.method ); + CHECK( g_called.method ); handler.Unbind( MyEventType, &MyHandler::OnEvent, &handler ); g_called.Reset(); handler.ProcessEvent(e); - CPPUNIT_ASSERT( !g_called.method ); + CHECK( !g_called.method ); handler.Bind( MyEventType, &MyHandler::OnEvent, &handler, 0 ); handler.Unbind( MyEventType, &MyHandler::OnEvent, &handler, 0 ); @@ -393,19 +359,22 @@ void EvtHandlerTestCase::BindMethodUsingBaseEvent() } -void EvtHandlerTestCase::BindFunctionUsingBaseEvent() +TEST_CASE("Event::BindFunctionUsingBaseEvent", "[event][bind]") { + MyHandler handler; + MyEvent e; + // test connecting a function taking just wxEvent and not MyEvent: this // should work too if we don't need any MyEvent-specific information in the // handler handler.Bind( MyEventType, GlobalOnEvent ); g_called.Reset(); handler.ProcessEvent(e); - CPPUNIT_ASSERT( g_called.function ); + CHECK( g_called.function ); handler.Unbind( MyEventType, GlobalOnEvent ); g_called.Reset(); handler.ProcessEvent(e); - CPPUNIT_ASSERT( !g_called.function ); + CHECK( !g_called.function ); handler.Bind( MyEventType, GlobalOnEvent, 0 ); handler.Unbind( MyEventType, GlobalOnEvent, 0 ); @@ -416,22 +385,25 @@ void EvtHandlerTestCase::BindFunctionUsingBaseEvent() -void EvtHandlerTestCase::BindNonHandler() +TEST_CASE("Event::BindNonHandler", "[event][bind]") { + MyHandler handler; + MyEvent e; + // class method tests for class not derived from wxEvtHandler MySink sink; handler.Bind( MyEventType, &MySink::OnMyEvent, &sink ); g_called.Reset(); handler.ProcessEvent(e); - CPPUNIT_ASSERT( g_called.method ); + CHECK( g_called.method ); handler.Unbind( MyEventType, &MySink::OnMyEvent, &sink ); g_called.Reset(); handler.ProcessEvent(e); - CPPUNIT_ASSERT( !g_called.method ); + CHECK( !g_called.method ); } -void EvtHandlerTestCase::InvalidBind() +TEST_CASE("Event::InvalidBind", "[event][bind]") { // these calls shouldn't compile but we unfortunately can't check this // automatically, you need to uncomment them manually and test that @@ -482,7 +454,7 @@ struct Handler1 // Although this handler is bound, the second one below is bound // later and so will be called first and will disconnect this one // before it has a chance to be called. - CPPUNIT_FAIL("shouldn't be called"); + FAIL("shouldn't be called"); } }; @@ -510,8 +482,11 @@ private: wxDECLARE_NO_COPY_CLASS(Handler2); }; -void EvtHandlerTestCase::UnbindFromHandler() +TEST_CASE("Event::UnbindFromHandler", "[event][bind][unbind]") { + MyHandler handler; + MyEvent e; + Handler1 h1; handler.Bind(MyEventType, &Handler1::OnDontCall, &h1); From 1afd2248d74a4f24d54e280bcfcb651ce6999a1f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 16 Nov 2020 21:08:19 +0100 Subject: [PATCH 2/3] Enable C++11-only Bind() unit test for MSVS 2015+ too This might compile with even earlier MSVS versions, but it definitely does with the recent ones, so enable the test for this compiler, even if it still doesn't define __cplusplus to indicate C++11 conformance by default. --- tests/events/evthandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/events/evthandler.cpp b/tests/events/evthandler.cpp index 2a1856d7f7..7472b74241 100644 --- a/tests/events/evthandler.cpp +++ b/tests/events/evthandler.cpp @@ -501,7 +501,7 @@ TEST_CASE("Event::UnbindFromHandler", "[event][bind][unbind]") // result in compilation errors. // Note that this test will work only on C++11 compilers, so we test this only // for such compilers. -#if __cplusplus >= 201103 +#if __cplusplus >= 201103 || wxCHECK_VISUALC_VERSION(14) class HandlerNonPublic : protected wxEvtHandler { public: From f48099e00aa45ceeeba0a4b51b11fb9bf651d5f8 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 16 Nov 2020 22:41:47 +0100 Subject: [PATCH 3/3] Make Connect() work with overloaded event handlers in C++17 This used to work previously but got broken when using C++17 by c3810da549 (Allow using noexcept methods with event tables macros, 2020-04-09), as cast added to deal with noexcept handlers relied on argument type deduction which can't be done for overloaded functions. Fix this by extracting the event argument type from the function pointer type and specifying it explicitly, while still letting the compiler deduce the class. Add a test case checking that using overloaded event handlers compiles. See #18721. Closes #18896. --- include/wx/event.h | 57 +++++++++++++++++++++++-------------- tests/events/evthandler.cpp | 11 +++++++ 2 files changed, 47 insertions(+), 21 deletions(-) diff --git a/include/wx/event.h b/include/wx/event.h index fb58167edb..f71c271345 100644 --- a/include/wx/event.h +++ b/include/wx/event.h @@ -144,43 +144,58 @@ inline wxEventFunction wxEventFunctionCast(void (wxEvtHandler::*func)(T&)) wxGCC_WARNING_RESTORE_CAST_FUNCTION_TYPE() } -// Special hack to remove "noexcept" from the function type when using C++17 or -// later: static_cast<> below would fail to cast a member function pointer to a -// member of a derived class to the base class if noexcept is specified for the -// former, so remove it first if necessary. +// In good old pre-C++17 times we could just static_cast the event handler, +// defined in some class deriving from wxEvtHandler, to the "functype" which is +// a type of wxEvtHandler method. But with C++17 this doesn't work when the +// handler is a noexcept function, so we need to cast it to a noexcept function +// pointer first. #if __cplusplus >= 201703L namespace wxPrivate { -// Generic template version, doing nothing. -template -struct RemoveNoexceptEventHandler +// Cast to noexcept function type first if necessary. +template +constexpr auto DoCast(void (C::*pmf)(E&)) { - using type = T; + return static_cast(pmf); +} + +template +constexpr auto DoCast(void (C::*pmf)(E&) noexcept) +{ + return static_cast( + static_cast(pmf) + ); +} + +// Helper used to recover the type of the handler argument from the function +// type. This is required in order to explicitly pass this type to DoCast<> as +// the compiler would be unable to deduce it for overloaded functions. + +// Generic template version, doing nothing. +template +struct EventArgOf; + +// Specialization sufficient to cover all event handler function types. +template +struct EventArgOf +{ + using type = E; }; -// Specialization removing noexcept when it's specified. -// -// Note that this doesn't pretend to be generally suitable, this is just enough -// to work in our particular case. -template -struct RemoveNoexceptEventHandler -{ - using type = R (C::*)(E&); -}; } // namespace wxPrivate -#define wxREMOVE_NOEXCEPT_EVENT_HANDLER(pmf) \ - static_cast::type>(pmf) +#define wxEventHandlerNoexceptCast(functype, pmf) \ + wxPrivate::DoCast::type>(pmf) #else -#define wxREMOVE_NOEXCEPT_EVENT_HANDLER(pmf) pmf +#define wxEventHandlerNoexceptCast(functype, pmf) static_cast(pmf) #endif // Try to cast the given event handler to the correct handler type: #define wxEVENT_HANDLER_CAST( functype, func ) \ - wxEventFunctionCast(static_cast(wxREMOVE_NOEXCEPT_EVENT_HANDLER(&func))) + wxEventFunctionCast(wxEventHandlerNoexceptCast(functype, &func)) // The tag is a type associated to the event type (which is an integer itself, diff --git a/tests/events/evthandler.cpp b/tests/events/evthandler.cpp index 7472b74241..65616d4e48 100644 --- a/tests/events/evthandler.cpp +++ b/tests/events/evthandler.cpp @@ -101,6 +101,9 @@ public: void OnEvent(wxEvent&) { g_called.method = true; } void OnAnotherEvent(AnotherEvent&); void OnIdle(wxIdleEvent&) { g_called.method = true; } + + void OnOverloadedHandler(wxIdleEvent&) { } + void OnOverloadedHandler(wxThreadEvent&) { } }; // we can also handle events in classes not deriving from wxEvtHandler @@ -203,6 +206,14 @@ TEST_CASE("Event::LegacyConnect", "[event][connect]") handler.Disconnect( 0, 0, LegacyEventType, (wxObjectEventFunction)&MyHandler::OnEvent, NULL, &handler ); } +TEST_CASE("Event::ConnectOverloaded", "[event][connect]") +{ + MyHandler handler; + + handler.Connect(wxEVT_IDLE, wxIdleEventHandler(MyHandler::OnOverloadedHandler)); + handler.Connect(wxEVT_THREAD, wxThreadEventHandler(MyHandler::OnOverloadedHandler)); +} + TEST_CASE("Event::DisconnectWildcard", "[event][connect][disconnect]") { MyHandler handler;