[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038 --- Comment #21 from Harald van Dijk --- (In reply to Segher Boessenkool from comment #20) > That is still not what I said, so don't pretend I did please. > > Those are also not false positives: in all these cases, the program does > in fact skip some initialisation. "Pretend"? When you misrepresented my words, I did not accuse you of any type of deception, I assumed it was simply a misunderstanding and responded accordingly. I'd appreciate the same treatment from you. You made an ambiguous comment which I did not recognise as ambiguous at the time. You wrote: > -Wall please, unless there are frequent false positives. [...] "False positive" can be interpreted in two ways. It can be read as a false positive wrt -Wjump-misses-init, meaning a warning where no initialisation is jumped over. It turns out this is what you meant. It can also be read as I did, which is a false positive wrt -Wall, which is documented as: > This enables all the warnings about constructions that some users > consider questionable, and that are easy to avoid (or modify to > prevent the warning), even in conjunction with macros. [...] A false positive here would mean that the warning is either about a construct that users do not consider questionable, or that is not easy to avoid. I was responding based on the assumption that this is what you meant. Initialisation is commonly jumped over in large bodies of C code, which is a good indication that it is not largely considered questionable, and of the cases I'd found, although all can be worked around to suppress the warning, quite a lot of them would get a lot uglier. It did not even scan to me that you might be saying that the only reason you'd not propose -Wjump-misses-init be included in -Wall is if -Wjump-misses-init doesn't do what it's supposed to. > But, it seems -Wjump-misses-init is not what you want; you do not want a > warning for jumping over initialisers, it warns for a lot of harmless code. > What you want is a warning that only warns if the var is (potentially) used > without initialisation? Yes, at least for -Wall. Since -Wjump-misses-init triggers too often for commonly used C patterns, I do not think it is appropriate to include it in -Wall. At the same time, for the originally reported code, it does seem like *some* warning at -Wall would be appropriate. A warning about the use of the potentially-uninitialised variable, as clang does, seems sensible to me.
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038 --- Comment #20 from Segher Boessenkool --- That is still not what I said, so don't pretend I did please. Those are also not false positives: in all these cases, the program does in fact skip some initialisation. But, it seems -Wjump-misses-init is not what you want; you do not want a warning for jumping over initialisers, it warns for a lot of harmless code. What you want is a warning that only warns if the var is (potentially) used without initialisation?
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038
--- Comment #19 from Harald van Dijk ---
(In reply to Segher Boessenkool from comment #18)
> (In reply to Harald van Dijk from comment #15)
> > (In reply to Segher Boessenkool from comment #10)
> > > The initialisation (the call to f1) could have a side effect, but the
> > > a==1 case skips that. GCC is right to warn here in my opinion.
>
> > And warnings that are likely to trigger on
> > perfectly valid code that behaves exactly as the author intended shouldn't
> > be included in -Wall, per your own comment.
>
> No, that is not what I said. *All* warnings are likely to trigger on valid
> code, after all. I said "frequent false positives".
Please re-read what I said. I didn't just say valid code, I said valid code
that behaves exactly as the author intended, or in other words code where the
fact that the initialisation is skipped over does not point to any problem, or
in other words a false positive.
I'm only at the start of a rebuild to see how commonly this warning will be
triggered, but I have found more false positives already almost immediately:
alsa-lib-1.1.7:
setup.c: In function 'snd_config_get_ctl_elem_value':
setup.c:361:5: error: jump skips variable initialization
[-Werror=jump-misses-init]
goto _bad_content;
^~~~
setup.c:313:4: note: label '_bad_content' defined here
_bad_content:
^~~~
setup.c:311:17: note: 'idx' declared here
unsigned int idx = 0;
^~~
(and more)
anjuta-3.28.0:
jsparse.c: In function 'print_node':
jsparse.c:49:4: error: switch jumps over variable initialization
[-Werror=jump-misses-init]
case TOK_RC:
^~~~
jsparse.c:32:4: note: switch starts here
switch ((JSTokenType)node->pn_type)
^~
jsparse.c:43:9: note: 'k' declared here
int k = 0;
^
(and more)
at-spi2-core-2.30.0:
../at-spi2-core-2.30.0/dbind/dbind-any.c: In function 'dbind_any_demarshal':
../at-spi2-core-2.30.0/dbind/dbind-any.c:642:5: error: switch jumps over
variable initialization [-Werror=jump-misses-init]
case DBUS_TYPE_VARIANT:
^~~~
../at-spi2-core-2.30.0/dbind/dbind-any.c:541:5: note: switch starts here
switch (**type) {
^~
../at-spi2-core-2.30.0/dbind/dbind-any.c:618:21: note: 'offset' declared here
int offset = 0, stralign;
^~
(and more)
busybox-1.29.3:
archival/bbunzip.c: In function 'bbunpack':
archival/bbunzip.c:68:6: error: jump skips variable initialization
[-Werror=jump-misses-init]
goto free_name;
^~~~
archival/bbunzip.c:183:2: note: label 'free_name' defined here
free_name:
^
archival/bbunzip.c:139:10: note: 'del' declared here
char *del = new_name;
^~~
In file included from archival/bzip2.c:101:
archival/libarchive/bz/compress.c: In function 'generateMTFValues':
archival/libarchive/bz/compress.c:286:3: error: jump skips variable
initialization [-Werror=jump-misses-init]
goto process_zPend; /* "process it and come back here" */
^~~~
archival/libarchive/bz/compress.c:251:2: note: label 'process_zPend' defined
here
process_zPend:
^
archival/libarchive/bz/compress.c:235:11: note: 'll_i' declared here
uint8_t ll_i = ll_i; /* gcc 4.3.1 thinks it may be used w/o init */
^~~~
cargo-vendor-0.1.15:
/h/cargo-vendor-src-0.1.15/vendor/libssh2-sys/libssh2/src/packet.c: In function
'_libssh2_packet_add':
/h/cargo-vendor-src-0.1.15/vendor/libssh2-sys/libssh2/src/packet.c:454:9:
error: jump skips variable initialization [-Werror=jump-misses-init]
goto libssh2_packet_add_jump_point5;
^~~~
/h/cargo-vendor-src-0.1.15/vendor/libssh2-sys/libssh2/src/packet.c:588:19:
note: label 'libssh2_packet_add_jump_point5' defined here
libssh2_packet_add_jump_point5:
^~
/h/cargo-vendor-src-0.1.15/vendor/libssh2-sys/libssh2/src/packet.c:574:31:
note: 'want_reply' declared here
unsigned char want_reply=0;
^~
(and more)
chromium-71.0.3578.10:
../../third_party/angle/third_party/vulkan-loader/src/loader/trampoline.c: In
function 'vkEnumeratePhysicalDevices':
../../third_party/angle/third_party/vulkan-loader/src/loader/trampoline.c:658:9:
error: jump skips variable initialization [-Werror=jump-misses-init]
goto out;
^~~~
../../third_party/angle/third_party/vulkan-loader/src/loader/trampoline.c:696:1:
note: label 'out' defined here
out:
^~~
../../third_party/angle/third_party/vulkan-loader/src/loader/trampoline.c:671:14:
note: 'setup_res' declared here
VkResult setup_res = setupLoaderTrampPhysDevs(instance);
^
(and more)
efivar-36:
efivarfs.c: In function 'efivarfs_get_variable_size':
efivarfs.c:182:3: error: jump skips variable initialization
[-Werror=jump-misses-init]
goto err;
^~~~
efivarfs.c:195:1: note: label 'err' defined here
err:
^~~
efivarfs.c:185:14:
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038 --- Comment #18 from Segher Boessenkool --- (In reply to Harald van Dijk from comment #15) > (In reply to Segher Boessenkool from comment #10) > > The initialisation (the call to f1) could have a side effect, but the > > a==1 case skips that. GCC is right to warn here in my opinion. > And warnings that are likely to trigger on > perfectly valid code that behaves exactly as the author intended shouldn't > be included in -Wall, per your own comment. No, that is not what I said. *All* warnings are likely to trigger on valid code, after all. I said "frequent false positives".
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038 --- Comment #17 from Harald van Dijk --- (In reply to Steinar H. Gunderson from comment #16) > Since you're asking (presumably rhethorically): I would certainly recommend > that this code be changed, yes, and I don't find the author's intent obvious > at all. I'm surprised by that. I'm not the one who wrote the code, but it seemed obvious to me when I started working with it. > The fix that comes to mind is to scope the case clause: Okay, that's cleaner than what I had suggested. Still, I hope you will check how much software uses this before deciding on whether to include the warning in -Wall. I can probably try a rebuild of my current system with that warning turned into an error to get an estimate of how big the impact will be, if you like, though it will not be as extensive as if one of the distros were willing to include it in their next mass rebuild.
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038
--- Comment #16 from Steinar H. Gunderson ---
Since you're asking (presumably rhethorically): I would certainly recommend
that this code be changed, yes, and I don't find the author's intent obvious at
all. The fix that comes to mind is to scope the case clause:
void f4(int a) {
switch (a) {
case 0: {
int i = f1();
f3(i);
break;
}
case 1: {
int i = f2();
f3(i);
break;
}
}
}
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038 --- Comment #15 from Harald van Dijk --- (In reply to Segher Boessenkool from comment #10) > The initialisation (the call to f1) could have a side effect, but the > a==1 case skips that. GCC is right to warn here in my opinion. With an explicit -Wjump-misses-init, sure, that's the point of the warning. With -Wall, no, as there is nothing wrong with the code and it's quite intentional that the initialisation is skipped. Would you recommend to the author of whoever wrote that that the initialiser should be taken out of the declaration and put in an assignment on a separate line? That makes no sense to me. (And by the way, this is not hypothetical. This warning *does* trigger on code I work on.) The side effect is irrelevant, by the way, as the warning is triggered even if the initialiser is constant. And warnings that are likely to trigger on perfectly valid code that behaves exactly as the author intended shouldn't be included in -Wall, per your own comment.
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038 --- Comment #14 from Segher Boessenkool --- I am saying that no warning should warn for things that are valid C but invalid C++, except with -Wc++-compat; not for that reason, anyway. -Wjump-misses-init should warn here: the jump does miss the init! And it can be quite harmful in cases, and is trivial to work around. But warning for constructs that are not valid in C++, when compiling C code (and not asking for such a warning specifically) is a disservice to the user.
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038 --- Comment #13 from Steinar H. Gunderson --- That could be (I disagree, but that's another debate), but the question was whether allowing this case would improve the warning or not. If you change it to allowing such a case, you also make -Wc++-compat less good at its job (unless you add yet another warning of course).
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038 --- Comment #12 from Segher Boessenkool --- That is not a reason to have the warning in C, not without some "-Wc++-compat" or similar; and in C++ it should be an error you say, not a warning at all.
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038 --- Comment #11 from Steinar H. Gunderson --- It is also not legal (side effects or not) when compiling as C++, which is one of the reasons for having this warning.
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038 --- Comment #10 from Segher Boessenkool --- The initialisation (the call to f1) could have a side effect, but the a==1 case skips that. GCC is right to warn here in my opinion.
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038
Harald van Dijk changed:
What|Removed |Added
CC||harald at gigawatt dot nl
--- Comment #9 from Harald van Dijk ---
(In reply to Segher Boessenkool from comment #8)
> -Wall please, unless there are frequent false positives.
-Wjump-misses-init warns for harmless code such as
int f1();
int f2();
void f3(int);
void f4(int a) {
switch (a) {
case 0:;
int i = f1();
f3(i);
break;
case 1:
i = f2();
f3(i);
break;
}
}
Here, yes, the initialisation is jumped over, but the variable is never used
uninitialised. clang manages to avoid warning for this.
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038 Segher Boessenkool changed: What|Removed |Added CC||segher at gcc dot gnu.org --- Comment #8 from Segher Boessenkool --- -Wall please, unless there are frequent false positives. Things shouldn't move between warning levels unless there is a good reason for it (like, the accuracy of the warning was improved, or it was put in the wrong level in the first place). We shouldn't put warnings under less used levels first; this is not a popularity contest.
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038 Eric Gallager changed: What|Removed |Added Blocks||87656 --- Comment #7 from Eric Gallager --- (In reply to Steinar H. Gunderson from comment #4) > Oh, it exists? Yes, if so, please count this as a request for enabling on > -Wextra. Anyone know why it isn't already? Referenced Bugs: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87656 [Bug 87656] Useful flags to enable with -Wall or -Wextra
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038 Eric Gallager changed: What|Removed |Added CC||egallager at gcc dot gnu.org --- Comment #6 from Eric Gallager --- (In reply to Richard Biener from comment #5) > I think even -Wall makes sense. Maybe eventually, but I think a good compromise would be to put it in -Wextra for gcc 9, and then move it to -Wall for gcc 10, so people have time to adjust to the gradual strictening
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038 --- Comment #5 from Richard Biener --- I think even -Wall makes sense.
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038 --- Comment #4 from Steinar H. Gunderson --- Oh, it exists? Yes, if so, please count this as a request for enabling on -Wextra.
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038 --- Comment #3 from joseph at codesourcery dot com --- This is Wjump-misses-init. Is this a request to make some other option such as -Wall or -Wextra enable that option (rather than just -Wc++-compat as at present)?
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038
--- Comment #2 from Jonathan Wakely ---
And for the goto case Clang says:
87038.c:2:13: warning: variable 'foo' is used uninitialized whenever 'if'
condition is true [-Wsometimes-uninitialized]
if (x == 0) goto lbl;
^~
87038.c:5:31: note: uninitialized use occurs here
printf("foo is %d\n", foo);
^~~
87038.c:2:9: note: remove the 'if' if its condition is always false
if (x == 0) goto lbl;
^~~~
87038.c:3:9: note: variable 'foo' is declared here
int foo = 3;
^
1 warning generated.
[Bug c/87038] diagnostics: Please add warning for jumping over initializers with switch/case in C mode
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87038
Jonathan Wakely changed:
What|Removed |Added
Keywords||diagnostic
Status|UNCONFIRMED |NEW
Last reconfirmed||2018-08-21
Ever confirmed|0 |1
Severity|normal |enhancement
--- Comment #1 from Jonathan Wakely ---
Confirmed. Clang warns consistently, even at -O0:
87038.c:5:30: warning: variable 'foo' is used uninitialized whenever switch
case is taken [-Wsometimes-uninitialized]
case 0:
^
87038.c:6:55: note: uninitialized use occurs here
printf("foo is %d\n", foo);
^~~
87038.c:4:25: note: variable 'foo' is declared here
int foo = 3;
^
1 warning generated.
