DevHeads.net

Review Request 113189: lower (a lot) warning noise caused by undefined macros

Review request for kdelibs.

Repository: kdelibs

Description
There is a lot of warning noise in kdelibs build, this decreases number of warnings a lot.

I have 'git grep'ped the code and checked all usages, and found no problem.

Tested by compile/usage.

Diffs
kjs/wtf/Platform.h 843cfd2

Diff: <a href="http://git.reviewboard.kde.org/r/113189/diff/" title="http://git.reviewboard.kde.org/r/113189/diff/">http://git.reviewboard.kde.org/r/113189/diff/</a>

Testing

Thanks,

Jiří Pinkava

Comments

Re: Review Request 113189: lower (a lot) warning noise caused by

By Bernd Buschinski at 10/10/2013 - 06:59

This is a bit confussing...

lets sum up the background story:
- kjs(/wtf) loves gcc features
- kjs-cmake filters out (gcc) pedantic

now comes the problem... whats with other compilers?

if you look at the commit message that caused the warnings:

"According to CPP standards (defined(X) && X) should be the same of just (X). On the other hand it is undefined behavior if 'defined' appears in a macro expansion. gcc -pedantic and icl evaluates them always to false."
( <a href="http://quickgit.kde.org/?p=kdelibs.git&amp;a=commit&amp;h=75fa90c16232186bbd287a2ac79839ea34da32c4" title="http://quickgit.kde.org/?p=kdelibs.git&amp;a=commit&amp;h=75fa90c16232186bbd287a2ac79839ea34da32c4">http://quickgit.kde.org/?p=kdelibs.git&amp;a=commit&amp;h=75fa90c16232186bbd287a...</a> )

-> "it is undefined behavior if 'defined' appears in a macro expansion" <-

So? Whats the correct solution here?
And yes, I agree the warnings ARE annoying.

But I would like to hear a second opinion here :)

- Bernd Buschinski

On Oct. 10, 2013, 10:52 a.m., Jiří Pinkava wrote:

Re: Review Request 113189: lower (a lot) warning noise caused by

By Andrea Iacovitti at 12/10/2013 - 13:41

It appears the same patch has been already applied to KDE/4.12 branch:
<a href="http://commits.kde.org/kdelibs/96cde24b38765bcc96d0efe462bd8c2f950ab47f" title="http://commits.kde.org/kdelibs/96cde24b38765bcc96d0efe462bd8c2f950ab47f">http://commits.kde.org/kdelibs/96cde24b38765bcc96d0efe462bd8c2f950ab47f</a>

- Andrea

On Oct. 10, 2013, 10:52 a.m., Jiří Pinkava wrote:

Re: Review Request 113189: lower (a lot) warning noise caused by

By =?utf-8?b?SmnFm... at 10/12/2013 - 18:12

The other option is add something like

#if !defined(SOMETHING)
#define SOMETHING 0
#endif

this is common pattern in other (non-kde) projects.

- Jiří

On Oct. 10, 2013, 10:52 a.m., Jiří Pinkava wrote:

Re: Review Request 113189: lower (a lot) warning noise caused by

By Michael Pyne at 10/10/2013 - 15:16

IIRC when I was trying to filter out these warnings to debug some Clang issues, the define flags in question are always non-zero if they are defined at all. So it might be possible to replace #if with #ifdef instead.

- Michael

On Oct. 10, 2013, 10:52 a.m., Jiří Pinkava wrote:

Re: Review Request 113189: lower (a lot) warning noise caused by

By Martin Klapetek at 10/10/2013 - 06:29

Same patch is being submitted into the frameworks branch, see <a href="https://git.reviewboard.kde.org/r/113173/diff/" title="https://git.reviewboard.kde.org/r/113173/diff/">https://git.reviewboard.kde.org/r/113173/diff/</a>

I'm not sure if the frameworks branch is still being merged to master or not, but you might want to just cherry-pick that.

- Martin Klapetek

On Oct. 10, 2013, 10:52 a.m., Jiří Pinkava wrote: