Re: [boost] [xint] Fourth release, requesting preliminary review again

Subject: Re: [boost] [xint] Fourth release, requesting preliminary review again
From: Chad Nelson (chad.thecomfychair_at_[hidden])
Date: 2010-06-03 00:30:59

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/02/2010 05:53 PM, Marius Stoica wrote:

>>> The documentation got a bit harder to read since it redundantly
>>> repeats the template arguments. Can something be done about that?
>>
>> I've thought the same thing, and I don't yet know what to do about it.
>> The only way I can think to fix it is to rewrite all of the functions as
>> non-templates, purely for documentation purposes.
> 
> Not worth the effort... and what would be the purpose of using doxygen then ?
> I was thinking there migth be some doxygen option to disable this ? I would 
> think this is not the only library to have this issue. Maby make a feature 
> request to the doxigen bug tracker ? Frankly it's not that big an issue.

I found a way, thanks to a suggestion from John Bytheway. :-)

>>> Related to that maby you can consider using boost parameter for the
>>> template parameters?
>>
>> Would that make the documentation easier to read? I don't see any other
>> benefit to using it here; there are only three optional parameters, and
>> two of them have the same type (bool) so they couldn't benefit from
>> Boost.Parameter's automatic deduction.
> 
> Why not ? As the parameter docs points out having two bool parameters can get 
> confusing. And i don't like having to specify the allocator every time .
> Of course is't just some syntactic sugar.

Since you'll probably use typedefs, it's not something you'll need to
deal with often. Easy enough to change it, but I'm not sure I see the
need yet.

>>> I think you should make clear in the documentation what threadsafe
>>> means. If it still only means that you can't use copies of xints
>>> across threads then some could still want to use them in
>>> multithreaded code.
>>
>> That's still what it means. I've added further documentation on it, and
>> modified the copy constructors to allow optional thread-safe copying of
>> non-thread-safe objects; I've uploaded those changes to the sandbox now,
>> if you want to see them.
> 
> It seems to be missing threadsafe.html

<sigh> Problem found and fixed -- it's there now, along with the latest
changes. Sorry about that.

>>> From an user's point of view making nothrow just a template argument
>>> would make sense .
>>
>> Would it? Though similar, they act very differently in the face of
>> errors. The nothrow version also needs to support the Not-a-Number
>> value, which requires a couple functions that don't make sense for the
>> other types.
> 
> It's just a compile-time option like the other ones.

Not entirely, due to those two extra functions. And logically it's a
distinct type.

>>> I consider using size_t for size in bits a little
>>> confusing/annoying. Maby you should typedef size_t bits_t; ?
>>
>> I'm not sure I understand the complaint. size_t is the standard type for
>> sizes. Wouldn't introducing a new type, which is just an alias for an
>> existing and well-known type, be more confusing to people?
> 
> It's just that the size_t is used to count in bytes everywere. That's how you 
> usually count syze.

I still don't really understand the problem, but I've changed it to
bitsize_t. That should be close enough that it's intuitively obvious
what it's based on.

>>> data_t seems quite a bloated class to me .
>>
>> It's the central class of the entire library. Reducing its size would
>> just mean moving the complexity somewhere else. Most of its size is due
>> directly to the requirement that it support allocators, which I was told
>> is necessary to be accepted as a Boost library.
> 
> I was thinking of maby doing something like this.
> 
> bitfield flags;
> union {
> 	intmax_t val;
> 	data_t* data;
> }
> 
> and use a flag to see if it's using a val or data. that should make sure that 
> small values don't use any allocatin and use little memory.
> some of the bools in data_t could be put in the bitfield.

That might save a few bytes of memory, but it would make the entire
library slower, because it would have to check to see whether to use the
data_t object or not before every operation. Memory isn't so precious
that I'd consider that a good trade-off.
- -- 
Chad Nelson
Oak Circle Software, Inc.
*
*
*
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkwHMAMACgkQp9x9jeZ9/wQuPgCggIGud4Q3l421tuh6LuaWZQzn
+JQAoJFS5bNdcaSCtdoqugujY0z+gLYH
=sWhv
-----END PGP SIGNATURE-----