From 405cfe7f3273869de173198ae3e98b1123ba9c3d Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 19 May 2021 19:14:57 +0100 Subject: [PATCH 1/4] Add ASSERT_NO_IGNORED_FLAGS() macro and use it in wxBoxSizer code No real changes, just refactor the asserts a bit before the upcoming changes and also try to make the messages more clear and useful. --- src/common/sizer.cpp | 65 ++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/src/common/sizer.cpp b/src/common/sizer.cpp index 127a557ed6..7b701059ef 100644 --- a/src/common/sizer.cpp +++ b/src/common/sizer.cpp @@ -146,6 +146,13 @@ static const int SIZER_FLAGS_MASK = #endif // wxDEBUG_LEVEL +#define ASSERT_NO_IGNORED_FLAGS_IMPL(f, value, name, explanation) \ + wxASSERT_MSG( !((f) & (value)), \ + name " will be ignored in this sizer: " explanation ) + +#define ASSERT_NO_IGNORED_FLAGS(f, flags, explanation) \ + ASSERT_NO_IGNORED_FLAGS_IMPL(f, flags, #flags, explanation) + #define ASSERT_INCOMPATIBLE_NOT_USED_IMPL(f, f1, n1, f2, n2) \ wxASSERT_MSG(((f) & (f1 | f2)) != (f1 | f2), \ n1 " and " n2 " can't be used together") @@ -1473,7 +1480,7 @@ wxSizerItem *wxGridSizer::DoInsert(size_t index, wxSizerItem *item) ( !(flags & (wxALIGN_BOTTOM | wxALIGN_CENTRE_VERTICAL)) || !(flags & (wxALIGN_RIGHT | wxALIGN_CENTRE_HORIZONTAL)), - wxS("wxEXPAND flag will be overridden by alignment flags") + "wxEXPAND flag will be overridden by alignment flags" ); } @@ -2083,10 +2090,10 @@ wxSizerItem *wxBoxSizer::DoInsert(size_t index, wxSizerItem *item) const int flags = item->GetFlag(); if ( IsVertical() ) { - wxASSERT_MSG + ASSERT_NO_IGNORED_FLAGS ( - !(flags & wxALIGN_BOTTOM), - wxS("Vertical alignment flags are ignored in vertical sizers") + flags, wxALIGN_BOTTOM, + "only horizontal alignment flags can be used in vertical sizers" ); // We need to accept wxALIGN_CENTRE_VERTICAL when it is combined with @@ -2094,49 +2101,43 @@ wxSizerItem *wxBoxSizer::DoInsert(size_t index, wxSizerItem *item) // and we accept it historically in wxSizer API. if ( !(flags & wxALIGN_CENTRE_HORIZONTAL) ) { - wxASSERT_MSG + ASSERT_NO_IGNORED_FLAGS ( - !(flags & wxALIGN_CENTRE_VERTICAL), - wxS("Vertical alignment flags are ignored in vertical sizers") - ); - } - - // Note that using alignment with wxEXPAND can make sense if wxSHAPED - // is also used, as the item doesn't necessarily fully expand in the - // other direction in this case. - if ( (flags & wxEXPAND) && !(flags & wxSHAPED) ) - { - wxASSERT_MSG - ( - !(flags & (wxALIGN_RIGHT | wxALIGN_CENTRE_HORIZONTAL)), - wxS("Horizontal alignment flags are ignored with wxEXPAND") + flags, wxALIGN_CENTRE_VERTICAL, + "only horizontal alignment flags can be used in vertical sizers" ); } } else // horizontal { - wxASSERT_MSG + ASSERT_NO_IGNORED_FLAGS ( - !(flags & wxALIGN_RIGHT), - wxS("Horizontal alignment flags are ignored in horizontal sizers") + flags, wxALIGN_RIGHT, + "only vertical alignment flags can be used in horizontal sizers" ); if ( !(flags & wxALIGN_CENTRE_VERTICAL) ) { - wxASSERT_MSG + ASSERT_NO_IGNORED_FLAGS ( - !(flags & wxALIGN_CENTRE_HORIZONTAL), - wxS("Horizontal alignment flags are ignored in horizontal sizers") + flags, wxALIGN_CENTRE_HORIZONTAL, + "only vertical alignment flags can be used in horizontal sizers" ); } + } - if ( (flags & wxEXPAND) && !(flags & wxSHAPED) ) - { - wxASSERT_MSG( - !(flags & (wxALIGN_BOTTOM | wxALIGN_CENTRE_VERTICAL)), - wxS("Vertical alignment flags are ignored with wxEXPAND") - ); - } + // Note that using alignment with wxEXPAND can make sense if wxSHAPED + // is also used, as the item doesn't necessarily fully expand in the + // other direction in this case. + if ( (flags & wxEXPAND) && !(flags & wxSHAPED) ) + { + ASSERT_NO_IGNORED_FLAGS + ( + flags, + wxALIGN_RIGHT | wxALIGN_CENTRE_HORIZONTAL | + wxALIGN_BOTTOM | wxALIGN_CENTRE_VERTICAL, + "wxEXPAND overrides alignment flags in box sizers" + ); } return wxSizer::DoInsert(index, item); From 5502d2d86bdf76ee25f9071d7cc9c196efca7319 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 19 May 2021 23:49:25 +0100 Subject: [PATCH 2/4] Make sizer flag asserts even more verbose and hopefully helpful Try to indicate that these asserts are informative and don't indicate a fatal problem. --- src/common/sizer.cpp | 53 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/src/common/sizer.cpp b/src/common/sizer.cpp index 7b701059ef..a06bba72a0 100644 --- a/src/common/sizer.cpp +++ b/src/common/sizer.cpp @@ -144,18 +144,55 @@ static const int SIZER_FLAGS_MASK = wxADD_FLAG(wxSHAPED, 0)))))))))))))))))); +namespace +{ + +wxString MakeFlagsCheckMessage(const char* start, const char* whatToRemove) +{ + return wxString::Format + ( + "%s" + "\n\nDO NOT PANIC !!\n\n" + "If you're an end user running a program not developed by you, " + "please ignore this message, it is harmless, and please try " + "reporting the problem to the program developers.\n" + "\n" + "If you're the developer, simply remove %s from your code to " + "avoid getting this message.\n", + start, + whatToRemove + ); +} + +} // anonymous namespace + #endif // wxDEBUG_LEVEL #define ASSERT_NO_IGNORED_FLAGS_IMPL(f, value, name, explanation) \ - wxASSERT_MSG( !((f) & (value)), \ - name " will be ignored in this sizer: " explanation ) + wxASSERT_MSG \ + ( \ + !((f) & (value)), \ + MakeFlagsCheckMessage \ + ( \ + name " will be ignored in this sizer: " explanation, \ + "this flag" \ + ) \ + ) #define ASSERT_NO_IGNORED_FLAGS(f, flags, explanation) \ ASSERT_NO_IGNORED_FLAGS_IMPL(f, flags, #flags, explanation) -#define ASSERT_INCOMPATIBLE_NOT_USED_IMPL(f, f1, n1, f2, n2) \ - wxASSERT_MSG(((f) & (f1 | f2)) != (f1 | f2), \ - n1 " and " n2 " can't be used together") +#define ASSERT_INCOMPATIBLE_NOT_USED_IMPL(f, f1, n1, f2, n2) \ + wxASSERT_MSG \ + ( \ + ((f) & (f1 | f2)) != (f1 | f2), \ + MakeFlagsCheckMessage \ + ( \ + "One of " n1 " and " n2 " will be ignored in this sizer: " \ + "they are incompatible and cannot be used together", \ + "one of these flags" \ + ) \ + ) #define ASSERT_INCOMPATIBLE_NOT_USED(f, f1, f2) \ ASSERT_INCOMPATIBLE_NOT_USED_IMPL(f, f1, #f1, f2, #f2) @@ -1480,7 +1517,11 @@ wxSizerItem *wxGridSizer::DoInsert(size_t index, wxSizerItem *item) ( !(flags & (wxALIGN_BOTTOM | wxALIGN_CENTRE_VERTICAL)) || !(flags & (wxALIGN_RIGHT | wxALIGN_CENTRE_HORIZONTAL)), - "wxEXPAND flag will be overridden by alignment flags" + MakeFlagsCheckMessage + ( + "wxEXPAND flag will be overridden by alignment flags", + "either wxEXPAND or the alignment in at least one direction" + ) ); } From 2e289d7231fee91051ccda9bfe68d9d5a2368bdc Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 20 May 2021 00:20:54 +0100 Subject: [PATCH 3/4] Add wxSizerFlags::DisableConsistencyChecks() This allows to (hopefully temporarily) disable size flag check asserts. --- docs/changes.txt | 4 +++- include/wx/sizer.h | 3 +++ interface/wx/sizer.h | 23 +++++++++++++++++++++++ src/common/sizer.cpp | 21 +++++++++++++++++---- 4 files changed, 46 insertions(+), 5 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index 21d6b8fc75..a189daf42e 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -25,7 +25,9 @@ Changes in behaviour not resulting in compilation errors (it used to succeed in wxMSW). - Using invalid flags with wxBoxSizer or wxGridSizer items now triggers asserts - when done from the code or error messages when done in XRC. + when done from the code or error messages when done in XRC. These asserts are + best avoided by fixing the flags, but wxSizerFlags::DisableConsistencyChecks() + can be used to globally suppress them until this can be done. - wxWS_EX_VALIDATE_RECURSIVELY is now the default behaviour, i.e. calling Validate() or TransferData{From,To}Window() will now also call the same diff --git a/include/wx/sizer.h b/include/wx/sizer.h index c1f234734f..16af934419 100644 --- a/include/wx/sizer.h +++ b/include/wx/sizer.h @@ -238,6 +238,9 @@ public: int GetFlags() const { return m_flags; } int GetBorderInPixels() const { return m_borderInPixels; } + // Disablee sizer flags (in)consistency asserts. + static void DisableConsistencyChecks(); + private: #ifdef wxNEEDS_BORDER_IN_PX static float DoGetDefaultBorderInPx(); diff --git a/interface/wx/sizer.h b/interface/wx/sizer.h index b2dab8d0ac..a1f7508e94 100644 --- a/interface/wx/sizer.h +++ b/interface/wx/sizer.h @@ -1480,6 +1480,29 @@ public: */ wxSizerFlags& CentreVertical(); + /** + Globally disable checks for sizer flag consistency in debug builds. + + By default, sizer classes such as wxBoxSizer and wxFlexGridSizer assert + when passed invalid flags, even though doing this usually doesn't + result in any catastrophic consequences and the invalid flags are + simply ignored later. Due to this, and the fact that these checks were + only added in wxWidgets 3.1, existing code may run into multiple + asserts warning about incorrect sizer flags use. Using this function + provides a temporary solution for avoiding such asserts when upgrading + to wxWidgets 3.1 from a previous version and will prevent such checks + from being done. + + Please do note that correcting the code by removing the invalid flags + remains a much better solution as these asserts may be very helpful to + understand why some code using sizer flags doesn't work as expected, so + using this function, especially permanently, rather than a temporary + workaround, is @e not recommended. + + @since 3.1.6 + */ + static void DisableConsistencyChecks(); + /** Sets the border in the given @a direction having twice the default border size. diff --git a/src/common/sizer.cpp b/src/common/sizer.cpp index a06bba72a0..6ed7b1d46d 100644 --- a/src/common/sizer.cpp +++ b/src/common/sizer.cpp @@ -147,6 +147,8 @@ static const int SIZER_FLAGS_MASK = namespace { +bool gs_disableFlagChecks = false; + wxString MakeFlagsCheckMessage(const char* start, const char* whatToRemove) { return wxString::Format @@ -158,7 +160,9 @@ wxString MakeFlagsCheckMessage(const char* start, const char* whatToRemove) "reporting the problem to the program developers.\n" "\n" "If you're the developer, simply remove %s from your code to " - "avoid getting this message.\n", + "avoid getting this message. You can also call " + "wxSizerFlags::DisableConsistencyChecks() to globally disable " + "all such checks, but this is strongly not recommended.", start, whatToRemove ); @@ -171,7 +175,7 @@ wxString MakeFlagsCheckMessage(const char* start, const char* whatToRemove) #define ASSERT_NO_IGNORED_FLAGS_IMPL(f, value, name, explanation) \ wxASSERT_MSG \ ( \ - !((f) & (value)), \ + gs_disableFlagChecks || !((f) & (value)), \ MakeFlagsCheckMessage \ ( \ name " will be ignored in this sizer: " explanation, \ @@ -185,7 +189,7 @@ wxString MakeFlagsCheckMessage(const char* start, const char* whatToRemove) #define ASSERT_INCOMPATIBLE_NOT_USED_IMPL(f, f1, n1, f2, n2) \ wxASSERT_MSG \ ( \ - ((f) & (f1 | f2)) != (f1 | f2), \ + gs_disableFlagChecks || ((f) & (f1 | f2)) != (f1 | f2), \ MakeFlagsCheckMessage \ ( \ "One of " n1 " and " n2 " will be ignored in this sizer: " \ @@ -203,6 +207,14 @@ wxString MakeFlagsCheckMessage(const char* start, const char* whatToRemove) ASSERT_INCOMPATIBLE_NOT_USED(f, wxALIGN_CENTRE_VERTICAL, wxALIGN_BOTTOM) +/* static */ +void wxSizerFlags::DisableConsistencyChecks() +{ +#if wxDEBUG_LEVEL + gs_disableFlagChecks = true; +#endif // wxDEBUG_LEVEL +} + void wxSizerItem::Init(const wxSizerFlags& flags) { Init(); @@ -1515,7 +1527,8 @@ wxSizerItem *wxGridSizer::DoInsert(size_t index, wxSizerItem *item) // Check that expansion will happen in at least one of the directions. wxASSERT_MSG ( - !(flags & (wxALIGN_BOTTOM | wxALIGN_CENTRE_VERTICAL)) || + gs_disableFlagChecks || + !(flags & (wxALIGN_BOTTOM | wxALIGN_CENTRE_VERTICAL)) || !(flags & (wxALIGN_RIGHT | wxALIGN_CENTRE_HORIZONTAL)), MakeFlagsCheckMessage ( From 1d6c740f3b35def74374505d13ca34d37f7d6808 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 20 May 2021 00:29:43 +0100 Subject: [PATCH 4/4] Disable sizer flag checks if WXSUPPRESS_SIZER_FLAGS_CHECK is set This provides a less intrusive, and also usable by the end users rather than only by the developers, way of doing the same thing as the just added wxSizerFlags::DisableConsistencyChecks() does. --- docs/changes.txt | 4 +++- docs/doxygen/overviews/envvars.h | 5 +++++ interface/wx/sizer.h | 4 ++++ src/common/sizer.cpp | 33 ++++++++++++++++++++++++++------ 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index a189daf42e..e92d92064d 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -27,7 +27,9 @@ Changes in behaviour not resulting in compilation errors - Using invalid flags with wxBoxSizer or wxGridSizer items now triggers asserts when done from the code or error messages when done in XRC. These asserts are best avoided by fixing the flags, but wxSizerFlags::DisableConsistencyChecks() - can be used to globally suppress them until this can be done. + can be used to globally suppress them until this can be done. Even less + intrusively, environment variable WXSUPPRESS_SIZER_FLAGS_CHECK can be set (to + any value) to achieve the same effect. - wxWS_EX_VALIDATE_RECURSIVELY is now the default behaviour, i.e. calling Validate() or TransferData{From,To}Window() will now also call the same diff --git a/docs/doxygen/overviews/envvars.h b/docs/doxygen/overviews/envvars.h index c869bc5faa..52c8b2e3c8 100644 --- a/docs/doxygen/overviews/envvars.h +++ b/docs/doxygen/overviews/envvars.h @@ -32,5 +32,10 @@ wxWidgets programs. set it to @c "CURL" to force using libcurl-based implementation under MSW or macOS platforms where the native implementation would be chosen by default.} +@itemdef{WXSUPPRESS_SIZER_FLAGS_CHECK, + If set, disables asserts about using invalid sizer flags in the code. + This can be helpful when running older programs recompiled with + wxWidgets 3.1 or later, as these asserts are mostly harmless and can + be safely ignored if the code works as expected.} */ diff --git a/interface/wx/sizer.h b/interface/wx/sizer.h index a1f7508e94..b9aa47c65f 100644 --- a/interface/wx/sizer.h +++ b/interface/wx/sizer.h @@ -1499,6 +1499,10 @@ public: using this function, especially permanently, rather than a temporary workaround, is @e not recommended. + Notice that the same effect as calling this function can be achieved by + setting the environment variable @c WXSUPPRESS_SIZER_FLAGS_CHECK to any + value. + @since 3.1.6 */ static void DisableConsistencyChecks(); diff --git a/src/common/sizer.cpp b/src/common/sizer.cpp index 6ed7b1d46d..755ba7fd3e 100644 --- a/src/common/sizer.cpp +++ b/src/common/sizer.cpp @@ -147,7 +147,23 @@ static const int SIZER_FLAGS_MASK = namespace { -bool gs_disableFlagChecks = false; +int gs_disableFlagChecks = -1; + +// Check condition taking gs_disableFlagChecks into account. +// +// Note that because this is not a macro, the condition is always evaluated, +// even if gs_disableFlagChecks is 0, but this shouldn't matter because the +// conditions used with this function are just simple bit checks. +bool CheckSizerFlags(bool cond) +{ + // Once-only initialization: check if disabled via environment. + if ( gs_disableFlagChecks == -1 ) + { + gs_disableFlagChecks = wxGetEnv("WXSUPPRESS_SIZER_FLAGS_CHECK", NULL); + } + + return gs_disableFlagChecks || cond; +} wxString MakeFlagsCheckMessage(const char* start, const char* whatToRemove) { @@ -159,6 +175,9 @@ wxString MakeFlagsCheckMessage(const char* start, const char* whatToRemove) "please ignore this message, it is harmless, and please try " "reporting the problem to the program developers.\n" "\n" + "You may also set WXSUPPRESS_SIZER_FLAGS_CHECK environment " + "variable to suppress all such checks when running this program.\n" + "\n" "If you're the developer, simply remove %s from your code to " "avoid getting this message. You can also call " "wxSizerFlags::DisableConsistencyChecks() to globally disable " @@ -175,7 +194,7 @@ wxString MakeFlagsCheckMessage(const char* start, const char* whatToRemove) #define ASSERT_NO_IGNORED_FLAGS_IMPL(f, value, name, explanation) \ wxASSERT_MSG \ ( \ - gs_disableFlagChecks || !((f) & (value)), \ + CheckSizerFlags(!((f) & (value))), \ MakeFlagsCheckMessage \ ( \ name " will be ignored in this sizer: " explanation, \ @@ -189,7 +208,7 @@ wxString MakeFlagsCheckMessage(const char* start, const char* whatToRemove) #define ASSERT_INCOMPATIBLE_NOT_USED_IMPL(f, f1, n1, f2, n2) \ wxASSERT_MSG \ ( \ - gs_disableFlagChecks || ((f) & (f1 | f2)) != (f1 | f2), \ + CheckSizerFlags(((f) & (f1 | f2)) != (f1 | f2)), \ MakeFlagsCheckMessage \ ( \ "One of " n1 " and " n2 " will be ignored in this sizer: " \ @@ -1527,9 +1546,11 @@ wxSizerItem *wxGridSizer::DoInsert(size_t index, wxSizerItem *item) // Check that expansion will happen in at least one of the directions. wxASSERT_MSG ( - gs_disableFlagChecks || - !(flags & (wxALIGN_BOTTOM | wxALIGN_CENTRE_VERTICAL)) || - !(flags & (wxALIGN_RIGHT | wxALIGN_CENTRE_HORIZONTAL)), + CheckSizerFlags + ( + !(flags & (wxALIGN_BOTTOM | wxALIGN_CENTRE_VERTICAL)) || + !(flags & (wxALIGN_RIGHT | wxALIGN_CENTRE_HORIZONTAL)) + ), MakeFlagsCheckMessage ( "wxEXPAND flag will be overridden by alignment flags",