Fix Clang warnings and build failure on OSX 10.12

[Floessie]

Over at pixls.us user Partha has fixed his OSX 10.12 build. I've kind of incorporated his fixes and had master compile with Clang 3.8 under Debian Stretch, which reported some warnings that I also fixed (apart from missing enum values in switch statements).

[Beep6581]

Compiles and runs fine here - Gentoo, GCC-5.4.0, CMake-3.5.2.

[heckflosse]

Compiles and runs fine. Win7, GCC 5.3.0, CMake 3.4.1

[Floessie]

Thank you both for testing. I'll wait for Partha's feedback before I'll merge. If you have any objections, I'll wait after RT5 is released.

[Beep6581]

I don't want to include unnecessary last-minute changes, but if this fixes a build failure then it would be good to have it for RT5, in which case the faster its committed the sooner it gets properly tested (by Open Build Service which builds for many distributions, etc.)

[Floessie]

@Beep6581 Agreed. Waiting for Partha's response, will merge then.

[Partha1b]

I checked out the fix-clang-build branch. I rebuilt rawtherapee and it went without errors. Some warnings about using std::abs I can live with. More importantly, you also fixed the annoying gtkmm warning on launch. That was an important fix!

I opened rawtherapee and was able to export an image to McGimp. This is good.

From my pov, you are ready to merge with master.

Thanks,
Partha

[Floessie]

@Partha1b Thanks for taking the time and reporting back! I've merged the changes now.

More importantly, you also fixed the annoying gtkmm warning on launch. That was an important fix!

Hm, that one wasn't by me. But @Hombre57 and @Beep6581 will be glad to here it. 😄

Some warnings about using std::abs I can live with.

Although this PR is closed now, you can still comment on it, and I'd be glad, if you would share those warnings with us, as a wrong type for std::abs() could well be critical. To verbosely inline those warnings here, use Markdown's tripple backtick variant (see the Code tab in the examples).

Thanks for your hints!

Best
Flössie

[Partha1b]

Sure thing! Here are all the warnings I get while compiling:

[ 50%] Building CXX object rtgui/CMakeFiles/rth.dir/filebrowserentry.cc.o
/Users/partha/projects/src/graphics/RT/RawTherapee/rtgui/filebrowserentry.cc:566:13: warning: 6 enumeration values not handled in switch: 'CropWinButtons',
      'CropToolBar', 'ColorPicker'... [-Wswitch]
    switch (a) {
            ^
1 warning generated.
...
[ 53%] Building CXX object rtgui/CMakeFiles/rth.dir/thresholdselector.cc.o
/Users/partha/projects/src/graphics/RT/RawTherapee/rtgui/thresholdselector.cc:554:57: warning: using integer absolute value function 'abs' when argument is of
      floating point type [-Wabsolute-value]
                if (cursorX > positions[TS_TOPRIGHT] || abs(cursorX - positions[TS_TOPRIGHT]) < abs(cursorX - positions[TS_TOPLEFT])) {
                                                        ^
/Users/partha/projects/src/graphics/RT/RawTherapee/rtgui/thresholdselector.cc:554:57: note: use function 'std::abs' instead
                if (cursorX > positions[TS_TOPRIGHT] || abs(cursorX - positions[TS_TOPRIGHT]) < abs(cursorX - positions[TS_TOPLEFT])) {
                                                        ^~~
                                                        std::abs
/Users/partha/projects/src/graphics/RT/RawTherapee/rtgui/thresholdselector.cc:554:97: warning: using integer absolute value function 'abs' when argument is of
      floating point type [-Wabsolute-value]
                if (cursorX > positions[TS_TOPRIGHT] || abs(cursorX - positions[TS_TOPRIGHT]) < abs(cursorX - positions[TS_TOPLEFT])) {
                                                                                                ^
/Users/partha/projects/src/graphics/RT/RawTherapee/rtgui/thresholdselector.cc:554:97: note: use function 'std::abs' instead
                if (cursorX > positions[TS_TOPRIGHT] || abs(cursorX - positions[TS_TOPRIGHT]) < abs(cursorX - positions[TS_TOPLEFT])) {
                                                                                                ^~~
                                                                                                std::abs
/Users/partha/projects/src/graphics/RT/RawTherapee/rtgui/thresholdselector.cc:567:60: warning: using integer absolute value function 'abs' when argument is of
      floating point type [-Wabsolute-value]
                if (cursorX > positions[TS_BOTTOMRIGHT] || abs(cursorX - positions[TS_BOTTOMRIGHT]) < abs(cursorX - positions[TS_BOTTOMLEFT])) {
                                                           ^
/Users/partha/projects/src/graphics/RT/RawTherapee/rtgui/thresholdselector.cc:567:60: note: use function 'std::abs' instead
                if (cursorX > positions[TS_BOTTOMRIGHT] || abs(cursorX - positions[TS_BOTTOMRIGHT]) < abs(cursorX - positions[TS_BOTTOMLEFT])) {
                                                           ^~~
                                                           std::abs
/Users/partha/projects/src/graphics/RT/RawTherapee/rtgui/thresholdselector.cc:567:103: warning: using integer absolute value function 'abs' when argument is of
      floating point type [-Wabsolute-value]
                if (cursorX > positions[TS_BOTTOMRIGHT] || abs(cursorX - positions[TS_BOTTOMRIGHT]) < abs(cursorX - positions[TS_BOTTOMLEFT])) {
                                                                                                      ^
/Users/partha/projects/src/graphics/RT/RawTherapee/rtgui/thresholdselector.cc:567:103: note: use function 'std::abs' instead
                if (cursorX > positions[TS_BOTTOMRIGHT] || abs(cursorX - positions[TS_BOTTOMRIGHT]) < abs(cursorX - positions[TS_BOTTOMLEFT])) {
                                                                                                      ^~~
                                                                                                      std::abs
4 warnings generated.
[ 54%] Building CXX object rtgui/CMakeFiles/rth.dir/thresholdadjuster.cc.o
...
[ 62%] Building CXX object rtgui/CMakeFiles/rth.dir/exifpanel.cc.o
/Users/partha/projects/src/graphics/RT/RawTherapee/rtgui/exifpanel.cc:433:35: warning: comparison of constant 0 with expression of type 'bool' is always false
      [-Wtautological-constant-out-of-range-compare]
        if (tcombo->get_active () < 0) {
            ~~~~~~~~~~~~~~~~~~~~~ ^ ~
1 warning generated.

[Floessie]

/Users/partha/projects/src/graphics/RT/RawTherapee/rtgui/thresholdselector.cc:554:57: note: use function 'std::abs' instead
                if (cursorX > positions[TS_TOPRIGHT] || abs(cursorX - positions[TS_TOPRIGHT]) < abs(cursorX - positions[TS_TOPLEFT])) {
                                                        ^~~
                                                        std::abs

Yep, thought it: This is wrong, indeed. Great warning. This is a common fallout when converting from C++03 to C++11.

@Beep6581 I'll push a new PR and branch for this tonight.

/Users/partha/projects/src/graphics/RT/RawTherapee/rtgui/exifpanel.cc:433:35: warning: comparison of constant 0 with expression of type 'bool' is always false
      [-Wtautological-constant-out-of-range-compare]
        if (tcombo->get_active () < 0) {
            ~~~~~~~~~~~~~~~~~~~~~ ^ ~

@Hombre57 I think, Clang is right here, too. get_active() returns a TreeIter which has no operator <(int) but a non-explicit operator bool(). So shouldn't this be spelled

 if (!tcombo->get_active()) {

or am I missing something?

Best
Flössie

[Floessie]

@heckflosse Clang also warns about this line in dcraw.cpp. Could you have a look?

[Hombre57]

I think, Clang is right here, too. get_active() returns a TreeIter which has no operator <(int) but a non-explicit operator bool(). So shouldn't this be spelled

if (!tcombo->get_active()) {

or am I missing something?

I think that get_active should be replaced by get_active_row_number, which returns -1 for inconsistent values.