explicitly signed chars for struct MidiMapping for ARM

Interesting, I never thought of defining based on char(-1)...then no need to change the data structure.

What define method is the most appropriate way for musescore code base? Where scopewise should I put it (at the top of score.h?)? I was initially going to make a #define, but I didn't see other defines like that.

Can I put the define in score.h immediately above or below the MidiMapping struct?

well unfortunately Score::getNextFreeMidiMapping(int p, int ch) does use -1 in that same manner, so using such a define wouldn't work for that function. I could leave that function alone. Although I'd like to change it to use char as well. But that will cause a make world.

I used #define FREE char(-1)

https://github.com/musescore/MuseScore/compare/master...ericfont:96626-…

I tried #defining NONE, but that wouldn't work because someone in score.cpp used SelState::NONE. I think FREE might make more sense anyway when reading. I don't know what is the best name for char(-1), any suggestions?

note that Part::setMidiChannel also uses this -1 terminology but in this case won't bother compiler since using ints: https://github.com/musescore/MuseScore/blob/6674938d52466dac4c1ebb9994e…

There is nothing really wrong adding signed char in MidiMapping definition. A lot of these type of equivalent solutions I will defer to what you guys think is easier to read.

So here is just modifying MidiMapping struct and enum class StaffTypes : signed char
https://github.com/musescore/MuseScore/compare/master...ericfont:96626-…

that fixes those specific compiler warnings.

ok, here is PR (for MidiMapping and StaffTypes): https://github.com/musescore/MuseScore/pull/2374

There will be more PRs as I go down the list of compiler warnings, and I will make new issue for them.

I looked over the code surrounding the rest of those compiler warnings, and none of them seem to have any relation to ARM. I think they're benign, except for the fact that they pollute the build output and hence hide other important warnings by making them hard to spot. There are a few warnings for -Wmaybe-uninitialized which can be dealt with by initializing the variables, one -Wunused-variable for some preferences.cpp text that is never used. There are 23 warnings "-Wformat" for qDebug messages that convert from an enum to int.

The following "-Wstrict-overflow" warning is something that I didn't understand at first:

/home/e/MuseScore/mscore/importmidi/importmidi_voice.cpp: In function 'int Ms::MidiVoice::findMaxOccupiedVoiceInBar(const iterator&, const std::multimap&)':
/home/e/MuseScore/mscore/importmidi/importmidi_voice.cpp:704:5: warning: assuming signed overflow does not occur when assuming that (X + c) < X is always false [-Wstrict-overflow]
 int findMaxOccupiedVoiceInBar( 

After reading http://stackoverflow.com/questions/12984861/dont-understand-assuming-si… it seems to be about the compiler wanting to do certain optimizations that would for the most part be ok unless the values got big enough to overflow. I don't see any chars in that function, so I think this particular warning can be suppressed.

There are 2 benign -Wempty-body warnings:

/home/e/MuseScore/libmscore/element.cpp: In member function 'virtual QPointF Ms::Element::pagePos() const':
/home/e/MuseScore/libmscore/element.cpp:521:34: warning: suggest braces around empty body in an 'else' statement [-Wempty-body]
                   Q_ASSERT(false);
                                  ^
/home/e/MuseScore/libmscore/element.cpp: In member function 'virtual QPointF Ms::Element::canvasPos() const':
/home/e/MuseScore/libmscore/element.cpp:556:34: warning: suggest braces around empty body in an 'else' statement [-Wempty-body]
                   Q_ASSERT(false);

which occur because when Debug mode is disabled, those Q_ASSERTS are removed (I believe by pre-preprocessor), so those else statements are empty.

Anyway, I'm going to keep looking for the slur error, just letting you know it is not one of these compiler warnings.

I'm going to say mscore.h has lots of enums defined as (unspecified) char. I noticed when building in *debug mode* on qtcreator on my ARMv7-A machine, I get another -Wtype-limits error on line 1282 of style.cpp that is fixed by changing enum TextStyleType from char into an explicitly signed char. The error is due to a Q_ASSERT testing that enum >= 0, which I presume someone wrote because might have used -1 to indicate an invalid textstyle, but as we all know now, that Q_ASSERT is broken on ARM. Anyway, I think someone should make another PR going through every enum in mscore.h and decide if is really an unspecified char, because if at anytime someone uses -1 to represent invalid, then that char enum should probably be explicitly signed. Maybe there needs to be a musescore programming rule saying only use unspecified chars if sign doesn't matter for that char.

You know better than I which enums in mscore.h should be explicitly signed...

EDIT: It will take me too long to study all their usages, so I'll let you decide if you want and let you make any changes for what should be explicitly signed in mscore.h. Most of them look like sign doesn't matter, in which plain char is ok, but if any ever deal with any negative values, then should be explicitly signed.

Actually that "enum class TextStyleType : char" seems to be the only enum that is creating additional compile messages I notice for latest git in qtcreator debug mode on my ARMv7-A archlinux.

yes, changing TextStyleType to signed char fixes the compiler -Wtype-limit warning on my ARM (when building in qtcreator debug mode). I'll add it to my PR now...

ok, that PR passed travis and is ready for merge.

Regarding the slur & pedal issue in reunion: I thought it was an ARM-specific issue, but I've discovered that the issue also happens on x86-64 windows and arch linux nightly. So something must have been recently introduced since 2.0.2 that caused this. Also apparently, the pedal issue is present in 2.0.2 x86-64 windows & linux, so something else caused that issue. I've made a new forum post on it here: https://musescore.org/en/node/96816 so please comment there.