CMakeLists.txt and VC++

[afabri]

• add_definitions(-O3) triggers a warning for VC++
• -Wall creates too many warnings and should maybe not be the default.

[PhiBabin]

We should change the optimization flag, if it's a Windows build. This seem to be the modern way of setting compiler flags.

As for -Wall can you be more precise about the kind of warning you want to be suppressed? -Wall -Wextra is, as far as I know, the best practice for warning verbosity for C++. The problem we have with warning is most of them are not caused by libpointmatcher, but by Eigen. There are way of suppressing warning coming from Eigen.

[afabri]

The warnings are not in Eigen, but triggered by how you use Eigen. When you write const int pointsCount(features.cols()); you get the warning warning C4244: 'initializing': conversion from 'Eigen::EigenBase<Derived>::Index' to 'int', possible loss of data So either you static_cast or use the right type.

In my opinion there are too many warnings so Wall is not really helpful.

[PhiBabin]

I misunderstood, I thought you meant that -wall was too verbose on all platform. You meant that it is too verbose with VC++. On linux with gcc, the only warnings are from yaml-cpp and from an unused variable. That specific Eigen warning is not visible with gcc, unless you add -Wconversion. Adding that flag creates an huge amount of warning, since we convert Eigen index into int all over the place.

I suggest adding a flag to silence integer conversion warning in VC++ (maybe -Wno-conversion?). The correct solution would be to fix every conversion from Eigen::Index to int. However, it will take a while, since almost every file do multiple conversions.

[afabri]

I just realized that you already try to silence it using SYSTEM for include_directories and Eigen, so probably I should switch to the latest version of VC++?

You could have written add_definitions( /wd4244 /wd4267) but it then also silences warnings concerning your own code.