make: ignore fallthroughs errors with GCC 7.x#8430
Conversation
jnohlgard
left a comment
There was a problem hiding this comment.
Looks good for the pkgs, but why do we need to change the default implicit fall through regex choice?
|
|
||
| MODULE:=$(PKG_NAME) | ||
|
|
||
| CFLAGS += -Wno-implicit-fallthrough |
There was a problem hiding this comment.
Add a comment above this line why it is necessary.
| PKG_VERSION=ef41ce5d02d64cec0751882ae8fd95f6c32bc018 | ||
| PKG_LICENSE=BSD-2-Clause | ||
|
|
||
| CFLAGS += -Wno-implicit-fallthrough |
There was a problem hiding this comment.
Add a comment above this line describing why this is necessary
Makefile.include
Outdated
| export WERROR | ||
| ifeq ($(WERROR),1) | ||
| CFLAGS += -Wall -Werror -Wextra | ||
| CFLAGS += -Wall -Werror -Wextra -Werror=implicit-fallthrough=2 |
There was a problem hiding this comment.
Why do we need this? Are some comments wrongly formatted to match the gcc default regex?
There was a problem hiding this comment.
Not wrongly formatted, but default is -Werror=implicit-fallthrough=3, which only allows (or matches) on one-line comments with very limited (and case sensitive) wording, e.g., /* falls through */, but some like to add an explanation like /* falls through intentionally because of XYZ */, see here for instance, which does not match and hence causes an error.
However, considering 17000++ errors this gives us for older compilers that do not recognise this option - we better fix/adapt the comments - I guess.
|
new record! ;) |
I always aim for that, where is my medal 😉 🏅 |
bbce413 to
b3fde34
Compare
|
reworked and added some more fixes for GCC 7 |
miri64
left a comment
There was a problem hiding this comment.
Please also add a fallthrough ignore to pkg/emb6/Makefile.include
| PKG_LICENSE=BSD-2-Clause | ||
|
|
||
| # GCC 7.x fails on (intentional) fallthrough, thus disable implicit-fallthrough. | ||
| CFLAGS += -Wno-implicit-fallthrough |
There was a problem hiding this comment.
Shouldn't this rather go in pkg/libcoap/Makefile.include?
There was a problem hiding this comment.
mhm, @gebart was suggesting to use Makefile not Makefile.include so the option is only effective when compiling the package. IIRC putting it into Makefile.include exposes this to RIOT completely.
There was a problem hiding this comment.
Ah, you are right... But with emb6 this doesn't work somehow.
There was a problem hiding this comment.
Ah, because emb6 has submodules and libcoap and oonf_api do not... So at least for those submodules we somehow also need to activate -Wno-implicit-fallthrough... I think it is easier just to take the patch from smlng#3
for the moment, since porting to newest emb6 isn't that simple ;-).
| MODULE:=$(PKG_NAME) | ||
|
|
||
| # GCC 7.x fails on (intentional) fallthrough, thus disable implicit-fallthrough. | ||
| CFLAGS += -Wno-implicit-fallthrough |
There was a problem hiding this comment.
Dito for pkg/oonf_api/Makefile.include?
|
emb6 is separately fixed in #8571, the Murdock error is unrelated I'll retriever |
|
Murdock is happy now -> go |
Contribution description
This PR fixes several implicit-fallthrough errors when using GCC 7.x. That is, for libcoap and oonf_api package the errors are ignored, and this PR also relaxes
-Werror=implicit-fallthrough=2to allow more descriptive comments. See also hereIssues/PRs references
#8265