RFC: int bFoo -> bool bFoo for local vars - simplest case
Author: Kurt Schwehr schwehr@google.com / schwehr@gmail.com (osgeo id: goatbar)
Started: 2016-May-09
Status: Open for discussion
Discussion: https://lists.osgeo.org/pipermail/gdal-dev/2016-June/044511.html
Link: https://goo.gl/hdzhXD Google doc link
Consider:
int bFoo = FALSE; int bBar = expThatIsFalse || expThatIsTrue; int bBaz = CPLReturnsIntTrueFalse(); int bHadRuntimeTrouble = -1;
bHadRuntimeTrouble = expression; |
Drawbacks:
- Compiler and static analyzers may have to track more than 0 or 1.
- The reader has to be wary that these variables might be assigned to -1 (e.g. gmlreader.cpp:GMLReader::SetupParser() ) or other values outside of TRUE and FALSE to represent undefined/optional not set state or something totally unexpected. But most TRUE / FALSE / int uses are just what they seem. The only way to know that something is unusual is to check through all uses of that variable.
- TRUE and FALSE are C preprocessor #defines and only show as the number when debugging
- While very unlikely, it is possible that other code might rely on values from FALSE and/or TRUE that are not 0 and 1.
Advantages:
- Is the same as in the pure C code
Proposed solution:
bool bFoo = false; bool bBar = expThatIsFalse || expThatIsTrue; bool bBaz = CPL_TO_BOOL(CPLReturnsIntTrueFalse()); bool bHadRuntimeTrouble = false; bool bCheckedHadRuntimeTrouble = false; … bHadRuntimeTrouble = expression; bCheckedHadRuntimeTrouble = true; |
Benefits:
Drawbacks:
- Must use CPL_TO_BOOL to convert ints to bool for compilers that have implicit conversion warnings enabled (e.g. MS VC compilers). CPL_TO_BOOL added in r31594.
Cases not covered:
Alternatives:
For bool bFoo = false;
- #define BOOL int - Which will most certainly collide with other packages. So it would have to be something like CPL_BOOL or GDAL_BOOL
- enum
- enum class
- char / uchar / GByte / short
- And what other weird tricks?
- bitset cplusplus.com/bitset
- Boost:dynamic_bitset
- C bool in stdbool.h wikipedia bool
- C99 _Bool #include <stdbool.h>
- Use OGRBoolean, which is just an int. Suppose we could make OGRBoolean an uint8, but that still is more complicated than a two state bool value.
Converting from int to bool with CPL_TO_BOOL:
- CPLReturnsIntTrueFalse() == TRUE
- CPLReturnsIntTrueFalse() != FALSE
- bool(CPLReturnsIntTrueFalse) // “functional cast expression” which is equivalent to C casting Why C++ casting?
- (bool)CPLReturnsIntTrueFalse() // C style cast Why C++ casting?
- static_cast<bool>(CPLReturnsIntTrueFalse())
- And?
For int bHadRuntimeTrouble = -1;
Scale of change:
Medium
In most of the cases in GDAL, the behavior will be exactly the same. The old convention occurs throughout the code base, but is really pretty simple. There will be no externally visible changes except for possible performance changes (which should almost always be faster). It is possible that a rare case could make it harder for a compiler to cleanly layout memory, but those cases should only occur with changes from int to bool that are excluded from this RFC. In general, this should make compilers, analyzers and debuggers have an easier time working with the code and resulting binary objects. Memory usage may decrease very slightly.
Code samples from GDAL and autotest2:
- r34195 - Local variable
- r34199 - Local variable requiring CPL_TO_BOOL
Not all of these uses of TRUE and FALSE can be converted to true and false, but most of them can.
find . -name \*.cpp | xargs egrep 'TRUE|FALSE' | wc -l
14335
find . -name \*.cpp | xargs egrep 'true|false' | wc -l
6563
Notes to readers for context:
- Most GDAL code uses a modified version of Hungarian notation as defined in RFC 8 - Developer Guidelines. The “b” of bFoo means that the variable is a boolean. “n” denotes an integer and “i” is for a for indexing into zero based arrays or loop counters.
- Some parts of GDAL have to interface with C where the bool type does not exist.
- GDAL’s bool helpers: https://gist.github.com/schwehr/46287294726cfee81854efbbdc22dd80
- See the “cases not covered” below to see how much this doc tries to restrict the discussion. I plan for follow on documents to address those cases.
http://www.possibility.com/Cpp/CppCodingStandard.html#boolean When was this true?
The new C++ standard defines a native boolean type. Until all compilers support bool, and existing code is changed to use it, we must still deal with the cruel world.
This must be from long long ago. All compilers that will build GDAL support the C++ bool type.
See also:
Other RFCs like this one for reference: