remove -Wformat compiler complaints from qDebug statements in capella.cpp and capxml.cpp when build debug

ericfontainejazz

• Feb 4, 2016 - 06:41

Type

Functional

Severity

S4 - Minor

Status

closed

Project

MuseScore

I want to keep the build output clean. When I build debug in qtcreator, I get 23 warnings "-Wformat=" which seem to all occur for qDebug statements in capella.cpp and capxml.cpp and all have the form:

warning: format '%hhd' expects argument of type 'int', but argument N has type 'Ms::______'

and result from the compiler having to cast from char enums into ints when printing these debug statments. Attached is full build output, and note these same warning occur on my x86-64 & ARMv7-A arch linux machines. But there are no other warnings when building debug, so if I get rid of these, I should have a clean debug build output (although there are a few extra warnings when building release).

Attachment	Size
compiler-complaints-x86-64.txt	71.93 KB

Comments

ericfontainejazz

• Feb 4, 2016 - 07:07

to remove the warnings, I can enclose each of those problematic variables of type enum inside int(). Or I could enclose them inside either char() or signed char() depending on how the enum is defined. For simplicity I think I'm just going to use int(). Is there another preferred way?

[DELETED] 5

• Feb 4, 2016 - 07:25

Casting the enum to int and using %d is fine by me.

Jojo-Schmitz

• Feb 4, 2016 - 07:51

I believe the printf format specifier for unsigned char is %hhu, for signed char is %hhd, there doesn't seem to be a format specifier for plain char.

The %hh[du] stuff had been changed to from %d to avoid compiler warnings on Mac, see 61eed114

I believe for those qDebug() statements we could easily live with %d and casting them to int. This has been done in other places, by Werner, for example in 6f2cf4c2

I don't understand though the difference between int() and static_cast<int>()

ericfontainejazz

• Feb 4, 2016 - 07:41

Status (old)

active

⇒

patch (code needs review)

Well I've explicitly cast them to ints.

But I've left the string %formatting specifier as is, which are either %hhd (will print as signed char) or %hhu (will print as unsigned char), and the warnings are removed.

https://github.com/musescore/MuseScore/pull/2375

Jojo-Schmitz

• Feb 4, 2016 - 07:45

When casting to int, better change to %d (or %u, when casting to unsigned int?) too

ericfontainejazz

• Feb 4, 2016 - 07:51

~~I'll do that Jojo now, so those are always %d. Standby for updated PR.~~

Wait, now I'm a little confused. Do you want %u when the original enum is unsigned char?

Jojo-Schmitz

• Feb 4, 2016 - 07:52

right, only %hhd is the problem here (not all, not the ones that are explicitly signed char), places where %hhu is used are save, as those are explicitly unsigned char.

I believe the %hhu places don't need any change at all, or do you get warnings from them?

ericfontainejazz

• Feb 4, 2016 - 07:54

the warnings are all fixed by simply adding int(), even though the formatting strings may be %hhd or %hhu.

Jojo-Schmitz

• Feb 4, 2016 - 07:57

still the compiler expects them to be (signed/unsigned) char, but gets (signed) int instead

ericfontainejazz

• Feb 4, 2016 - 08:07

No. That may be the case with printf. But according to the debug build output (see attached .txt in my issue description), all these -Wformat with qDebug are saying compiler expects plain int. Never says it expects anything else like unsigned int or unsigned char, even if the formatting string is %hhu.

Jojo-Schmitz

• Feb 4, 2016 - 08:16

Hmm, OK... keep the %u then, so it gets interpreted as unsigned, which it is.

ericfontainejazz

• Feb 4, 2016 - 09:09

I'm leaving my PR as I initially had it, which just encloses the enums with int(). As you can see from my qtcreator debug build output for both x86-64 and ARMv7, there are no more -Wformat complaints after simply doing int().

I would think if the original data is signed char of -1 (0xFF), then when cast to 32-bit int will remain -1 (0xFFFFFFFF), and then when printed as %hhd will remain as "-1" but even when printed as %d will remain as "-1".

While if original data is unsigned char of 255 (0xFF), then when is cast to int will remain as 255 (0x000000FF), and then when printed as %hhu will remain as "255" but even when printed as %d will remain as "255".

So I think everything is fine either way (leaving those formatting strings as is or changing those formatting strings to %d).

My PR does not change any behavior. I'm just letting the compiler know it is indeed OK to treat those enums as ints here.