REGoth-project / REGoth

OpenSource-Reimplementation of the zEngine, used by the game "Gothic"
GNU General Public License v3.0
631 stars 62 forks source link

Datatype confusion #238

Closed ErrorFlexXx closed 7 years ago

ErrorFlexXx commented 7 years ago

I found some if expressions, which are marked as warning and I can understand the compiler.

In file DATFile.cpp Method in line 394: iterateSymbolsOfClass There is this one if(s.parent == -1 || s.properties.elemProps.type != EParType_Instance) And the compiler complains about an signed - unsigend comparison. s.parent is unsigned, so the left part of expression will never become true. if(p.properties.elemProps.type == EParType_Prototype && p.parent != -1) In this one the right expression is always true.

Maybe you know more about the reason of this expressions. Just want to mention that.

markusobi commented 7 years ago

s.parent == -1 can be true, even if s.parent is unsigned. When comparing signed int with unsigned int the following rule is applied:

Otherwise, if the operand that has unsigned integer type has rank greater or equal to the rank of the type of the other operand, then the operand with signed integer type is converted to the type of the operand with unsigned integer type.

https://stackoverflow.com/q/7711124/2056153

Both types have rank 3 (rank of signed/unsigned int), so int gets converted to unsigned int. static_cast<unsigned int>(-1) is well defined since unsigned integral types are guaranteed to wrap around.

The STL uses this at some point: static const size_type npos = -1 http://de.cppreference.com/w/cpp/string/basic_string/npos

ErrorFlexXx commented 7 years ago

Thank you for that. So we could remove the warning with an explicit typecast ? (unsigned int)(-1) for example ?

markusobi commented 7 years ago

Yes, explicit casting -1 would be appropriate. IMHO it's good practice to not use c-style casts. Use static_cast<> for numerical conversions.