[Bug libstdc++/109922] New: <iomanip>: consider removing operator>> for istream >> setfill(c)

[Bug libstdc++/109922] New: <iomanip>: consider removing operator>> for istream >> setfill(c)

[fsb4000 at yandex dot ru]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109922

            Bug ID: 109922
           Summary: <iomanip>: consider removing operator>> for istream >>
                    setfill(c)
           Product: gcc
           Version: 13.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: fsb4000 at yandex dot ru
  Target Milestone: ---

Hi.

This is a mirror of a bug report: https://github.com/microsoft/STL/issues/3724

I noticed that libstdc++ has the same bug but LLVM libc++ doesn't have.

Look at the code

#include <iostream>
#include <iomanip>

int main()
{
    std::cin >> std::setfill('*');
}

According to the
standard(https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/n4944.pdf),
std::setfill is only for ostreams. Therefore, istream >> setfill(c) has no
well-defined meaning and can lead to confusion:
https://stackoverflow.com/questions/42530114/stdsetfill-and-stdsetw-for-input-streams

Initially reported by @achabense (https://github.com/achabense)

[Bug libstdc++/109922] <iomanip>: consider removing operator>> for istream >> setfill(c)

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109922

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2023-05-22

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Hmm, this is interesting. The spec has only mentioned setfill for output since
C++98, but Langer & Kreft says that setfill is for both input and output, and
says it applies to basic_ios<charT> (rather than basic_ostream).

I wonder if pre-standard iostreams defined this for istream, and that changed,
which explains the behaviour of libstdc++ and MSVC. I'll dig further ...

[Bug libstdc++/109922] <iomanip>: consider removing operator>> for istream >> setfill(c)

[de34 at live dot cn]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109922

Jiang An <de34 at live dot cn> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |de34 at live dot cn

--- Comment #2 from Jiang An <de34 at live dot cn> ---
In some prehistoric working drafts, several manipulators were required to
return a single smanip type, and only the common base class (ios_base, or ios
in earlier drafts) is used in the wording (e.g.
https://open-std.org/JTC1/SC22/WG21/docs/wp/html/jun96/lib-iostreams.html),
which made them usable with both both input and output.

The requirements was removed by N0958
(https://www.open-std.org/jtc1/sc22/wg21/docs/papers/1996/N0958.pdf), and then
setfill and its friends are only required to be usable with operator<<.

[Bug libstdc++/109922] <iomanip>: consider removing operator>> for istream >> setfill(c)

[de34 at live dot cn]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109922

--- Comment #3 from Jiang An <de34 at live dot cn> ---
> setfill and its friends

I was wrong. Only setfill itself is so relaxed.

[Bug libstdc++/109922] <iomanip>: consider removing operator>> for istream >> setfill(c)

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109922

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jiang An from comment #2)
> The requirements was removed by N0958
> (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/1996/N0958.pdf), and
> then setfill and its friends are only required to be usable with operator<<.

I don't think the 27-652 change from N0958 was applied to the draft, but 27-651
in https://www.open-std.org/jtc1/sc22/wg21/docs/papers/1996/N0964.pdf changed
setfill.

[Bug libstdc++/109922] <iomanip>: consider removing operator>> for istream >> setfill(c)

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109922

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #4)
> I don't think the 27-652 change from N0958 was applied to the draft, but

It wasn't applied verbatim, because the proposed resolution only talked about
basic_ostream. The final change applied to the draft  was corrected to refer to
both basic_ostream and basic_istream.

> 27-651 in https://www.open-std.org/jtc1/sc22/wg21/docs/papers/1996/N0964.pdf
> changed setfill.

I wonder if this was just an editing conflict, and the change for 27-651 should
actually have included basic_istream, as the change for 27-652 did.

Either way, it's not in the standard now and has no useful purpose, so I think
we should deprecate it for GCC 14 and remove it in a future release.

[Bug libstdc++/109922] <iomanip>: consider removing operator>> for istream >> setfill(c)

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109922

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:979f8bfd3164d23e6bd192a00e89eb1fc8f5b4a3

commit r14-1430-g979f8bfd3164d23e6bd192a00e89eb1fc8f5b4a3
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu May 25 10:06:14 2023 +0100

    libstdc++: Deprecate std::setfill for std::basic_istream [PR109922]

    Prior to N0966 (July 1996) the std::setfill manipulator was specified to
    work with both input and output streams. In the final C++98 standard it
    is only specified to work with output streams.

    We have always supported it for input streams, despite that never being
    in the standard, and having no meaning for any input streams defined by
    the standard. This commit adds a deprecated attribute to the overload
    for input streams, so that we can stop supporting this some day.

    libstdc++-v3/ChangeLog:

            PR libstdc++/109922
            * include/std/iomanip (operator>>(basic_istream&, _Setfill)):
            Add deprecated attribute to non-standard overload.
            * doc/xml/manual/evolution.xml: Document deprecation.
            * doc/html/*: Regenerate.
            * testsuite/27_io/manipulators/standard/char/1.cc: Add
            dg-warning for expected deprecated warning.
            * testsuite/27_io/manipulators/standard/char/2.cc: Likewise.
            * testsuite/27_io/manipulators/standard/wchar_t/1.cc: Likewise.
            * testsuite/27_io/manipulators/standard/wchar_t/2.cc: Likewise.

[Bug libstdc++/109922] <iomanip>: consider removing operator>> for istream >> setfill(c)

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109922

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Deprecated for GCC 14. We can remove it for GCC 15 or 16.

Thanks for pointing out this historical oddity!