From 176b9dde903476d528688d2282fb2f5823c14f2f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 10 May 2020 22:57:41 +0200 Subject: [PATCH 1/3] Fix wxString iterator comparison in C++20 In C++20 the reverse comparison operators are also considered when searching for the operator to use and a wrong operator was selected for comparisons between iterator and const_iterator, that would result in an infinite recursion at run-time. Fix this, thanks to the nice gcc 10 warning about it, by explicitly defining the operators for this overload set too instead of relying on implicit conversions. Although not all these overloads are necessary, and they are only necessary in C++20, it seems better to define all of them and always just to be perfectly explicit and clear, as this code is not exactly simple to follow. --- include/wx/string.h | 40 ++++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/include/wx/string.h b/include/wx/string.h index 418dd0d704..d95710424d 100644 --- a/include/wx/string.h +++ b/include/wx/string.h @@ -884,10 +884,18 @@ public: const_iterator operator-(ptrdiff_t n) const { return const_iterator(str(), wxStringOperations::AddToIter(m_cur, -n)); } - // Notice that comparison operators taking non-const iterator are not - // needed here because of the implicit conversion from non-const iterator - // to const ones ensure that the versions for const_iterator declared - // inside WX_STR_ITERATOR_IMPL can be used. + // Until C++20 we could avoid defining these comparison operators because + // the implicit conversion from iterator to const_iterator was used to + // reuse the operators defined inside WX_STR_ITERATOR_IMPL. However in + // C++20 the operator overloads with reversed arguments would be used + // instead, resulting in infinite recursion, so we do need them and, just + // for consistency, define them in all cases. + bool operator==(const iterator& i) const; + bool operator!=(const iterator& i) const; + bool operator<(const iterator& i) const; + bool operator>(const iterator& i) const; + bool operator<=(const iterator& i) const; + bool operator>=(const iterator& i) const; private: // for internal wxString use only: @@ -954,10 +962,13 @@ public: const_iterator operator-(ptrdiff_t n) const { return const_iterator(wxStringOperations::AddToIter(m_cur, -n)); } - // As in UTF-8 case above, we don't need comparison operators taking - // iterator because we have an implicit conversion from iterator to - // const_iterator so the operators declared by WX_STR_ITERATOR_IMPL will - // be used. + // See comment for comparison operators in the UTF-8 case above. + bool operator==(const iterator& i) const; + bool operator!=(const iterator& i) const; + bool operator<(const iterator& i) const; + bool operator>(const iterator& i) const; + bool operator<=(const iterator& i) const; + bool operator>=(const iterator& i) const; private: // for internal wxString use only: @@ -3934,6 +3945,19 @@ inline bool operator!=(const wxString& s, wchar_t c) { return !s.IsSameAs(c); } // wxString iterators comparisons +inline bool wxString::const_iterator::operator==(const iterator& i) const + { return *this == const_iterator(i); } +inline bool wxString::const_iterator::operator!=(const iterator& i) const + { return *this != const_iterator(i); } +inline bool wxString::const_iterator::operator<(const iterator& i) const + { return *this < const_iterator(i); } +inline bool wxString::const_iterator::operator>(const iterator& i) const + { return *this > const_iterator(i); } +inline bool wxString::const_iterator::operator<=(const iterator& i) const + { return *this <= const_iterator(i); } +inline bool wxString::const_iterator::operator>=(const iterator& i) const + { return *this >= const_iterator(i); } + inline bool wxString::iterator::operator==(const const_iterator& i) const { return i == *this; } inline bool wxString::iterator::operator!=(const const_iterator& i) const From 63626acbe4086e1c58ad5ffec26d998421a81c09 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 10 May 2020 23:00:00 +0200 Subject: [PATCH 2/3] Fix std::ostream::operator<<(wxScopedWCharBuffer) This never worked correctly as using operator<<() with wchar_t pointer just fell back to the overload for void pointers, i.e. printed out the address of the wide string, which wasn't especially useful, but with C++20 it doesn't even compile, as this overload is explicitly deleted. Fix both problems at once by actually doing something useful for it instead and printing out data in either current encoding or UTF-8 if converting it to the current encoding failed. --- include/wx/string.h | 2 -- src/common/string.cpp | 8 +++++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/wx/string.h b/include/wx/string.h index d95710424d..fe6394dbe5 100644 --- a/include/wx/string.h +++ b/include/wx/string.h @@ -4030,9 +4030,7 @@ namespace std WXDLLIMPEXP_BASE wxSTD ostream& operator<<(wxSTD ostream&, const wxString&); WXDLLIMPEXP_BASE wxSTD ostream& operator<<(wxSTD ostream&, const wxCStrData&); WXDLLIMPEXP_BASE wxSTD ostream& operator<<(wxSTD ostream&, const wxScopedCharBuffer&); -#ifndef __BORLANDC__ WXDLLIMPEXP_BASE wxSTD ostream& operator<<(wxSTD ostream&, const wxScopedWCharBuffer&); -#endif #if wxUSE_UNICODE && defined(HAVE_WOSTREAM) diff --git a/src/common/string.cpp b/src/common/string.cpp index d2bcac8b03..7f0a657366 100644 --- a/src/common/string.cpp +++ b/src/common/string.cpp @@ -207,12 +207,14 @@ wxSTD ostream& operator<<(wxSTD ostream& os, const wxScopedCharBuffer& str) return os << str.data(); } -#ifndef __BORLANDC__ wxSTD ostream& operator<<(wxSTD ostream& os, const wxScopedWCharBuffer& str) { - return os << str.data(); + // There is no way to write wide character data to std::ostream directly, + // but we need to define this operator for compatibility, as we provided it + // since basically always, even if it never worked correctly before. So do + // the only reasonable thing and output it as UTF-8. + return os << wxConvWhateverWorks.cWC2MB(str.data()); } -#endif #if wxUSE_UNICODE && defined(HAVE_WOSTREAM) From 499252ace84d6d4f683590f458c9e7f01cb40c52 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 10 May 2020 23:05:41 +0200 Subject: [PATCH 3/3] Fix unit tests compilation in C++20 There is no more overload of std::ostream::operator<<() for wchar_t in C++20, i.e. it is explicitly deleted, so we need to define some other way of printing wchar_t out from Catch macros. Do it by specializing Catch::StringMaker<> for it and outputting it either as a (7 bit) ASCII character, if this is what it is, or as a Unicode character code otherwise, as this will probably be more useful in case of a test failure. --- tests/testprec.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/testprec.h b/tests/testprec.h index 38e2ce14e0..0eaf103cfc 100644 --- a/tests/testprec.h +++ b/tests/testprec.h @@ -55,6 +55,31 @@ #define wxDEFAULT_MANTISSA_SIZE_3 #endif +// Many tests use wide characters or wide strings inside Catch macros, which +// requires converting them to string if the check fails. This falls back to +// std::ostream::operator<() by default, which never worked correctly, as there +// never was any overload for wchar_t and so it used something else, but in C++ +// 20 this overload is explicitly deleted, so it results in compile-time error. +// +// Hence define this specialization to allow compiling such comparisons. +namespace Catch +{ + +template <> +struct StringMaker +{ + static std::string convert(wchar_t wc) + { + if ( wc < 0x7f ) + return std::string(static_cast(wc), 1); + + return wxString::Format("U+%06X", wc).ToStdString(); + } +}; + +} // namespace Catch + + // thrown when assert fails in debug build class TestAssertFailure {