| Message ID | 24052956-4978-21a7-b364-f5d06e7e6b49@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Add -C when using -Wimplicit-fallthrough and --save-temps (PR preprocessor/78497). | expand |
On Tue, Apr 03, 2018 at 02:29:37PM +0200, Martin Liška wrote: > Hi. > > This helps the warning with --save-temps. Doing that one needs to preserve comments > in preprocessed source file. Do we really want to only use -C when -Wimplicit-fallthrough is in effect? I mean, shouldn't we always use -C when -save-temps? Marek
On 04/04/2018 09:31 PM, Marek Polacek wrote: > On Tue, Apr 03, 2018 at 02:29:37PM +0200, Martin Liška wrote: >> Hi. >> >> This helps the warning with --save-temps. Doing that one needs to preserve comments >> in preprocessed source file. > > Do we really want to only use -C when -Wimplicit-fallthrough is in effect? I > mean, shouldn't we always use -C when -save-temps? Why not, Jakub what do you think? Note that it was originally Jakub's idea to do that. Martin > > Marek >
On Thu, Apr 05, 2018 at 02:51:22PM +0200, Martin Liška wrote: > On 04/04/2018 09:31 PM, Marek Polacek wrote: > > On Tue, Apr 03, 2018 at 02:29:37PM +0200, Martin Liška wrote: > >> Hi. > >> > >> This helps the warning with --save-temps. Doing that one needs to preserve comments > >> in preprocessed source file. > > > > Do we really want to only use -C when -Wimplicit-fallthrough is in effect? I > > mean, shouldn't we always use -C when -save-temps? > > Why not, Jakub what do you think? Note that it was originally Jakub's idea to do that. I'd prefer to do that only when we actually care about the comments, it is a behavior change in any case, and might be undesirable to some people. Note that we do not care about the comments if -Wimplicit-fallthrough=0 or -Wimplicit-fallthrough=5, but do care for: -Wimplicit-fallthrough -Wimplicit-fallthrough=1 -Wimplicit-fallthrough=2 -Wimplicit-fallthrough=3 -Wimplicit-fallthrough=4 -Wextra -W (unless -Wno-implicit-fallthrough). So, it would be desirable to: 1) swap the order, put save-temps to the outer level 2) use {Wimplicit-fallthrough*:{!Wimplicit-fallthrough=0:{!Wimplicit-fallthrough=5:...}}} 3) verify (including adding testcases) that it doesn't emit comments for the =0, =5 or -W -Wno-implicit-fallthrough cases, but does for -W etc. Jakub
On 04/05/2018 03:00 PM, Jakub Jelinek wrote: > On Thu, Apr 05, 2018 at 02:51:22PM +0200, Martin Liška wrote: >> On 04/04/2018 09:31 PM, Marek Polacek wrote: >>> On Tue, Apr 03, 2018 at 02:29:37PM +0200, Martin Liška wrote: >>>> Hi. >>>> >>>> This helps the warning with --save-temps. Doing that one needs to preserve comments >>>> in preprocessed source file. >>> >>> Do we really want to only use -C when -Wimplicit-fallthrough is in effect? I >>> mean, shouldn't we always use -C when -save-temps? >> >> Why not, Jakub what do you think? Note that it was originally Jakub's idea to do that. > > I'd prefer to do that only when we actually care about the comments, it is a > behavior change in any case, and might be undesirable to some people. > > Note that we do not care about the comments if -Wimplicit-fallthrough=0 > or -Wimplicit-fallthrough=5, but do care for: > -Wimplicit-fallthrough > -Wimplicit-fallthrough=1 > -Wimplicit-fallthrough=2 > -Wimplicit-fallthrough=3 > -Wimplicit-fallthrough=4 > -Wextra > -W Or you can trigger the warning via -Werror=implicit-fallthrough* which would complicate the option matching. > (unless -Wno-implicit-fallthrough). So, it would be desirable to: > 1) swap the order, put save-temps to the outer level > 2) use > {Wimplicit-fallthrough*:{!Wimplicit-fallthrough=0:{!Wimplicit-fallthrough=5:...}}} > 3) verify (including adding testcases) that it doesn't emit comments for the > =0, =5 or -W -Wno-implicit-fallthrough cases, but does for -W etc. That would also complicate the exclusion of negative forms. Sigh. I'm thinking if really worth it.. Martin > > Jakub >
On 04/03/2018 06:29 AM, Martin Liška wrote: > Hi. > > This helps the warning with --save-temps. Doing that one needs to preserve comments > in preprocessed source file. > > Ready for trunk? > Martin > > gcc/ChangeLog: > > 2018-04-03 Martin Liska <mliska@suse.cz> > > PR preprocessor/78497 > * gcc.c: Add -C when using -Wimplicit-fallthrough and --save-temps. > > gcc/testsuite/ChangeLog: > > 2018-04-03 Martin Liska <mliska@suse.cz> > > PR preprocessor/78497 > * c-c++-common/Wimplicit-fallthrough-37.c: New test. OK. jeff
diff --git a/gcc/gcc.c b/gcc/gcc.c index a716f708259..f641c249e27 100644 --- a/gcc/gcc.c +++ b/gcc/gcc.c @@ -1131,7 +1131,8 @@ static const char *cpp_options = "%(cpp_unique_options) %1 %{m*} %{std*&ansi&trigraphs} %{W*&pedantic*} %{w}\ %{f*} %{g*:%{%:debug-level-gt(0):%{g*}\ %{!fno-working-directory:-fworking-directory}}} %{O*}\ - %{undef} %{save-temps*:-fpch-preprocess}"; + %{undef} %{save-temps*:-fpch-preprocess}\ + %{Wimplicit-fallthrough*|Werror=implicit-fallthrough*:%{save-temps*:-C}}"; /* This contains cpp options which are not passed when the preprocessor output will be used by another program. */ diff --git a/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-37.c b/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-37.c new file mode 100644 index 00000000000..ca9d21fc70e --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wimplicit-fallthrough-37.c @@ -0,0 +1,22 @@ +/* PR preprocessor/78497 */ +/* { dg-do compile } */ +/* { dg-options "-Wimplicit-fallthrough --save-temps" } */ + +int main (int argc, char **argv) +{ + int a; + switch (argc) + { + case 1: + a = 1; + break; + case 2: + a = 2; + /* FALLTHROUGH */ + case 3: + a = 3; + break; + } + + return a; +}