Re: [C PATCH] Print header hints (PR c/59717)
On 10/07/2014 11:04 AM, Jakub Jelinek wrote: adding a hint in this case is less obvious than in the C case, because, what if this wasn't supposed to be ::abort (), but std::abort (), or some other namespace abort, or some class abort () method etc.? It still seems reasonable to offer a hint if no declaration was found. Jason
Re: [C PATCH] Print header hints (PR c/59717)
On Tue, 7 Oct 2014, Marek Polacek wrote: > 2014-10-07 Marek Polacek > > PR c/59717 > * c-decl.c (header_for_builtin_fn): New function. > (implicitly_declare): Suggest which header to include. > > * gcc.dg/pr59717.c: New test. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [C PATCH] Print header hints (PR c/59717)
On Tue, Oct 07, 2014 at 05:00:26PM +0200, Richard Biener wrote: > On Tue, Oct 7, 2014 at 4:51 PM, Marek Polacek wrote: > > On Tue, Oct 07, 2014 at 04:39:55PM +0200, Richard Biener wrote: > >> Why not annotate builtins.def with the info? > > > > Because I think that would be more hairy, I'd have to change DEF_BUILTIN > > and all the builtins. That seemed superfluous given that this hint is > > only for a C FE... > > All C family frontends, no? And builtins.def is used by (and only by) > all C family frontends... As Jakub pointed out, only C and ObjC for now. > Well - just a suggestion ;) Thanks - builtins.def was where I originally started. Marek
Re: [C PATCH] Print header hints (PR c/59717)
On Tue, Oct 07, 2014 at 05:00:05PM +0200, Jakub Jelinek wrote: > On Tue, Oct 07, 2014 at 04:51:31PM +0200, Marek Polacek wrote: > > On Tue, Oct 07, 2014 at 04:39:55PM +0200, Richard Biener wrote: > > > Why not annotate builtins.def with the info? > > > > Because I think that would be more hairy, I'd have to change DEF_BUILTIN > > and all the builtins. That seemed superfluous given that this hint is > > only for a C FE... > > Guess it depends on how many DEF_*_BUILTIN classes would this affect, At least the following: DEF_LIB_BUILTIN DEF_C94_BUILTIN DEF_C99_BUILTIN DEF_C11_BUILTIN DEF_C99_COMPL_BUILTIN DEF_C99_C90RES_BUILTIN I think that is quite a lot. > if just a couple, you could add DEF_*_BUILTIN_WITH_C_HINT, with an extra > arg. But as the builtins.def info already has quite long lines, making them > even longer might not be best. So perhaps the switch is good enough. Yeah, that the lines are long enough already was one of the things that discouraged me from tweaking builtins.def. Marek
Re: [C PATCH] Print header hints (PR c/59717)
On Tue, Oct 07, 2014 at 05:00:26PM +0200, Richard Biener wrote:
> On Tue, Oct 7, 2014 at 4:51 PM, Marek Polacek wrote:
> > On Tue, Oct 07, 2014 at 04:39:55PM +0200, Richard Biener wrote:
> >> Why not annotate builtins.def with the info?
> >
> > Because I think that would be more hairy, I'd have to change DEF_BUILTIN
> > and all the builtins. That seemed superfluous given that this hint is
> > only for a C FE...
>
> All C family frontends, no? And builtins.def is used by (and only by)
> all C family frontends...
Well, the C++ FE on say:
void
bar (void)
{
abort ();
}
just errors out:
/tmp/a.c: In function ‘void bar()’:
/tmp/a.c:4:10: error: ‘abort’ was not declared in this scope
abort ();
^
adding a hint in this case is less obvious than in the C case, because,
what if this wasn't supposed to be ::abort (), but std::abort (), or
some other namespace abort, or some class abort () method etc.?
Jakub
Re: [C PATCH] Print header hints (PR c/59717)
On Tue, Oct 7, 2014 at 4:51 PM, Marek Polacek wrote: > On Tue, Oct 07, 2014 at 04:39:55PM +0200, Richard Biener wrote: >> Why not annotate builtins.def with the info? > > Because I think that would be more hairy, I'd have to change DEF_BUILTIN > and all the builtins. That seemed superfluous given that this hint is > only for a C FE... All C family frontends, no? And builtins.def is used by (and only by) all C family frontends... Well - just a suggestion ;) I'd like to see some easier to grok specification of the number of arguments expected to the builtins for example (for the match-and-simplify work). Richard. > Marek
Re: [C PATCH] Print header hints (PR c/59717)
On Tue, Oct 07, 2014 at 04:51:31PM +0200, Marek Polacek wrote: > On Tue, Oct 07, 2014 at 04:39:55PM +0200, Richard Biener wrote: > > Why not annotate builtins.def with the info? > > Because I think that would be more hairy, I'd have to change DEF_BUILTIN > and all the builtins. That seemed superfluous given that this hint is > only for a C FE... Guess it depends on how many DEF_*_BUILTIN classes would this affect, if just a couple, you could add DEF_*_BUILTIN_WITH_C_HINT, with an extra arg. But as the builtins.def info already has quite long lines, making them even longer might not be best. So perhaps the switch is good enough. Jakub
Re: [C PATCH] Print header hints (PR c/59717)
On Tue, Oct 07, 2014 at 04:39:55PM +0200, Richard Biener wrote: > Why not annotate builtins.def with the info? Because I think that would be more hairy, I'd have to change DEF_BUILTIN and all the builtins. That seemed superfluous given that this hint is only for a C FE... Marek
Re: [C PATCH] Print header hints (PR c/59717)
On Tue, Oct 7, 2014 at 2:53 PM, Marek Polacek wrote:
> PR59717 is a request for hints which header to include if the compiler warns
> about incompatible implicit declarations. E.g., if one uses abort
> without declaring it first, we now say
> note: include ‘’ or provide a declaration of ‘abort’
> I've added hints only for standard functions which means we won't display
> the hint for functions such as mempcpy.
>
> The implementation is based on a function that just maps built_in_function
> codes to header names.
>
> Two remarks:
> * header_for_builtin_fn is long and I don't want to unnecessarily
> inflate already big c-decl.c file, so it might make sense to move
> the function into c-errors.c;
> * we don't issue "incompatible implicit declaration of built-in function"
> warning for functions that return int and whose parameter types don't need
> default promotions - for instance putc, fputs, ilogb, strcmp, vprintf,
> isnan,
> isalpha, ... Therefore for such functions we don't print the hint neither.
> header_for_builtin_fn is ready for them, though. (The cases for
> and could be removed.)
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
Why not annotate builtins.def with the info?
Richard.
> 2014-10-07 Marek Polacek
>
> PR c/59717
> * c-decl.c (header_for_builtin_fn): New function.
> (implicitly_declare): Suggest which header to include.
>
> * gcc.dg/pr59717.c: New test.
>
> diff --git gcc/c/c-decl.c gcc/c/c-decl.c
> index ce5a8de..e23284a 100644
> --- gcc/c/c-decl.c
> +++ gcc/c/c-decl.c
> @@ -2968,6 +2968,189 @@ implicit_decl_warning (location_t loc, tree id, tree
> olddecl)
> }
> }
>
> +/* This function represents mapping of a function code FCODE
> + to its respective header. */
> +
> +static const char *
> +header_for_builtin_fn (enum built_in_function fcode)
> +{
> + switch (fcode)
> +{
> +CASE_FLT_FN (BUILT_IN_ACOS):
> +CASE_FLT_FN (BUILT_IN_ACOSH):
> +CASE_FLT_FN (BUILT_IN_ASIN):
> +CASE_FLT_FN (BUILT_IN_ASINH):
> +CASE_FLT_FN (BUILT_IN_ATAN):
> +CASE_FLT_FN (BUILT_IN_ATANH):
> +CASE_FLT_FN (BUILT_IN_ATAN2):
> +CASE_FLT_FN (BUILT_IN_CBRT):
> +CASE_FLT_FN (BUILT_IN_CEIL):
> +CASE_FLT_FN (BUILT_IN_COPYSIGN):
> +CASE_FLT_FN (BUILT_IN_COS):
> +CASE_FLT_FN (BUILT_IN_COSH):
> +CASE_FLT_FN (BUILT_IN_ERF):
> +CASE_FLT_FN (BUILT_IN_ERFC):
> +CASE_FLT_FN (BUILT_IN_EXP):
> +CASE_FLT_FN (BUILT_IN_EXP2):
> +CASE_FLT_FN (BUILT_IN_EXPM1):
> +CASE_FLT_FN (BUILT_IN_FABS):
> +CASE_FLT_FN (BUILT_IN_FDIM):
> +CASE_FLT_FN (BUILT_IN_FLOOR):
> +CASE_FLT_FN (BUILT_IN_FMA):
> +CASE_FLT_FN (BUILT_IN_FMAX):
> +CASE_FLT_FN (BUILT_IN_FMIN):
> +CASE_FLT_FN (BUILT_IN_FMOD):
> +CASE_FLT_FN (BUILT_IN_FREXP):
> +CASE_FLT_FN (BUILT_IN_HYPOT):
> +CASE_FLT_FN (BUILT_IN_ILOGB):
> +CASE_FLT_FN (BUILT_IN_LDEXP):
> +CASE_FLT_FN (BUILT_IN_LGAMMA):
> +CASE_FLT_FN (BUILT_IN_LLRINT):
> +CASE_FLT_FN (BUILT_IN_LLROUND):
> +CASE_FLT_FN (BUILT_IN_LOG):
> +CASE_FLT_FN (BUILT_IN_LOG10):
> +CASE_FLT_FN (BUILT_IN_LOG1P):
> +CASE_FLT_FN (BUILT_IN_LOG2):
> +CASE_FLT_FN (BUILT_IN_LOGB):
> +CASE_FLT_FN (BUILT_IN_LRINT):
> +CASE_FLT_FN (BUILT_IN_LROUND):
> +CASE_FLT_FN (BUILT_IN_MODF):
> +CASE_FLT_FN (BUILT_IN_NAN):
> +CASE_FLT_FN (BUILT_IN_NEARBYINT):
> +CASE_FLT_FN (BUILT_IN_NEXTAFTER):
> +CASE_FLT_FN (BUILT_IN_NEXTTOWARD):
> +CASE_FLT_FN (BUILT_IN_POW):
> +CASE_FLT_FN (BUILT_IN_REMAINDER):
> +CASE_FLT_FN (BUILT_IN_REMQUO):
> +CASE_FLT_FN (BUILT_IN_RINT):
> +CASE_FLT_FN (BUILT_IN_ROUND):
> +CASE_FLT_FN (BUILT_IN_SCALBLN):
> +CASE_FLT_FN (BUILT_IN_SCALBN):
> +CASE_FLT_FN (BUILT_IN_SIN):
> +CASE_FLT_FN (BUILT_IN_SINH):
> +CASE_FLT_FN (BUILT_IN_SINCOS):
> +CASE_FLT_FN (BUILT_IN_SQRT):
> +CASE_FLT_FN (BUILT_IN_TAN):
> +CASE_FLT_FN (BUILT_IN_TANH):
> +CASE_FLT_FN (BUILT_IN_TGAMMA):
> +CASE_FLT_FN (BUILT_IN_TRUNC):
> +case BUILT_IN_ISINF:
> +case BUILT_IN_ISNAN:
> + return "";
> +CASE_FLT_FN (BUILT_IN_CABS):
> +CASE_FLT_FN (BUILT_IN_CACOS):
> +CASE_FLT_FN (BUILT_IN_CACOSH):
> +CASE_FLT_FN (BUILT_IN_CARG):
> +CASE_FLT_FN (BUILT_IN_CASIN):
> +CASE_FLT_FN (BUILT_IN_CASINH):
> +CASE_FLT_FN (BUILT_IN_CATAN):
> +CASE_FLT_FN (BUILT_IN_CATANH):
> +CASE_FLT_FN (BUILT_IN_CCOS):
> +CASE_FLT_FN (BUILT_IN_CCOSH):
> +CASE_FLT_FN (BUILT_IN_CEXP):
> +CASE_FLT_FN (BUILT_IN_CIMAG):
> +CASE_FLT_FN (BUILT_IN_CLOG):
> +CASE_FLT_FN (BUILT_IN_CONJ):
> +CASE_FLT_FN (BUILT_IN_CPOW):
> +CASE_FLT_FN (BUILT_IN_CPROJ):
> +CASE_FLT_FN (BUILT_IN_CREAL):
> +CASE_FLT_FN (BUILT_IN_CSIN):
> +CASE_FLT_FN (BUILT_IN_CSINH):
> +CASE_FLT_FN (BUILT_IN_CSQRT):
> +CASE_FLT_FN (BUILT_IN_CTAN):
> +CASE_FLT_FN (BUILT_IN_CTANH):
> + return "";
> +case BUILT_IN_MEMCHR:
> +case BUILT_IN_M
[C PATCH] Print header hints (PR c/59717)
PR59717 is a request for hints which header to include if the compiler warns
about incompatible implicit declarations. E.g., if one uses abort
without declaring it first, we now say
note: include ‘’ or provide a declaration of ‘abort’
I've added hints only for standard functions which means we won't display
the hint for functions such as mempcpy.
The implementation is based on a function that just maps built_in_function
codes to header names.
Two remarks:
* header_for_builtin_fn is long and I don't want to unnecessarily
inflate already big c-decl.c file, so it might make sense to move
the function into c-errors.c;
* we don't issue "incompatible implicit declaration of built-in function"
warning for functions that return int and whose parameter types don't need
default promotions - for instance putc, fputs, ilogb, strcmp, vprintf, isnan,
isalpha, ... Therefore for such functions we don't print the hint neither.
header_for_builtin_fn is ready for them, though. (The cases for
and could be removed.)
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2014-10-07 Marek Polacek
PR c/59717
* c-decl.c (header_for_builtin_fn): New function.
(implicitly_declare): Suggest which header to include.
* gcc.dg/pr59717.c: New test.
diff --git gcc/c/c-decl.c gcc/c/c-decl.c
index ce5a8de..e23284a 100644
--- gcc/c/c-decl.c
+++ gcc/c/c-decl.c
@@ -2968,6 +2968,189 @@ implicit_decl_warning (location_t loc, tree id, tree
olddecl)
}
}
+/* This function represents mapping of a function code FCODE
+ to its respective header. */
+
+static const char *
+header_for_builtin_fn (enum built_in_function fcode)
+{
+ switch (fcode)
+{
+CASE_FLT_FN (BUILT_IN_ACOS):
+CASE_FLT_FN (BUILT_IN_ACOSH):
+CASE_FLT_FN (BUILT_IN_ASIN):
+CASE_FLT_FN (BUILT_IN_ASINH):
+CASE_FLT_FN (BUILT_IN_ATAN):
+CASE_FLT_FN (BUILT_IN_ATANH):
+CASE_FLT_FN (BUILT_IN_ATAN2):
+CASE_FLT_FN (BUILT_IN_CBRT):
+CASE_FLT_FN (BUILT_IN_CEIL):
+CASE_FLT_FN (BUILT_IN_COPYSIGN):
+CASE_FLT_FN (BUILT_IN_COS):
+CASE_FLT_FN (BUILT_IN_COSH):
+CASE_FLT_FN (BUILT_IN_ERF):
+CASE_FLT_FN (BUILT_IN_ERFC):
+CASE_FLT_FN (BUILT_IN_EXP):
+CASE_FLT_FN (BUILT_IN_EXP2):
+CASE_FLT_FN (BUILT_IN_EXPM1):
+CASE_FLT_FN (BUILT_IN_FABS):
+CASE_FLT_FN (BUILT_IN_FDIM):
+CASE_FLT_FN (BUILT_IN_FLOOR):
+CASE_FLT_FN (BUILT_IN_FMA):
+CASE_FLT_FN (BUILT_IN_FMAX):
+CASE_FLT_FN (BUILT_IN_FMIN):
+CASE_FLT_FN (BUILT_IN_FMOD):
+CASE_FLT_FN (BUILT_IN_FREXP):
+CASE_FLT_FN (BUILT_IN_HYPOT):
+CASE_FLT_FN (BUILT_IN_ILOGB):
+CASE_FLT_FN (BUILT_IN_LDEXP):
+CASE_FLT_FN (BUILT_IN_LGAMMA):
+CASE_FLT_FN (BUILT_IN_LLRINT):
+CASE_FLT_FN (BUILT_IN_LLROUND):
+CASE_FLT_FN (BUILT_IN_LOG):
+CASE_FLT_FN (BUILT_IN_LOG10):
+CASE_FLT_FN (BUILT_IN_LOG1P):
+CASE_FLT_FN (BUILT_IN_LOG2):
+CASE_FLT_FN (BUILT_IN_LOGB):
+CASE_FLT_FN (BUILT_IN_LRINT):
+CASE_FLT_FN (BUILT_IN_LROUND):
+CASE_FLT_FN (BUILT_IN_MODF):
+CASE_FLT_FN (BUILT_IN_NAN):
+CASE_FLT_FN (BUILT_IN_NEARBYINT):
+CASE_FLT_FN (BUILT_IN_NEXTAFTER):
+CASE_FLT_FN (BUILT_IN_NEXTTOWARD):
+CASE_FLT_FN (BUILT_IN_POW):
+CASE_FLT_FN (BUILT_IN_REMAINDER):
+CASE_FLT_FN (BUILT_IN_REMQUO):
+CASE_FLT_FN (BUILT_IN_RINT):
+CASE_FLT_FN (BUILT_IN_ROUND):
+CASE_FLT_FN (BUILT_IN_SCALBLN):
+CASE_FLT_FN (BUILT_IN_SCALBN):
+CASE_FLT_FN (BUILT_IN_SIN):
+CASE_FLT_FN (BUILT_IN_SINH):
+CASE_FLT_FN (BUILT_IN_SINCOS):
+CASE_FLT_FN (BUILT_IN_SQRT):
+CASE_FLT_FN (BUILT_IN_TAN):
+CASE_FLT_FN (BUILT_IN_TANH):
+CASE_FLT_FN (BUILT_IN_TGAMMA):
+CASE_FLT_FN (BUILT_IN_TRUNC):
+case BUILT_IN_ISINF:
+case BUILT_IN_ISNAN:
+ return "";
+CASE_FLT_FN (BUILT_IN_CABS):
+CASE_FLT_FN (BUILT_IN_CACOS):
+CASE_FLT_FN (BUILT_IN_CACOSH):
+CASE_FLT_FN (BUILT_IN_CARG):
+CASE_FLT_FN (BUILT_IN_CASIN):
+CASE_FLT_FN (BUILT_IN_CASINH):
+CASE_FLT_FN (BUILT_IN_CATAN):
+CASE_FLT_FN (BUILT_IN_CATANH):
+CASE_FLT_FN (BUILT_IN_CCOS):
+CASE_FLT_FN (BUILT_IN_CCOSH):
+CASE_FLT_FN (BUILT_IN_CEXP):
+CASE_FLT_FN (BUILT_IN_CIMAG):
+CASE_FLT_FN (BUILT_IN_CLOG):
+CASE_FLT_FN (BUILT_IN_CONJ):
+CASE_FLT_FN (BUILT_IN_CPOW):
+CASE_FLT_FN (BUILT_IN_CPROJ):
+CASE_FLT_FN (BUILT_IN_CREAL):
+CASE_FLT_FN (BUILT_IN_CSIN):
+CASE_FLT_FN (BUILT_IN_CSINH):
+CASE_FLT_FN (BUILT_IN_CSQRT):
+CASE_FLT_FN (BUILT_IN_CTAN):
+CASE_FLT_FN (BUILT_IN_CTANH):
+ return "";
+case BUILT_IN_MEMCHR:
+case BUILT_IN_MEMCMP:
+case BUILT_IN_MEMCPY:
+case BUILT_IN_MEMMOVE:
+case BUILT_IN_MEMSET:
+case BUILT_IN_STRCAT:
+case BUILT_IN_STRCHR:
+case BUILT_IN_STRCMP:
+case BUILT_IN_STRCPY:
+case BUILT_IN_STRCSPN:
+case BUILT_IN_STRLEN:
+case BUILT_IN_STRNCAT:
+case BUILT_IN_STRNCMP:
+case BUILT_IN_STRNCPY:
+case BUILT_IN_STRPBRK:
+ca
