[PATCH] c-family: Add fix-it suggestions for <stdlib.h> names [PR101052]

[PATCH] c-family: Add fix-it suggestions for <stdlib.h> names [PR101052]

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 583 bytes --]

	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?


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3824 bytes --]

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.

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)

Re: [PATCH] c-family: Add fix-it suggestions for <stdlib.h> names [PR101052]

[Martin Sebor]

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

Re: [PATCH] c-family: Add fix-it suggestions for <stdlib.h> names [PR101052]

[Jonathan Wakely]

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.

Re: [PATCH] c-family: Add fix-it suggestions for <stdlib.h> names [PR101052]

[Jakub Jelinek]

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

Re: [PATCH] c-family: Add fix-it suggestions for <stdlib.h> names [PR101052]

[David Malcolm]

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