Added C_ONLY_FLAG

[brendena]

So the problem is that the .cc files take all the CFLAGS because of that it also grabs the "-std=GNUXX" flags.

So i added another flag to separate out the flags that can only exist on a c file.

Error Message

CC obj/libs/tensorflow/tensorflow/lite/core/api/error_reporter.cc.o cc1plus: warning: ‘-Werror=’ argument ‘-Werror=-std=gnu99’ is not valid for C++

[gfwilliams]

Thanks - it's definitely good to avoid those issues.

Just thinking, we have CFLAGS/CPPFLAGS/LDFLAGS/OPTIMIZEFLAGS - and C_ONLY_FLAGS looks a bit different as a name choice. What about CFLAGS_ONLY or something like that?

But before we merge this you're saying the error is:

cc1plus: warning: ‘-Werror=’ argument ‘-Werror=-std=gnu99’ is not valid for C++

Any chance of running with V=1 make to see what the command-line was?

Because in the Makefile we set -std=gnu11, but not ‘-Werror=-std=gnu99. It's possible there is an empty -Werror= and then -std=gnu99 got tacked on the end of it when it shouldn't have been?

[brendena]

So i tested back on main and this was the error i got. I don't exactly know why it differed from the thing i original posted. But the error makes more sense.

cc1plus: warning: ‘-Werror=’ argument ‘-Werror=implicit-function-declaration’ is not valid for C++ cc1plus: warning: command-line option ‘-std=gnu99’ is valid for C/ObjC but not for C++

Full line

gcc -Wall -Wextra -Wconversion -Werror=implicit-function-declaration -fno-strict-aliasing -g -Wno-packed-bitfield-compat -std=gnu99 -c -DGIT_COMMIT=5ee8eb215 -DBUILDNUMBER="107" -DSYSFS_GPIO_DIR="/sys/class/gpio" -DLINUX -DESPR_UNICODE_SUPPORT=1 -DUSE_FONT_6X8 -DGRAPHICS_PALETTED_IMAGES -DGRAPHICS_ANTIALIAS -DESPR_PBF_FONTS -DSPIFLASH_BASE=0 -DSPIFLASH_LENGTH=FLASH_SAVED_CODE_LENGTH -DUSE_DEBUGGER -DUSE_TAB_COMPLETE -DUSE_HEATSHRINK -DUSE_FILESYSTEM -DUSE_GRAPHICS -DUSE_NET -DUSE_NETWORK_JS -DUSE_TELNET -DUSE_CRYPTO -DUSE_SHA256 -DUSE_SHA512 -DUSE_TLS -DUSE_AES -DUSE_TENSORFLOW=1 -DLINUX -DUSB -I/home/brendena/code/newCode/Espruino -I/home/brendena/code/newCode/Espruino/targets -I/home/brendena/code/newCode/Espruino/src -Igen -I/home/brendena/code/newCode/Espruino/libs/compression -I/home/brendena/code/newCode/Espruino/libs/compression/heatshrink -I/home/brendena/code/newCode/Espruino/libs/filesystem -I/home/brendena/code/newCode/Espruino/libs/graphics -I/home/brendena/code/newCode/Espruino/libs/network -I/home/brendena/code/newCode/Espruino/libs/network -I/home/brendena/code/newCode/Espruino/libs/network/http -I/home/brendena/code/newCode/Espruino/libs/network/js -I/home/brendena/code/newCode/Espruino/libs/network/linux -I/home/brendena/code/newCode/Espruino/libs/network/telnet -I/home/brendena/code/newCode/Espruino/libs/crypto -I/home/brendena/code/newCode/Espruino/libs/crypto/mbedtls -I/home/brendena/code/newCode/Espruino/libs/crypto/mbedtls/include -Ilibs/tensorflow -Ilibs/tensorflow/tensorflow -Ilibs/tensorflow/third_party/gemmlowp -Ilibs/tensorflow/third_party/kissfft -Ilibs/tensorflow/third_party/flatbuffers/include -Ilibs/tensorflow/third_party/ruy -I/home/brendena/code/newCode/Espruino/targets/linux -DESPR_DEFINES_ON_COMMANDLINE libs/tensorflow/tensorflow/lite/kernels/internal/quantization_util.cc -o obj/libs/tensorflow/tensorflow/lite/kernels/internal/quantization_util.cc.o

[gfwilliams]

Ok, thanks! Yes, that looks good. Seems like there was just some copy/paste issue in the original message

Presumably -Werror=implicit-function-declaration needs moving as well?

[brendena]

Yep moved it and pushed up.

Also changed the name to "CFLAGS_C_COMPILER"

Little long. But i think it very descriptive.

[gfwilliams]

Thanks - sorry for the delay merging!

[fanoush]

Sorry, just noticed this issue.

Also changed the name to "CFLAGS_C_COMPILER"

Well CFLAGS are only meant to be for C compiler so the "_C_COMPILER" is confusing and redundant CXXFLAGS are for c++ files (.cc and .cpp) see https://www.gnu.org/software/make/manual/make.html#Catalogue-of-Rules

also CPPFLAGS are for preprocessor so are passed to both c and c++ compilers

So the problem is that the .cc files take all the CFLAGS because of that it also grabs the "-std=GNUXX" flags.

if CFLAGS are passed to c++ than it is probably our mistake in Makefiles somewhere(?) so it may be better to fix that instead of adding yet another one, CFLAGS are not supposed to be passed to *.cc files

[gfwilliams]

Looking again it's not ideal - as far as I can see despite renaming CFLAGS the compiler for .cpp files is still called just as is? https://github.com/espruino/Espruino/blob/master/Makefile#L852-L852 It looks like for tensorflow we have cc files and those are called differently, with CCFLAGS

I guess then the question is if we have CFLAGS for C, CCFLAGS/CPPFLAGS for C++, what define do we use for all languages?

@fanoush if you're willing to do another PR that does it properly I'm very willing to revert this and merge that in instead?

[fanoush]

those are called differently, with CCFLAGS

I guess then the question is if we have CFLAGS for C, CCFLAGS/CPPFLAGS for C++, what define do we use for all languages?

CCFLAGS (double C) is actually not used in standard make(?), only CFLAGS for C and CXXFLAGS for C++, so CCFLAGS name is free to use, here https://stackoverflow.com/a/56290397 (and also one answer below it) someone mentions CCFLAGS are used when options are meant for both. So if we use it like this too then at some point we may += common CCFLAGS to both CFLAGS and CXXFLAGS or we run compiler with both (CFLAGS and CCFLAGS for .c and CXXFLAGS and CCFLAGS for c++ = .cc .cpp .C)

@fanoush if you're willing to do another PR that does it properly I'm very willing to revert this and merge that in instead?

may take few days to look into it

[fanoush]

Well, looks like we are using CFLAGS for both c and c++ and changing this would touch a lot of places so what about filtering out known C only flags before running C++ compiler (and vice versa if there is something that breaks C)? Makefiles can use $(filter ..) and $(filter-out ...) as per https://stackoverflow.com/a/70599306 or https://www.gnu.org/software/make/manual/html_node/Text-Functions.html so we could pattern match and filter-out on case by case basis as we use them. Then we could still have them mixed in one CFLAGS. Separating them into CFLAGS_C_COMPILER feels like too much noise to me.

Would pattern matching "-std=%" and "-W%=implicit-function-declaration" out from CFLAGS into CXXFLAGS for c++ be good enough or is it more complicated? I agree it is a hack and workaround but the complexity introduced by this PR is maybe even worse(?)

just tried to modify example from stackoverflow post

CXXFLAGS := $(foreach arg, $(CFLAGS), $(and \
        $(filter-out -std=%,$(arg)), \
        $(filter-out -W%=implicit-function-declaration,$(arg)) \
         ))

all:
        @echo CXXFLAGS == $(CXXFLAGS)

and it seems to work

$ make CFLAGS="-Wall -I/path/to/include -W -UMAC -DFOO=bar -o foo.o -lm -std=gnuxx -lxx -Werror=implicit-function-declaration  -Werror=xx"
CXXFLAGS == -Wall -I/path/to/include -W -UMAC -DFOO=bar -o foo.o -lm -lxx  -Werror=xx

[gfwilliams]

Thanks - yes, I like that solution! I think it makes it less likely someone accidentally adds some compile flag just to C and not C++