
Apologies for lack of proper review structure, I'm working with last bits of free time to post before the deadline... IMO, the proposed XInt, as can be downloaded from the vault, should NOT be accepted into Boost. Rationale: The reasons were mostly already covered by other boost.devel posters and pretty much all fall into the categories of a too coupled/inflexible and inefficient design. My primary concern is lack of support for true, efficient, no-fail-guarantee fixed-size integers: the provided fixed-size support still uses dynamic memory allocation (which can fail and requires/introduces EH code) and the nothrow policy does not in fact disable the use of exceptions (it only wraps and 'eats' them with try-catch blocks...which is just plain inefficient...this is like implementing malloc in terms of new instead of the other way around). For such low level operations, as fixed-sized integer arithmetic, it is quite reasonable to expect the implementation to be able to work even without linking to the CRT (i.e. no use of keywords new, virtual or throw)... This might at first be viewed as a possibly 'conditional yes' vote but since Chad has repeatedly said that he considers fixed-sized integers to be 'second class citizens' of his library I vote no for such a library... I am also not convinced by the rationale behind the different-than-built-in-behaviour (sign-magnitude, overflow NaN...).
"Chad Nelson" <chad.thecomfychair@gmail.com> wrote in message news:20110302202855.5cd8067f@ubuntu...
On Wed, 02 Mar 2011 18:18:33 +0100 Mathias Gaunard <mathias.gaunard@ens-lyon.org> wrote:
[...] Also, it seems that the object always contains the copy count and a readonly boolean, which total 2 words, even if COW is disabled.
Eliminating sixteen bits of overhead, on an integer that is almost always going to be larger than 64 bits (and often *much* larger), seems to be a premature optimization at best.
2 words are 64 bits on a 32 bit system and 128 bits on a 64 bit system...that's 25% of a 512 bit number. In fact, for a fixed-sized integer with disabled COW, there are, AFAICT, two redundant std:size_ts and two bools (in addition to the data pointer and dynamic memory allocation) per integer...that certainly is not so easily dismissible, especially on a 64 bit system and especially if we also consider the code that maintains those 'useless' data members...Quite to the contrary, I would sooner accuse your approach of premature pessimization (have you even examined the assembly output of your code?). In one post you complained that you are already tired of all the criticism on this list but please try to learn from this. The above misunderstanding and/or unwarranted dismissal of data layout/storage issues along with things like thinking that virtual inheritance is resolved at compile time, not knowing about the CRTP and misunderstanding of when to 'pass by value for performance' shows that you have holes in your knowledge which should make you more humble before honest criticism... You also complained about not getting enough feedback in the last iteration of your library, IMHO your attitude (of fixing your library on your own set of use cases) again may have something to do with that. I remember way back then people were already criticising the hardwired dynamic allocation and data-layout&storage coupling design. I proposed that you extract algorithms into free functions that simply take and work on data ranges as this would both eliminate template bloat (in your current design the algorithms get replicated for every integer_t instantiation) and allow for cleaner implementation of different data storage strategies. As far as I remember you gave little heed to these concerns/suggestions or complained that that would be too much time consuming to implement while OTOH you spent who knows how much time on implementing COW which AFAICT turned to be a misguided premature optimization (of an a priori inefficient design)... When I finally saw that you actually defended your hardwired dyn.mem. allocation by saying that it enables faster swaps I simply gave up on replying at that time deciding I'd wait for an official review (when boost.devel input has to be taken into consideration else you don't get accepted)... ps. I'm sorry if some/all of the above mentioned issues were already resolved (I did not manage to read through the whole discussion before the review deadline)... pps. I saw FFT being mentioned, you migh want to check out this http://drdobbs.com/cpp/199500857 ppps. You use a member-function-pointer based "unspecified bool type" implementation which produces suboptimal code on MSVC <10...IMO a plain operator bool would probably do just fine here as the built-in integral types also implicitly convert to bool... -- "What Huxley teaches is that in the age of advanced technology, spiritual devastation is more likely to come from an enemy with a smiling face than from one whose countenance exudes suspicion and hate." Neil Postman