jeanluct / braidlab

Matlab package for analyzing data using braids
GNU General Public License v3.0
23 stars 9 forks source link

Compile warnings on macOS (compact_helper in particular) #146

Closed mbudisic closed 4 years ago

mbudisic commented 4 years ago

After a long while, I'm compiling braidlab on Mac (10.14.6 Mojave, with Apple's Clang v11 compiler), and I get the following warnings on compact_helper.cpp.

mex -largeArrayDims -O -DBRAIDLAB_USE_GMP CFLAGS="-O -DMATLAB_MEX_FILE -fPIC" CXXFLAGS="-O -DMATLAB_MEX_FILE -fPIC -std=c++0x" compact_helper.cpp \
        -I../../../extern/cbraid/include -L../../../extern/cbraid/lib -lcbraid-mex
Building with 'Xcode Clang++'.
/Users/marko/Work/Software/braidlab/+braidlab/@braid/private/compact_helper.cpp:190:26: warning: taking the absolute value of unsigned type 'bool' has no effect [-Wabsolute-value]
                         abs(b[i+2*dir] == n-1));
                         ^
/Users/marko/Work/Software/braidlab/+braidlab/@braid/private/compact_helper.cpp:273:10: note: in instantiation of function template specialization 'commute_and_cancel<std::__1::vector<int, std::__1::allocator<int> > >' requested here
  while (commute_and_cancel(bw,1,n,false,annular) ||
         ^
/Users/marko/Work/Software/braidlab/+braidlab/@braid/private/compact_helper.cpp:190:26: note: remove the call to 'abs' since unsigned values cannot be negative
                         abs(b[i+2*dir] == n-1));
                         ^~~
1 warning generated.

Note in particular parentheses in line 190 - I think this is a clear typo (comparing with above), but just wanted to check. Moving the closing parenthesis of abs ahead of == would result that (-n+1) is correctly matched. Right now, I don't think this is happening.

The other warning is a bit more arcane, I am not sure if the compiled code "does the right thing".

There are a bunch of other warnings about deprecations, but I don't think it needs attention asap.

I can fix it, but would like a second opinion.

jeanluct commented 4 years ago

That's a little disturbing. Let me check.

jeanluct commented 4 years ago

This was fixed in c82bb2c on develop on 1 August 2018, maybe not merged in master yet?

commit c82bb2c761949e72a177c3caec7ee35198d44e75
Author: Jean-Luc Thiffeault <jeanluc@mailaps.org>
Date:   Wed Aug 1 14:17:15 2018 -0500

    Misplaced parenthesis.

 +braidlab/@braid/private/compact_helper.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
jeanluct commented 4 years ago

This was indeed fixed after the 3.2.3 release: 3.2.3 - 2018-04-04.