Suggestions for libstdc++-2.90.6

[Russell Davidson <russell@ehess.cnrs-mrs.fr>]

Over the weekend I was trying out the new snapshot of
libstdc++-2.90.6, which I built with no problems on my
i686-pc-linux-gnu system using the gcc 2.95 release of last week. A
couple of the tests failed, 26_numerics/modf_float.cc and
27_io/istream_extractor_char.cc, but I gather these failures are well
known.

I then moved on to using the new library to try to compile a program
used for econometrics. I am very keen to use the new library for
this, because it makes extensive use of stringbufs, etc., the stuff
in <sstream>. Up to now, I have had to use the useful, but
out-of-date, programs in P.J. Plauger's draft C++ library book.

Not everything worked (not surprising!), and so I would like to
submit some suggestions that made at least a number of things compile
and work correctly. Consider the following program:

*************************

#include <vector>

template class vector<double>;

*************************

Compiling this fails, with complaints about lines 669, 678, and 681
of stl_vector.h. The following diff fixes things:

*************************

669c669
<     copy_backward(__position, _M_finish - 2, _M_finish - 1);
---
>     copy_backward(__position, iterator(_M_finish - 2), iterator(_M_finish-1));
678c678,679
<       __new_finish = uninitialized_copy(_M_start, __position, __new_start);
---
>       __new_finish = uninitialized_copy(iterator(_M_start), __position,
>          __new_start);
681c682,683
<       __new_finish = uninitialized_copy(__position, _M_finish, __new_finish);
---
>       __new_finish = uninitialized_copy(__position, iterator(_M_finish),
>          __new_finish);

*************************

The problem is evidently due to the "evil" use of pointers as
iterators, more specifically, a mix of pointers and iterators of an
iterator class.

Next problem: I wanted to copy the contents of one file into another.
The following program sketches my procedure:

*************************

#include <iostream>
#include <fstream>

int main(int argc, char* argv[])
{
   if (argc < 3) { cerr << "No filenames" << endl; return 1; }
   filebuf fbin, fbout;
   fbin.open(argv[1], ios::in);
   if (!fbin.is_open()) 
    { cerr << "Cannot open " << argv[1] << endl; return 2; }
   fbout.open(argv[2], ios::out|ios::trunc);
   if (!fbout.is_open())
    { cerr << "Cannot open " << argv[2] << endl; return 3; }

   istream is(&fbin);
   is.unsetf(ios::skipws);
   is >> &fbout;

   fbout.close();
   fbin.close();

   return 0;
}

*************************

The program compiles with no problem, but the line fbin.close() gives
rise to a segmentation fault. The problem seems to be that, in the
close() function, there is a call to
_M_really_overflow(traits_type::eof()), which is fine for files used
for output. The test for whether to call this function is based on
the _M_out pointers, and, since _M_buf_unified is true for filebufs,
the _M_out_cur pointer gets moved when the file is read. It seems to
me that the logical test checks also for whether the file mode
include ios_base::out. The following diff, for the file fstream.tcc,
makes everything work fine. (I did not recompile the library for
this. I just put a specialization of the template function in another
file that I linked together with the one above.)

*************************

133c133,134
< 	  bool __testput = _M_out_cur && _M_out_beg < _M_out_cur;
---
>           bool __testput = (_M_mode & ios_base::out) &&
>              _M_out_cur && _M_out_beg < _M_out_cur;
157d157
< 

*************************

The next problem was related to istreams with arithmetic extractors,
in particular
  istream& istream::operator>>(int&);

This program illustrates the difficulty:

*************************

#include <iostream>
#include <fstream>

int main()
{
   filebuf fb;
   fb.open("number", ios::in);
   if (!fb.is_open()) { cerr << "No file" << endl; return 1; }
   istream is(&fb);
   int n = 15;
   is >> n;
   cout << "That was " << n << endl;
   char c = is.peek();
   cout << "Next character: " << (int) c << endl;
   return 0;
}

*************************

The file called number contained something like:

*************************
20000AB
*************************

If the program above is compiled with the library that comes with gcc
2.95, then the output is

*************************
That was 20000
Next character: 65
*************************

but, with the snapshot, it gave:

*************************
That was 15
Next character: -1
*************************

With more difficult input, in number, I could cause segmentation
faults.

In the file locale_facets.tcc, I find the following intention:

*************************
  // The thought was to encapsulate the conversion
  // into this one function, and thus the num_get::do_get member
  // functions can just adjust for the type of the overloaded
  // argument and process the char array returned from _M_extract.
*************************

With respect, I don't see how this can work without some additional
machinery. The template functions num_get::do_get are based on the
use of iterators, in the present context, of istreambuf_iterators.
Section 24.5.1 of the standard, point #2, tells us that "The
practical consequence of this fact is that istream iterators can
be used only for one-pass algorithms". The same seems to be true for
istreambuf_iterators.

There was a first problem, easily avoided, caused by the function
_M_extract eating up a character after each number it has to read.
This is not OK with the file number above, where AB comes right after
the number, with no white space.

The more fundamental problem is that, if extracting integer types,
the decimal point cannot be a part of the number, and should cause
_M_extract to stop reading. In addition, things like the "AB" above
are read, because 'A' and 'B' could be hex digits. If the base is
octal, even '8'and '9' should cause reading to stop.

The minimal extra machinery I could think of was to give an extra
boolean argument to _M_extract, true by default, but set to false for
extracting integer rather than floating-point types. _M_extract can
figure out by itself what the base is, but not the type of number to
be extracted, and a one-pass algorithm needs to know. Here is my diff
for the file locale_facets.tcc:

*************************

291a292,293
>       typedef _Format_cache<char> __format_type;
> 
313c315,318
< 	  if (strchr(__fmt->_S_literals, __c))
---
>           char* __p;
>           const char* __lits = __fmt->_S_literals;
> 
>           if ((__p = strchr(__fmt->_S_literals, __c)))
314a320,328
>               if (!__c) break;  // strchr returns true for __c == 0x0
>               if ((__p >= &__lits[__format_type::_S_digits+__base]
>                 && __p < &__lits[__format_type::_S_digits_end]) ||
>                 (__p >= &__lits[__format_type::_S_udigits+__base]
>                 && __p < &__lits[__format_type::_S_udigits_end]))
>                 {
>                   if (!(fl && (__p == &__lits[__format_type::_S_ee] ||
>                     __p == &__lits[__format_type::_S_Ee]))) break;
>                 }
324a339
> 
326a342
>               if (!fl) break;
330a347
>           if (!__valid) break;  // for otherwise we consume another character
341a359,360
> 
>   
452c471,472
<       _M_extract(__beg, __end, __io, __err, __xtrc, __base);
---
>       // Call _M_extract with last argument false, for int
>       _M_extract(__beg, __end, __io, __err, __xtrc, __base, false);

*************************

The last part above is the changed call to _M_extract in do_get for
an int& argument. For clarity, I have not completed the diff for all
the other integer types, short, size_t, etc, but something similar
must be done for them as well. The floating-point extractors don't
need to be changed, since the default value for the boolean
argument is correct for them.

In the earlier part, I make use of the enum defined in class
_Format_cache in order to get more information about what characters
have been extracted, and then take appropriate action.

The line if (!__valid) break; is essential, because otherwise the
++__beg in the 3rd statement of the for loop will be executed before
the condition (__valid) in the 2nd statement is seen to be false, and
the loop terminated. This was what was eating up the extra
character.

The change in the signature of _M_extract necessitates a couple of
changes in locale_facets.h, in the declaration of the function, as
follows:

*************************

942c942
< 		 ios_base::iostate& __err, char* __xtrc, int& __base) const;
---
> 	 ios_base::iostate& __err, char* __xtrc, int& __base, bool fl = true) const;
997c997
< 	       ios_base::iostate& __err, char* __xtrc, int& __base) const;
---
> 	ios_base::iostate& __err, char* __xtrc, int& __base, bool fl) const;

*************************

Two more points, and I'll be done for now. In my various experiments,
I used the unformatted extractor istream::read(char*, streamsize n)
at one point. As written, this function reads n+1 characters. The
diff, for the file std_istream.h, is:

*************************

476c476
< 		  for (; _M_gcount <= __n; ++_M_gcount)
---
> 		  for (; _M_gcount < __n; ++_M_gcount)

*************************

where the inequality must be strict.

When reading characters in a binary file, either by get or by
operator>>, I had trouble because eof would be returned prematurely
whenever the char (byte) read was 0xff. The various means of
extracting characters all use this->rdbuf()->sbumpc(), which returns
an int, in order to get the raw character. For an ordinary streambuf,
sbumpc gets a char __c from the stream, and returns
traits_type::to_int_type(__c). Since we are dealing with a char
rather than an unsigned char, both the byte 0xff and the eof symbol
0xffffffff end up as 0xffffffff after this. This seems undesirable,
since sbumpc shouldn't be returning eof unless it really has reached
the end of the stream. The solution, I believe, is to change the
definition of to_int_type in char_traits<char>, by the following
diff to char_traits.h:

*************************

202c202
<       { return static_cast<int_type>(__c); }
---
>       { return static_cast<int_type>(static_cast<unsigned char>(__c)); }

*************************

This change returns the byte 0xff as the int 0x000000ff, and eof as
0xffffffff, as desired. For most purposes, it is also better that it
returns the byte 0xfe as 0x000000fe rather than as 0xfffffffe,
although it often doesn't matter.

Well, that's it; sorry the list is so long. As the BUGS list says,
istream::get(char) is not working for cin. I'm afraid I'll have to
leave that to you, although I hope someone fixes it soon!

Russell

Russell Davidson                    email: russell@ehess.cnrs-mrs.fr
GREQAM,
Centre de la Vieille Charite,       telephone: +33-4.91.14.07.40 
F-13002 Marseille                   fax:       +33-4.91.90.02.27