Re: dlw_int/dlw_gcd review summary

From: Daryle Walker (darylew_at_[hidden])
Date: 2001-10-08 19:16:44

on 9/30/01 10:39 PM, David Abrahams at david.abrahams_at_[hidden] wrote:

> As I reported several days ago, Daryle Walker's dlw_int and dlw_gcd
> submissions were unanimously accepted.
> 
> Here are a few of the comments which I found most salient. Daryle of course
> has the option to ignore any of these, but I'd advise against it. Consensus
> makes the world go around ;-)
> 
> ------------- dlw_int -------------
> 
> Eric Ford:
> 
> static_lb should probably be called "static_log2"
> It might be worth considering providing coverage for long long types on
> compilers that can handle it.

Did the name change.

For the dual long types ("long long" and "unsigned long long"), they won't
be supported in any of the compile-time value templates.  Why?  Because they
are non-standard and not guaranteed to be supported as value-based template
parameters (so I can't use boost::(u)intmax_t either).

> Fernando Cacciola:
> 
> There are broken links and a typo in the documentation
> Consider using an unsigned type for values specifying a number of bits.
> Consider supplying a separate header for forward declarations (presumably to
> reduce compilation dependencies).

Can you be more specific where the problems are, if they still exist in the
latest version.

The templates that take a number of bits were designed by someone before me,
so I shouldn't change their parameter type from under them.  Besides, the
std::numeric_traits template returns an "int" for its bit sizes.

Added the forward declaration header.

> Ed Brey:
> 
> None of this code compiles with VC6sp5. As review manager, I suggest you
> /consider/ recruiting someone to help you port it. Although this compiler
> has numerous conformance problems, enough people use it that your library
> will have a considerably smaller audience than it otherwise might.

Since I don't use Windows (or an IBM-style PC), it's very hard for me to
play "guess the MSVC6 fix."  However, I want to support all the platforms
that the rational and pool libraries did, since I'm trying to replace the
specialty GCD/LCM routines they had.  So if someone could provide a fix
that's close to my style, that's OK.

> --------------- dlw_gcd -------------
> 
> Fernando Cacciola:
> 
> The use of the unary '+' operator adds unneccessary dependencies; consider
> removing it.

A numeric class should have an unary operator+, and it isn't hard to add
one.  It gives the result expression a pretty symmetry.  :^)

> Ed Brey:
> 
> The same comments apply to dlw_gcd w.r.t. VC6sp5 as apply to dlw_int.
> Ed also had these documentation corrections to offer. Daryle, I strongly
> suggest you follow up on them:
> 
> "GCD Function Object:
> - even if any -> even if either
> - should (partially) specialize -> should specialize [since full
> specialization
> is fine; the word "partial" confused me more than it helped].
> In the paragraph describing usage of Compile-Time Extrema Templates, Usage,
> two
> sentences are comma splices (run-on sentences)."

See above for the MSVC policy.

I did most of the documentation corrections.  I didn't do the last one
because I don't understand what you're saying there.

> Hubert Holin:
> 
> M. Holin had several remarks about this submission which are detailed enough
> that I won't post them here. They were posted in
> http://groups.yahoo.com/group/boost/message/17852. There is also a short
> ensuing thread that is worth following. Since Daryle never responded to this
> thread on the boost list, I would very much like to know that he has at
> least considered the issues. As Jens suggested in the thread, if no change
> to the library results, it would be good to cover some of the issues in the
> documentation, to show that they've been considered.

Added some overflow checking.  The new code is so large that I moved it to a
pending header.

-- 
Daryle Walker
Mac, Internet, and Video Game Junkie
darylew AT mac DOT com