search

lab-cosmo / librascal

A scalable and versatile library to generate representations for atomic-scale learning
https://lab-cosmo.github.io/librascal/
GNU Lesser General Public License v2.1
80 stars 18 forks source link

Rework/redesign the compiler flags #167

Open mastricker opened 5 years ago

mastricker commented 5 years ago

As discussed with @agoscinski and @Luthaf, we need to look at the compiler flags, especially e.g. an updated version of the -Weffc++.

Opened an issue so we can discuss it here.

Luthaf commented 5 years ago

Another thing to consider is the unconditional use of "-Werror": upon the release of a new compiler, or when using a different one, this can prevent users from building rascal, which can make a bad impression (this code does not even compile). This happened to me when using intel compiler, which implement a slightly different version of -Weffc++ than GCC, causing the whole build to fail.

We could instead have a ENABLE_WERROR cmake configuration option, that is OFF by default, and that developers are supposed to enable. We can then enforce it by always enabling it on CI.

Luthaf commented 5 years ago

See also this: https://stackoverflow.com/a/10471626/3041318, related to the need for https://github.com/cosmo-epfl/librascal/blob/9554260e0728f3ca9201d56aeb8408ff6aa72360/CMakeLists.txt#L108

felixmusil commented 5 years ago

We could instead have a ENABLE_WERROR

I like this idea since we would still be warning free on our tested configurations.

Luthaf commented 4 years ago

Ran into this again, while trying to compile the code on a CentOS 7 container while testing #266. I get the following error due to -Weff-c++ and -Werror interaction.

/librascal/src/rascal/external/json.hpp:1196:8: error: base class 'struct nlohmann::detail::is_compatible_integer_type_impl<long unsigned int, std::move_iterator<nlohmann::basic_json<>*>, void>' has a non-virtual destructor [-Werror=effc++]
 struct is_compatible_integer_type