| Message ID | YMdIxgUERRTksgaf@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c-family: Add fix-it suggestions for <stdlib.h> names [PR101052] | expand |
On 6/14/21 6:17 AM, Jonathan Wakely via Gcc-patches wrote: > PR c++/101052 > > gcc/c-family/ChangeLog: > > * known-headers.cc (get_stdlib_header_for_name): Add known > headers for EXIT_FAILURE, EXIT_SUCCESS, abort, atexit, calloc, > exit, and getenv. > > gcc/testsuite/ChangeLog: > > * g++.dg/spellcheck-stdlib.C: Add checks for <cstdlib> names. > * gcc.dg/spellcheck-stdlib.c: Likewise. > > There are no C tests for the functions because the C front-end doesn't > give fix-it hints for them, it complains about mismatched implicit > declarations. I assume this is why there were no existing tests for > malloc, free, etc. > > Tested powerpc64le-linux. OK for trunk? A while back I noticed some other symbol wasn't giving a nice hint. I think it was one of the <stdint.h> macros that I can never remember if it's in <inttypes.h> or <limits.h>. So decided to add them all, and while I was there, I noticed that the function does a linear search through the array. That seems suboptimal so I started optimizing it and never finished the work. So I'm wondering if you stopped at the seven above for the same reason or some other? FWIW, I think the symbols that are hardcoded there now are some of the most obvious ones. The ones that are harder to remember tend to be missing. (E.g., I'd probably include <wchar.h> for wcstombs and would have to look it up if I got an error. YMMV of course.) This is not an objection to adding just a handful of new names, just a comment on the general state of incompleteness and the inefficient data structure in case that matters. If we don't care enough about the inefficiency to avoid completing the list then I'd suggest to add all the missing <stdlib.h> names, not just the seven above. Martin
On Mon, Jun 14, 2021 at 01:17:10PM +0100, Jonathan Wakely via Gcc-patches wrote: > commit 8f211596b34662f4b9a041fdf8634f69cf8efe4c > Author: Jonathan Wakely <jwakely@redhat.com> > Date: Mon Jun 14 11:38:11 2021 > > c-family: Add fix-it suggestions for <stdlib.h> names [PR101052] > > PR c++/101052 > > gcc/c-family/ChangeLog: > > * known-headers.cc (get_stdlib_header_for_name): Add known > headers for EXIT_FAILURE, EXIT_SUCCESS, abort, atexit, calloc, > exit, and getenv. > > gcc/testsuite/ChangeLog: > > * g++.dg/spellcheck-stdlib.C: Add checks for <cstdlib> names. > * gcc.dg/spellcheck-stdlib.c: Likewise. LGTM, thanks. Jakub
On Mon, 14 Jun 2021 at 18:03, Martin Sebor <msebor@gmail.com> wrote: > > On 6/14/21 6:17 AM, Jonathan Wakely via Gcc-patches wrote: > > PR c++/101052 > > > > gcc/c-family/ChangeLog: > > > > * known-headers.cc (get_stdlib_header_for_name): Add known > > headers for EXIT_FAILURE, EXIT_SUCCESS, abort, atexit, calloc, > > exit, and getenv. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/spellcheck-stdlib.C: Add checks for <cstdlib> names. > > * gcc.dg/spellcheck-stdlib.c: Likewise. > > > > There are no C tests for the functions because the C front-end doesn't > > give fix-it hints for them, it complains about mismatched implicit > > declarations. I assume this is why there were no existing tests for > > malloc, free, etc. > > > > Tested powerpc64le-linux. OK for trunk? > > A while back I noticed some other symbol wasn't giving a nice hint. > I think it was one of the <stdint.h> macros that I can never remember > if it's in <inttypes.h> or <limits.h>. So decided to add them all, > and while I was there, I noticed that the function does a linear > search through the array. That seems suboptimal so I started > optimizing it and never finished the work. Yes, I noticed that. We only search the array after compilation fails, so performance doesn't really matter. > > So I'm wondering if you stopped at the seven above for the same > reason or some other? I lost the motivation to add the rest of <stdlib.h> when testing it for C turned out to be difficult due to C allowing implicit declarations. > > FWIW, I think the symbols that are hardcoded there now are some of > the most obvious ones. The ones that are harder to remember tend > to be missing. (E.g., I'd probably include <wchar.h> for wcstombs > and would have to look it up if I got an error. YMMV of course.) Yes, that's why I wanted to add aligned_alloc, at_quick_exit and quick_exit, in a new c11_cxx11_hints array. They're maybe less "well known" than exit and malloc. > This is not an objection to adding just a handful of new names, just > a comment on the general state of incompleteness and the inefficient > data structure in case that matters. If we don't care enough about > the inefficiency to avoid completing the list then I'd suggest to > add all the missing <stdlib.h> names, not just the seven above.
On Mon, 2021-06-14 at 13:17 +0100, Jonathan Wakely wrote: > PR c++/101052 > > gcc/c-family/ChangeLog: > > * known-headers.cc (get_stdlib_header_for_name): Add known > headers for EXIT_FAILURE, EXIT_SUCCESS, abort, atexit, > calloc, > exit, and getenv. > > gcc/testsuite/ChangeLog: > > * g++.dg/spellcheck-stdlib.C: Add checks for <cstdlib> names. > * gcc.dg/spellcheck-stdlib.c: Likewise. > > There are no C tests for the functions because the C front-end > doesn't > give fix-it hints for them, it complains about mismatched implicit > declarations. I assume this is why there were no existing tests for > malloc, free, etc. > > Tested powerpc64le-linux. OK for trunk? > LGTM
diff --git a/gcc/c-family/known-headers.cc b/gcc/c-family/known-headers.cc index 85924718c8b..a3912468a8d 100644 --- a/gcc/c-family/known-headers.cc +++ b/gcc/c-family/known-headers.cc @@ -162,7 +162,14 @@ get_stdlib_header_for_name (const char *name, enum stdlib lib) {"stdout", {"<stdio.h>", "<cstdio>"} }, /* <stdlib.h> and <cstdlib>. */ + {"EXIT_FAILURE", {"<stdlib.h>", "<cstdlib>"} }, + {"EXIT_SUCCESS", {"<stdlib.h>", "<cstdlib>"} }, + {"abort", {"<stdlib.h>", "<cstdlib>"} }, + {"atexit", {"<stdlib.h>", "<cstdlib>"} }, + {"calloc", {"<stdlib.h>", "<cstdlib>"} }, + {"exit", {"<stdlib.h>", "<cstdlib>"} }, {"free", {"<stdlib.h>", "<cstdlib>"} }, + {"getenv", {"<stdlib.h>", "<cstdlib>"} }, {"malloc", {"<stdlib.h>", "<cstdlib>"} }, {"realloc", {"<stdlib.h>", "<cstdlib>"} }, diff --git a/gcc/testsuite/g++.dg/spellcheck-stdlib.C b/gcc/testsuite/g++.dg/spellcheck-stdlib.C index 31e91fe8d1b..87736b25e54 100644 --- a/gcc/testsuite/g++.dg/spellcheck-stdlib.C +++ b/gcc/testsuite/g++.dg/spellcheck-stdlib.C @@ -138,6 +138,24 @@ void test_cstdlib (void *q) // { dg-message "'#include <cstdlib>'" "" { target *-*-* } .-1 } q = realloc (q, 1024); // { dg-error "was not declared" } // { dg-message "'#include <cstdlib>'" "" { target *-*-* } .-1 } + q = calloc (8, 8); // { dg-error "was not declared" } + // { dg-message "'#include <cstdlib>'" "" { target *-*-* } .-1 } + + void callback (); + atexit (callback); // { dg-error "was not declared" } + // { dg-message "'#include <cstdlib>'" "" { target *-*-* } .-1 } + int i; + i = EXIT_SUCCESS; // { dg-error "was not declared" } + // { dg-message "'#include <cstdlib>'" "" { target *-*-* } .-1 } + i = EXIT_FAILURE; // { dg-error "was not declared" } + // { dg-message "'#include <cstdlib>'" "" { target *-*-* } .-1 } + exit (i); // { dg-error "was not declared" } + // { dg-message "'#include <cstdlib>'" "" { target *-*-* } .-1 } + abort (); // { dg-error "was not declared" } + // { dg-message "'#include <cstdlib>'" "" { target *-*-* } .-1 } + + getenv ("foo"); // { dg-error "was not declared" } + // { dg-message "'#include <cstdlib>'" "" { target *-*-* } .-1 } } /* Verify that we don't offer suggestions to stdlib globals names when diff --git a/gcc/testsuite/gcc.dg/spellcheck-stdlib.c b/gcc/testsuite/gcc.dg/spellcheck-stdlib.c index 1ae3b5e33ab..7297a92368f 100644 --- a/gcc/testsuite/gcc.dg/spellcheck-stdlib.c +++ b/gcc/testsuite/gcc.dg/spellcheck-stdlib.c @@ -38,6 +38,16 @@ void test_stdio_h (void) /* { dg-message "'EOF' is defined in header '<stdio.h>'; did you forget to '#include <stdio.h>'?" "" { target *-*-* } .-1 } */ } +/* Missing <stdlib.h>. */ + +void test_stdlib (int i) +{ + i = EXIT_SUCCESS; /* { dg-error "'EXIT_SUCCESS' undeclared" } */ + /* { dg-message "'EXIT_SUCCESS' is defined in header '<stdlib.h>'; did you forget to '#include <stdlib.h>'?" "" { target *-*-* } .-1 } */ + i = EXIT_FAILURE; /* { dg-error "'EXIT_FAILURE' undeclared" } */ + /* { dg-message "'EXIT_FAILURE' is defined in header '<stdlib.h>'; did you forget to '#include <stdlib.h>'?" "" { target *-*-* } .-1 } */ +} + /* Missing <errno.h>. */ int test_errno_h (void)
PR c++/101052 gcc/c-family/ChangeLog: * known-headers.cc (get_stdlib_header_for_name): Add known headers for EXIT_FAILURE, EXIT_SUCCESS, abort, atexit, calloc, exit, and getenv. gcc/testsuite/ChangeLog: * g++.dg/spellcheck-stdlib.C: Add checks for <cstdlib> names. * gcc.dg/spellcheck-stdlib.c: Likewise. There are no C tests for the functions because the C front-end doesn't give fix-it hints for them, it complains about mismatched implicit declarations. I assume this is why there were no existing tests for malloc, free, etc. Tested powerpc64le-linux. OK for trunk? commit 8f211596b34662f4b9a041fdf8634f69cf8efe4c Author: Jonathan Wakely <jwakely@redhat.com> Date: Mon Jun 14 11:38:11 2021 c-family: Add fix-it suggestions for <stdlib.h> names [PR101052] PR c++/101052 gcc/c-family/ChangeLog: * known-headers.cc (get_stdlib_header_for_name): Add known headers for EXIT_FAILURE, EXIT_SUCCESS, abort, atexit, calloc, exit, and getenv. gcc/testsuite/ChangeLog: * g++.dg/spellcheck-stdlib.C: Add checks for <cstdlib> names. * gcc.dg/spellcheck-stdlib.c: Likewise.