[pool] Warnings in MSVC 7.1 and 8.0
Hello, First, I want to thank Stephen Cleary for the Boost.Pool library. Recently it saves our project by dramatically reducing memory usage in std::list<> of many small objects. However, there were about a hundred of warnings in the pool.hpp file in lines that contain this code: static_cast<bool>(total_req_size % partition_size); There are three such a places in this file. I've refactored this code in one function, removed the cast: size_type calc_num_chunks(const size_type n) { const size_type partition_size = alloc_size(); const size_type total_req_size = n * requested_size; return total_req_size / partition_size + (total_req_size % partition_size) != 0 ? 1 : 0; } and replaced all three occurrences of this code snippet by a call to this simple function. All warnings have disappeared, my code still works fine. Please, make this little change in the upcoming boost release, if it is not too late. Best, Oleg Abrosimov.
----- Mensaje original ----- De: Oleg Abrosimov <olegabr@mail.ru> Fecha: Sábado, Diciembre 2, 2006 12:59 pm Asunto: [boost] [pool] Warnings in MSVC 7.1 and 8.0 Para: boost@lists.boost.org
Hello,
First, I want to thank Stephen Cleary for the Boost.Pool library. Recently it saves our project by dramatically reducing memory usage in std::list<> of many small objects.
However, there were about a hundred of warnings in the pool.hpp file in lines that contain this code:
static_cast<bool>(total_req_size % partition_size); [...]
Oleg, looks like Harmut Kaiser has already fixed this problem in the CVS version of pool.hpp. Would you mind downloading this newer version from http://boost.cvs.sourceforge.net/*checkout*/boost/boost/boost/pool/pool .hpp?revision=1.15 and reporting whether the warnings you had go away? Thank you, Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
JOAQUIN LOPEZ MU?Z wrote:
Oleg, looks like Harmut Kaiser has already fixed this problem in the CVS version of pool.hpp. Would you mind downloading this newer version from
http://boost.cvs.sourceforge.net/*checkout*/boost/boost/boost/pool/pool .hpp?revision=1.15
and reporting whether the warnings you had go away?
I've examined it and found the following code snippet: template <typename UserAllocator> void * pool<UserAllocator>::ordered_malloc(const size_type n) { const size_type partition_size = alloc_size(); const size_type total_req_size = n * requested_size; const size_type num_chunks = total_req_size / partition_size + static_cast<bool>(total_req_size % partition_size); It suggests that warnings would persist. The problem is that this lines are copy-pasted in three different places and it is a maintainability issue that Harmut Kaiser was catched by. Second, he replaced this code in two other places by the following: const size_type partition_size = alloc_size(); const size_type total_req_size = n * requested_size; const size_type num_chunks = total_req_size / partition_size + ((total_req_size % partition_size) ? true : false); I failed to see how it is better then original version, because the problem is in implicit conversion from bool to integer type during promotion in operator+ Just replace true with 1 and false with 0 and add an explicit !=0 test and you'll have what I've originally proposed: size_type calc_num_chunks(const size_type n) { const size_type partition_size = alloc_size(); const size_type total_req_size = n * requested_size; return total_req_size / partition_size + (total_req_size % partition_size) != 0 ? 1 : 0; } Best, Oleg Abrosimov
----- Mensaje original ----- De: Oleg Abrosimov <olegabr@mail.ru> Fecha: Sábado, Diciembre 2, 2006 5:21 pm Asunto: Re: [boost] [pool] Warnings in MSVC 7.1 and 8.0 Para: boost@lists.boost.org
JOAQUIN LOPEZ MU?Z wrote:
Oleg, looks like Harmut Kaiser has already fixed this problem in the CVS version of pool.hpp. Would you mind downloading this newer version from
http://boost.cvs.sourceforge.net/*checkout*/boost/boost/boost/pool/pool> .hpp?revision=1.15
and reporting whether the warnings you had go away?
I've examined it and found the following code snippet:
template <typename UserAllocator> void * pool<UserAllocator>::ordered_malloc(const size_type n) { const size_type partition_size = alloc_size(); const size_type total_req_size = n * requested_size; const size_type num_chunks = total_req_size / partition_size + static_cast<bool>(total_req_size % partition_size);
It suggests that warnings would persist.
Better than guessing, please try this version on your local environment: do warnings still show up? After all, the change was labeled by Harmut like "Removed VC7.1 warning 4800", so it's reasonable to explicitly verify whether they make a difference.
The problem is that this lines are copy-pasted in three different places and it is a maintainability issue that Harmut Kaiser was catched by.
Second, he replaced this code in two other places by the following:
const size_type partition_size = alloc_size(); const size_type total_req_size = n * requested_size; const size_type num_chunks = total_req_size / partition_size + ((total_req_size % partition_size) ? true : false);
I failed to see how it is better then original version, because the problem is in implicit conversion from bool to integer type during promotion in operator+
As I said above, there must be a reason for Harmut to have committed such changes, only actually trying this version will tell for sure. Let's do the following: please try v1.15 of pool.hpp on your local environment and check if the warnings persist or not: if the former, please post a .diff between v1.15 and your proposed modification and I can see that it makes it to the CVS next Monday. How does this look yo you? Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
Oleg Abrosimov ha escrito:
The smallest change I've made to suppress warnings is:
503a504
505c506 < static_cast<bool>(total_req_size % partition_size); ---
((total_req_size % partition_size) ? true : false);
The command was:
diff pool_1.15.hpp pool.hpp > pool.diff
I've just commited this to HEAD (v1.16) and RC_1_34_0 (v1.14.6.2). I'd appreciate if you can verify that warnings are gone now, as I don't have MSVC 7.1 nor 8.0 here. Excuse my being so picky about the process, but I tend to be extracareful when dealing with someone else's code. Thank you, Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
Joaquín Mª López Muñoz wrote:
I've just commited this to HEAD (v1.16) and RC_1_34_0 (v1.14.6.2). I'd appreciate if you can verify that warnings are gone now, as I don't have MSVC 7.1 nor 8.0 here. Excuse my being so picky about the process, but I tend to be extracareful when dealing with someone else's code.
It works now without any warnings. Thank you, Joaquín. Best, Oleg Abrosimov.
participants (3)
-
"JOAQUIN LOPEZ MU?Z" -
Joaquín Mª López Muñoz
-
Oleg Abrosimov