[xint] Fifth release -- one more round of reviews, please
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 One more time... the next iteration of the Extended Integer library (a arbitrary-length integer math library) is now available in the sandbox (at <https://svn.boost.org/svn/boost/sandbox/xint>) and the Vault (at <http://www.boostpro.com/vault/index.php?action=downloadfile&filename=xint.zip&directory=&>). It incorporates almost all of the suggestions offered on this list. The changes between this release and the last one are: * All three types (integer, nothrow_integer, and fixed_integer) have now been consolidated into a single template, using Boost.Parameter for the options. This release, like the last one, includes a slightly older version of Ion Gaztañaga's preliminary Move library too. I haven't tested it with the current version, but it should work with that as well. The Move library is not used by default, and makes only a slight difference to the speed when thread-safety is enabled. Still on my immediate to-do list: * Add an "unsigned" option, so that the mod operation will work as expected. * Add support for GCC's -fno-exceptions option. * Add more examples. - -- 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/ iEYEARECAAYFAkwTu5sACgkQp9x9jeZ9/wS2AQCfekF0zJW4TjDUhFNP/Cts6ZLC xXkAoNOchGNvIbJcimXnYo3PrSIF68or =4Knn -----END PGP SIGNATURE-----
Attempting to stream an integer_t to/from a wide stream (e.g. std::wcout, std::wcin) fails to compile on MSVC2010 because wide streams don't have insertion/extraction operator overloads defined for std::string, only for std::wstring. To take this a bit further, as you probably know, Windows development is heavily biased towards usage of wchar_t over char, so it feels somewhat limited (or at least clumsy) that integer_t doesn't work directly with wchar_t strings/ streams/pointers. For example, integer_t has constructors that take char const*, but no constructors that take wchar_t const*; also, xint::to_string explicitly returns std::string, with no direct way to get a std::wstring instead. It would be nice if this was remedied so that working with wchar_t-based types was as painless as it currently is for char-based types.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 06/12/2010 09:11 PM, Adam Merz wrote:
Attempting to stream an integer_t to/from a wide stream (e.g. std::wcout, std::wcin) fails to compile on MSVC2010 because wide streams don't have insertion/extraction operator overloads defined for std::string, only for std::wstring.
Oops... I'd completely forgotten wide-character strings.
To take this a bit further, as you probably know, Windows development is heavily biased towards usage of wchar_t over char,
Can you tell I do most of my development work under Linux these days? ;-)
so it feels somewhat limited (or at least clumsy) that integer_t doesn't work directly with wchar_t strings/ streams/pointers. For example, integer_t has constructors that take char const*, but no constructors that take wchar_t const*; also, xint::to_string explicitly returns std::string, with no direct way to get a std::wstring instead. It would be nice if this was remedied so that working with wchar_t-based types was as painless as it currently is for char-based types.
Done. There's one new function (to_wstring), everything else should just work the way you'd expect it to, whether you're using ASCII strings or wide-character ones. The changes have just been uploaded to the Sandbox (haven't updated the Vault yet). Please take a look and let me know if you see any problems with the implementation. There shouldn't be. - -- 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/ iEYEARECAAYFAkwUXEwACgkQp9x9jeZ9/wQOMgCfRtIj8NWu1Eoa5FD0f8tL9w0T uU4AoIGdwefEp7ZhhHctnCqxIE2bDw8r =RGX2 -----END PGP SIGNATURE-----
Chad Nelson <chad.thecomfychair <at> gmail.com> writes:
Done. There's one new function (to_wstring), everything else should just work the way you'd expect it to, whether you're using ASCII strings or wide-character ones.
The changes have just been uploaded to the Sandbox (haven't updated the Vault yet). Please take a look and let me know if you see any problems with the implementation. There shouldn't be.
Looks good to me -- the compilation errors were solved, which was my only real concern. A few nitpicks though, since you asked: ;-] Functions templated on charT taking std::basic_string<charT> should be templated on charT, traitsT, allocT and take std::basic_string<charT, traitsT, allocT>. xint::detail::nan_text should return 'std::basic_string<charT> const&' rather than just 'std::basic_string<charT>'. This way callers (many integer_t constructors as well as xint::to_string variants and stream operators) can call it like 'std::basic_string<charT> const& tnan = detail::nan_text<charT>();' and avoid making hundreds/thousands of pointless copies of the string. For functions taking 'charT const*, charT**', this is a bit C-ish; obviously this is pedantic/stylistic concern, but charT*& is preferred over charT**. The functions templated on charT are fine, but for the functions specifically overloaded on std::string/std::wstring (e.g. to_string, to_wstring), consider also providing additional overloads for char16_t and char32_t based on BOOST_NO_CHAR16_T and BOOST_NO_CHAR32_T. to_string and to_wstring have identical implementations except for the character type they hardcode in. Refactor these into a function template so there's a single common implementation, and have outer functions defer to the template.
A few more: integer_t constructors taking 'const charT *, charT **' don't compile, due to const correctness issues (assigning a charT const* to a charT*). convert.hpp line 140 works around it with const_cast, but the signature should just be changed to 'const charT *, const charT **' (or better yet 'const charT *, const charT *&', as previously mentioned). raw_integer line 131 & convert.hpp line 103, from_string takes char** rather than charT**. std::size_t should be used universally instead of size_t.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 06/13/2010 04:53 AM, Adam Merz wrote:
The changes have just been uploaded to the Sandbox (haven't updated the Vault yet). Please take a look and let me know if you see any problems with the implementation. There shouldn't be.
Looks good to me -- the compilation errors were solved, which was my only real concern. A few nitpicks though, since you asked: ;-]
Functions templated on charT taking std::basic_string<charT> should be templated on charT, traitsT, allocT and take std::basic_string<charT, traitsT, allocT>.
Okay, done.
xint::detail::nan_text should return 'std::basic_string<charT> const&' rather than just 'std::basic_string<charT>'. This way callers (many integer_t constructors as well as xint::to_string variants and stream operators) can call it like 'std::basic_string<charT> const& tnan = detail::nan_text<charT>();' and avoid making hundreds/thousands of pointless copies of the string.
That makes sense. Done.
For functions taking 'charT const*, charT**', this is a bit C-ish; obviously this is pedantic/stylistic concern, but charT*& is preferred over charT**.
It's a bit C-ish because it was based on a C function. :-) There's no similar function that would confuse the compiler (or the user), so I've changed it, but I'm ambivalent about it... having to add the address-of operator was a clue that it was an out-parameter. Without it, you can't tell that it is unless you look at the source code or documentation. A minor quibble though.
The functions templated on charT are fine, but for the functions specifically overloaded on std::string/std::wstring (e.g. to_string, to_wstring), consider also providing additional overloads for char16_t and char32_t based on BOOST_NO_CHAR16_T and BOOST_NO_CHAR32_T.
to_string and to_wstring have identical implementations except for the character type they hardcode in. Refactor these into a function template so there's a single common implementation, and have outer functions defer to the template.
I've made a new template function, to_stringtype, that takes a type to make the string from. to_string and to_wstring are now just thin shells around it. I'm not sure how often the other two types will be used (and I doubt anyone is at this point). I'm also not sure what I'd call those functions... to_char16_string seems esthetically unpleasant. It's the kind of thing that, if I ran across it in an otherwise well-designed library that I wanted to use, I'd write a function or macro to rename it and hide its ugliness. So I propose leaving it as it is. If someone needs that capability, to_stringtype will provide it, and they can write whatever wrapper function or macro they want around it. Combining messages...
integer_t constructors taking 'const charT *, charT **' don't compile, due to const correctness issues (assigning a charT const* to a charT*). convert.hpp line 140 works around it with const_cast, but the signature should just be changed to 'const charT *, const charT **' (or better yet 'const charT *, const charT *&', as previously mentioned).
I wondered about that, but I guess I got distracted before I could find a satisfying fix for it. In any case, it's fixed now.
raw_integer line 131 & convert.hpp line 103, from_string takes char** rather than charT**.
Oops... sorry, I was in a bit of a hurry, and there was no test to point that out. Both of those problems are now fixed.
std::size_t should be used universally instead of size_t.
As a defensive measure, it makes sense, though I'd question the sanity of any developer who made his own different size_t type. Done. These changes have been uploaded to the Sandbox now. If you have the time to spare, please check them and ensure that I haven't made a mistake. And thanks for the feedback, I really appreciate it. - -- 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/ iEYEARECAAYFAkwVDRMACgkQp9x9jeZ9/wSfdwCgqDsSmDfXiL/YkeWRkQP6ve4J sJ4An3BMnuwx0UFCD7nvJfVOup2EpCFA =XxGb -----END PGP SIGNATURE-----
A quick request: can you give integer_t a safe bool conversion operator? integer_t's comparison operators don't have overloads to compare directly to primitive integral types (which *would* be nice to have, but I'm not formally requesting that), so checking for a value with 'if (i != 0) { ... }' creates a temporary. Since integer_t has operator! but no bool conversion operator, I've been using 'if (!!i) { ... }' to avoid creation of a temporary, which looks a bit silly.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 06/15/2010 06:02 AM, Adam Merz wrote:
A quick request: can you give integer_t a safe bool conversion operator? [...]
Done. Changes are in the Sandbox.
Since integer_t has operator! but no bool conversion operator, I've been using 'if (!!i) { ... }' to avoid creation of a temporary, which looks a bit silly.
It looks perfectly normal to us grizzled veterans of C. :-) - -- 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/ iEYEARECAAYFAkwX5LYACgkQp9x9jeZ9/wSlMQCgm4j1ioj/KRoh5Gv3KK2GOgKx KbYAn3y48sPyKxfRj5DfvXGwB/laCM9h =sUMd -----END PGP SIGNATURE-----
participants (2)
-
Adam Merz -
Chad Nelson