On 1/31/19 7:13 AM, David Malcolm wrote:
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?
FWIW, I agree that your patch is an improvement (thank you!) I tend
to forget that FLT_MAX is not defined in <limits.h> and I'm pretty
sure others do too, so these hints are helpful. Especially for some
of the less commonly used symbols like the INT_LEASTN_MIN kind (in
<stdint.h>) vs INT_MIN (in <limits.h>) et al.
Just as food for thought for now, in C++, to avoid errors due to
-Wnarrowing, I think the hints will ultimately need to consider
types at least in some cases. E.g., in
int dmax { D_MAX };
when D_MAX is not defined, if it were to be replaced by DBL_MAX as
the hint suggests, the declaration would fail to compile.
Martin
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
