opensim-core icon indicating copy to clipboard operation
opensim-core copied to clipboard

Size_t variable being considered smaller than 0

Open vandrw opened this issue 2 years ago • 3 comments

Problem

Hello! I am trying to build opensim-core using the instructions provided in the Continuous Integration action, and I am encountering the following error during make -j8:

In file included from /home/user/software/opensim/opensim-core/OpenSim/Common/DataAdapter.h:28,
                 from /home/user/software/opensim/opensim-core/OpenSim/Common/Adapters.h:23,
                 from /home/user/software/opensim/opensim-core/OpenSim/Common/Test/testTRCFileAdapter.cpp:23:
/home/user/software/opensim/opensim-core/OpenSim/Common/TimeSeriesTable.h: In instantiation of ‘size_t OpenSim::TimeSeriesTable_<ETY>::getRowIndexBeforeTime(const double&) const [with ETY = SimTK::Vec<3>; size_t = long unsigned int]’:
/home/user/software/opensim/opensim-core/OpenSim/Common/TimeSeriesTable.h:450:28:   required from ‘void OpenSim::TimeSeriesTable_<ETY>::trim(const double&, const double&) [with ETY = SimTK::Vec<3>]’
/home/user/software/opensim/opensim-core/OpenSim/Common/Test/testTRCFileAdapter.cpp:194:32:   required from here
/home/user/software/opensim/opensim-core/OpenSim/Common/TimeSeriesTable.h:346:36: error: comparison of unsigned expression in ‘< 0’ is always false [-Werror=type-limits]
  346 |         OPENSIM_THROW_IF(candidate < 0, TimeOutOfRange, time,
      |                          ~~~~~~~~~~^~~
/home/user/software/opensim/opensim-core/OpenSim/Common/Exception.h:79:8: note: in definition of macro ‘OPENSIM_THROW_IF’
   79 |     if(CONDITION)                                                    \
      |        ^~~~~~~~~
cc1plus: all warnings being treated as errors
make[2]: *** [OpenSim/Common/Test/CMakeFiles/testTRCFileAdapter.dir/testTRCFileAdapter.cpp.o] Error 1
make[1]: *** [OpenSim/Common/Test/CMakeFiles/testTRCFileAdapter.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....

After looking at the code, candidate is defined as a size_t variable, which would always be > 0. Is there something else I can do here other than build with the -Wno-error flag?

Build commands

mkdir software/
mkdir software/opensim/
cd software/opensim/

git clone https://github.com/opensim-org/opensim-core.git

mkdir build_deps/
cd build_deps/

cmake ../opensim-core/dependencies/ -LAH \
      -DCMAKE_INSTALL_PREFIX=~/software/opensim/opensim_dependencies_install \
      -DCMAKE_BUILD_TYPE=Release \
      -DSUPERBUILD_ezc3d=ON \
      -DOPENSIM_WITH_TROPTER=ON \
      -DOPENSIM_WITH_CASADI=ON

make -j8

cd ..
mkdir build/
cd build/

export docopt_DIR=/home/user/software/opensim/opensim_dependencies_install/docopt/lib64/cmake

cmake ../opensim-core -LAH \
      -DCMAKE_INSTALL_PREFIX=~/software/opensim/opensim-core-install \
      -DCMAKE_BUILD_TYPE=Release \
      -DOPENSIM_DEPENDENCIES_DIR=~/software/opensim/opensim_dependencies_install \
      -DOPENSIM_C3D_PARSER=ezc3d \
      -DBUILD_PYTHON_WRAPPING=ON \
      -DSWIG_DIR=/software/software/SWIG/4.0.2-GCCcore-10.2.0/share/swig \
      -DSWIG_EXECUTABLE=/software/software/SWIG/4.0.2-GCCcore-10.2.0/bin/swig \
      -DOPENSIM_INSTALL_UNIX_FHS=OFF \
      -DOPENSIM_DOXYGEN_USE_MATHJAX=off \
      -DOPENSIM_SIMBODY_DOXYGEN_LOCATION="https://simbody.github.io/simtk.org/api_docs/simbody/latest/" \
      -DCMAKE_CXX_FLAGS="-Werror"

make -j8

vandrw avatar Apr 22 '22 11:04 vandrw

What compiler/version are you using, as compared to ci? It's possible a more recent compiler is catching this now. My understanding is that size_t is unsigned, not sure why the compiler is suddenly complaining.

aymanhab avatar Apr 25 '22 20:04 aymanhab

Not sure what was intended at TimeSeriesTable.h line 346… perhaps candidate was an int in a former life. However, candidate is a size_t which can never be less than zero. The OPENSIM_THROW_IF check can (should) be removed as only the "false" branch of this condition can ever be reached.

tkuchida avatar Apr 25 '22 20:04 tkuchida

@aymanhab I am using GCC 10.2.0, with cmake 3.20.1. This warning could have been produced by the newer compiler (the CI GCC seems to be 4:7.4.0-1ubuntu2.3).

vandrw avatar Apr 26 '22 08:04 vandrw