Re: [boost] [system] system-0.2 request for comments

From: Gennaro Prota (gennaro_prota_at_[hidden])
Date: 2006-07-24 11:02:59

On Sat, 22 Jul 2006 12:10:21 -0400, Beman Dawes <bdawes_at_[hidden]>
wrote:

>system-0.2.zip has been posted in the file vault. See the "system" 
>directory at http://boost-consulting.com/vault/
>
>This is a major upgrade, reflecting comments and suggestions from many 
>Boosters, but in particular Chris Kohlhoff and Peter Dimov.

First of all I'd like to thank you for your careful work on this
library. I see it constantly and quickly improving in design and
usefulness. And I believe we are very near to the final version.

The only major flow I can see concerns i18n. Here are my comments (I'm
deliberately excluding optimization, particularly of lookup in the
native_to_errno[] array, and other implementation details):

* Microsoft CRT/SCL-secure warnings: to be transparent to users I
guess we would better #pragma push/pragma pop around disable-C4996.

* The usage of L'\n', L'\r' etc. doesn't give the desired effect
(unless the effect I desire is incorrect :-)). Similarly for functions
having string and wstring variations.

That's an issue I faced for dynamic_bitset<> with the help of James
Kanze; and what I've learned from him is basically the reason why I
suggested not to use strings everywhere a const char* is accepted now
in the standard.

Sticking to the point:

a) the conversion of the character literals and string literals
depends on the compile-time locale; for the run-time locale you need a
facet, and that's why one should use streams, not strings. After all,
the implementation of std::endl is an example:

 os.put(L’\n’); os.flush();  // not this way

b) there's no guarantee that operator== can be used on a generic char
type (and no guarantee that one can construct a CharT from a char).
The only viable char->CharT paths are ctype::widen and codecvt::in.
For comparison there's Traits::eq()

c) no way to spell a "generic character literal", except for the null
character, which is guaranteed to be CharT() for any character type

* Though you took care to not let escape exceptions from the what()
function, that doesn't remove the root problem: having a string
subobject in system_error objects, which can make its construction
fail directly at the throw site. That would end up reporting a
different exception (coming from std::string) than system_error.

* Did you consider using virtual inheritance, as advised in
more/error_handling.html?

* [very minor] you don't need #include <iosfwd> in identifier.hpp, do
you? As far as I can see the implementation puts constraints on the
right hand operand of << and >>, not on the left hand one, which may
well not be a stream.

That's all, I think :-) Please, don't beat me!

--
[ Gennaro Prota, C++ developer for hire ]