Re: [PATCH] libstdc++: Fix infinite loop in std::istream::ignore(n, delim) [PR93672]

[Jonathan Wakely <jwakely@redhat.com>]

On Thu, 4 Apr 2024 at 17:55, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Thu, 4 Apr 2024 at 17:30, Jonathan Wakely <jwakely@redhat.com> wrote:
> Ulrich's suggestion is to allow the buggy user code to Just Work (for
> all cases except (char)-1 on a platform where char is signed). That is
> maybe the least surprising behaviour for users.
>
> On the other hand, I'm not sure we really want to support:
>
> cin.ignore(n, -10);
> cin.ignore(n, -999);
> cin.ignore(n, +999);
>
> What are these trying to do? Those are just nonsense arguments to this
> function. But if we try to make the testcase in the bug report Just
> Work, we end up also accepting (at least) the -10 case, because it's
> indistinguishable from the char '\xf6'. Depending how we do it, we
> might also make the other cases work, treating -999 as '\x19', and
> +999 as '\xe7'.

So maybe what we want is to add a new overload:

basic_istream&
ignore(streamsize n, char_type c)
{
  return ignore(n, traits_type::to_int_type(c));
}

This will only accept the stream's char_type, not -999 or +999, and
will do the required conversion to int_type correctly, not as an
implicit conversion.

Calls that pass eof() will still select the other overload and do the
right thing. Calls using a char (or for wstream, a wchar_t) will
select the new overload.

This new overload will make some calls ambiguous, e.g. ignore(1, 1L)
compiles today, but would become ambiguous. That seems fine, since the
second argument should not be type long (what is it even trying to
do?)

If that's a problem, we can make it a constrained template that only
gets called for the exact char_type:

basic_istream&
ignore(streamsize n, same_as<char_type> auto c)

This would still Do The Right Thing for ignore(n, '\x80') but would
not change the result of ignore(1, 1L) which would still select the
original overload taking int_type for the second parameter.

I think I'll raise this idea with the committee. For now though, I
think my patch fixes the bug and conforms to the standard, and doesn't
do anything inventive.