From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Paul Eggert Newsgroups: gmane.emacs.devel Subject: Re: #2 [Was: Re: Function attributes for make-docfile] Date: Thu, 15 Jan 2015 20:50:11 -0800 Organization: UCLA Computer Science Department Message-ID: <54B89883.6030401@cs.ucla.edu> References: <54B348E8.7080203@yandex.ru> <54B3604E.9020304@cs.ucla.edu> <54B3BDE3.8030602@yandex.ru> <54B4C971.7010209@cs.ucla.edu> <54B5A991.8010509@cs.ucla.edu> <54B6C71A.1040101@cs.ucla.edu> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------050902020602030103060401" X-Trace: ger.gmane.org 1421383839 26486 80.91.229.3 (16 Jan 2015 04:50:39 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 16 Jan 2015 04:50:39 +0000 (UTC) Cc: Dmitry Antipov , Emacs development discussions To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Jan 16 05:50:39 2015 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1YByrm-00012x-54 for ged-emacs-devel@m.gmane.org; Fri, 16 Jan 2015 05:50:38 +0100 Original-Received: from localhost ([::1]:53937 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YByrl-0003h8-4v for ged-emacs-devel@m.gmane.org; Thu, 15 Jan 2015 23:50:37 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:47020) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YByre-0003fK-Rf for emacs-devel@gnu.org; Thu, 15 Jan 2015 23:50:32 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YByrZ-0002Kr-T6 for emacs-devel@gnu.org; Thu, 15 Jan 2015 23:50:30 -0500 Original-Received: from smtp.cs.ucla.edu ([131.179.128.62]:54574) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YByrZ-0002KR-HX for emacs-devel@gnu.org; Thu, 15 Jan 2015 23:50:25 -0500 Original-Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 92529A60011; Thu, 15 Jan 2015 20:50:22 -0800 (PST) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Original-Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id cFCyDEw3gCnC; Thu, 15 Jan 2015 20:50:19 -0800 (PST) Original-Received: from [192.168.1.9] (pool-173-55-11-52.lsanca.fios.verizon.net [173.55.11.52]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 49C91A60006; Thu, 15 Jan 2015 20:50:19 -0800 (PST) User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 In-Reply-To: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 131.179.128.62 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:181320 Archived-At: This is a multi-part message in MIME format. --------------050902020602030103060401 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit 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. --------------050902020602030103060401 Content-Type: text/x-patch; name="0001-Give-up-on-Wsuggest-attribute-const.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-Give-up-on-Wsuggest-attribute-const.patch" >From 64cdb38ed4483c089fff6f3ef6f1ad01f0a2a8d1 Mon Sep 17 00:00:00 2001 From: Paul Eggert 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 +-- src/xmenu.c | 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 + + 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 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 + + 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 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 + + 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 * 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; diff --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 --------------050902020602030103060401--