Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Jim Meyering]

Eric Blake wrote:

> On 11/29/2011 02:05 PM, Paul Eggert wrote:
>> On 11/29/11 09:35, Jim Meyering wrote:
>>> +  gl_WARN_ADD([-Wno-unsuffixed-float-constants])
>>
>> How about if we remove -Wunsuffixed-float-constants from
>> manywarnings.m4?  In practice it typically causes more
>> trouble than it cures (the above-quoted gzip patch is
>> one example, but I've run into it elsewhere).
>
> I've seen -Wunsuffixed-float-constants trigger in a couple projects now,
> so it really is a new warning that pops up on existing code; but it's
> also fairly mechanical to correct (for example, while fixing it for
> libvirt, I found at least one case where it was sufficient to use int
> instead of floating point).
>
> I'm not convinced about removing it from manywarnings.m4 - it's not that
> hard to disable the warning if you don't want it, but leaving it in
> leads to smaller executable size for the cases where 1.0F is sufficient
> (compared to the extra size required to represent 1.0 which is 1.0D).
> By explicitly marking F or D to all float constants, it shows you've
> thought about which precision is worth using; omitting the suffix could
> be the sign of sloppy code that has other problems with misuse of
> floating point.

Inspired by that, I went to see what would be required for coreutils to pass.
Are these "D" and "F" really worth it?

Unless there are objections (portability?)
I'll fix at least the affected header files: intprops.h, timespec.h.
At which point, I might as well fix the others, too.
There aren't many, considering the amount of code.

First the gnulib changes:

diff --git a/lib/hash.c b/lib/hash.c
index 1dd657a..9b6e4db 100644
--- a/lib/hash.c
+++ b/lib/hash.c
@@ -113,8 +113,8 @@ struct hash_table
    1.0).  The growth threshold defaults to 0.8, and the growth factor
    defaults to 1.414, meaning that the table will have doubled its size
    every second time 80% of the buckets get used.  */
-#define DEFAULT_GROWTH_THRESHOLD 0.8
-#define DEFAULT_GROWTH_FACTOR 1.414
+#define DEFAULT_GROWTH_THRESHOLD 0.8F
+#define DEFAULT_GROWTH_FACTOR 1.414F

 /* If a deletion empties a bucket and causes the ratio of used buckets to
    table size to become smaller than the shrink threshold (a number between
@@ -122,8 +122,8 @@ struct hash_table
    number greater than the shrink threshold but smaller than 1.0).  The shrink
    threshold and factor default to 0.0 and 1.0, meaning that the table never
    shrinks.  */
-#define DEFAULT_SHRINK_THRESHOLD 0.0
-#define DEFAULT_SHRINK_FACTOR 1.0
+#define DEFAULT_SHRINK_THRESHOLD 0.0F
+#define DEFAULT_SHRINK_FACTOR 1.0F

 /* Use this to initialize or reset a TUNING structure to
    some sensible values. */
@@ -238,7 +238,7 @@ hash_print_statistics (const Hash_table *table, FILE 
*stream)
   fprintf (stream, "# buckets:         %lu\n", (unsigned long int) n_buckets);
   fprintf (stream, "# buckets used:    %lu (%.2f%%)\n",
            (unsigned long int) n_buckets_used,
-           (100.0 * n_buckets_used) / n_buckets);
+           (100.0F * n_buckets_used) / n_buckets);
   fprintf (stream, "max bucket length: %lu\n",
            (unsigned long int) max_bucket_length);
 }
diff --git a/lib/intprops.h b/lib/intprops.h
index 1f6a539..7bce5db 100644
--- a/lib/intprops.h
+++ b/lib/intprops.h
@@ -35,7 +35,7 @@

 /* True if the arithmetic type T is an integer type.  bool counts as
    an integer.  */
-#define TYPE_IS_INTEGER(t) ((t) 1.5 == 1)
+#define TYPE_IS_INTEGER(t) ((t) 1.5F == 1)

 /* True if negative values of the signed integer type T use two's
    complement, ones' complement, or signed magnitude representation,
diff --git a/lib/timespec.h b/lib/timespec.h
index acf815c..6b783e3 100644
--- a/lib/timespec.h
+++ b/lib/timespec.h
@@ -73,7 +73,7 @@ struct timespec dtotimespec (double);
 static inline double
 timespectod (struct timespec a)
 {
-  return a.tv_sec + a.tv_nsec / 1e9;
+  return a.tv_sec + a.tv_nsec / 1e9D;
 }

 void gettime (struct timespec *);

=======================================================================
And then the coreutils diffs:

diff --git a/configure.ac b/configure.ac
index 8c4fff4..9ca2ce4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -122,7 +122,6 @@ if test "$gl_gcc_warnings" = yes; then
   gl_WARN_ADD([-Wsuggest-attribute=const])
   gl_WARN_ADD([-Wsuggest-attribute=noreturn])
   gl_WARN_ADD([-Wno-format-nonliteral])
-  gl_WARN_ADD([-Wno-unsuffixed-float-constants])

   # Enable this warning only with gcc-4.7 and newer.  With 4.6.2 20111027,
   # it suggests test.c's advance function may be pure, even though it
diff --git a/src/sleep.c b/src/sleep.c
index d32daa4..70e5deb 100644
--- a/src/sleep.c
+++ b/src/sleep.c
@@ -100,7 +100,7 @@ int
 main (int argc, char **argv)
 {
   int i;
-  double seconds = 0.0;
+  double seconds = 0.0D;
   bool ok = true;

   initialize_main (&argc, &argv);
diff --git a/src/sort.c b/src/sort.c
index 4a87148..02bf89f 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -987,7 +987,7 @@ pipe_fork (int pipefds[2], size_t tries)
 #if HAVE_WORKING_FORK
   struct tempnode *saved_temphead;
   int saved_errno;
-  double wait_retry = 0.25;
+  double wait_retry = 0.25D;
   pid_t pid IF_LINT ( = -1);
   struct cs_status cs;

diff --git a/src/stat.c b/src/stat.c
index 4e5dbce..b20da0b 100644
--- a/src/stat.c
+++ b/src/stat.c
@@ -544,7 +544,7 @@ static int
 out_minus_zero (char *pformat, size_t prefix_len)
 {
   make_format (pformat, prefix_len, "'-+ 0", ".0f");
-  return printf (pformat, -0.25);
+  return printf (pformat, -0.25D);
 }

 /* Output the number of seconds since the Epoch, using a format that
diff --git a/src/tail.c b/src/tail.c
index 1641a12..ade0b36 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -2090,7 +2090,7 @@ main (int argc, char **argv)
   /* The number of seconds to sleep between iterations.
      During one iteration, every file name or descriptor is checked to
      see if it has changed.  */
-  double sleep_interval = 1.0;
+  double sleep_interval = 1.0D;

   initialize_main (&argc, &argv);
   set_program_name (argv[0]);