Re: [PATCH v2] Fix comparator bug in cooked index

[Simon Marchi <simark@simark.ca>]

On 1/30/23 09:56, Tom Tromey via Gdb-patches wrote:
> Simon pointed out that the cooked index template-matching patch
> introduced a failure in libstdc++ debug mode.  In particular, the new
> code violates the assumption of std::lower_bound and std::upper_bound
> that the range is sorted with respect to the comparison.
> 
> When I first debugged this, I thought the problem was unfixable as-is
> and that a second layer of filtering would have to be done.  However,
> on irc, Simon pointed out that it could perhaps be solved if the
> comparison function were assured that one operand always came from the
> index, with the other always being the search string.
> 
> This patch implements this idea.
> 
> First, a new mode is introduced: a sorting mode for
> cooked_index_entry::compare.  In this mode, strings are compared
> case-insensitively, but we're careful to always sort '<' before any
> other printable character.  This way, two names like "func" and
> "func<param>" will be sorted next to each other -- i.e., "func1" will
> not be seen between them.  This is important when searching.
> 
> Second, the compare function is changed to work in a strcmp-like way.
> This makes it easier to test and (IMO) understand.
> 
> Third, the compare function is modified so that in non-sorting modes,
> the index entry is always the first argument.  This allows consistency
> in compares.
> 
> I regression tested this in libstdc++ debug mode on x86-64 Fedora 36.
> It fixes the crash that Simon saw.
> 
> This is v2.  I believe it addresses the review comments, except for
> the 'enum class' change, as I mentioned in email on the list.

This LGTM:

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon