VS 2005 warnings
Author
For personal projects, I always work with g++. I always enable compiler options -W -Wall -Wextra -pedantic -ansi. This turns on pretty much all warnings and makes sure that my code is standard C++.
I take the warnings of g++ very seriously, and I want to see empty output from the compiler in the console after compiling my code. Therefore, my code generates 0 warnings in g++.
g++ warnings have helped me spot bugs, e.g. signed<->unsigned warnings, and especially warnings about uninitialized variables or exiting a function that should return a value without return.
However, even though this code is standard C++, some people appear to have minor troubles compiling it with visual studio 2005 (and major troubles with visual studio 6). So I downloaded my code at work and compiled it in the VS2005 there. It generated 57 warnings, and all of them were due to conversions from one integer type to another.
E.g. in the line "file.read((char*)(&buffer[0]), size);", where size is of type size_t and file of type "std::ifstream", it gives the warning: warning C4267: 'argument' : conversion from 'size_t' to 'std::streamsize', possible loss of data.
Do you think that this warning is justified? Should I use "std::streamsize" instead of size_t for the size of the file?
Almost all other warnings are conversions between unsigned long and size_t. Do you generally fix these warnings? Or do you lower the warning setting?
I usually fix them, because warnings are important and I don't want them to increase the signal-to-noise ratio of warnings.
There's a certain religious element to anyone's answer, I suspect.
My religion dictates that you should change the code to fix the warning. There really is a danger with implicit conversions. But in your case you know what you're doing. So I would let the compiler know that you know best and use a cast.
Slightly OT: I would use a C++-style cast, too, such as static_cast<> (or whatever is appropriate) for similar reasons; if your cast masks overflow in an implicit conversion, it's much easier to find all casts that would do that and fix the problem. Searching for C-style casts is near-impossible. static_cast/reinterpret_cast/const_cast/dynamic_cast are all much easier to find.
EDIT: borland 5.82 throws up even more warnings of this kind than Visual-C++, in my experience!
Edd
My religion dictates that you should change the code to fix the warning. There really is a danger with implicit conversions. But in your case you know what you're doing. So I would let the compiler know that you know best and use a cast.
Slightly OT: I would use a C++-style cast, too, such as static_cast<> (or whatever is appropriate) for similar reasons; if your cast masks overflow in an implicit conversion, it's much easier to find all casts that would do that and fix the problem. Searching for C-style casts is near-impossible. static_cast/reinterpret_cast/const_cast/dynamic_cast are all much easier to find.
EDIT: borland 5.82 throws up even more warnings of this kind than Visual-C++, in my experience!
Edd
Quote:Original post by Lode
E.g. in the line "file.read((char*)(&buffer[0]), size);", where size is of type size_t and file of type "std::ifstream", it gives the warning: warning C4267: 'argument' : conversion from 'size_t' to 'std::streamsize', possible loss of data.
Do you think that this warning is justified? Should I use "std::streamsize" instead of size_t for the size of the file?
Almost all other warnings are conversions between unsigned long and size_t. Do you generally fix these warnings? Or do you lower the warning setting?
I would definitely fix it. The warnings are there for a reason. AFAIK the only thing the standard guarantees is that std::streamsize is a typedef of one of the basic integral types. In a footnote the standard adds the following, "streamsize is used in most places where ISO C would use size_t. Most of the uses of streamsize could use size_t, except for the strstreambuf constructors, which require negative values. It should probably be the signed type corresponding to size_t (which is what Posix.2 calls ssize_t)."
I don't think the size_t->streamsize conversion is likely to create a bug, but why not just be safe.
The unsigned long to size_t conversions could very easily go wrong however. It isn't hard to find an implementation where some values are representable with an unsigned long, but not with a size_t. So I would also fix those.
Author
Quote:Original post by CTarThe unsigned long to size_t conversions could very easily go wrong however. It isn't hard to find an implementation where some values are representable with an unsigned long, but not with a size_t. So I would also fix those.
I don't entirely understand what you mean. Do you mean everything should be unsigned long? The problem is however that I use a lot of std containers and their size in the code, and their size requires size_t.
I hope I'm correct in paraphrasing CTar as follows:
std::streamsize must be a signed type on any implementation, whereas size_t is unsigned. The conversion whether implicit (without cast) or explicit (with cast) is unlikely to cause any trouble in the given context.
Use a cast to let the compiler know you've considered this.
Edd
std::streamsize must be a signed type on any implementation, whereas size_t is unsigned. The conversion whether implicit (without cast) or explicit (with cast) is unlikely to cause any trouble in the given context.
Use a cast to let the compiler know you've considered this.
Edd
Personally, I never use size_t.
If I need to index the string, I use std::string::size_type, if I need to index a vector - std::vector< someType >::size_type, and so on.
If I need to index the string, I use std::string::size_type, if I need to index a vector - std::vector< someType >::size_type, and so on.
Quote:Original post by LodeQuote:Original post by CTarThe unsigned long to size_t conversions could very easily go wrong however. It isn't hard to find an implementation where some values are representable with an unsigned long, but not with a size_t. So I would also fix those.
I don't entirely understand what you mean. Do you mean everything should be unsigned long? The problem is however that I use a lot of std containers and their size in the code, and their size requires size_t.
size_t and unsigned long can represent different range of values. For example std::basic_string<T>::npos is -1 which is representable with a size_t, but not with an unsigned long. Therefore you should not carelessly allow implicit conversions between them.
You store everything in the type which seems to make the most sense, which could be size_t, unsigned long or std::streamsize depending on the context. If you wish to represent the size of file then why not do it as a std::streamsize? That is what the type was made for.
Some conversion is unavoidable and not dangerous as long as you make sure your value can be represented by both the source and target type. The conversion should of course be done explicitely (with a static_cast) to show that you know a conversion is happening (this should also get rid of any warnings).
Quote:Original post by Lode
However, even though this code is standard C++, some people appear to have minor troubles compiling it with visual studio 2005
Well, GCC isn't standards compliant. It comes fairly close, as does VS2k5, but neither are *really* standards compliant. Which means your code may not be either, if you only judge it by "whether or not G++ complains about it"
Quote:E.g. in the line "file.read((char*)(&buffer[0]), size);", where size is of type size_t and file of type "std::ifstream", it gives the warning: warning C4267: 'argument' : conversion from 'size_t' to 'std::streamsize', possible loss of data.
Do you think that this warning is justified? Should I use "std::streamsize" instead of size_t for the size of the file?
I'm guessing you should add a static cast. It's no different than the warnings about signed vs unsigned you already said you fixed. Add a cast to tell the compiler you're aware that the conversion happens, so it won't warn you.
Personally I always fix warnings. In fact we enable the "treat errors as warnings" option to enforce this.
Conversions between variables of (potentially) different sizes can be risky, though equally, you may be able (by examining the code) to definitively say it's never going to be out of range. That's fine - a static_cast will do the job nicely, or if you prefer, you can use #pragma warning( disable ) or ( suppress ) to silence it - suppress being preferable as it only affects the next line rather than an entire compilation unit.
That said, VC's variable conversion warnings are quite noisy, and you wouldn't be the only person in the world to disable them if that's what you chose to do.
Personally I always run at W4, and also enable the following warnings that (for some reason) are not enabled by default (check http://msdn2.microsoft.com/en-us/library/23k5d385(VS.80).aspx):
C4255: 'function' : no function prototype given: converting '()' to '(void)'
C4263: 'function' : member function does not override any base class virtual member function
C4264: 'virtual_function' : no override available for virtual member function from base 'class'; function is hidden
C4265: 'class' : class has virtual functions, but destructor is not virtual (REALLY important, no idea why it's disabled by default!)
C4266: 'function' : no override available for virtual member function from base 'type'; function is hidden
C4296: 'operator' : expression is always false [or true]
C4431: missing type specifier - int assumed. Note: C no longer supports default-int
C4555: expression has no effect; expected expression with side-effect
C4623: 'derived class' : default constructor could not be generated because a base class default constructor is inaccessible
C4905: wide string literal cast to 'LPSTR'
C4906: string literal cast to 'LPWSTR'
C4928: illegal copy-initialization; more than one user-defined conversion has been implicitly applied
C4946: reinterpret_cast used between related classes: 'class1' and 'class2'
The only warning on by default I explicitly disable as a matter of course is:
C4100: unreferenced formal parameter
...though it can easily enough be avoided with static_cast<void>(param) or by not naming the parameter.
Conversions between variables of (potentially) different sizes can be risky, though equally, you may be able (by examining the code) to definitively say it's never going to be out of range. That's fine - a static_cast will do the job nicely, or if you prefer, you can use #pragma warning( disable ) or ( suppress ) to silence it - suppress being preferable as it only affects the next line rather than an entire compilation unit.
That said, VC's variable conversion warnings are quite noisy, and you wouldn't be the only person in the world to disable them if that's what you chose to do.
Personally I always run at W4, and also enable the following warnings that (for some reason) are not enabled by default (check http://msdn2.microsoft.com/en-us/library/23k5d385(VS.80).aspx):
C4255: 'function' : no function prototype given: converting '()' to '(void)'
C4263: 'function' : member function does not override any base class virtual member function
C4264: 'virtual_function' : no override available for virtual member function from base 'class'; function is hidden
C4265: 'class' : class has virtual functions, but destructor is not virtual (REALLY important, no idea why it's disabled by default!)
C4266: 'function' : no override available for virtual member function from base 'type'; function is hidden
C4296: 'operator' : expression is always false [or true]
C4431: missing type specifier - int assumed. Note: C no longer supports default-int
C4555: expression has no effect; expected expression with side-effect
C4623: 'derived class' : default constructor could not be generated because a base class default constructor is inaccessible
C4905: wide string literal cast to 'LPSTR'
C4906: string literal cast to 'LPWSTR'
C4928: illegal copy-initialization; more than one user-defined conversion has been implicitly applied
C4946: reinterpret_cast used between related classes: 'class1' and 'class2'
The only warning on by default I explicitly disable as a matter of course is:
C4100: unreferenced formal parameter
...though it can easily enough be avoided with static_cast<void>(param) or by not naming the parameter.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement