makefiles/cflags.inc.mk: Use a template for CFLAGS testing#9358
makefiles/cflags.inc.mk: Use a template for CFLAGS testing#9358miri64 merged 3 commits intoRIOT-OS:masterfrom
Conversation
ffc0a02 to
ebffd54
Compare
|
#9357 was merged. Please rebase. |
ebffd54 to
0b2fbc6
Compare
|
rebased |
There was a problem hiding this comment.
As requested inline, I would just the commit split in two and some other tester on mac.
I tested on Linux/gcc with wsn430-v1_3b, iotlab-m3, arduino-mega2560, pic32-wifire and it worked.
-fdiagnostics-color is not added for wsn320-v1_3b for both master and this PR.
I also tried to add some new gcc7 warning (-Wduplicated-branches) to the foreach line and they are correctly only added for gcc7 compilers.
I used this diff for testing:
diff --git a/makefiles/cflags.inc.mk b/makefiles/cflags.inc.mk
index edd578b..94d6769 100644
--- a/makefiles/cflags.inc.mk
+++ b/makefiles/cflags.inc.mk
@@ -25,6 +25,8 @@ ifeq ($(shell $(CC) -fno-delete-null-pointer-checks -E - 2>/dev/null >/dev/null
endif
endif
+OLD_CFLAGS := $(CFLAGS)
+
# Template for testing a compiler flag and adding it to CFLAGS (errors usually
# happens when using older toolchains which do not support the given flags)
define cflags_test_and_add
@@ -43,6 +45,8 @@ endif
# Worse yet they hide errors by accepting wildcard argument types.
$(foreach flag,-Wstrict-prototypes -Wold-style-definition,$(eval $(call cflags_test_and_add,$(flag))))
+$(info DIFF: $(filter-out $(OLD_CFLAGS),$(CFLAGS)))
+
# Unwanted flags for c++
CXXUWFLAGS += -std=%
CXXUWFLAGS += -Wstrict-prototypes -Wold-style-definitionCan someone test on mac/llvm ? @kYc0o ?
| ifeq ($(shell $(CC) -Wstrict-prototypes -Werror=strict-prototypes -Wold-style-definition -Werror=old-style-definition -E - 2>/dev/null >/dev/null </dev/null ; echo $$?),0) | ||
| # duplicated parameters don't hurt | ||
| CFLAGS += -Wstrict-prototypes -Wold-style-definition | ||
| CXXUWFLAGS += -Wstrict-prototypes -Wold-style-definition |
There was a problem hiding this comment.
Just for clarity, I would prefer moving the CXXUWFLAGS and removing the useless WERROR case to a separate "cleanup" commit.
Works correctly. |
|
I can do the split and push it here if you want. |
WERROR already sets -Werror so this is not necessary.
The excluded patterns can always be defined as they only set `CFLAGS` that should not be passed to `CXX`. This prepares for replacing the cflags support detection by a function.
0b2fbc6 to
843cec5
Compare
|
I reviewed and tested the changes but I also contributed by splitting the commits so I cannot ACK. Can someone else ACK the changes ? |
|
Thanks @cladmi |
|
Tested with both |
|
Thanks |
Contribution description
Using a makefile template reduces the code duplication in cflags.inc.mk
Issues/PRs references
depends on
#9357