[libc++] Implement LWG3464(istream::gcount() can overflow)

[yronglin]

Summary

Implement LWG3464 https://wg21.link/LWG3464

[yronglin]

I'm so sorry that I have no idea how to test stream size overflow in this change now.

[Mordante]

In D158749#4614243, @yronglin wrote:

I'm so sorry that I have no idea how to test stream size overflow in this change now.

I also see no good way to test this.

libcxx/include/istream

220

This is a new function, so no need to change the behaviour after V1.

220

Why is this function protected and not private?

[yronglin]

Addres comments.

[Mordante]

libcxx/include/istream

224

Please use my suggestion, that matches our latest coding standard.

Nit: I prefer to have private, protected, public in that order, especially since there is already a private section.

[yronglin]

Address comments.

[yronglin]

Many thanks for your review! @Mordante

libcxx/include/istream

224

Sorry, somehow I have missed this comments.

[Mordante]

LGTM!

[philnik]

Shouldn't this be possible to actually test? If not, please add a comment why this change was done.

[yronglin]

In D158749#4619918, @philnik wrote:

Shouldn't this be possible to actually test? If not, please add a comment why this change was done.

Sorry, It may be because I didn't refresh the browser page, so I didn't see the status change to Request change. I apologize for that. I'll revert this change to resolve your concerns. I have no idea that how to test this, can you please give some guide? many thanks!

[yronglin]

I have reverted this patch and reopen.

[Mordante]

In D158749#4619918, @philnik wrote:

Shouldn't this be possible to actually test? If not, please add a comment why this change was done.

Do you have a suggestion how to test this. It depends on overflowing streamsize. This is an alias of ptrdiff_t.

In D158749#4619979, @yronglin wrote:

In D158749#4619918, @philnik wrote:

Shouldn't this be possible to actually test? If not, please add a comment why this change was done.

Sorry, It may be because I didn't refresh the browser page, so I didn't see the status change to Request change. I apologize for that. I'll revert this change to resolve your concerns. I have no idea that how to test this, can you please give some guide? many thanks!

For the future it's possible to address these comments after landing the patch. It was not really needed to revert it. But now let's land if after @philnik is happy.

[philnik]

In D158749#4621858, @Mordante wrote:

In D158749#4619918, @philnik wrote:

Shouldn't this be possible to actually test? If not, please add a comment why this change was done.

Do you have a suggestion how to test this. It depends on overflowing streamsize. This is an alias of ptrdiff_t.

Ah sorry, I missed that. I though it depends the char traits. In that case I'm happy with a short explanation in the commit message. Sorry for the churn.

LGTM with a comment in the commit message.

[yronglin]

Thanks for your review! @Mordante @philnik

libcxx/docs/Status/Cxx23Issues.csv

 "Issue #","Issue Name","Meeting","Status","First released version","Labels"
 "`2839 <https://wg21.link/LWG2839>`__","Self-move-assignment of library types, again","November 2020","|Nothing To Do|",""
 "`3117 <https://wg21.link/LWG3117>`__","Missing ``packaged_task`` deduction guides","November 2020","|Complete|","16.0"
 "`3143 <https://wg21.link/LWG3143>`__","``monotonic_buffer_resource`` growth policy is unclear","November 2020","",""
 "`3195 <https://wg21.link/LWG3195>`__","What is the stored pointer value of an empty ``weak_ptr``?","November 2020","|Nothing To Do|",""
 "`3211 <https://wg21.link/LWG3211>`__","``std::tuple<>`` should be trivially constructible","November 2020","|Complete|","9.0"
 "`3236 <https://wg21.link/LWG3236>`__","Random access iterator requirements lack limiting relational operators domain to comparing those from the same range","November 2020","|Nothing To Do|",""
 "`3265 <https://wg21.link/LWG3265>`__","``move_iterator``'s conversions are more broken after P1207","November 2020","Fixed by `LWG3435 <https://wg21.link/LWG3435>`__",""
 "`3435 <https://wg21.link/LWG3435>`__","``three_way_comparable_with<reverse_iterator<int*>, reverse_iterator<const int*>>``","November 2020","|Complete|","13.0"
 "`3432 <https://wg21.link/LWG3432>`__","Missing requirement for ``comparison_category``","November 2020","|Complete|","16.0","|spaceship|"
 "`3447 <https://wg21.link/LWG3447>`__","Deduction guides for ``take_view`` and ``drop_view`` have different constraints","November 2020","|Complete|","14.0","|ranges|"
 "`3450 <https://wg21.link/LWG3450>`__","The const overloads of ``take_while_view::begin/end`` are underconstrained","November 2020","|Complete|","16.0","|ranges|"
-"`3464 <https://wg21.link/LWG3464>`__","``istream::gcount()`` can overflow","November 2020","",""
+"`3464 <https://wg21.link/LWG3464>`__","``istream::gcount()`` can overflow","November 2020","|Complete|","18.0"
 "`2731 <https://wg21.link/LWG2731>`__","Existence of ``lock_guard<MutexTypes...>::mutex_type`` typedef unclear","November 2020","|Complete|","5.0"
 "`2743 <https://wg21.link/LWG2743>`__","P0083R3 ``node_handle`` private members missing ""exposition only"" comment","November 2020","|Nothing To Do|",""
 "`2820 <https://wg21.link/LWG2820>`__","Clarify ``<cstdint>`` macros","November 2020","|Nothing To Do|",""
 "`3120 <https://wg21.link/LWG3120>`__","Unclear behavior of ``monotonic_buffer_resource::release()``","November 2020","",""
 "`3170 <https://wg21.link/LWG3170>`__","``is_always_equal`` added to ``std::allocator`` makes the standard library treat derived types as always equal","November 2020","",""
 "`3036 <https://wg21.link/LWG3036>`__","``polymorphic_allocator::destroy`` is extraneous","November 2020","",""
 "`3171 <https://wg21.link/LWG3171>`__","LWG2989 breaks ``directory_entry`` stream insertion","November 2020","|Complete|","14.0"
 "`3306 <https://wg21.link/LWG3306>`__","``ranges::advance`` violates its preconditions","November 2020","|Complete|","14.0","|ranges|"

libcxx/include/istream

 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <class _CharT, class _Traits>
 class _LIBCPP_TEMPLATE_VIS basic_istream
     : virtual public basic_ios<_CharT, _Traits>
 {
     streamsize __gc_;
+
+    _LIBCPP_HIDE_FROM_ABI void __inc_gcount() {
+      if (__gc_ < numeric_limits<streamsize>::max())
+        ++__gc_;
+    }
 public:
     // types (inherited from basic_ios (27.5.4)):
     typedef _CharT                         char_type;
     typedef _Traits                        traits_type;
     typedef typename traits_type::int_type int_type;
     typedef typename traits_type::pos_type pos_type;
     typedef typename traits_type::off_type off_type;
 
protected:
     basic_istream& operator=(basic_istream&& __rhs);
 
     inline _LIBCPP_HIDE_FROM_ABI_AFTER_V1
     void swap(basic_istream& __rhs) {
       _VSTD::swap(__gc_, __rhs.__gc_);
       basic_ios<char_type, traits_type>::swap(__rhs);
     }
 
     basic_istream           (const basic_istream& __rhs) = delete;
     basic_istream& operator=(const basic_istream& __rhs) = delete;
 public:
 
     // 27.7.1.1.3 Prefix/suffix:
     class _LIBCPP_TEMPLATE_VIS sentry;
 
     // 27.7.1.2 Formatted input:
     inline _LIBCPP_HIDE_FROM_ABI_AFTER_V1
     basic_istream& operator>>(basic_istream& (*__pf)(basic_istream&))
     { return __pf(*this); }
 
     inline _LIBCPP_HIDE_FROM_ABI_AFTER_V1
#endif // _LIBCPP_HAS_NO_EXCEPTIONS
                     {
                        __state |= ios_base::eofbit;
                        break;
                     }
                     if (traits_type::eq_int_type(
                             __sb->sputc(traits_type::to_char_type(__i)),
                             traits_type::eof()))
                         break;
-                    ++__gc_;
+                    __inc_gcount();
                     this->rdbuf()->sbumpc();
                 }
                 if (__gc_ == 0)
                    __state |= ios_base::failbit;
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
             }
             catch (...)
             {
#endif
                     {
                        __state |= ios_base::eofbit;
                        break;
                     }
                     char_type __ch = traits_type::to_char_type(__i);
                     if (traits_type::eq(__ch, __dlm))
                         break;
                     *__s++ = __ch;
-                    ++__gc_;
+                    __inc_gcount();
                      this->rdbuf()->sbumpc();
                 }
                 if (__gc_ == 0)
                    __state |= ios_base::failbit;
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
             }
             catch (...)
             {
#endif // _LIBCPP_HAS_NO_EXCEPTIONS
                    __state |= ios_base::eofbit;
                    break;
                 }
                 char_type __ch = traits_type::to_char_type(__i);
                 if (traits_type::eq(__ch, __dlm))
                     break;
                 if (traits_type::eq_int_type(__sb.sputc(__ch), traits_type::eof()))
                     break;
-                ++__gc_;
+                __inc_gcount();
                 this->rdbuf()->sbumpc();
             }
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
         }
         catch (...)
         {
             __state |= ios_base::badbit;
             // according to the spec, exceptions here are caught but not rethrown
#endif // _LIBCPP_HAS_NO_EXCEPTIONS
                 {
                    __state |= ios_base::eofbit;
                    break;
                 }
                 char_type __ch = traits_type::to_char_type(__i);
                 if (traits_type::eq(__ch, __dlm))
                 {
                     this->rdbuf()->sbumpc();
-                    ++__gc_;
+                    __inc_gcount();
                     break;
                 }
                 if (__gc_ >= __n-1)
                 {
                     __state |= ios_base::failbit;
                     break;
                 }
                 *__s++ = __ch;
                 this->rdbuf()->sbumpc();
-                ++__gc_;
+                __inc_gcount();
             }
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
         }
         catch (...)
         {
             __state |= ios_base::badbit;
             this->__setstate_nothrow(__state);
             if (this->exceptions() & ios_base::badbit)
#endif // _LIBCPP_HAS_NO_EXCEPTIONS
                 while (true)
                 {
                     typename traits_type::int_type __i = this->rdbuf()->sbumpc();
                     if (traits_type::eq_int_type(__i, traits_type::eof()))
                     {
                        __state |= ios_base::eofbit;
                        break;
                     }
-                    ++__gc_;
+                    __inc_gcount();
                     if (traits_type::eq_int_type(__i, __dlm))
                         break;
                 }
             }
             else
             {
                 while (__gc_ < __n)
                 {
                     typename traits_type::int_type __i = this->rdbuf()->sbumpc();
                     if (traits_type::eq_int_type(__i, traits_type::eof()))
                     {
                        __state |= ios_base::eofbit;
                        break;
                     }
-                    ++__gc_;
+                    __inc_gcount();
                     if (traits_type::eq_int_type(__i, __dlm))
                         break;
                 }
             }
 #ifndef _LIBCPP_HAS_NO_EXCEPTIONS
         }
         catch (...)
         {