search

yhirose / cpp-peglib

A single file C++ header-only PEG (Parsing Expression Grammars) library
MIT License
900 stars 112 forks source link

Fix missing return statement in lambda #99

Closed halirutan closed 4 years ago

halirutan commented 4 years ago

This PR fixes a warning with g++ 9.0 and handles, when e.g. an element is added to Loop::Type without adapting the switch-statement.

It will fix https://github.com/yhirose/cpp-peglib/issues/98

yhirose commented 4 years ago

@halirutan, thank you for your contribution!

yhirose commented 4 years ago

@halirutan, it now shows the following warning on my machine...

[master] ~/Projects/cpp-peglib/build$ make
Scanning dependencies of target test-main
[  5%] Building CXX object test/CMakeFiles/test-main.dir/test1.cc.o
In file included from /Users/hirose/Projects/cpp-peglib/test/test1.cc:2:
/Users/hirose/Projects/cpp-peglib/test/../peglib.h:3141:9: warning: default label in switch which covers all
      enumeration values [-Wcovered-switch-default]
        default:
        ^
^Cmake[2]: *** [test/CMakeFiles/test-main.dir/test1.cc.o] Interrupt: 2
make[1]: *** [test/CMakeFiles/test-main.dir/all] Interrupt: 2
make: *** [all] Interrupt: 2

Could you show me what is the original warning with g++ 9.0 again? Thanks!

halirutan commented 4 years ago

That is unfortunate. I had posted the exact warning for GCC in the issue I opened.

I just spent half an hour reading through a discussion on the GCC list. It all comes down to that GCC and clang have opposing warnings. This seems to be such a discussion point that clang actually mentions it on its coding standard page:

A knock-on effect of this stylistic requirement is that when building LLVM with GCC you may get warnings related to “control may reach end of non-void function” if you return from each case of a covered switch-over-enum because GCC assumes that the enum expression may take any representable value, not just those of individual enumerators. To suppress this warning, use llvm_unreachable after the switch.

I'm definitely no language advocate, and I did not know that fixing the warning for GCC will introduce a new warning for clang. The simple solution is that you revert my changes, so that you, as the developer, get no contradictory warnings. If you want to ensure that GCC users don't get a warning when they compile cpp-peglib, you could add -Wno-return-type in the GNU section of your CMake build file.

Honestly, I'm a bit shocked that GCC and clang could not agree on what is the right way to use switch with an enum.

yhirose commented 4 years ago

@halirutan, don't worry about it, because tools can never be perfect. :) I really appreciate your fine research, it's so helpful. I'll install gcc 9.0 on my machine when I have time, and try to find a solution for both compilers. Thanks a lot!