On 22/05/21 22:08 +0200, François Dumont via Libstdc++ wrote:
Here is the part of the libbacktrace patch with the enhancement to
the rendering of assert message.
It only contains one real fix, the rendering of address. In 2
places it was done with "0x%p", so resulting in something like:
0x0x012345678
Otherwise it is just enhancements, mostly avoiding intermediate
buffering.
I am adding the _Parameter::_Named type to help on the rendering.
I hesitated in doing the same for the _M_iterator type but
eventually managed it with a template function.
libstdc++: [_GLIBCXX_DEBUG] Enhance rendering of assert message
Avoid building an intermediate buffer to print to stderr, push
directly to stderr.
libstdc++-v3/ChangeLog:
* include/debug/formatter.h
(_Error_formatter::_Parameter::_Named): New.
(_Error_formatter::_Parameter::_Type): Inherit latter.
(_Error_formatter::_Parameter::_M_integer): Likewise.
(_Error_formatter::_Parameter::_M_string): Likewise.
* src/c++11/debug.cc: Include <cstring>.
(_Print_func_t): New.
(print_raw(PrintContext&, const char*, ptrdiff_t)): New.
(print_word): Use '%.*s' format in fprintf to render
only expected number of chars.
(pretty_print(PrintContext&, const char*,
_Print_func_t)): New.
(print_type): Rename in...
(print_type_info): ...this. Use pretty_print.
(print_address, print_integer): New.
(print_named_name, print_iterator_constness,
print_iterator_state): New.
(print_iterator_seq_type): New.
(print_named_field, print_type_field,
print_instance_field, print_iterator_field): New.
(print_field): Use latters.
(print_quoted_named_name, print_type_type, print_type,
print_instance): New.
(print_string(PrintContext&, const char*, const
_Parameter*, size_t)):
Change signature to...
(print_string(PrintContext&, const char*, ptrdiff_t,
const _Parameter*, size_t)):
...this and adapt. Remove intermediate buffer to
render input string.
(print_string(PrintContext&, const char*, ptrdiff_t)): New.
Ok to commit ?
François
+ void
+ pretty_print(PrintContext& ctx, const char* str, _Print_func_t
print_func)
+ {
+ const char cxx1998[] = "__cxx1998::";
+ const char uglification[] = "__";
+ for (;;)
+ {
+ auto idx = strstr(str, uglification);
This is confusing. strstr returns a pointer, not an index into the
string.
+ if (idx)
+ {
+ size_t offset =
+ (idx == strstr(str, cxx1998)
+ ? sizeof(cxx1998) : sizeof(uglification)) - 1;
This is a bit inefficient. Consider "int __foo(__cxx1998::bar)". The
first strstr returns a pointer to "__foo" and then the second one
searches the entire string from the beginning looking for
"__cxx1998::", and checks if it is the same position as "__foo".
The second strstr doesn't need to search from the beginning, and it
doesn't need to look all the way to the end. It should be memcmp.
if (auto pos = strstr(str, uglification))
{
if (pos != str)
print_func(ctx, str, pos - str);
if (memcmp(pos, cxx1998, sizeof(cxx1998)-1) == 0)
str = pos + (sizeof(cxx1998) - 1);
else
str = pos + (sizeof(uglification) - 1);
while (*str && isspace((unsigned char)*str))
++str;
if (!*str)
break;
}
else
It doesn't even need to search from the position found by the first
strstr, because we already know it starts with "__", so:
for (;;)
{
if (auto pos = strstr(str, "__"))
{
if (pos != str)
print_func(ctx, str, pos - str);
pos += 2; // advance past "__"
if (memcmp(pos, "cxx1998::", 9) == 0)
str = pos + 9; // advance past "cxx1998::"
while (*str && isspace((unsigned char)*str))
++str;
if (!*str)
break;
}
else
But either is OK. Just not doing a second strstr through the entire
string again to look for "__cxx1998::".
+
+ if (idx != str)
+ print_func(ctx, str, idx - str);
+
+ str = idx + offset;
+
+ while (*str && isspace((unsigned char)*str))
+ ++str;
Is this really needed?
Why would there be whitespace following "__" or "__cxx1998::" and why
would we want to skip it?
