Re: [PATCH 1/4] Revert "2017-10-04 Petr Ovtchenkov <p...@void-ptr.info>"

2017-11-16 Thread Petr Ovtchenkov 
On Thu, 16 Nov 2017 11:39:30 +
Jonathan Wakely  wrote:

> On 16/11/17 14:35 +0300, Petr Ovtchenkov wrote:
> >On Thu, 16 Nov 2017 10:56:29 +
> >Jonathan Wakely  wrote:
> >
> >> On 10/10/17 22:55 +0300, Petr Ovtchenkov wrote:
> >> >This reverts commit 0dfbafdf338cc6899d146add5161e52efb02c067
> >> >(svn r253417).
> >>
> >> I'm not even going to bother to review patches sent without any
> >> explanation or rationale for the change.
> >
> >https://gcc.gnu.org/ml/libstdc++/2017-11/msg00044.html
> >
> >Along with "violate principles of C++ objects life cycle",
> >the side-effect is
> >
> >  - Make istreambuf_iterator::_M_sbuf immutable
> >  - streambuf_iterator: avoid debug-dependent behaviour
> >
> >I should underline, that "_M_sbuf = 0" when istreambuf_iterator
> >see eof, lead to cripple lifecycle of istreambuf_iterator
> >object and [almost] block usage of istreambuf_iterator
> >for entities other then immutable files.
> >
> >All tests from 24_iterators and 25_algorithms passed,
> >so I expect it conform to Standard.
> >
> >This is series of patches, not single patch because
> >I keep in mind technology aspect---easy transfer
> >to branches other then trunk.
> >
> >>
> >> I will repeat what Paolo said: changing the ABI is not acceptable.
> >
> >I will repeat special for you:
> >
> >
> >Is we really worry about frozen sizeof of instantiated template?
> >(Removed private template member).
> >
> >If yes, than
> >
> >   int_type __dummy;
> >
> >is our all.
> >
> >
> >I.e. problem can be easy resolved---i.e. ABI will not suffer, if we will
> 
> What about other translation units which have inlined the old
> definition of the template, and expect to find a buffered character in
> that member?

I can say that I can write

  int_type _M_c;

but you see, this is a _private_ member of template, so we should (may?) worry 
only
about size of object.

Just for clarification: Do you made accent on "buffered" or on "character" 
("symbol" in ELF)?

> 
> >reach some consensus on the main issue.
> 
> We don't have any consensus, in fact I don't see anybody agreeing with
> you, and I've previously stated I don't want to support your use case:
> https://gcc.gnu.org/ml/libstdc++/2017-09/msg00100.html
>

Re: [PATCH 1/4] Revert "2017-10-04 Petr Ovtchenkov <p...@void-ptr.info>"

2017-11-16 Thread Jonathan Wakely 

On 16/11/17 11:39 +, Jonathan Wakely wrote:

On 16/11/17 14:35 +0300, Petr Ovtchenkov wrote:

On Thu, 16 Nov 2017 10:56:29 +
Jonathan Wakely  wrote:


On 10/10/17 22:55 +0300, Petr Ovtchenkov wrote:

This reverts commit 0dfbafdf338cc6899d146add5161e52efb02c067
(svn r253417).


I'm not even going to bother to review patches sent without any
explanation or rationale for the change.


https://gcc.gnu.org/ml/libstdc++/2017-11/msg00044.html

Along with "violate principles of C++ objects life cycle",
the side-effect is

- Make istreambuf_iterator::_M_sbuf immutable
- streambuf_iterator: avoid debug-dependent behaviour

I should underline, that "_M_sbuf = 0" when istreambuf_iterator
see eof, lead to cripple lifecycle of istreambuf_iterator
object and [almost] block usage of istreambuf_iterator
for entities other then immutable files.

All tests from 24_iterators and 25_algorithms passed,
so I expect it conform to Standard.

This is series of patches, not single patch because
I keep in mind technology aspect---easy transfer
to branches other then trunk.



I will repeat what Paolo said: changing the ABI is not acceptable.


I will repeat special for you:


Is we really worry about frozen sizeof of instantiated template?
(Removed private template member).

If yes, than

 int_type __dummy;

is our all.


I.e. problem can be easy resolved---i.e. ABI will not suffer, if we will


What about other translation units which have inlined the old
definition of the template, and expect to find a buffered character in
that member?


In other words, the ABI is not just the "frozen sizeof".


reach some consensus on the main issue.


We don't have any consensus, in fact I don't see anybody agreeing with
you, and I've previously stated I don't want to support your use case:
https://gcc.gnu.org/ml/libstdc++/2017-09/msg00100.html

Re: [PATCH 1/4] Revert "2017-10-04 Petr Ovtchenkov <p...@void-ptr.info>"

2017-11-16 Thread Jonathan Wakely 

On 16/11/17 14:35 +0300, Petr Ovtchenkov wrote:

On Thu, 16 Nov 2017 10:56:29 +
Jonathan Wakely  wrote:


On 10/10/17 22:55 +0300, Petr Ovtchenkov wrote:
>This reverts commit 0dfbafdf338cc6899d146add5161e52efb02c067
>(svn r253417).

I'm not even going to bother to review patches sent without any
explanation or rationale for the change.


https://gcc.gnu.org/ml/libstdc++/2017-11/msg00044.html

Along with "violate principles of C++ objects life cycle",
the side-effect is

 - Make istreambuf_iterator::_M_sbuf immutable
 - streambuf_iterator: avoid debug-dependent behaviour

I should underline, that "_M_sbuf = 0" when istreambuf_iterator
see eof, lead to cripple lifecycle of istreambuf_iterator
object and [almost] block usage of istreambuf_iterator
for entities other then immutable files.

All tests from 24_iterators and 25_algorithms passed,
so I expect it conform to Standard.

This is series of patches, not single patch because
I keep in mind technology aspect---easy transfer
to branches other then trunk.



I will repeat what Paolo said: changing the ABI is not acceptable.


I will repeat special for you:


Is we really worry about frozen sizeof of instantiated template?
(Removed private template member).

If yes, than

  int_type __dummy;

is our all.


I.e. problem can be easy resolved---i.e. ABI will not suffer, if we will


What about other translation units which have inlined the old
definition of the template, and expect to find a buffered character in
that member?


reach some consensus on the main issue.


We don't have any consensus, in fact I don't see anybody agreeing with
you, and I've previously stated I don't want to support your use case:
https://gcc.gnu.org/ml/libstdc++/2017-09/msg00100.html

Re: [PATCH 1/4] Revert "2017-10-04 Petr Ovtchenkov <p...@void-ptr.info>"

2017-11-16 Thread Petr Ovtchenkov 
On Thu, 16 Nov 2017 10:56:29 +
Jonathan Wakely  wrote:

> On 10/10/17 22:55 +0300, Petr Ovtchenkov wrote:
> >This reverts commit 0dfbafdf338cc6899d146add5161e52efb02c067
> >(svn r253417).
> 
> I'm not even going to bother to review patches sent without any
> explanation or rationale for the change.

https://gcc.gnu.org/ml/libstdc++/2017-11/msg00044.html

Along with "violate principles of C++ objects life cycle",
the side-effect is

  - Make istreambuf_iterator::_M_sbuf immutable
  - streambuf_iterator: avoid debug-dependent behaviour

I should underline, that "_M_sbuf = 0" when istreambuf_iterator
see eof, lead to cripple lifecycle of istreambuf_iterator
object and [almost] block usage of istreambuf_iterator
for entities other then immutable files.

All tests from 24_iterators and 25_algorithms passed,
so I expect it conform to Standard.

This is series of patches, not single patch because
I keep in mind technology aspect---easy transfer
to branches other then trunk.

> 
> I will repeat what Paolo said: changing the ABI is not acceptable.

I will repeat special for you:


Is we really worry about frozen sizeof of instantiated template?
(Removed private template member).

If yes, than

   int_type __dummy;

is our all.


I.e. problem can be easy resolved---i.e. ABI will not suffer, if we will
reach some consensus on the main issue. 

>

Re: [PATCH 1/4] Revert "2017-10-04 Petr Ovtchenkov <p...@void-ptr.info>"

2017-11-16 Thread Jonathan Wakely 

On 10/10/17 22:55 +0300, Petr Ovtchenkov wrote:

This reverts commit 0dfbafdf338cc6899d146add5161e52efb02c067
(svn r253417).


I'm not even going to bother to review patches sent without any
explanation or rationale for the change.

I will repeat what Paolo said: changing the ABI is not acceptable.

[PATCH 1/4] Revert "2017-10-04 Petr Ovtchenkov <p...@void-ptr.info>"

2017-11-15 Thread Petr Ovtchenkov 
This reverts commit 0dfbafdf338cc6899d146add5161e52efb02c067
(svn r253417).
---
 libstdc++-v3/include/bits/streambuf_iterator.h | 59 ++
 1 file changed, 33 insertions(+), 26 deletions(-)

diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h 
b/libstdc++-v3/include/bits/streambuf_iterator.h
index 081afe5..69ee013 100644
--- a/libstdc++-v3/include/bits/streambuf_iterator.h
+++ b/libstdc++-v3/include/bits/streambuf_iterator.h
@@ -95,7 +95,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // NB: This implementation assumes the "end of stream" value
   // is EOF, or -1.
   mutable streambuf_type*  _M_sbuf;
-  int_type _M_c;
+  mutable int_type _M_c;
 
 public:
   ///  Construct end of input stream iterator.
@@ -122,29 +122,28 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   char_type
   operator*() const
   {
-   int_type __c = _M_get();
-
 #ifdef _GLIBCXX_DEBUG_PEDANTIC
// Dereferencing a past-the-end istreambuf_iterator is a
// libstdc++ extension
-   __glibcxx_requires_cond(!_S_is_eof(__c),
+   __glibcxx_requires_cond(!_M_at_eof(),
_M_message(__gnu_debug::__msg_deref_istreambuf)
._M_iterator(*this));
 #endif
-   return traits_type::to_char_type(__c);
+   return traits_type::to_char_type(_M_get());
   }
 
   /// Advance the iterator.  Calls streambuf.sbumpc().
   istreambuf_iterator&
   operator++()
   {
-   __glibcxx_requires_cond(_M_sbuf &&
-   (!_S_is_eof(_M_c) || 
!_S_is_eof(_M_sbuf->sgetc())),
+   __glibcxx_requires_cond(!_M_at_eof(),
_M_message(__gnu_debug::__msg_inc_istreambuf)
._M_iterator(*this));
-
-   _M_sbuf->sbumpc();
-   _M_c = traits_type::eof();
+   if (_M_sbuf)
+ {
+   _M_sbuf->sbumpc();
+   _M_c = traits_type::eof();
+ }
return *this;
   }
 
@@ -152,14 +151,16 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   istreambuf_iterator
   operator++(int)
   {
-   __glibcxx_requires_cond(_M_sbuf &&
-   (!_S_is_eof(_M_c) || 
!_S_is_eof(_M_sbuf->sgetc())),
+   __glibcxx_requires_cond(!_M_at_eof(),
_M_message(__gnu_debug::__msg_inc_istreambuf)
._M_iterator(*this));
 
istreambuf_iterator __old = *this;
-   __old._M_c = _M_sbuf->sbumpc();
-   _M_c = traits_type::eof();
+   if (_M_sbuf)
+ {
+   __old._M_c = _M_sbuf->sbumpc();
+   _M_c = traits_type::eof();
+ }
return __old;
   }
 
@@ -175,21 +176,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   int_type
   _M_get() const
   {
-   int_type __ret = _M_c;
-   if (_M_sbuf && _S_is_eof(__ret) && _S_is_eof(__ret = _M_sbuf->sgetc()))
- _M_sbuf = 0;
+   const int_type __eof = traits_type::eof();
+   int_type __ret = __eof;
+   if (_M_sbuf)
+ {
+   if (!traits_type::eq_int_type(_M_c, __eof))
+ __ret = _M_c;
+   else if (!traits_type::eq_int_type((__ret = _M_sbuf->sgetc()),
+  __eof))
+ _M_c = __ret;
+   else
+ _M_sbuf = 0;
+ }
return __ret;
   }
 
   bool
   _M_at_eof() const
-  { return _S_is_eof(_M_get()); }
-
-  static bool
-  _S_is_eof(int_type __c)
   {
const int_type __eof = traits_type::eof();
-   return traits_type::eq_int_type(__c, __eof);
+   return traits_type::eq_int_type(_M_get(), __eof);
   }
 };
 
@@ -367,14 +373,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   typedef typename __is_iterator_type::traits_type traits_type;
   typedef typename __is_iterator_type::streambuf_type  streambuf_type;
   typedef typename traits_type::int_type   int_type;
-  const int_type __eof = traits_type::eof();
 
   if (__first._M_sbuf && !__last._M_sbuf)
{
  const int_type __ival = traits_type::to_int_type(__val);
  streambuf_type* __sb = __first._M_sbuf;
  int_type __c = __sb->sgetc();
- while (!traits_type::eq_int_type(__c, __eof)
+ while (!traits_type::eq_int_type(__c, traits_type::eof())
 && !traits_type::eq_int_type(__c, __ival))
{
  streamsize __n = __sb->egptr() - __sb->gptr();
@@ -391,9 +396,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__c = __sb->snextc();
}
 
- __first._M_c = __eof;
+ if (!traits_type::eq_int_type(__c, traits_type::eof()))
+   __first._M_c = __c;
+ else
+   __first._M_sbuf = 0;
}
-
   return __first;
 }
 
-- 
2.10.1