AMDG On 03/09/2011 06:13 AM, Chad Nelson wrote:
boost::xint::integer is totally not thread-safe, because of the race on the reference count. I recently swore to eat my keyboard without salt if anyone found a flaw in the thread-safety design (rather than an implementation bug). I like
On Mon, 7 Mar 2011 17:15:58 -0800 Steven Watanabe<watanabesj@gmail.com> wrote: this keyboard, but I doubt it would taste very good, especially without any salt.
Every integer_t object will have unique storage when it's returned to user code (assuming the threadsafe option isn't disabled),
... by anything except the copy constructor. The copy constructor does *not* call make_unique.
and no two threads will ever have access the same integer_t object within the library. With that in mind, should I get the knife and fork ready?
Stepping through this code in the debugger shows copy_count=2 at the end of main: #include <boost/xint/xint.hpp> int main() { typedef boost::xint::integer_t<boost::xint::options::threadsafe> test_t; test_t test1(1); test_t test2(test1); }
Can I use a Boost.Interprocess allocator to put an integer_t in shared memory? I doubt it since I don't see any constructors taking an allocator. I've never tried it, but if it has the standard allocator interface, I believe you can. Just specify the allocator class as a parameter to the integer_t class template. (The documentation for that has already been improved for the next release.)
Interprocess has a few extra requirements on allocators: * The allocators are stateful. You cannot assume that allocators of the same type are equivalent. * The containers must store the Allocator::pointer type, not T*.
Why can the comparison operators throw? I would have expected that they could be implemented to never throw. So far as I know, the only reason they would ever throw is if they're passed a Not-a-Number value, or if something unpredictable has gone seriously wrong.
If the integer is nothrow, then comparison doesn't throw. Otherwise, the argument cannot be NaN. There should be nothing unpredictable, since you don't call user-defined code, nor is there any memory allocation. Exceptions don't randomly come out of nowhere.
The comparison and arithmetic operators should be restricted with SFINAE. As written, they will allow comparison with a string. Is that intended? It would be highly unusual, but not specifically an error, assuming the string contains a base-ten numeric value.
I just think that the extra operators that you allow should match the implicit conversions. Since the constructor that takes a string is explicit, I should have to cast the string to an integer.
When you test for integer_t<BOOST_XINT_APARAMS>::Nothrow&& n.is_nan(), you could just test n.is_nan() and make sure that n.is_nan() is always false if Nothrow is true. Couldn't that prevent some less-intelligent compilers from optimizing out the test?
I wouldn't worry about it. This is a fairly easy optimization.
Of course this is rather hypothetical, since we always work with powers of 2. Those types, and the originally-arcane methods used to come up with them, were intended to support *any* platform, no matter how odd. I've heard horror stories of systems with seventeen-bit words and nine-bit characters. :-)
I think it won't quite work, and I have little confidence that it will ever work unless you actually test on such a system. It's too easy to make hidden assumptions.
magnitude_t: * It probably needs to be POD for you to make assumptions about the layout. The "struct hack" is a standard C idiom, and has been for as long as C has been around. It only became explicitly documented behavior in C99, but it works with all known C and C++ compilers; I checked very thoroughly, and even asked on this list (where one might reasonably expect to find people with experience with really odd architectures and compiler setups), and wasn't able to find a single report of a compiler where it would fail.
I don't object to the 'struct hack.' I object to using it for something that isn't a struct.
gcd.hpp: The behavior of invmod when the inverse doesn't exist is to return zero. Wouldn't an exception/NaN be more consistant? I prefer to reserve exceptions for exceptional behavior, primarily errors. A number that doesn't have an inverse isn't really exceptional, there are many such values.
I think that returning 0 has the potential to be more surprising. From the name I would have assumed that the postcondition of invmod is that it returns the inverse. If it cannot meet this, then it should throw. This is safer for anyone who tries to use it without thinking about whether the inverse exists. If I call invmod when there is no inverse, it is probably wrong to go on my merry way as though the inverse were 0.
I don't think std::showbase works correctly with std::setw. (This is a very common problem with ostream operators) [...] It looks like the standard library mostly works the way my code does, but has some quirks with zero values that I'm not presently emulating, according to <http://stackoverflow.com/questions/3273654/ostream-showbase-non-present-for-zero-value-and-internal-doesnt-work-for-handle>.
The issue is that if you split up the streaming into multiple parts, std::setw will be applied to the first string that you print, rather than the whole, which kind of messes up the formatting.
I've written a few extra test cases. The ones called fail_*.cpp should be marked as compile-fail, the ones called test should pass. GCC balks at some of those tests here (haven't tried MSVC yet). Maybe you can see why? In test_invalid_overload.cpp:
Line 34 says "result.message() =", which the compiler won't accept. I'm not sure what is intended there.
The compiler balked at lines 54 and onward ("error: no matching function for call to ‘boost::xint::integer_t<>::integer_t(const unrelated_type&)’"). No idea how to fix it, other than removing unrelated_type from the mpl::vector.
The test_complement case reports "error: cannot pass objects of non-trivially-copyable type ‘class boost::xint::integer’ through ‘...’".
I only tested with MSVC 10. Looks like gcc is pickier about it. Try changing the constructor of any to be a template instead of using '...'.
I was able to get test_arithmetic_conversion.cpp to compile and work, but only after changing the uniform_int_distribution.hpp include to uniform_int.hpp instead, and tweaking the first two lines of the test_arithmetic function. I'm not using the very latest Boost yet, but I'm using a very recent one (1.45), so I'm not sure what happened there.
I'm using the trunk which has some major updates to Boost.Random. uniform_int is fine, but you'll need to specify the bounds explicitly.
Does fail_adl_detail.cpp compile on your system? It shouldn't, so far as I can see, but on mine it does.
It compiles for me too. This is one of my favorite tests which almost always fails unless you've thought about it. In Christ, Steven Watanabe