From: Paul Eggert <eggert@cs.ucla.edu>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: Dmitry Antipov <dmantipov@yandex.ru>,
Emacs development discussions <emacs-devel@gnu.org>
Subject: Re: #2 [Was: Re: Function attributes for make-docfile]
Date: Thu, 15 Jan 2015 20:50:11 -0800 [thread overview]
Message-ID: <54B89883.6030401@cs.ucla.edu> (raw)
In-Reply-To: <jwva91ky5ky.fsf-monnier+emacs@gnu.org>
[-- Attachment #1: Type: text/plain, Size: 1217 bytes --]
Dmitry Antipov wrote:
> Why not use #pragma in the way similar to bytecode.c?
I'd rather not head down that direction, as it is ugly in a different way: seven
lines' worth of preprocessor directives to silence one bogus diagnostic?!
> '--enable-gcc-warnings --with-x-toolkit=no' build should have the
> Fnext_read_file_uses_dialog_p-like issue with Fx_uses_old_gtk_dialog.
> But I don't see it, and don't understand why
I don't know why either. Apparently GCC does not do a perfect job of suggesting
the attribute.
Stefan Monnier wrote:
> Easy, then: either you live with this warning, or you don't
> enable -Wsuggest-attribute=const.
Let's do the latter. A couple of things. First, I checked, and the attribute
doesn't significantly help Emacs's performance, so it appears to be more trouble
than it's worth for us. Second, I audited the code and found five instances
where we blindly followed GCC's advice and added ATTRIBUTE_CONST even though
this was incorrect on some platforms; these mistakes suggest that we need to be
more skeptical of GCC's diagnostics in this area on the rare occasions when we
do use -Wsuggest-attribute=const.
I installed the attached patch to try to fix all this.
[-- Attachment #2: 0001-Give-up-on-Wsuggest-attribute-const.patch --]
[-- Type: text/x-patch, Size: 6761 bytes --]
From 64cdb38ed4483c089fff6f3ef6f1ad01f0a2a8d1 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 15 Jan 2015 20:37:05 -0800
Subject: [PATCH] Give up on -Wsuggest-attribute=const
The attribute doesn't help performance significantly, and the
warning seems to be more trouble than it's worth. See the thread at:
http://lists.gnu.org/archive/html/emacs-devel/2015-01/msg00361.html
* configure.ac (WERROR_CFLAGS): Don't use -Wsuggest-attribute=const.
* lib-src/make-docfile.c (write_globals):
Remove special hack for Fnext_read_file_uses_dialog_p.
* src/decompress.c (Fzlib_available_p):
* src/gnutls.c (Fgnutls_available_p):
* src/gtkutil.h (xg_uses_old_file_dialog):
* src/xdisp.c (Ftool_bar_height):
* src/xmenu.c (popup_activated):
No longer const, since it's not const on at lest some
configurations, and we shouldn't lie to the compiler.
---
ChangeLog | 8 ++++++++
configure.ac | 5 ++++-
lib-src/ChangeLog | 6 ++++++
lib-src/make-docfile.c | 12 ------------
src/ChangeLog | 11 +++++++++++
src/decompress.c | 3 +--
src/gnutls.c | 3 +--
src/gtkutil.h | 2 +-
src/xdisp.c | 3 +--
| 2 +-
10 files changed, 34 insertions(+), 21 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index cca9100..309b04f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2015-01-16 Paul Eggert <eggert@cs.ucla.edu>
+
+ Give up on -Wsuggest-attribute=const
+ The attribute doesn't help performance significantly, and the
+ warning seems to be more trouble than it's worth. See the thread at:
+ http://lists.gnu.org/archive/html/emacs-devel/2015-01/msg00361.html
+ * configure.ac (WERROR_CFLAGS): Don't use -Wsuggest-attribute=const.
+
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..9db4bde 100644
--- a/configure.ac
+++ b/configure.ac
@@ -892,6 +892,10 @@ else
# Emacs's use of alloca inhibits protecting the stack.
nw="$nw -Wstack-protector"
+ # Emacs's use of partly-const functions such as Fgnutls_available_p
+ # make this option problematic.
+ nw="$nw -Wsuggest-attribute=const"
+
# Emacs's use of partly-pure functions such as CHECK_TYPE make this
# option problematic.
nw="$nw -Wsuggest-attribute=pure"
@@ -1974,7 +1978,6 @@ fi
if test "$window_system" = none && test "$gl_gcc_warnings" = yes; then
# Too many warnings for now.
nw=
- nw="$nw -Wsuggest-attribute=const"
nw="$nw -Wsuggest-attribute=noreturn"
gl_MANYWARN_COMPLEMENT([WARN_CFLAGS], [$WARN_CFLAGS], [$nw])
diff --git a/lib-src/ChangeLog b/lib-src/ChangeLog
index e9205fd..7cbf327 100644
--- a/lib-src/ChangeLog
+++ b/lib-src/ChangeLog
@@ -1,3 +1,9 @@
+2015-01-16 Paul Eggert <eggert@cs.ucla.edu>
+
+ Give up on -Wsuggest-attribute=const
+ * make-docfile.c (write_globals):
+ Remove special hack for Fnext_read_file_uses_dialog_p
+
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 40d8b26..ae634f3 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,14 @@
+2015-01-16 Paul Eggert <eggert@cs.ucla.edu>
+
+ Give up on -Wsuggest-attribute=const
+ * decompress.c (Fzlib_available_p):
+ * gnutls.c (Fgnutls_available_p):
+ * gtkutil.h (xg_uses_old_file_dialog):
+ * xdisp.c (Ftool_bar_height):
+ * xmenu.c (popup_activated):
+ No longer const, since it's not const on at lest some
+ configurations, and we shouldn't lie to the compiler.
+
2015-01-15 Eli Zaretskii <eliz@gnu.org>
* fileio.c: Include binary-io.h.
diff --git a/src/decompress.c b/src/decompress.c
index b78dace..b14f0a2 100644
--- a/src/decompress.c
+++ b/src/decompress.c
@@ -88,8 +88,7 @@ unwind_decompress (void *ddata)
}
DEFUN ("zlib-available-p", Fzlib_available_p, Szlib_available_p, 0, 0, 0,
- doc: /* Return t if zlib decompression is available in this instance of Emacs. */
- attributes: const)
+ doc: /* Return t if zlib decompression is available in this instance of Emacs. */)
(void)
{
#ifdef WINDOWSNT
diff --git a/src/gnutls.c b/src/gnutls.c
index 909542f..35f0eb4 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -1619,8 +1619,7 @@ This function may also return `gnutls-e-again', or
#endif /* HAVE_GNUTLS */
DEFUN ("gnutls-available-p", Fgnutls_available_p, Sgnutls_available_p, 0, 0, 0,
- doc: /* Return t if GnuTLS is available in this instance of Emacs. */
- attributes: const)
+ doc: /* Return t if GnuTLS is available in this instance of Emacs. */)
(void)
{
#ifdef HAVE_GNUTLS
diff --git a/src/gtkutil.h b/src/gtkutil.h
index 7d712c9..0ac49ca 100644
--- a/src/gtkutil.h
+++ b/src/gtkutil.h
@@ -78,7 +78,7 @@ typedef struct xg_menu_item_cb_data_
} xg_menu_item_cb_data;
-extern bool xg_uses_old_file_dialog (void) ATTRIBUTE_CONST;
+extern bool xg_uses_old_file_dialog (void);
extern char *xg_get_file_name (struct frame *f,
char *prompt,
diff --git a/src/xdisp.c b/src/xdisp.c
index 041a022..f006f8e 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -12276,8 +12276,7 @@ DEFUN ("tool-bar-height", Ftool_bar_height, Stool_bar_height,
0, 2, 0,
doc: /* Return the number of lines occupied by the tool bar of FRAME.
If FRAME is nil or omitted, use the selected frame. Optional argument
-PIXELWISE non-nil means return the height of the tool bar in pixels. */
- attributes: const)
+PIXELWISE non-nil means return the height of the tool bar in pixels. */)
(Lisp_Object frame, Lisp_Object pixelwise)
{
int height = 0;
--git a/src/xmenu.c b/src/xmenu.c
index 9063a8a..fdf1f6f 100644
--- a/src/xmenu.c
+++ b/src/xmenu.c
@@ -2288,7 +2288,7 @@ x_menu_show (struct frame *f, int x, int y, int menuflags,
/* Detect if a dialog or menu has been posted. MSDOS has its own
implementation on msdos.c. */
-int ATTRIBUTE_CONST
+int
popup_activated (void)
{
return popup_activated_flag;
--
2.1.0
prev parent reply other threads:[~2015-01-16 4:50 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-12 4:09 Function attributes for make-docfile Dmitry Antipov
2015-01-12 5:33 ` Stefan Monnier
2015-01-12 5:49 ` Paul Eggert
2015-01-12 12:28 ` #2 [Was: Re: Function attributes for make-docfile] Dmitry Antipov
2015-01-13 7:29 ` Paul Eggert
2015-01-13 10:21 ` Dmitry Antipov
2015-01-13 17:43 ` Paul Eggert
2015-01-13 19:45 ` Stefan Monnier
2015-01-13 23:26 ` Paul Eggert
2015-01-14 17:49 ` Stefan Monnier
2015-01-14 19:44 ` Paul Eggert
2015-01-15 3:27 ` Dmitry Antipov
2015-01-15 3:54 ` Dmitry Antipov
2015-01-15 5:33 ` Stefan Monnier
2015-01-16 4:50 ` Paul Eggert [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54B89883.6030401@cs.ucla.edu \
--to=eggert@cs.ucla.edu \
--cc=dmantipov@yandex.ru \
--cc=emacs-devel@gnu.org \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.