diff mbox series

[v2,5/8] c: Do not ignore some forms of -Wimplicit-int in system headers

Message ID 78cec351fe80ff14ef5881e726aa53d1ec69c750.1699983736.git.fweimer@redhat.com
State Unresolved
Headers
Series Turn some C warnings into errors by default |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Florian Weimer Nov. 14, 2023, 5:50 p.m. UTC 
  Most -Wimplicit-int warnings were unconditionally disabled for system
headers.  Only missing types for parameters in old-style function
definitions resulted in warnings.  This is inconsistent with the
treatment of other permerrors, which are active in system headers.

gcc/c/

	* c-decl.cc (grokdeclarator): Do not skip -Wimplicit-int
	warnings or errors in system headers.

gcc/testsuite/

	* gcc.dg/permerror-system.c: Expect all -Wimplicit-int
	permerrors.
---
 gcc/c/c-decl.cc                         | 2 +-
 gcc/testsuite/gcc.dg/permerror-system.c | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Sam James Nov. 15, 2023, 5:03 a.m. UTC | #1 
Florian Weimer <fweimer@redhat.com> writes:

> Most -Wimplicit-int warnings were unconditionally disabled for system
> headers.  Only missing types for parameters in old-style function
> definitions resulted in warnings.  This is inconsistent with the
> treatment of other permerrors, which are active in system headers.

The situation with system headers is kind of a mess still. I went
looking for a bug for the -Wimplicit-int behaviour but I only found
PR78000 for -Wimplicit-function-declaration. But in the bug, Joseph
makes the same observation.

I don't suppose he'll want to block on that at this late point though.

Do you know offhand what Clang's behaviour is wrt warnings in system headers?

>
> gcc/c/
>
> 	* c-decl.cc (grokdeclarator): Do not skip -Wimplicit-int
> 	warnings or errors in system headers.
>
> gcc/testsuite/
>
> 	* gcc.dg/permerror-system.c: Expect all -Wimplicit-int
> 	permerrors.
> ---
>  gcc/c/c-decl.cc                         | 2 +-
>  gcc/testsuite/gcc.dg/permerror-system.c | 5 +++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> index 893e24f7dc6..d16958113a8 100644
> --- a/gcc/c/c-decl.cc
> +++ b/gcc/c/c-decl.cc
> @@ -6845,7 +6845,7 @@ grokdeclarator (const struct c_declarator *declarator,
>  
>    /* Diagnose defaulting to "int".  */
>  
> -  if (declspecs->default_int_p && !in_system_header_at (input_location))
> +  if (declspecs->default_int_p)
>      {
>        /* Issue a warning if this is an ISO C 99 program or if
>  	 -Wreturn-type and this is a function, or if -Wimplicit;
> diff --git a/gcc/testsuite/gcc.dg/permerror-system.c b/gcc/testsuite/gcc.dg/permerror-system.c
> index 60c65ffc1d4..cad48c93f90 100644
> --- a/gcc/testsuite/gcc.dg/permerror-system.c
> +++ b/gcc/testsuite/gcc.dg/permerror-system.c
> @@ -10,7 +10,12 @@
>  
>  /* { dg-error "'f1' \\\[-Wimplicit-function-declaration\\\]" "" { target *-*-* } 10 } */
>  
> +/* { dg-error "'implicit_int_1' \\\[-Wimplicit-int\\\]" "" { target *-*-* } 13 } */
> +/* { dg-error "'implicit_int_2' \\\[-Wimplicit-int\\\]" "" { target *-*-* } 14 } */
> +/* { dg-error "'implicit_int_3' \\\[-Wimplicit-int\\]" "" { target *-*-* } 15 } */
> +/* { dg-error "return type defaults to 'int' \\\[-Wimplicit-int\\\]" "" { target *-*-* } 16 } */
>  /* { dg-error "type of 'i' defaults to 'int' \\\[-Wimplicit-int\\\]" "" { target *-*-*} 16 } */
> +/* { dg-error "type defaults to 'int' in type name \\\[-Wimplicit-int\\\]" "" { target *-*-* } 19 } */
>  
>  /* { dg-error "pointer/integer type mismatch in conditional expression \\\[-Wint-conversion\\\]" "" { target *-*-* } 29 } */
>  /* { dg-error "pointer/integer type mismatch in conditional expression \\\[-Wint-conversion\\\]" "" { target *-*-* } 30 } */
Florian Weimer Nov. 15, 2023, 5:12 a.m. UTC | #2 
* Sam James:

> Florian Weimer <fweimer@redhat.com> writes:
>
>> Most -Wimplicit-int warnings were unconditionally disabled for system
>> headers.  Only missing types for parameters in old-style function
>> definitions resulted in warnings.  This is inconsistent with the
>> treatment of other permerrors, which are active in system headers.
>
> The situation with system headers is kind of a mess still. I went
> looking for a bug for the -Wimplicit-int behaviour but I only found
> PR78000 for -Wimplicit-function-declaration. But in the bug, Joseph
> makes the same observation.
>
> I don't suppose he'll want to block on that at this late point though.
>
> Do you know offhand what Clang's behaviour is wrt warnings in system
> headers?

Clang ignores these new errors in system headers by default.  I don't
know if that's deliberate or a bug.  Our permerrors are deliberately
active in system headers.  As the test shows, -Wimplicit-int really was
the outlier here because of that check outside the permerror machinery.

I expect system headers are quite clean actually because they have to be
for C++ compatibility.  Some things have improved in the last 25 years.

Thanks,
Florian
Sam James Nov. 15, 2023, 5:13 a.m. UTC | #3 
Florian Weimer <fweimer@redhat.com> writes:

> * Sam James:
>
>> Florian Weimer <fweimer@redhat.com> writes:
>>
>>> Most -Wimplicit-int warnings were unconditionally disabled for system
>>> headers.  Only missing types for parameters in old-style function
>>> definitions resulted in warnings.  This is inconsistent with the
>>> treatment of other permerrors, which are active in system headers.
>>
>> The situation with system headers is kind of a mess still. I went
>> looking for a bug for the -Wimplicit-int behaviour but I only found
>> PR78000 for -Wimplicit-function-declaration. But in the bug, Joseph
>> makes the same observation.
>>
>> I don't suppose he'll want to block on that at this late point though.
>>
>> Do you know offhand what Clang's behaviour is wrt warnings in system
>> headers?
>
> Clang ignores these new errors in system headers by default.  I don't
> know if that's deliberate or a bug.  Our permerrors are deliberately
> active in system headers.  As the test shows, -Wimplicit-int really was
> the outlier here because of that check outside the permerror machinery.

Thanks - my assumption was that it was more widespread because of a few
lingering bugs I've seen a few projects complain about, but maybe they
were using old versions or similar.

>
> I expect system headers are quite clean actually because they have to be
> for C++ compatibility.  Some things have improved in the last 25 years.
>

Oh, duh. Thank you!
Jason Merrill Nov. 21, 2023, 9:18 p.m. UTC | #4 
On 11/15/23 00:13, Sam James wrote:
> Florian Weimer <fweimer@redhat.com> writes:
>> * Sam James:
>>> Florian Weimer <fweimer@redhat.com> writes:
>>>
>>>> Most -Wimplicit-int warnings were unconditionally disabled for system
>>>> headers.  Only missing types for parameters in old-style function
>>>> definitions resulted in warnings.  This is inconsistent with the
>>>> treatment of other permerrors, which are active in system headers.
>>>
>>> The situation with system headers is kind of a mess still. I went
>>> looking for a bug for the -Wimplicit-int behaviour but I only found
>>> PR78000 for -Wimplicit-function-declaration. But in the bug, Joseph
>>> makes the same observation.
>>>
>>> I don't suppose he'll want to block on that at this late point though.
>>>
>>> Do you know offhand what Clang's behaviour is wrt warnings in system
>>> headers?
>>
>> Clang ignores these new errors in system headers by default.  I don't
>> know if that's deliberate or a bug.  Our permerrors are deliberately
>> active in system headers.  As the test shows, -Wimplicit-int really was
>> the outlier here because of that check outside the permerror machinery.
> 
> Thanks - my assumption was that it was more widespread because of a few
> lingering bugs I've seen a few projects complain about, but maybe they
> were using old versions or similar.

The permerror behavior was chosen for C++, which was the only front-end 
that used it before; we might want to reconsider if this patch set runs 
into problems with system C headers.

Jason
diff mbox series

Patch

diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 893e24f7dc6..d16958113a8 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -6845,7 +6845,7 @@  grokdeclarator (const struct c_declarator *declarator,
 
   /* Diagnose defaulting to "int".  */
 
-  if (declspecs->default_int_p && !in_system_header_at (input_location))
+  if (declspecs->default_int_p)
     {
       /* Issue a warning if this is an ISO C 99 program or if
 	 -Wreturn-type and this is a function, or if -Wimplicit;
diff --git a/gcc/testsuite/gcc.dg/permerror-system.c b/gcc/testsuite/gcc.dg/permerror-system.c
index 60c65ffc1d4..cad48c93f90 100644
--- a/gcc/testsuite/gcc.dg/permerror-system.c
+++ b/gcc/testsuite/gcc.dg/permerror-system.c
@@ -10,7 +10,12 @@ 
 
 /* { dg-error "'f1' \\\[-Wimplicit-function-declaration\\\]" "" { target *-*-* } 10 } */
 
+/* { dg-error "'implicit_int_1' \\\[-Wimplicit-int\\\]" "" { target *-*-* } 13 } */
+/* { dg-error "'implicit_int_2' \\\[-Wimplicit-int\\\]" "" { target *-*-* } 14 } */
+/* { dg-error "'implicit_int_3' \\\[-Wimplicit-int\\]" "" { target *-*-* } 15 } */
+/* { dg-error "return type defaults to 'int' \\\[-Wimplicit-int\\\]" "" { target *-*-* } 16 } */
 /* { dg-error "type of 'i' defaults to 'int' \\\[-Wimplicit-int\\\]" "" { target *-*-*} 16 } */
+/* { dg-error "type defaults to 'int' in type name \\\[-Wimplicit-int\\\]" "" { target *-*-* } 19 } */
 
 /* { dg-error "pointer/integer type mismatch in conditional expression \\\[-Wint-conversion\\\]" "" { target *-*-* } 29 } */
 /* { dg-error "pointer/integer type mismatch in conditional expression \\\[-Wint-conversion\\\]" "" { target *-*-* } 30 } */