Fail to build with gcc 5

[petterreinholdtsen]

Hi. I just got a bug report in Debian about lmms failing to build with gcc 5. See https://bugs.debian.org/777989 for the details. This is the error reported:

/usr/bin/c++   -DQT_CORE_LIB -DQT_GUI_LIB -DQT_NO_DEBUG -DQT_XML_LIB
-D_REENTRANT -O2 -g -fno-exceptions -Wall -Werror=unused-function
-Wno-sign-compare -Wno-strict-overflow -Werror -g -O2 -fstack-protector-strong
-Wformat -Werror=format-security -D_FORTIFY_SOURCE=2  -fPIC -DPIC -isystem
/usr/include/qt4 -isystem /usr/include/qt4/QtGui -isystem
/usr/include/qt4/QtXml -isystem /usr/include/qt4/QtCore
-I/«PKGBUILDDIR»/obj-x86_64-linux-gnu
-I/«PKGBUILDDIR»/obj-x86_64-linux-gnu/include -I/«PKGBUILDDIR»
-I/«PKGBUILDDIR»/include -I/usr/include/SDL   
-D'QT_TRANSLATIONS_DIR="/usr/share/qt4/translations"'
-D'LIB_DIR="../lib/x86_64-linux-gnu/"'
-D'PLUGIN_DIR="../lib/x86_64-linux-gnu/lmms/"' -o
CMakeFiles/lmms.dir/src/core/AutomatableModel.o -c
/«PKGBUILDDIR»/src/core/AutomatableModel.cpp
/«PKGBUILDDIR»/src/core/AutomatableModel.cpp: In member function 'void
AutomatableModel::setAutomatedValue(float)':
/«PKGBUILDDIR»/src/core/AutomatableModel.cpp:224:36: error: logical not is
only applied to the left hand side of comparison
[-Werror=logical-not-parentheses]
     !(*it)->fittedValue( m_value ) !=
                    ^

Are you aware of this issue?

[tresf]

Are you aware of this issue?

No we were not. Thanks for reporting it. That certainly looks peculiar having !value != something.

-Tres

[tresf]

@petterreinholdtsen,

The offending line is here AutomatableModel.cpp#L321.

We're likely to run into other strictness checks, so probably best to identify all of them all in this bug report.

Do you mind fixing this on your end and reporting the next few compilation problems?

@curlymorphic @lukas-w can you help bring some sense to the offending statement AutomatableModel.cpp#L321.?

At first glance I think it's trying to do(!(value)) != something, but from googling it appears this may actually be evaluating to !(value != something), which actually makes quite a bit of sense on why it needs to be eradicated from the language. 😄

[curlymorphic]

I am already sat here reading the changes to gcc 5. I am going to setup a VM with GCC 5.

I did note this line from https://bugs.debian.org/777989

Found in version lmms/1.0.3-5

[lukas-w]

That's a warning that has been showing up in Clang for a long time. I'm not sure about the original intention of that (obviously wrong) statement…

I believe it should just say (*it)->fittedValue( m_value ) != (*it)->m_value. See AutomatableModel.cpp#L225 which is almost the same statement.

[tresf]

This line of code was around at least as far back as 2009 1d5cb23; the last commit to this file I can find in GitHub, so this is quite an old bug.

Am I correct in my findings that this is only used by VSTs (instruments and effects) and AutomationPattern? this could be an age old bug that no one has reported.

I assume also that this bug should actually be observable (right?) But I guess this also depends on how the order of evaluation happens. Can C++ negate a float (for example?) Does it cast the float to a boolean and then back to a float, such as !3.14 evaluates to 0.0.0? If that is the case, this bug would only surface when comparing values are zero or one (right?)

I vote we just delete the !.