Re: #2 [Was: Re: Function attributes for make-docfile]

[Paul Eggert <eggert@cs.ucla.edu>]

[-- Attachment #1: Type: text/plain, Size: 1026 bytes --]

On 01/14/2015 09:49 AM, Stefan Monnier wrote:
> This is hideous.  Why do we need that?

Assuming --enable-gcc-warnings, it's a combination of things. First, gcc 
-Wsuggest-attribute=const complains if it can deduce that an extern 
function is const but the function wasn't declared const. Second, this 
particular function is const on some platforms and not on others, so we 
can't simply declare it to be const everywhere. Third, the DEFUN syntax 
doesn't give us a way to declare a function const only on some 
platforms.  Fourth, gcc -Wredundant-decls won't let us supply another 
extern declaration by hand, with the const attribute on platforms that 
need const, because GCC complains if there are duplicate extern declations.

There are several possibilities for how to fix this in a less-ugly way, 
including:

1.  Omit -Wsuggest-attribute=const.

2.  Omit -Wredundant-decls.

3.  Make make-docfile.c even smarter and/or trickier.

The attached patch would do (2), as I expect this is simplest. Would you 
prefer this?

[-- Attachment #2: 0001-Give-up-on-Wredundant-decls.patch --]
[-- Type: text/x-patch, Size: 4732 bytes --]

From ed8c06dcd1442d2543a5272e741e1ec97f9f9090 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 14 Jan 2015 11:42:43 -0800
Subject: [PATCH] Give up on -Wredundant-decls

This removes the need for the ugly make-docfile.c hack involving
Fnext_read_file_uses_dialog_p.
* configure.ac (WARN_CFLAGS): Omit -Wredundant-decls.
* lib-src/make-docfile.c (write_globals): Undo previous change.
* src/fileio.c (READ_FILE_NAME_MAY_USE_DIALOG): New constant.
(Fnext_read_file_uses_dialog_p): Use it instead of an #ifdef.
Use a redundant decl to declare this function to be const, on
platforms where it's const.
---
 ChangeLog              |  5 +++++
 configure.ac           |  1 -
 lib-src/ChangeLog      |  7 +++++++
 lib-src/make-docfile.c | 12 ------------
 src/ChangeLog          |  8 ++++++++
 src/fileio.c           | 14 ++++++++++----
 6 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index cca9100..a28b68e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2015-01-14  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Give up on -Wredundant-decls
+	* configure.ac (WARN_CFLAGS): Omit -Wredundant-decls.
+
 2015-01-11  Paul Eggert  <eggert@cs.ucla.edu>
 
 	Default to 'configure --enable-silent-rules'
diff --git a/configure.ac b/configure.ac
index 4cad214..3600e00 100644
--- a/configure.ac
+++ b/configure.ac
@@ -912,7 +912,6 @@ else
   for w in $ws; do
     gl_WARN_ADD([$w])
   done
-  gl_WARN_ADD([-Wredundant-decls])     # Prefer this, as we don't use Bison.
   gl_WARN_ADD([-Wno-missing-field-initializers]) # We need this one
   gl_WARN_ADD([-Wno-sign-compare])     # Too many warnings for now
   gl_WARN_ADD([-Wno-type-limits])      # Too many warnings for now
diff --git a/lib-src/ChangeLog b/lib-src/ChangeLog
index e9205fd..e4408c0 100644
--- a/lib-src/ChangeLog
+++ b/lib-src/ChangeLog
@@ -1,3 +1,10 @@
+2015-01-14  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Give up on -Wredundant-decls
+	This removes the need for the ugly make-docfile.c hack involving
+	Fnext_read_file_uses_dialog_p.
+	* make-docfile.c (write_globals): Undo previous change.
+
 2015-01-13  Paul Eggert  <eggert@cs.ucla.edu>
 
 	Don't say Fnext_read_file_uses_dialog_p is const
diff --git a/lib-src/make-docfile.c b/lib-src/make-docfile.c
index 741fa4b..79d421a 100644
--- a/lib-src/make-docfile.c
+++ b/lib-src/make-docfile.c
@@ -729,18 +729,6 @@ write_globals (void)
 
 	  if (globals[i].flags & DEFUN_const)
 	    fputs (" ATTRIBUTE_CONST", stdout);
-	  else if (strcmp (globals[i].name, "Fnext_read_file_uses_dialog_p")
-		   == 0)
-	    {
-	      /* It would be nice to have a cleaner way to deal with this
-		 special hack.  */
-	      fputs (("\n"
-		      "#if ! (defined USE_GTK || defined USE_MOTIF \\\n"
-		      "       || defined HAVE_NS || defined HAVE_NTGUI)\n"
-		      "\tATTRIBUTE_CONST\n"
-		      "#endif\n"),
-		     stdout);
-	    }
 
 	  puts (";");
 	}
diff --git a/src/ChangeLog b/src/ChangeLog
index b2588f1..b4929d0 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,11 @@
+2015-01-14  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Give up on -Wredundant-decls
+	* fileio.c (READ_FILE_NAME_MAY_USE_DIALOG): New constant.
+	(Fnext_read_file_uses_dialog_p): Use it instead of an #ifdef.
+	Use a redundant decl to declare this function to be const, on
+	platforms where it's const.
+
 2015-01-14  Eli Zaretskii  <eliz@gnu.org>
 
 	* w32fns.c (w32_wnd_proc): Ignore MENUITEMINFO's dwItemData data
diff --git a/src/fileio.c b/src/fileio.c
index 6c443c9..3237d74 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -5736,6 +5736,14 @@ then any auto-save counts as "recent".  */)
 
 /* Reading and completing file names.  */
 
+#if (defined USE_GTK || defined USE_MOTIF \
+     || defined HAVE_NS || defined HAVE_NTGUI)
+enum { READ_FILE_NAME_MAY_USE_DIALOG = true };
+#else
+enum { READ_FILE_NAME_MAY_USE_DIALOG = false };
+EXFUN (Fnext_read_file_uses_dialog_p, 0) ATTRIBUTE_CONST;
+#endif
+
 DEFUN ("next-read-file-uses-dialog-p", Fnext_read_file_uses_dialog_p,
        Snext_read_file_uses_dialog_p, 0, 0, 0,
        doc: /* Return t if a call to `read-file-name' will use a dialog.
@@ -5743,14 +5751,12 @@ The return value is only relevant for a call to `read-file-name' that happens
 before any other event (mouse or keypress) is handled.  */)
   (void)
 {
-#if (defined USE_GTK || defined USE_MOTIF \
-     || defined HAVE_NS || defined HAVE_NTGUI)
-  if ((NILP (last_nonmenu_event) || CONSP (last_nonmenu_event))
+  if (READ_FILE_NAME_MAY_USE_DIALOG
+      && (NILP (last_nonmenu_event) || CONSP (last_nonmenu_event))
       && use_dialog_box
       && use_file_dialog
       && window_system_available (SELECTED_FRAME ()))
     return Qt;
-#endif
   return Qnil;
 }
 
-- 
2.1.0