Error even w/warnings disabled for no-return fcns

[earlephilhower]

A function whose prototype says it will return a value but doesn't
is undefined behaviour in C++. GCC 10 will generate code that crashes
in this case.

In warnings==None mode, insterad of turning off all warnings with
-w, explicitly list all G++ possible warnings except for the
no-return warning which catches this programming error.

[earlephilhower]

With this PR, the following code generates a compile error even with File->Prefs->Warnings->None

int test() {
    Serial.println("test");
}
    
void setup() {
  test();
}

void loop() {
}

output:

...
Compiling sketch...
/home/earle/Arduino/hardware/esp8266com/esp8266/tools/python3/python3 -I /home/earle/Arduino/hardware/esp8266com/esp8266/tools/signing.py --mode header --publickey /home/earle/Arduino/sketch_feb21a/public.key --out /tmp/arduino_build_467286/core/Updater_Signing.h
/home/earle/Arduino/hardware/esp8266com/esp8266/tools/xtensa-lx106-elf/bin/xtensa-lx106-elf-g++ -D__ets__ -DICACHE_FLASH -U__STRICT_ANSI__ -D_GNU_SOURCE -DESP8266 -I/home/earle/Arduino/hardware/esp8266com/esp8266/tools/sdk/include -I/home/earle/Arduino/hardware/esp8266com/esp8266/tools/sdk/lwip2/include -I/home/earle/Arduino/hardware/esp8266com/esp8266/tools/sdk/libc/xtensa-lx106-elf/include -I/tmp/arduino_build_467286/core -c @/home/earle/Arduino/hardware/esp8266com/esp8266/tools/gcc-no-warnings -Werror=return-type -Os -g -free -fipa-pta -mlongcalls -mtext-section-literals -fno-rtti -falign-functions=4 -std=gnu++17 -MMD -ffunction-sections -fdata-sections -fno-exceptions -DMMU_IRAM_SIZE=0x8000 -DMMU_ICACHE_SIZE=0x8000 -DNONOSDK22x_190703=1 -DF_CPU=80000000L -DLWIP_OPEN_SRC -DTCP_MSS=536 -DLWIP_FEATURES=1 -DLWIP_IPV6=0 -DARDUINO=10816 -DARDUINO_ESP8266_WEMOS_D1MINI -DARDUINO_ARCH_ESP8266 "-DARDUINO_BOARD=\"ESP8266_WEMOS_D1MINI\"" -DFLASHMODE_DIO "-DSTASSID=\"STA\"" "-DSTAPSK=\"PSK\"" -I/home/earle/Arduino/hardware/esp8266com/esp8266/cores/esp8266 -I/home/earle/Arduino/hardware/esp8266com/esp8266/variants/d1_mini /tmp/arduino_build_467286/sketch/sketch_feb21a.ino.cpp -o /tmp/arduino_build_467286/sketch/sketch_feb21a.ino.cpp.o
/home/earle/Arduino/sketch_feb21a/sketch_feb21a.ino: In function 'int test()':
sketch_feb21a:3:5: error: no return statement in function returning non-void [-Werror=return-type]
    3 |     }
      |     ^
cc1plus: some warnings being treated as errors
exit status 1
no return statement in function returning non-void [-Werror=return-type]

[mcspr]

I guess this will also work :)
Since there are no -Wall or -Wextra, can the list be reduced to only contain stuff tagged [enabled] from the gcc -Q --help=warnings output?

[earlephilhower]

It's a lot of lines, but really just the output of

./tools/xtensa-lx106-elf/bin/xtensa-lx106-elf-g++ --help=warnings | awk '{print $1}' | sed 's/-W/-Wno-/' | grep -v = | grep -v -- -f > tools/gcc-no-warnings

and about 3 minutes manually removing warnings that aren't C/C++ compatible (GCC gives a warning for each non-C warning, so just do a compile and scan and dd in vi.) So for GCC 10.3, I'd rather just leave it as-is with everything. If we go to 11.2 in the future, we can revisit and I can add a grep '[enabled]' to the pipeline.

[earlephilhower]

Some more checks show that I have some C++ warnings disabled on the 4-5 C code files we have, so this isn't quite there yet. Let me try @mcspr's suggestion and see if the common set of enabled ones is legit for both C and C++.

[earlephilhower]

Latest push has differing C and C++ warnings files for Mode==None. Tested all warning modes with complete restart between (to ensure core is rebuilt).

A slightly-hackish way is used to allow the compiler type to be used as a parameter in compiler.warning_flags. The GCC or G++ compile recipe now appends gcc/g++ immediately after the warning_flags. So you end up with @file-name-gcc or @file-name-g++. For the Default...All levels, finish the warning flags with a -D__unused_define= so you end up with a harmless -D__unused_define={gcc,g++} on the command line.

[mcspr]

I wonder if it would be worth to go all-in at this point

diff --git a/platform.txt b/platform.txt
index 39414b98..c0fa81b2 100644
--- a/platform.txt
+++ b/platform.txt
@@ -21,11 +21,11 @@ runtime.tools.mkdir={runtime.platform.path}/tools/mkdir.py
 runtime.tools.cp={runtime.platform.path}/tools/cp.py
 runtime.tools.eboot={runtime.platform.path}/bootloaders/eboot/eboot.elf

-compiler.warning_flags=@{runtime.platform.path}/tools/no-warnings-
-compiler.warning_flags.none=@{runtime.platform.path}/tools/no-warnings-
-compiler.warning_flags.default=-D__COMPILER=
-compiler.warning_flags.more=-Wall -D__COMPILER=
-compiler.warning_flags.all=-Wall -Wextra -D__COMPILER=
+compiler.warning_flags={runtime.platform.path}/tools/warnings/none
+compiler.warning_flags.none={runtime.platform.path}/tools/warnings/none
+compiler.warning_flags.default={runtime.platform.path}/tools/warnings/default
+compiler.warning_flags.more={runtime.platform.path}/tools/warnings/more
+compiler.warning_flags.all={runtime.platform.path}/tools/warnings/all

 build.lwip_lib=-llwip_gcc
 build.lwip_include=lwip/include
@@ -68,18 +68,18 @@ compiler.cpreprocessor.flags=-D__ets__ -DICACHE_FLASH -U__STRICT_ANSI__ -D_GNU_S
 compiler.libraries.ldflags=

 compiler.c.cmd=xtensa-lx106-elf-gcc
-compiler.c.flags=-c {compiler.warning_flags}gcc -std=gnu17 {build.stacksmash_flags} -Os -g -free -fipa-pta -Werror=return-type -Wpointer-arith -Wno-implicit-function-declaration -Wl,-EL -fno-inline-functions -nostdlib -mlongcalls -mtext-section-literals -falign-functions=4 -MMD -ffunction-sections -fdata-sections {build.exception_flags} {build.sslflags} {build.mmuflags} {build.non32xferflags}
+compiler.c.flags=-c @{compiler.warning_flags}-gcc -std=gnu17 {build.stacksmash_flags} -Os -g -free -fipa-pta -Werror=return-type -Wpointer-arith -Wno-implicit-function-declaration -Wl,-EL -fno-inline-functions -nostdlib -mlongcalls -mtext-section-literals -falign-functions=4 -MMD -ffunction-sections -fdata-sections {build.exception_flags} {build.sslflags} {build.mmuflags} {build.non32xferflags}

 compiler.S.cmd=xtensa-lx106-elf-gcc
 compiler.S.flags=-c -g -x assembler-with-cpp -MMD -mlongcalls "-I{runtime.tools.xtensa-lx106-elf-gcc.path}/include/"

-compiler.c.elf.flags=-g {compiler.warning_flags}gcc -Os -nostdlib -Wl,--no-check-sections -u app_entry {build.float} -Wl,-static "-L{compiler.sdk.path}/lib" "-L{compiler.sdk.path}/lib/{build.sdk}" "-L{build.path}" "-L{compiler.libc.path}/lib" "-Tlocal.eagle.flash.ld" -Wl,--gc-sections -Wl,-wrap,system_restart_local -Wl,-wrap,spi_flash_read
+compiler.c.elf.flags=-g @{compiler.warning_flags}-gcc -Os -nostdlib -Wl,--no-check-sections -u app_entry {build.float} -Wl,-static "-L{compiler.sdk.path}/lib" "-L{compiler.sdk.path}/lib/{build.sdk}" "-L{build.path}" "-L{compiler.libc.path}/lib" "-Tlocal.eagle.flash.ld" -Wl,--gc-sections -Wl,-wrap,system_restart_local -Wl,-wrap,spi_flash_read

 compiler.c.elf.cmd=xtensa-lx106-elf-gcc
 compiler.c.elf.libs=-lhal -lphy -lpp -lnet80211 {build.lwip_lib} -lwpa -lcrypto -lmain -lwps -lbearssl -lespnow -lsmartconfig -lairkiss -lwpa2 {build.stdcpp_lib} -lm -lc -lgcc

 compiler.cpp.cmd=xtensa-lx106-elf-g++
-compiler.cpp.flags=-c {compiler.warning_flags}g++ {build.stacksmash_flags} -Os -g -free -fipa-pta -Werror=return-type -mlongcalls -mtext-section-literals -fno-rtti -falign-functions=4 {build.stdcpp_level} -MMD -ffunction-sections -fdata-sections {build.exception_flags} {build.sslflags} {build.mmuflags} {build.non32xferflags}
+compiler.cpp.flags=-c @{compiler.warning_flags}-g++ {build.stacksmash_flags} -Os -g -free -fipa-pta -Werror=return-type -mlongcalls -mtext-section-literals -fno-rtti -falign-functions=4 {build.stdcpp_level} -MMD -ffunction-sections -fdata-sections {build.exception_flags} {build.sslflags} {build.mmuflags} {build.non32xferflags}

 compiler.as.cmd=xtensa-lx106-elf-as

where files are tools/warnings/${warnings-name}-${compiler-name}

[earlephilhower]

I wonder if it would be worth to go all-in at this point ...snip...

I thought about doing it that way at first, and while on the +ve side it gets rid of the kludge -Dunusedstuff I think it obfuscates this since the files themselves will be a max of 2 lines. More files == more places to lose things in...

I could go either way on this, though. Thoughts, @d-a-v?

[d-a-v]

It is worth adding a ref here to #8421.
I'm OK for either way,

More files == more places to lose things in...

... and it would be nice in both case to have a script which regenerates the warning-list files. I think it can come later with a compiler update.

Thank you both

[earlephilhower]

Thanks for the decisiveness, @d-a-v . 😆

Hold off on merging this. After consideration, I think @mcspr 's suggestion is cleaner. You would have to go through extra files to figure out WTH is going on, but you wouldn't have the random -Dblah which is probably worse.

I'll do an update w/separate option files for all modes shortly. Updated