RE: [boost] About "dlw_oprs.zip" version 25

From: Aleksey Gurtovoy (alexy_at_[hidden])
Date: 2001-04-30 02:42:49

Daryle Walker wrote:
> Version 25 of "dlw_oprs.zip" has a new trial name for the 
> unary operator+()
> helper class template.  It has adaptations for the newest 
> Boost release, and
> some general improvements.

IMO the new unary operator+() name - 'identity_querible' - is still
counter-intuitive. I would suggest to go with Dave's version -
'unary_plusable'; it's much more easy to remember, and, as Dave said,
accurate and consistent with the documented naming strategy; to my ears it's
not even ugly :).

Some other random comments:

1) I am against including the 'unsigned_integer_convertible',
'signed_integer_convertible', 'float_convertible' and their composite
operator classes into the library. First at all, I find it unnatural,
unnecessary and unsafe for a class to define an implicit conversion
operators in the way 'signed_integer_convertible' class template (for
example) does it:

operator signed char() const {
    return static_cast< ::boost::intmax_t >( static_cast<const T&>(*this) );
}
operator short() const {
    return static_cast< ::boost::intmax_t >( static_cast<const T&>(*this) );
}
operator int() const {
    return static_cast< ::boost::intmax_t >( static_cast<const T&>(*this) );
}
// etc.

It's unnatural, because defining an implicit type conversion operator for
the class is a statement of the close relationship between two types, and,
at least for me, it's confusing and it "washes out" the semantics of the
class, when there are too many types/classes for which such statement was
made. 

Taking the 'bitint<>' class template as an example ('bitint.zip' archive in
the vault; as far as I understand, it was the main motivation for proposing
the discussed templates into the library), when I write 'bitint<32>' I would
expect the class to define only _one_ implicit conversion operator - on my
platform, it would be 'operator int() const'. Also, I find it _technically_
wrong that with current version of the class I will also get "for free"
implicit conversion operators to 'short', 'char' etc. and can write
something like 

char c = bitint<32>(100000);

without getting a single warning from the compiler about data loss in my
code (also, see below).

It's also unnatural because it requires the target class to define a
conversion to the "biggest" signed or unsigned type instead of conversion to
the most suitable one (that in some cases can also lead to a performance
degradation). 

It's unnecessary, because providing just one conversion operator, e.g.
'operator int() const': 

struct my { operator int() const { return 0; } };

is enough for the following statements to compile successfully (except
warnings, see below):

char    c = my(); // 1
short   s = my(); // 2
int     i = my();
long    l = my();

It's unsafe, because the discussed templates move a dangerous code from a
client side to the library header, thus making it look less suspicious/more
easy to accept and, on some compilers, just turning off the diagnostics.
Taking the above code as an example, a good compiler will issue a warning
for the lines 1 and 2 about implicit arithmetic conversions from 'int' to
'short' and 'char' that can lead to a data loss, which is IMO a good thing.
But if, instead of providing a single conversion operator to the most
appropriate type, you decide to use 'signed_integer_convertible' class,

struct my : signed_integer_convertible<my>
{ operator long() const { return 0; } };

the warnings will be issued for the code in the library's header, or not
issued at all (!) (also often you don't even have a chance to find out which
lines in your code caused these warnings). The latter is a problem of those
particular compilers, of course, but the main point remains - hiding
implicit conversions, that can lead to a data loss, in the header is not a
thing that a good library should do.

2) All words in  the "Use of _Concepts_" section's title are capitalized,
despite that the very first sentence of the section says:
"The discussed concepts are not necessarily the standard library's concepts
(CopyConstructible, etc.), although some of them could be; they are what we
call concepts with a small 'c'." :). I think the title should be "Use of
_concepts_".

3) I remember that the name 'totally_comparable' was born as a compromise
between my suggestion of plain 'comparable' (with which Dave was
disagreeing) and the previous 'totally_ordered', but looking at it now, I
think that the compromise is worse that both of the alternatives.
Personally, I am still favor 'comparable', and we already have 'bitwise' and
'shiftable' as precedents :), but I can live with 'totaly_ordered' too.
Dave?

That's all for now. Besides the commented issues, I think the library looks
really good and I am looking forward to see it updated!

Aleksey