On Thu, 2019-01-31 at 10:35 +0100, Richard Biener wrote: > On Thu, Jan 31, 2019 at 12:09 AM David Malcolm <dmalc...@redhat.com> > wrote: > > > > PR c/89122 reports that we emit a bogus fix-it hint for the case > > where > > the code uses FLT_MAX, but has included <limits.h> rather than > > <float.h>: > > > > x.c:3:11: error: 'FLT_MAX' undeclared here (not in a function); did > > you > > mean 'INT_MAX'? > > 3 | float f = FLT_MAX; > > | ^~~~~~~ > > | INT_MAX > > > > This patch adds some knowledge of <float.h> (and <cfloat>) to > > known-headers.cc, fixing the issue: > > > > x.c:3:11: error: 'FLT_MAX' undeclared here (not in a function) > > 3 | float f = FLT_MAX; > > | ^~~~~~~ > > x.c:2:1: note: 'FLT_MAX' is defined in header '<float.h>'; did you > > forget > > to '#include <float.h>'? > > 1 | #include <limits.h> > > +++ |+#include <float.h> > > 2 | > > > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. > > > > Is this OK for trunk in stage 4? (presumably very low risk) > > What does it not say for > > int i = FLT_MAX; > > ? Hopefully it doesn't suggest to include float.h but suggests > to include limits.h and INT_MAX?
The suggestions code (lookup_name_fuzzy) doesn't take types into account, and has no knowledge of the "int-ness" of i. It's purely looking for matches for names that it didn't recognize (albeit with an enum to hint at whether it's looking for a type vs a function-like thing vs "anything", but that wouldn't help here). Trunk emits: y.c:1:9: error: 'FLT_MAX' undeclared here (not in a function) 1 | int i = FLT_MAX; | ^~~~~~~ With the patch it does indeed tell the user where FLT_MAX is: y.c:1:9: error: 'FLT_MAX' undeclared here (not in a function) 1 | int i = FLT_MAX; | ^~~~~~~ y.c:1:1: note: 'FLT_MAX' is defined in header '<float.h>'; did you forget to '#include <float.h>'? +++ |+#include <float.h> 1 | int i = FLT_MAX; This doesn't seem too bad to me (in that it's screaming "float" to the user); if the user ignores that and blindly follows the suggestion, they get: y.c:2:9: warning: overflow in conversion from 'float' to 'int' changes value from '3.40282347e+38f' to '2147483647' [-Woverflow] 2 | int i = FLT_MAX; | ^~~~~~~ y.c:2:1: warning: overflow in constant expression [-Woverflow] 2 | int i = FLT_MAX; | ^~~ It's not clear to me that we would want a fix-it hint for this case (where the compiler knows about FLT_MAX) - maybe the user meant to write that? Such a fix-it hint might look something like: y.c:2:9: warning: overflow in conversion from 'float' to 'int' changes value from '3.40282347e+38f' to '2147483647'; did you mean 'INT_MAX' [- Woverflow] 2 | int i = FLT_MAX; | ^~~~~~~ | INT_MAX ...or somesuch, but that's clearly too ambitious a patch for stage 4. Would that be reasonable for the next stage 1? In the meantime, is the original patch OK? Thanks Dave > > gcc/c-family/ChangeLog: > > PR c/89122 > > * known-headers.cc (get_stdlib_header_for_name): Add > > {FLT|DBL|LDBL}_{MAX|MIN} to "hints" array. > > > > gcc/testsuite/ChangeLog: > > PR c/89122 > > * g++.dg/spellcheck-stdlib.C (test_FLT_MAX): New test. > > * gcc.dg/spellcheck-stdlib.c (test_FLT_MAX): New test. > > --- > > gcc/c-family/known-headers.cc | 8 ++++++++ > > gcc/testsuite/g++.dg/spellcheck-stdlib.C | 5 +++++ > > gcc/testsuite/gcc.dg/spellcheck-stdlib.c | 5 +++++ > > 3 files changed, 18 insertions(+) > > > > diff --git a/gcc/c-family/known-headers.cc b/gcc/c-family/known- > > headers.cc > > index e3dcf73..c222f30 100644 > > --- a/gcc/c-family/known-headers.cc > > +++ b/gcc/c-family/known-headers.cc > > @@ -84,6 +84,14 @@ get_stdlib_header_for_name (const char *name, > > enum stdlib lib) > > {"ULONG_MAX", {"<limits.h>", "<climits>"} }, > > {"USHRT_MAX", {"<limits.h>", "<climits>"} }, > > > > + /* <float.h> and <cfloat>. */ > > + {"DBL_MAX", {"<float.h>", "<cfloat>"} }, > > + {"DBL_MIN", {"<float.h>", "<cfloat>"} }, > > + {"FLT_MAX", {"<float.h>", "<cfloat>"} }, > > + {"FLT_MIN", {"<float.h>", "<cfloat>"} }, > > + {"LDBL_MAX", {"<float.h>", "<cfloat>"} }, > > + {"LDBL_MIN", {"<float.h>", "<cfloat>"} }, > > + > > /* <stdarg.h> and <cstdarg>. */ > > {"va_list", {"<stdarg.h>", "<cstdarg>"} }, > > > > diff --git a/gcc/testsuite/g++.dg/spellcheck-stdlib.C > > b/gcc/testsuite/g++.dg/spellcheck-stdlib.C > > index 11a4e3e..31e91fe 100644 > > --- a/gcc/testsuite/g++.dg/spellcheck-stdlib.C > > +++ b/gcc/testsuite/g++.dg/spellcheck-stdlib.C > > @@ -77,6 +77,11 @@ int test_INT_MAX (void) > > // { dg-message "'INT_MAX' is defined in header '<climits>'; did > > you forget to '#include <climits>'?" "" { target *-*-* } > > INT_MAX_line } > > } > > > > +/* Missing <cfloat>. */ > > +float test_FLT_MAX = FLT_MAX; // { dg-line FLT_MAX_line } > > +// { dg-error "'FLT_MAX' was not declared" "" { target *-*-* } > > FLT_MAX_line } > > +// { dg-message "'FLT_MAX' is defined in header '<cfloat>'; did > > you forget to '#include <cfloat>'?" "" { target *-*-* } > > FLT_MAX_line } > > + > > /* Missing <cstring>. */ > > > > void test_cstring (char *dest, char *src) > > diff --git a/gcc/testsuite/gcc.dg/spellcheck-stdlib.c > > b/gcc/testsuite/gcc.dg/spellcheck-stdlib.c > > index 7474c9a..1ae3b5e 100644 > > --- a/gcc/testsuite/gcc.dg/spellcheck-stdlib.c > > +++ b/gcc/testsuite/gcc.dg/spellcheck-stdlib.c > > @@ -62,3 +62,8 @@ int test_INT_MAX (void) > > /* { dg-bogus "__INT_MAX__" "" { target *-*-* } INT_MAX_line } > > */ > > /* { dg-message "'INT_MAX' is defined in header '<limits.h>'; > > did you forget to '#include <limits.h>'?" "" { target *-*-* } > > INT_MAX_line } */ > > } > > + > > +/* Missing <float.h>. */ > > +float test_FLT_MAX = FLT_MAX; /* { dg-line FLT_MAX_line } */ > > +/* { dg-error "'FLT_MAX' undeclared" "" { target *-*-* } > > FLT_MAX_line } */ > > +/* { dg-message "'FLT_MAX' is defined in header '<float.h>'; did > > you forget to '#include <float.h>'?" "" { target *-*-* } > > FLT_MAX_line } */ > > -- > > 1.8.5.3 > >