solemnwarning/wxwidgets
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 4

Switch to a simpler wxString::Printf() implementation

Browse source 
Don't use std::index_sequence, as this has some advantages:

- This way doesn't require using another helper function, which is
  burdensome here and would be even more so if we had to do it for all
  the other wx vararg functions too.

- We don't need to require C++14 and can keep supporting even g++ 4.8.

The sole drawback is that we cannot pass the index of the argument to
wxArgNormalizer ctor any longer, which means that we can't validate it,
but this is addressed by validating the entire format string at once in
the new Validate() member function, which is also more efficient than
the old way for the format strings with more than one format specifier,
as they only need to be parsed once now, instead of having to do it for
each format specifier separately when GetArgumentType() is called.

This also means that we can't handle char arguments differently
depending on whether they are used with "%c" or "%d" format specifier.
This is a backwards-incompatible change, but should affect very few use
cases, so it seems to be worth breaking it to get the above benefits.
This commit is contained in:
Vadim Zeitlin 2022-11-22 19:42:43 +01:00
parent c453167c71
commit 275ab09ed7
3 changed files with 103 additions and 51 deletions
Download patch file Download diff file Expand all files Collapse all files

45
include/wx/string.h
View file

@ -43,7 +43,6 @@
#include "wx/beforestd.h"
#include <string>
#include <tuple>
#include <utility>
#include "wx/afterstd.h"
@ -2198,13 +2197,20 @@ public:
// formatted input/output
// as sprintf(), returns the number of characters written or < 0 on error
template <typename... Targs>
int Printf(const wxString& format, Targs... args)
int Printf(const wxFormatString& format, Targs... args)
{
// Package the arguments into a tuple to pass them on together with the
// indices that are needed by the implementation.
return DoPrintf(format,
std::make_tuple(args...),
std::index_sequence_for<Targs...>{});
format.Validate({wxFormatStringSpecifier<Targs>::value...});
#if wxUSE_UNICODE_UTF8
#if !wxUSE_UTF8_LOCALE_ONLY
if ( wxLocaleIsUtf8 )
#endif
return DoPrintfUtf8(format, wxArgNormalizerUtf8<Targs>{args, nullptr, 0}.get()...);
#endif // wxUSE_UNICODE_UTF8
#if !wxUSE_UTF8_LOCALE_ONLY
return DoPrintfWchar(format, wxArgNormalizerWchar<Targs>{args, nullptr, 0}.get()...);
#endif // !wxUSE_UTF8_LOCALE_ONLY
}
// as vprintf(), returns the number of characters written or < 0 on error
@ -3386,31 +3392,6 @@ public:
wxString& operator+=(wchar_t ch) { return *this += wxUniChar(ch); }
private:
template <typename ArgsTuple, std::size_t... Ns>
int DoPrintf(const wxFormatString& format,
const ArgsTuple& args,
std::index_sequence<Ns...>)
{
#if wxUSE_UNICODE_UTF8
#if !wxUSE_UTF8_LOCALE_ONLY
if ( wxLocaleIsUtf8 )
#endif
return DoPrintfUtf8
(
format,
wxNormalizeArgUtf8(std::get<Ns>(args), &format, Ns).get()...
);
#endif // wxUSE_UNICODE_UTF8
#if !wxUSE_UTF8_LOCALE_ONLY
return DoPrintfWchar
(
format,
wxNormalizeArgWchar(std::get<Ns>(args), &format, Ns).get()...
);
#endif // !wxUSE_UTF8_LOCALE_ONLY
}
#if !wxUSE_UTF8_LOCALE_ONLY
int DoPrintfWchar(const wxChar *format, ...);
#endif

35
include/wx/strvararg.h
View file

@ -20,6 +20,7 @@
#include <string>
#include <type_traits>
#include <vector>
#ifdef __cpp_lib_string_view
#include <string_view>
@ -195,6 +196,10 @@ public:
Arg_Unknown = 0x8000 // unrecognized specifier (likely error)
};
// Validate all format string parameters at once: the vector contains the
// format specifiers corresponding to the actually given arguments.
void Validate(const std::vector<int>& argTypes) const;
// returns the type of format specifier for n-th variadic argument (this is
// not necessarily n-th format specifier if positional specifiers are used);
// called by wxArgNormalizer<> specializations to get information about
@ -335,6 +340,13 @@ struct wxFormatStringArgumentFinder<wxWCharBuffer>
#define wxASSERT_ARG_TYPE(fmt, index, expected_mask) \
wxUnusedVar(fmt); \
wxUnusedVar(index)
// Also provide a trivial implementation of Validate() doing nothing in
// this case.
inline void
wxFormatString::Validate(const std::vector<int>& WXUNUSED(argTypes)) const
{
}
#endif // wxDEBUG_LEVEL/!wxDEBUG_LEVEL
@ -459,6 +471,10 @@ struct wxArgNormalizer
// to printf-like format string or nullptr if the variadic function doesn't
// use format string and 'index' is index of 'value' in variadic arguments
// list (starting at 1)
//
// Because the format string and index are used for checking for the format
// specifier mismatches and can be nullptr and 0, respectively, if they had
// been already checked using wxFormatString::Validate().
wxArgNormalizer(T value,
const wxFormatString *fmt, unsigned index)
: m_value(value)
@ -485,17 +501,6 @@ struct wxArgNormalizerWchar : public wxArgNormalizer<T>
: wxArgNormalizer<T>(value, fmt, index) {}
};
// Helper function for creating a wxArgNormalizerWchar<T> with type deduced
// from the type of the argument.
//
// NB: Unlike the class ctor, which uses 1-based indices, it takes 0-based
// index which makes it more convenient to use in pack expansions.
template<typename T>
wxArgNormalizerWchar<T>
wxNormalizeArgWchar(T value, const wxFormatString *fmt, unsigned indexFrom0)
{
return wxArgNormalizerWchar<T>(value, fmt, indexFrom0 + 1);
}
#endif // !wxUSE_UTF8_LOCALE_ONLY
// normalizer for passing arguments to functions working with UTF-8 encoded
@ -509,14 +514,6 @@ wxNormalizeArgWchar(T value, const wxFormatString *fmt, unsigned indexFrom0)
: wxArgNormalizer<T>(value, fmt, index) {}
};
// Same type-deducing helper as above, but for wxArgNormalizerUtf8<T>
template<typename T>
wxArgNormalizerUtf8<T>
wxNormalizeArgUtf8(T value, const wxFormatString *fmt, unsigned indexFrom0)
{
return wxArgNormalizerUtf8<T>(value, fmt, indexFrom0 + 1);
}
#define wxArgNormalizerNative wxArgNormalizerUtf8
#else // wxUSE_UNICODE_WCHAR
#define wxArgNormalizerNative wxArgNormalizerWchar

74
src/common/strvararg.cpp
View file

@ -732,6 +732,64 @@ wxFormatString::ArgumentType DoGetArgumentType(const CharType *format,
return ArgTypeFromParamType(parser.pspec[n-1]->m_type);
}
#if wxDEBUG_LEVEL
template<typename CharType>
void DoValidateFormat(const CharType* format, const std::vector<int>& argTypes)
{
wxPrintfConvSpecParser<CharType> parser(format);
// For the reasons mentioned in the comment in DoGetArgumentType() above,
// we ignore any extraneous argument types, so we only check that the
// format format specifiers we actually have match the types.
for ( unsigned n = 0; n < parser.nargs; ++n )
{
if ( n == argTypes.size() )
{
wxFAIL_MSG
(
wxString::Format
(
"Not enough arguments, %zu given but at least %u needed",
argTypes.size(),
parser.nargs
)
);
// Useless to continue further.
return;
}
auto const pspec = parser.pspec[n];
if ( !pspec )
{
wxFAIL_MSG
(
wxString::Format
(
"Missing format specifier for argument %u in \"%s\"",
n + 1, format
)
);
continue;
}
auto const ptype = ArgTypeFromParamType(pspec->m_type);
wxASSERT_MSG
(
(ptype & argTypes[n]) == ptype,
wxString::Format
(
"Format specifier mismatch for argument %u of \"%s\"",
n + 1, format
)
);
}
}
#endif // wxDEBUG_LEVEL
} // anonymous namespace
wxFormatString::ArgumentType wxFormatString::GetArgumentType(unsigned n) const
@ -748,3 +806,19 @@ wxFormatString::ArgumentType wxFormatString::GetArgumentType(unsigned n) const
wxFAIL_MSG( "unreachable code" );
return Arg_Unknown;
}
#if wxDEBUG_LEVEL
void wxFormatString::Validate(const std::vector<int>& argTypes) const
{
if ( m_char )
DoValidateFormat(m_char.data(), argTypes);
else if ( m_wchar )
DoValidateFormat(m_wchar.data(), argTypes);
else if ( m_str )
DoValidateFormat(m_str->wx_str(), argTypes);
else if ( m_cstr )
DoValidateFormat(m_cstr->AsInternal(), argTypes);
}
#endif // wxDEBUG_LEVEL