From 4ff436ecc1f4b5eae7137e7cd275af56b343ee62 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 22 Jun 2023 17:31:39 +0200 Subject: [PATCH 01/13] Fix xkb_rule_names struct initializer Use "{}" to initialize all struct fields to the default values instead of "{0}" which only initializes the first of them. This also fixes a warning given when building with -Wextra. --- src/gtk/window.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gtk/window.cpp b/src/gtk/window.cpp index 04dc7d9df4..139fd3a5d3 100644 --- a/src/gtk/window.cpp +++ b/src/gtk/window.cpp @@ -248,7 +248,7 @@ public: if ( !m_state ) { m_ctx = xkb_context_new(XKB_CONTEXT_NO_FLAGS); - struct xkb_rule_names names = {0}; + xkb_rule_names names{}; names.layout = "us"; m_keymap = xkb_keymap_new_from_names(m_ctx, &names, XKB_KEYMAP_COMPILE_NO_FLAGS); m_state = xkb_state_new(m_keymap); From c2de3b1a5e632ab0c40364ad8d41391e810a8fe2 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 22 Jun 2023 18:13:45 +0200 Subject: [PATCH 02/13] Rewrite generic wxPaletteRefData to use vector Don't bother with manual memory allocation of palette entries, just store them in a vector. No real changes. --- src/generic/paletteg.cpp | 63 +++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/src/generic/paletteg.cpp b/src/generic/paletteg.cpp index 9515d29a07..76d076df6c 100644 --- a/src/generic/paletteg.cpp +++ b/src/generic/paletteg.cpp @@ -15,6 +15,8 @@ #include "wx/palette.h" +#include + //----------------------------------------------------------------------------- // wxPalette //----------------------------------------------------------------------------- @@ -24,34 +26,20 @@ struct wxPaletteEntry unsigned char red, green, blue; }; -class wxPaletteRefData : public wxGDIRefData +struct wxPaletteRefData : public wxGDIRefData { -public: - wxPaletteRefData(); - wxPaletteRefData(const wxPaletteRefData& palette); - virtual ~wxPaletteRefData(); + wxPaletteRefData() = default; + wxPaletteRefData(const wxPaletteRefData& other); - int m_count; - wxPaletteEntry *m_entries; + std::vector m_entries; }; -wxPaletteRefData::wxPaletteRefData() -{ - m_count = 0; - m_entries = nullptr; -} - +// We need to define the copy ctor explicitly because the base class copy +// ctor is deleted. wxPaletteRefData::wxPaletteRefData(const wxPaletteRefData& palette) + : wxGDIRefData() { - m_count = palette.m_count; - m_entries = new wxPaletteEntry[m_count]; - for ( int i = 0; i < m_count; i++ ) - m_entries[i] = palette.m_entries[i]; -} - -wxPaletteRefData::~wxPaletteRefData() -{ - delete[] m_entries; + m_entries = palette.m_entries; } //----------------------------------------------------------------------------- @@ -77,7 +65,7 @@ wxPalette::~wxPalette() int wxPalette::GetColoursCount() const { if (m_refData) - return M_PALETTEDATA->m_count; + return M_PALETTEDATA->m_entries.size(); return 0; } @@ -90,15 +78,16 @@ bool wxPalette::Create(int n, UnRef(); m_refData = new wxPaletteRefData(); - M_PALETTEDATA->m_count = n; - M_PALETTEDATA->m_entries = new wxPaletteEntry[n]; + M_PALETTEDATA->m_entries.resize(n); - wxPaletteEntry *e = M_PALETTEDATA->m_entries; - for (int i = 0; i < n; i++, e++) + int i = 0; + for ( wxPaletteEntry& e : M_PALETTEDATA->m_entries ) { - e->red = red[i]; - e->green = green[i]; - e->blue = blue[i]; + e.red = red[i]; + e.green = green[i]; + e.blue = blue[i]; + + ++i; } return true; @@ -113,15 +102,17 @@ int wxPalette::GetPixel( unsigned char red, int closest = 0; double d,distance = 1000.0; // max. dist is 256 - wxPaletteEntry *e = M_PALETTEDATA->m_entries; - for (int i = 0; i < M_PALETTEDATA->m_count; i++, e++) + int i = 0; + for ( const wxPaletteEntry& e : M_PALETTEDATA->m_entries ) { - if ((d = 0.299 * abs(red - e->red) + - 0.587 * abs(green - e->green) + - 0.114 * abs(blue - e->blue)) < distance) { + if ((d = 0.299 * abs(red - e.red) + + 0.587 * abs(green - e.green) + + 0.114 * abs(blue - e.blue)) < distance) { distance = d; closest = i; } + + ++i; } return closest; } @@ -134,7 +125,7 @@ bool wxPalette::GetRGB(int pixel, if ( !m_refData ) return false; - if ( pixel < 0 || pixel >= M_PALETTEDATA->m_count ) + if ( pixel < 0 || (unsigned)pixel >= M_PALETTEDATA->m_entries.size() ) return false; wxPaletteEntry& p = M_PALETTEDATA->m_entries[pixel]; From 6881109a22dac915909061cdcc17185bf29bf2f6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 22 Jun 2023 18:16:48 +0200 Subject: [PATCH 03/13] Avoid always true comparison in wxThread::SetPriority() Don't compare unsigned priority value with wxPRIORITY_MIN which is 0, this is useless and results in -Wtype-limits enabled by -Wextra. --- src/unix/threadpsx.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/unix/threadpsx.cpp b/src/unix/threadpsx.cpp index 9eeda7943e..65f0fc790b 100644 --- a/src/unix/threadpsx.cpp +++ b/src/unix/threadpsx.cpp @@ -1392,7 +1392,10 @@ wxThreadError wxThread::Run() void wxThread::SetPriority(unsigned int prio) { - wxCHECK_RET( wxPRIORITY_MIN <= prio && prio <= wxPRIORITY_MAX, + // Don't compare with wxPRIORITY_MIN as long as it is 0, as the comparison + // would be always true. + static_assert( wxPRIORITY_MIN == 0, "update the check below" ); + wxCHECK_RET( /* wxPRIORITY_MIN <= prio && */ prio <= wxPRIORITY_MAX, wxT("invalid thread priority") ); wxCriticalSectionLocker lock(m_critsect); From b692eaa11291608a38a4b6c16b0903179f78ae03 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 23 Jun 2023 16:35:20 +0200 Subject: [PATCH 04/13] Avoid missing initializer warnings in wxrc Just suppress the warnings because adding all the missing initializers would make the command line initialization less readable for no real gain. --- utils/wxrc/wxrc.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/utils/wxrc/wxrc.cpp b/utils/wxrc/wxrc.cpp index 4af226ba11..795b59e753 100644 --- a/utils/wxrc/wxrc.cpp +++ b/utils/wxrc/wxrc.cpp @@ -252,6 +252,8 @@ wxIMPLEMENT_APP_CONSOLE(XmlResApp); int XmlResApp::OnRun() { + wxGCC_WARNING_SUPPRESS(missing-field-initializers) + static const wxCmdLineEntryDesc cmdLineDesc[] = { { wxCMD_LINE_SWITCH, "h", "help", "show help message", wxCMD_LINE_VAL_NONE, wxCMD_LINE_OPTION_HELP }, @@ -275,6 +277,8 @@ int XmlResApp::OnRun() wxCMD_LINE_DESC_END }; + wxGCC_WARNING_RESTORE(missing-field-initializers) + wxCmdLineParser parser(cmdLineDesc, argc, argv); switch (parser.Parse()) From 3b0aecf2547a7544013c1f056dd4fe7067ffa7f7 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 3 Jul 2023 21:25:05 +0200 Subject: [PATCH 05/13] Disable missing initializers warnings in wxGTK webview code Just suppress the warnings, this is better than specifying useless initializers. --- src/gtk/webview_webkit2_extension.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/gtk/webview_webkit2_extension.cpp b/src/gtk/webview_webkit2_extension.cpp index fd8c828da4..6c777d96b6 100644 --- a/src/gtk/webview_webkit2_extension.cpp +++ b/src/gtk/webview_webkit2_extension.cpp @@ -356,12 +356,16 @@ wxgtk_webview_handle_method_call(GDBusConnection*, } } // extern "C" +wxGCC_WARNING_SUPPRESS(missing-field-initializers) + static const GDBusInterfaceVTable interface_vtable = { wxgtk_webview_handle_method_call, nullptr, nullptr }; +wxGCC_WARNING_RESTORE(missing-field-initializers) + static gboolean wxgtk_webview_dbus_peer_is_authorized(GCredentials *peer_credentials) From 41b61aad59b067b6b3bf5db9753baf71837fb2fc Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 3 Jul 2023 21:23:40 +0200 Subject: [PATCH 06/13] Initialize base class in copy ctor in the other ports too This was already done in wxGTK, do it in wxX11 and wxQt too as not doing it triggers a (not very useful, but not completely specious) warning when -Wextra is in effect. --- src/qt/brush.cpp | 3 ++- src/qt/cursor.cpp | 3 ++- src/qt/region.cpp | 1 + src/x11/bitmap.cpp | 2 ++ src/x11/brush.cpp | 1 + src/x11/colour.cpp | 1 + src/x11/pen.cpp | 1 + src/x11/region.cpp | 1 + 8 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/qt/brush.cpp b/src/qt/brush.cpp index 1b8eb04a2f..1e05a6ffd3 100644 --- a/src/qt/brush.cpp +++ b/src/qt/brush.cpp @@ -68,7 +68,8 @@ class wxBrushRefData: public wxGDIRefData } wxBrushRefData( const wxBrushRefData& data ) - : m_qtBrush(data.m_qtBrush) + : wxGDIRefData(), + m_qtBrush(data.m_qtBrush) { m_style = data.m_style; } diff --git a/src/qt/cursor.cpp b/src/qt/cursor.cpp index c90beb7fb4..826aa696ee 100644 --- a/src/qt/cursor.cpp +++ b/src/qt/cursor.cpp @@ -50,7 +50,8 @@ class wxCursorRefData: public wxGDIRefData { public: wxCursorRefData() {} - wxCursorRefData( const wxCursorRefData& data ) : m_qtCursor(data.m_qtCursor) {} + wxCursorRefData( const wxCursorRefData& data ) + : wxGDIRefData(), m_qtCursor(data.m_qtCursor) {} wxCursorRefData( QCursor &c ) : m_qtCursor(c) {} QCursor m_qtCursor; diff --git a/src/qt/region.cpp b/src/qt/region.cpp index 9d73ed80fb..eb62850b1c 100644 --- a/src/qt/region.cpp +++ b/src/qt/region.cpp @@ -361,6 +361,7 @@ wxRegionIterator::wxRegionIterator(const wxRegion& region) } wxRegionIterator::wxRegionIterator(const wxRegionIterator& ri) + : wxObject() { m_qtRects = new QVector< QRect >( *ri.m_qtRects ); m_pos = ri.m_pos; diff --git a/src/x11/bitmap.cpp b/src/x11/bitmap.cpp index 745429f5ed..b7e58fb082 100644 --- a/src/x11/bitmap.cpp +++ b/src/x11/bitmap.cpp @@ -59,6 +59,7 @@ wxMask::wxMask() } wxMask::wxMask(const wxMask& mask) + : wxObject() { m_display = mask.m_display; if ( !mask.m_bitmap ) @@ -285,6 +286,7 @@ wxBitmapRefData::wxBitmapRefData() } wxBitmapRefData::wxBitmapRefData(const wxBitmapRefData& data) + : wxGDIRefData() { m_pixmap = 0; m_bitmap = 0; diff --git a/src/x11/brush.cpp b/src/x11/brush.cpp index 9808b86313..b80eaeede9 100644 --- a/src/x11/brush.cpp +++ b/src/x11/brush.cpp @@ -32,6 +32,7 @@ public: } wxBrushRefData( const wxBrushRefData& data ) + : wxGDIRefData() { m_style = data.m_style; m_stipple = data.m_stipple; diff --git a/src/x11/colour.cpp b/src/x11/colour.cpp index b222d2d124..662632bde6 100644 --- a/src/x11/colour.cpp +++ b/src/x11/colour.cpp @@ -38,6 +38,7 @@ public: } wxColourRefData(const wxColourRefData& data) + : wxGDIRefData() { m_color = data.m_color; m_colormap = data.m_colormap; diff --git a/src/x11/pen.cpp b/src/x11/pen.cpp index d54553e9d2..38d86b951f 100644 --- a/src/x11/pen.cpp +++ b/src/x11/pen.cpp @@ -37,6 +37,7 @@ public: } wxPenRefData( const wxPenRefData& data ) + : wxGDIRefData() { m_style = data.m_style; m_width = data.m_width; diff --git a/src/x11/region.cpp b/src/x11/region.cpp index 6d32e2ab0e..cb07d6d913 100644 --- a/src/x11/region.cpp +++ b/src/x11/region.cpp @@ -39,6 +39,7 @@ public: } wxRegionRefData(const wxRegionRefData& refData) + : wxGDIRefData() { m_region = XCreateRegion(); XUnionRegion( refData.m_region, m_region, m_region ); From 0f73473ae6d56a2ff8f811ed6f6a967fcd3aa9aa Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 3 Jul 2023 20:54:09 +0200 Subject: [PATCH 07/13] Update Scintilla submodules to avoid warnings with -Wextra No real changes, just suppress some warnings (that we don't really care about as we won't be fixing warnings in 3rd party code anyhow). --- src/stc/lexilla | 2 +- src/stc/scintilla | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/stc/lexilla b/src/stc/lexilla index 7cab74cefa..e1fc052077 160000 --- a/src/stc/lexilla +++ b/src/stc/lexilla @@ -1 +1 @@ -Subproject commit 7cab74cefa54851e99b8c06bd6e2cd659b4958ae +Subproject commit e1fc0520777932a6c2d5d82eca4911ee264e7b8d diff --git a/src/stc/scintilla b/src/stc/scintilla index b662e55b3d..0b90f31ced 160000 --- a/src/stc/scintilla +++ b/src/stc/scintilla @@ -1 +1 @@ -Subproject commit b662e55b3d8143bdad94e9a373679c0bfa3f834d +Subproject commit 0b90f31ced23241054e8088abb50babe9a44ae67 From 9952af8e9364b7a25387f896102196717e84ce2b Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 3 Jul 2023 23:58:23 +0200 Subject: [PATCH 08/13] Define explicit copy ctor for classes with assignment operator This avoids clang -Wdeprecated-copy enabled by -Wextra when building the test. --- tests/hashes/hashes.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/hashes/hashes.cpp b/tests/hashes/hashes.cpp index 82e9de6afc..b4fb8fb162 100644 --- a/tests/hashes/hashes.cpp +++ b/tests/hashes/hashes.cpp @@ -497,6 +497,8 @@ struct MyStruct class MyHash { public: + MyHash() = default; + MyHash(const MyHash&) = default; unsigned long operator()(const MyStruct& s) const { return m_dummy(s.ptr); } MyHash& operator=(const MyHash&) { return *this; } @@ -507,6 +509,8 @@ private: class MyEqual { public: + MyEqual() = default; + MyEqual(const MyEqual&) = default; bool operator()(const MyStruct& s1, const MyStruct& s2) const { return s1.ptr == s2.ptr; } MyEqual& operator=(const MyEqual&) { return *this; } From b6d082a7a6167f0a4094c78991abbf7e9d383acc Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 4 Jul 2023 01:43:55 +0200 Subject: [PATCH 09/13] Suppress warning in tests checking legacy Connect() This code can't be modified as it tests for compatibility with the existing code of this form, so just disable the warnings it now triggers with -Wextra. --- tests/events/evthandler.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/events/evthandler.cpp b/tests/events/evthandler.cpp index f7c5adc256..e0c60d4e36 100644 --- a/tests/events/evthandler.cpp +++ b/tests/events/evthandler.cpp @@ -165,10 +165,13 @@ TEST_CASE("Event::BuiltinConnect", "[event][connect]") handler.Connect(wxEVT_IDLE, wxIdleEventHandler(MyHandler::OnIdle), nullptr, &handler); handler.Disconnect(wxEVT_IDLE, wxIdleEventHandler(MyHandler::OnIdle), nullptr, &handler); - // using casts like this is even uglier than using wxIdleEventHandler but - // it should still continue to work for compatibility + // using casts like this is even uglier than using wxIdleEventHandler and + // results in warnings with gcc, but it should still continue to work for + // compatibility + wxGCC_WARNING_SUPPRESS_CAST_FUNCTION_TYPE() handler.Connect(wxEVT_IDLE, (wxObjectEventFunction)(wxEventFunction)&MyHandler::OnIdle); handler.Disconnect(wxEVT_IDLE, (wxObjectEventFunction)(wxEventFunction)&MyHandler::OnIdle); + wxGCC_WARNING_RESTORE_CAST_FUNCTION_TYPE() handler.Bind(wxEVT_IDLE, GlobalOnIdle); handler.Unbind(wxEVT_IDLE, GlobalOnIdle); From 008e72c3ac79a5f875b209ce975237febc3b94db Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 4 Jul 2023 01:47:07 +0200 Subject: [PATCH 10/13] Avoid using implicit assignment operator in wxAny test This implicitly declared operator is deprecated because MyClass has an explicitly defined copy ctor, so we either need to define the assignment explicitly too or just not use it at all, as done by this commit. --- tests/any/anytest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/any/anytest.cpp b/tests/any/anytest.cpp index 0e79561596..463bf2af73 100644 --- a/tests/any/anytest.cpp +++ b/tests/any/anytest.cpp @@ -479,8 +479,8 @@ class wxMyVariantData : public wxVariantData { public: wxMyVariantData(const MyClass& value) + : m_value(value) { - m_value = value; } virtual bool Eq(wxVariantData& WXUNUSED(data)) const override From 4040e35f41ba3a505b8ea6b104e6083969e344c9 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 4 Jul 2023 02:03:35 +0200 Subject: [PATCH 11/13] Use wxEVENT_HANDLER_CAST() in socket sample This macro avoids a -Wcast-function-type from gcc 8+. --- samples/sockets/baseclient.cpp | 2 +- samples/sockets/baseserver.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/sockets/baseclient.cpp b/samples/sockets/baseclient.cpp index 7143c10db5..9cfd83fa4f 100644 --- a/samples/sockets/baseclient.cpp +++ b/samples/sockets/baseclient.cpp @@ -27,7 +27,7 @@ #include "wx/thread.h" const wxEventType wxEVT_WORKER = wxNewEventType(); -#define EVT_WORKER(func) wxDECLARE_EVENT_TABLE_ENTRY( wxEVT_WORKER, -1, -1, (wxObjectEventFunction) (wxEventFunction) (WorkerEventFunction) & func, (wxObject *) nullptr ), +#define EVT_WORKER(func) wxDECLARE_EVENT_TABLE_ENTRY( wxEVT_WORKER, -1, -1, wxEVENT_HANDLER_CAST(WorkerEventFunction, func), (wxObject *) nullptr ), const int timeout_val = 1000; diff --git a/samples/sockets/baseserver.cpp b/samples/sockets/baseserver.cpp index 760387afc3..97cb2f9570 100644 --- a/samples/sockets/baseserver.cpp +++ b/samples/sockets/baseserver.cpp @@ -64,7 +64,7 @@ const char *GetSocketErrorMsg(int pSockError) //event sent by workers to server class //after client is served const wxEventType wxEVT_WORKER = wxNewEventType(); -#define EVT_WORKER(func) wxDECLARE_EVENT_TABLE_ENTRY( wxEVT_WORKER, -1, -1, (wxObjectEventFunction) (wxEventFunction) (WorkerEventFunction) & func, (wxObject *) nullptr ), +#define EVT_WORKER(func) wxDECLARE_EVENT_TABLE_ENTRY( wxEVT_WORKER, -1, -1, wxEVENT_HANDLER_CAST(WorkerEventFunction, func), (wxObject *) nullptr ), class WorkerEvent : public wxEvent { From e802eaa44d410e8e77e18006da12a3756919d1a5 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 4 Jul 2023 02:07:26 +0200 Subject: [PATCH 12/13] Suppress a warning for g_object_ref_sink() use in widgets sample The glib macro always triggers this warning when -Wextra is on, so we have no choice but to suppress it. --- samples/widgets/native.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/samples/widgets/native.cpp b/samples/widgets/native.cpp index 19e1c27b70..b27da69ed0 100644 --- a/samples/widgets/native.cpp +++ b/samples/widgets/native.cpp @@ -170,7 +170,9 @@ public: ); #endif // GTK+ 3.6/earlier + wxGCC_WARNING_SUPPRESS(ignored-qualifiers) g_object_ref_sink(widget); + wxGCC_WARNING_RESTORE(ignored-qualifiers) (void)Create(parent, wxID_ANY, widget); } From 78760c3538b41cbd7d576b860137c855f5355462 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 22 Jun 2023 17:36:25 +0200 Subject: [PATCH 13/13] Enable -Wextra for the CI builds Try detecting more warnings that can be useful when building the library. Don't use -Wextra with gcc 4.8 which generates several completely spurious warnings not generated by later compiler versions and which are not worth fixing/working around. --- .github/workflows/ci.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 829a872dcd..3bc6c9421b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -93,6 +93,7 @@ jobs: compiler: g++-4.8 container: ubuntu:18.04 configure_flags: --disable-shared + allow_extra_warnings: true use_xvfb: true - name: Ubuntu 18.04 wxGTK 3 compatible 3.0 runner: ubuntu-latest @@ -233,7 +234,10 @@ jobs: esac if [ -z ${{ matrix.allow_warnings }} ]; then - error_opts="-Werror $allow_warn_opt" + if [ -z ${{ matrix.allow_extra_warnings }} ]; then + error_opts="-Wextra" + fi + error_opts="$error_opts -Werror $allow_warn_opt" echo "wxMAKEFILE_ERROR_CXXFLAGS=$error_opts" >> $GITHUB_ENV echo "wxMAKEFILE_CXXFLAGS=$wxMAKEFILE_CXXFLAGS $error_opts" >> $GITHUB_ENV fi