JUCE doesn't compile cleanly with GCC recommended flags

Hi JUCE team,

Here is a popular set of flags some people use to compile C++ with GCC: https://www.artificialworlds.net/blog/2014/07/18/best-gcc-warning-flags-for-compiling-c/

However, JUCE doesn’t compile with these flags.

If I use GCC 7.4.0, which is the default C++ compiler on the current Ubuntu LTS, a lot of warnings show up in switch cases (missing default, unhandled cases, implicit fallthroughs). Additionally, some redundant declarations are being flagged.

JUCE only compiles if I remove the following flags:

-Wswitch-enum -Wswitch-default -Wredundant-decls

…and also additionally add the following flag:

-Wno-implicit-fallthrough

Would you consider fixing JUCE to compile cleanly with those flags? Or otherwise, what is your recommendation for GCC 7 flags? The above flags are very useful in our own codebase so I don’t feel super comfortable just removing them…

Thanks!
Timur

1 Like

JUCE should now compile with those warnings enabled in 59a058f. I know you aren’t using the Projucer, but there are a set of recommended warning flags for LLVM and GCC here which can be toggled on or off for projects. I’ve added -Wswitch-enum, -Wswitch-default, and -Wredundant-decls to these (where available) so they’ll get enabled on our CI builds and we can catch them. Unfortunately we can’t add -Wno-implicit-fallthrough by default as it’s only available in GCC 7 onwards and we need to support down to 4.8.

3 Likes

These recommended warnings flags can also be added to a project in FRUT by passing ADD_RECOMMENDED_COMPILER_WARNING_FLAGS and the desired value to jucer_export_target_configuration.

2 Likes

@ed95 wow, that is a very comprehensive fix indeed. Thank you so much!

@ed95 one more tiny fix…

with -Wconversion on gcc 7.5, I get the following two warnings:

/path/to/JUCE/modules/juce_core/text/juce_CharPointer_UTF8.h: In member function ‘juce::CharPointer_UTF8& juce::CharPointer_UTF8::operator++()’:
/path/to/JUCE/modules/juce_core/text/juce_CharPointer_UTF8.h:127:21: error: conversion to ‘juce::uint8 {aka unsigned char}’ from ‘int’ may alter its value [-Werror=conversion]
                 bit >>= 1;
                 ~~~~^~~~~

/path/to/JUCE/modules/juce_dsp/native/juce_fallback_SIMDNativeOps.h: In instantiation of ‘static ScalarType juce::dsp::SIMDFallbackOps<ScalarType, vSIMDType>::sum(vSIMDType) [with ScalarType = signed char; vSIMDType = __vector(2) long long int]’:
/path/to/JUCE/modules/juce_dsp/native/juce_sse_SIMDNativeOps.h:248:50:   required from here
/path/to/JUCE/modules/juce_dsp/native/juce_fallback_SIMDNativeOps.h:118:20: error: conversion to ‘signed char’ from ‘int’ may alter its value [-Werror=conversion]
             retval += a.s[i];
             ~~~~~~~^~~~~~~

May I suggest the following edits:

bit = static_cast<uint8> (bit >> 1);

and

retval = static_cast<ScalarType> (retval + a.s[i]);

which works. I confirmed with godbolt that this does not change the binary code generated in -O3.

Enabling -Wconversion on GCC flags up a lot of conversion from 'int' to 'float' may change value noise, and there doesn’t seem to be a way to disable those whilst also keeping the warnings that we care about like implicitly converting ints to short ints. I’ve tried to sift through and fix the imporant ones (as well as the ones you posted) here:

Thanks. This matches our experience. gcc-7 warnings seem to be way too aggressive compared to clang and msvc. I kinda gave up and started dropping some of those flags.