On Sat, Nov 6, 2010 at 11:35 AM, Christian Henning <chhenning@gmail.com> wrote:
I have done some testing to figure out if the code produced by the various compilers differ. But as far as I can see they produce the same code. What I did was to insert some local variables to check for their data type based on the template parameters.
I don't really understand why the newer compilers don't emit warnings. Do you have some ideas? I first ran into this problem several months ago, and my testing indicated that even VC8 was usually okay with narrowing during explicit conversion to a built-in type. The problem seemed the explicit conversion to the non-default type. Perhaps MSVC is more self-consistent in that way now?
NOTE: The patch I sent earlier has an error -- in copying my changes from 1.44 to the trunk I failed to replace value_type with type in a couple places. The patch attached to this email corrects that.
I'll apply your patch to the boost trunk and will update the release branch once the boost 1.45 is out.
Thanks!
I like the convenience of base_channel_type and I think it makes the code more readable. One change I did do is to remove the typedef for integer_t and the cast to integer_t in channel_converter_unsigned_integral_nondivisible<..>. I don't think that's needed anymore.
Yes, I wondered about that when writing the patch. I worried about removing it, but I was confused by its use in the original code and again in my patch. It seemed to me that it could cause integer overflow. The following sample demonstrates the problem in the trunk and (if I correctly understood your above comment) with your proposed change. In it I simply create a 30-bit channel type and convert it to 8-bit. ///BEGIN CODE demonstrating integer_t necessity & problem/// #include <boost/gil/gil_all.hpp> #include <iostream> namespace boost { namespace gil { ////////////////////////////////////////////////////////////////////////// // Byte-aligned 30 bit grayscale ////////////////////////////////////////////////////////////////////////// const uint32_t bits30_max_val = 1073741823; struct bits30_min { static uint32_t apply() { return 0; } }; struct bits30_max { static uint32_t apply() { return bits30_max_val; } }; typedef scoped_channel_value<uint32_t, bits30_min, bits30_max> bits30; namespace detail { template <> struct unsigned_integral_max_value<bits30> : public mpl::integral_c<uint32_t, bits30_max_val> {}; template <> struct unsigned_integral_num_bits<bits30> : public mpl::int_<30> {}; }//end detail GIL_DEFINE_BASE_TYPEDEFS(30, gray) } }//end boost::gil namespace gil = boost::gil; int main() { gil::gray30_pixel_t g30; gil::gray8_pixel_t g8(255); color_convert(g8, g30); std::cout << "Value: " << g30[0] << std::endl; std::cout << "Should be: " << gil::bits30_max_val << std::endl; return 0; } ///END CODE/// Output with trunk: Value: 12632255 Should be: 1073741823 I have an updated patch which fixes the above problem and also prevents a few more warnings. Since I dislike large amounts of code in a single message I'll send another shortly, describing how to reproduce the warning and with another patch which should correct it and everything else discussed thus far. Thanks for looking over my submission, Nate