* 27bb4de72b * Port cleanup attribute to Oracle Studio 12.5 @ 2017-06-14 10:29 Philipp Stephani 2017-06-14 18:25 ` Paul Eggert 0 siblings, 1 reply; 9+ messages in thread From: Philipp Stephani @ 2017-06-14 10:29 UTC (permalink / raw) To: Paul Eggert, Emacs developers [-- Attachment #1: Type: text/plain, Size: 298 bytes --] Hi, the commit message says "The C compiler should check the cleanup attribute in the next line anyway." But that's not the case: unknown attributes are silently ignored, at least in Clang. The verify(__has_attribute(cleanup)) or equivalent is absolutely required here. Can we revert that commit? [-- Attachment #2: Type: text/html, Size: 358 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 27bb4de72b * Port cleanup attribute to Oracle Studio 12.5 2017-06-14 10:29 27bb4de72b * Port cleanup attribute to Oracle Studio 12.5 Philipp Stephani @ 2017-06-14 18:25 ` Paul Eggert 2017-06-15 12:54 ` Philipp Stephani 0 siblings, 1 reply; 9+ messages in thread From: Paul Eggert @ 2017-06-14 18:25 UTC (permalink / raw) To: Philipp Stephani, Emacs developers [-- Attachment #1: Type: text/plain, Size: 1527 bytes --] On 06/14/2017 03:29 AM, Philipp Stephani wrote: > > the commit message says "The C compiler should check the cleanup > attribute in the next line anyway." But that's not the case: unknown > attributes are silently ignored, at least in Clang. The > verify(__has_attribute(cleanup)) or equivalent is absolutely required > here. Can we revert that commit? No, because the 'verify' breaks the build with Oracle Studio 12.5, where __has_attribute works only inside preprocessor conditionals. I installed the attached patch, which checks for __attribute__ (cleanup) in a different way. But while we're on the subject, wouldn't it be better if emacs-module.c were made to work (albeit perhaps less efficiently) even on compilers that do not support this nonstandard C extension? Another thing. Have you had a chance to think about related questions I asked about recently installed portability changes? Here are the URLs: http://lists.gnu.org/archive/html/emacs-devel/2017-06/msg00225.html http://lists.gnu.org/archive/html/emacs-devel/2017-06/msg00226.html Since then I see that you installed another patch (commit 32d8dba625fc50ccbe28e35afcf1f0529d611e00) to pacify Clang on macOS; this patch unfortunately could cause trouble on non-POSIX platforms where rlim_t is signed. Pacifying Clang shouldn't be at the cost of portability or unnecessary complexity. And I'm still puzzled as to why you're getting the Clang warnings but I am not. Are you using an older Clang? Are you passing it extra warning options? [-- Attachment #2: 0001-Port-cleanup-check-to-Oracle-Studio-12.5.patch --] [-- Type: text/x-patch, Size: 1982 bytes --] From 80f8f15b5e9d6776948adedc8b1ab25c2a41df6e Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Wed, 14 Jun 2017 11:01:02 -0700 Subject: [PATCH] Port cleanup check to Oracle Studio 12.5 * src/conf_post.h (__has_attribute_cleanup): Resurrect. * src/emacs-module.c: Verify __has_attribute (cleanup), but in an #if this time. --- src/conf_post.h | 5 ++++- src/emacs-module.c | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/conf_post.h b/src/conf_post.h index 18b096e..e1d6a93 100644 --- a/src/conf_post.h +++ b/src/conf_post.h @@ -57,10 +57,13 @@ typedef bool bool_bf; #endif /* Simulate __has_attribute on compilers that lack it. It is used only - on arguments like alloc_size that are handled in this simulation. */ + on arguments like alloc_size that are handled in this simulation. + __has_attribute should be used only in #if expressions, as Oracle + Studio 12.5's __has_attribute does not work in plain code. */ #ifndef __has_attribute # define __has_attribute(a) __has_attribute_##a # define __has_attribute_alloc_size GNUC_PREREQ (4, 3, 0) +# define __has_attribute_cleanup GNUC_PREREQ (3, 4, 0) # define __has_attribute_externally_visible GNUC_PREREQ (4, 1, 0) # define __has_attribute_no_address_safety_analysis false # define __has_attribute_no_sanitize_address GNUC_PREREQ (4, 8, 0) diff --git a/src/emacs-module.c b/src/emacs-module.c index de62329..5c413ee 100644 --- a/src/emacs-module.c +++ b/src/emacs-module.c @@ -170,6 +170,10 @@ static struct emacs_env_private global_env_private; internal_handler_##handlertype, \ internal_cleanup_##handlertype) +#if !__has_attribute (cleanup) + #error "__attribute__ ((cleanup)) not supported by this compiler; try GCC" +#endif + /* It is very important that pushing the handler doesn't itself raise a signal. Install the cleanup only after the handler has been pushed. Use __attribute__ ((cleanup)) to avoid -- 2.9.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: 27bb4de72b * Port cleanup attribute to Oracle Studio 12.5 2017-06-14 18:25 ` Paul Eggert @ 2017-06-15 12:54 ` Philipp Stephani 2017-06-15 18:46 ` Paul Eggert 0 siblings, 1 reply; 9+ messages in thread From: Philipp Stephani @ 2017-06-15 12:54 UTC (permalink / raw) To: Paul Eggert, Emacs developers [-- Attachment #1: Type: text/plain, Size: 2838 bytes --] Paul Eggert <eggert@cs.ucla.edu> schrieb am Mi., 14. Juni 2017 um 20:26 Uhr: > On 06/14/2017 03:29 AM, Philipp Stephani wrote: > > > > the commit message says "The C compiler should check the cleanup > > attribute in the next line anyway." But that's not the case: unknown > > attributes are silently ignored, at least in Clang. The > > verify(__has_attribute(cleanup)) or equivalent is absolutely required > > here. Can we revert that commit? > > No, because the 'verify' breaks the build with Oracle Studio 12.5, where > __has_attribute works only inside preprocessor conditionals. I installed > the attached patch, which checks for __attribute__ (cleanup) in a > different way. Thanks. > But while we're on the subject, wouldn't it be better if > emacs-module.c were made to work (albeit perhaps less efficiently) even > on compilers that do not support this nonstandard C extension? > I've considered a couple of options. I think the simplest and most portable one would be to compile as C++, which has destructors built into the language. If that's not possible for whatever reason, we could introduce a macro that would create an entire wrapper function, which could implement the cleanup as normal C function. That would require some preprocessor metaprogramming, though, or more boilerplate. Alternatively we could inline most of the macros and require discipline from the developers editing that code. > > Another thing. Have you had a chance to think about related questions I > asked about recently installed portability changes? Here are the URLs: > Yes, sorry for the slow response, I'm working through these now. > > http://lists.gnu.org/archive/html/emacs-devel/2017-06/msg00225.html > > http://lists.gnu.org/archive/html/emacs-devel/2017-06/msg00226.html > > Since then I see that you installed another patch (commit > 32d8dba625fc50ccbe28e35afcf1f0529d611e00) to pacify Clang on macOS; this > patch unfortunately could cause trouble on non-POSIX platforms where > rlim_t is signed. Pacifying Clang shouldn't be at the cost of > portability or unnecessary complexity. > Agreed. I wasn't aware of such platforms. How about using #pragma clang diagnostic push/pop/ignore to ignore the warnings in the specific statements where they arise and we know that they are false positives? I'd much prefer that over disabling them globally in configure, because most of the time the warnings are useful. > > And I'm still puzzled as to why you're getting the Clang warnings but I > am not. Are you using an older Clang? Are you passing it extra warning > options? > > I'm using the Apple fork on macOS. It's mostly identical to upstream Clang and compiles Emacs just fine, but it is a fork and not 100% identical. I also get some of the warnings only when building with -O3 (haven't checked other optimization levels). [-- Attachment #2: Type: text/html, Size: 4154 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 27bb4de72b * Port cleanup attribute to Oracle Studio 12.5 2017-06-15 12:54 ` Philipp Stephani @ 2017-06-15 18:46 ` Paul Eggert 2017-06-15 19:20 ` Philipp Stephani 0 siblings, 1 reply; 9+ messages in thread From: Paul Eggert @ 2017-06-15 18:46 UTC (permalink / raw) To: Philipp Stephani, Emacs developers [-- Attachment #1: Type: text/plain, Size: 9490 bytes --] On 06/15/2017 05:54 AM, Philipp Stephani wrote: > I've considered a couple of options. I think the simplest and most > portable one would be to compile as C++, which has destructors built > into the language. That would require some autoconf and makefile hacking, but it would work. That is, if modules are enabled, and __attribute__ ((cleanup)) is not available but a C++ compiler is available, then the build process could use the C++ compiler. Admittedly this is low priority. > How about using #pragma clang diagnostic push/pop/ignore to ignore the > warnings in the specific statements where they arise and we know that > they are false positives? I'd much prefer that over disabling them > globally in configure, because most of the time the warnings are useful. Although many warnings are useful, in my experience these particular warnings are not useful for Emacs. That is, the hassle they cause by false alarms costs more than the benefits of actual bugs that they fix. It's OK to disable such warnings globally. > > And I'm still puzzled as to why you're getting the Clang warnings > but I > am not. Are you using an older Clang? Are you passing it extra warning > options? > > > I'm using the Apple fork on macOS. It's mostly identical to upstream > Clang and compiles Emacs just fine, but it is a fork and not 100% > identical. I also get some of the warnings only when building with -O3 > (haven't checked other optimization levels). I just now applied the attached patch, configured with: ./configure --enable-gcc-warnings CC=clang CFLAGS='-O3' and had no problems on Fedora 25. If you have a problem with this patch, can you please send relevant output from 'clang --version' and 'make V=1'? Here's the output for me: clang version 3.9.1 (tags/RELEASE_391/final) Target: x86_64-unknown-linux-gnu Thread model: posix InstalledDir: /usr/bin make -C lib all make[1]: Entering directory '/home/eggert/src/gnu/emacs/static-checking/lib' clang -c -MMD -MF deps/strftime.d -MP -fno-common -W -Wabi -Waddress -Wall -Wattributes -Wbuiltin-macro-redefined -Wchar-subscripts -Wcomment -Wcomments -Wdangling-else -Wdate-time -Wdeprecated -Wdeprecated-declarations -Wdisabled-optimization -Wdiv-by-zero -Wduplicate-decl-specifier -Wempty-body -Wendif-labels -Wenum-compare -Wexpansion-to-defined -Wextra -Wformat-extra-args -Wformat-security -Wformat-y2k -Wformat-zero-length -Wignored-attributes -Wignored-qualifiers -Wimplicit -Wimplicit-function-declaration -Wimplicit-int -Wincompatible-pointer-types -Winit-self -Wint-conversion -Wint-to-pointer-cast -Winvalid-pch -Wlogical-not-parentheses -Wmain -Wmissing-declarations -Wmissing-include-dirs -Wmissing-prototypes -Wmultichar -Wnarrowing -Wnested-externs -Wnonnull -Wnull-dereference -Wodr -Wold-style-definition -Woverflow -Wpacked -Wparentheses -Wpointer-arith -Wpointer-sign -Wpointer-to-int-cast -Wpragmas -Wreturn-type -Wsequence-point -Wshift-count-negative -Wshift-count-overflow -Wshift-negative-value -Wsizeof-array-argument -Wsizeof-pointer-memaccess -Wstrict-aliasing -Wstrict-prototypes -Wswitch-bool -Wtautological-compare -Wtrigraphs -Wuninitialized -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label -Wunused-local-typedefs -Wunused-result -Wunused-value -Wunused-variable -Wvarargs -Wvariadic-macros -Wvolatile-register-var -Wwrite-strings -Wno-missing-field-initializers -Wredundant-decls -Wno-missing-field-initializers -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-format-nonliteral -Wno-missing-braces -Wno-tautological-compare -Wno-tautological-constant-out-of-range-compare -Werror -O3 -I. -I../src -I. -I./../src strftime.c [no warnings] ... clang -fno-common -W -Wabi -Waddress -Wall -Wattributes -Wbuiltin-macro-redefined -Wchar-subscripts -Wcomment -Wcomments -Wdangling-else -Wdate-time -Wdeprecated -Wdeprecated-declarations -Wdisabled-optimization -Wdiv-by-zero -Wduplicate-decl-specifier -Wempty-body -Wendif-labels -Wenum-compare -Wexpansion-to-defined -Wextra -Wformat-extra-args -Wformat-security -Wformat-y2k -Wformat-zero-length -Wignored-attributes -Wignored-qualifiers -Wimplicit -Wimplicit-function-declaration -Wimplicit-int -Wincompatible-pointer-types -Winit-self -Wint-conversion -Wint-to-pointer-cast -Winvalid-pch -Wlogical-not-parentheses -Wmain -Wmissing-declarations -Wmissing-include-dirs -Wmissing-prototypes -Wmultichar -Wnarrowing -Wnested-externs -Wnonnull -Wnull-dereference -Wodr -Wold-style-definition -Woverflow -Wpacked -Wparentheses -Wpointer-arith -Wpointer-sign -Wpointer-to-int-cast -Wpragmas -Wreturn-type -Wsequence-point -Wshift-count-negative -Wshift-count-overflow -Wshift-negative-value -Wsizeof-array-argument -Wsizeof-pointer-memaccess -Wstrict-aliasing -Wstrict-prototypes -Wswitch-bool -Wtautological-compare -Wtrigraphs -Wuninitialized -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label -Wunused-local-typedefs -Wunused-result -Wunused-value -Wunused-variable -Wvarargs -Wvariadic-macros -Wvolatile-register-var -Wwrite-strings -Wno-missing-field-initializers -Wredundant-decls -Wno-missing-field-initializers -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-format-nonliteral -Wno-missing-braces -Wno-tautological-compare -Wno-tautological-constant-out-of-range-compare -Werror -I. -I../src -I../lib -I. -I./../src -I./../lib -O3 make-docfile.c ../lib/libgnu.a -o make-docfile [no warnings] ... make[1]: Leaving directory '/home/eggert/src/gnu/emacs/static-checking/lib-src' make -C src VCSWITNESS='$(srcdir)/../.git/logs/HEAD' all make[1]: Entering directory '/home/eggert/src/gnu/emacs/static-checking/src' ... clang -c -Demacs -I. -I. -I../lib -I../lib -pthread -isystem /usr/include/gtk-3.0 -isystem /usr/include/at-spi2-atk/2.0 -isystem /usr/include/at-spi-2.0 -isystem /usr/include/dbus-1.0 -isystem /usr/lib64/dbus-1.0/include -isystem /usr/include/gtk-3.0 -isystem /usr/include/gio-unix-2.0/ -isystem /usr/include/cairo -isystem /usr/include/pango-1.0 -isystem /usr/include/harfbuzz -isystem /usr/include/pango-1.0 -isystem /usr/include/atk-1.0 -isystem /usr/include/cairo -isystem /usr/include/pixman-1 -isystem /usr/include/freetype2 -isystem /usr/include/libpng16 -isystem /usr/include/freetype2 -isystem /usr/include/libdrm -isystem /usr/include/libpng16 -isystem /usr/include/gdk-pixbuf-2.0 -isystem /usr/include/libpng16 -isystem /usr/include/glib-2.0 -isystem /usr/lib64/glib-2.0/include -isystem /usr/include/freetype2 -isystem /usr/include/libpng16 -isystem /usr/include/freetype2 -isystem /usr/include/libpng16 -isystem /usr/include/alsa -pthread -isystem /usr/include/librsvg-2.0 -isystem /usr/include/gdk-pixbuf-2.0 -isystem /usr/include/libpng16 -isystem /usr/include/cairo -isystem /usr/include/glib-2.0 -isystem /usr/lib64/glib-2.0/include -isystem /usr/include/pixman-1 -isystem /usr/include/freetype2 -isystem /usr/include/libpng16 -isystem /usr/include/freetype2 -isystem /usr/include/libdrm -isystem /usr/include/libpng16 -isystem /usr/include/libpng16 -isystem /usr/include/libxml2 -isystem /usr/include/dbus-1.0 -isystem /usr/lib64/dbus-1.0/include -pthread -isystem /usr/include/glib-2.0 -isystem /usr/lib64/glib-2.0/include -pthread -isystem /usr/include/gconf/2 -isystem /usr/include/dbus-1.0 -isystem /usr/lib64/dbus-1.0/include -isystem /usr/include/glib-2.0 -isystem /usr/lib64/glib-2.0/include -isystem /usr/include/glib-2.0 -isystem /usr/lib64/glib-2.0/include -isystem /usr/include/freetype2 -isystem /usr/include/libpng16 -isystem /usr/include/freetype2 -isystem /usr/include/libpng16 -isystem /usr/include/freetype2 -isystem /usr/include/libpng16 -isystem /usr/include/freetype2 -isystem /usr/include/libpng16 -MMD -MF deps/emacs.d -MP -isystem /usr/include/p11-kit-1 -Werror -fno-common -W -Wabi -Waddress -Wall -Wattributes -Wbuiltin-macro-redefined -Wchar-subscripts -Wcomment -Wcomments -Wdangling-else -Wdate-time -Wdeprecated -Wdeprecated-declarations -Wdisabled-optimization -Wdiv-by-zero -Wduplicate-decl-specifier -Wempty-body -Wendif-labels -Wenum-compare -Wexpansion-to-defined -Wextra -Wformat-extra-args -Wformat-security -Wformat-y2k -Wformat-zero-length -Wignored-attributes -Wignored-qualifiers -Wimplicit -Wimplicit-function-declaration -Wimplicit-int -Wincompatible-pointer-types -Winit-self -Wint-conversion -Wint-to-pointer-cast -Winvalid-pch -Wlogical-not-parentheses -Wmain -Wmissing-declarations -Wmissing-include-dirs -Wmissing-prototypes -Wmultichar -Wnarrowing -Wnested-externs -Wnonnull -Wnull-dereference -Wodr -Wold-style-definition -Woverflow -Wpacked -Wparentheses -Wpointer-arith -Wpointer-sign -Wpointer-to-int-cast -Wpragmas -Wreturn-type -Wsequence-point -Wshift-count-negative -Wshift-count-overflow -Wshift-negative-value -Wsizeof-array-argument -Wsizeof-pointer-memaccess -Wstrict-aliasing -Wstrict-prototypes -Wswitch-bool -Wtautological-compare -Wtrigraphs -Wuninitialized -Wunknown-pragmas -Wunused -Wunused-function -Wunused-label -Wunused-local-typedefs -Wunused-result -Wunused-value -Wunused-variable -Wvarargs -Wvariadic-macros -Wvolatile-register-var -Wwrite-strings -Wno-missing-field-initializers -Wredundant-decls -Wno-missing-field-initializers -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-format-nonliteral -Wno-missing-braces -Wno-tautological-compare -Wno-tautological-constant-out-of-range-compare -O3 emacs.c [no warnings] ... [-- Attachment #2: nowarnings.patch --] [-- Type: text/x-patch, Size: 2493 bytes --] diff --git a/lib-src/make-docfile.c b/lib-src/make-docfile.c index 85bcc8b..9470bd6 100644 --- a/lib-src/make-docfile.c +++ b/lib-src/make-docfile.c @@ -224,11 +224,7 @@ put_filename (char *filename) for (tmp = filename; *tmp; tmp++) { - /* Use separate variable to silence a Clang warning on macOS. - Clang takes offence of the additional set of parantheses - generated by the macro. */ - bool is_sep = IS_DIRECTORY_SEP (*tmp); - if (is_sep) + if (IS_DIRECTORY_SEP (*tmp)) filename = tmp + 1; } diff --git a/lib/strftime.c b/lib/strftime.c index 18c899d..99bee4e 100644 --- a/lib/strftime.c +++ b/lib/strftime.c @@ -1123,23 +1123,18 @@ __strftime_internal (STREAM_OR_CHAR_T *s, STRFTIME_ARG (size_t maxsize) if (modifier == L_('E')) goto bad_format; - { - /* Use a new variable here instead of reusing number_value - because Clang complains about the self-assignment - generated by DO_NUMBER. */ - ptrdiff_t n = ns; - if (width == -1) - width = 9; - else - { - /* Take an explicit width less than 9 as a precision. */ - int j; - for (j = width; j < 9; j++) - n /= 10; - } + number_value = ns; + if (width == -1) + width = 9; + else + { + /* Take an explicit width less than 9 as a precision. */ + int j; + for (j = width; j < 9; j++) + number_value /= 10; + } - DO_NUMBER (width, n); - } + DO_NUMBER (width, number_value); #endif case L_('n'): diff --git a/src/emacs.c b/src/emacs.c index 08430de..da8df1b 100644 --- a/src/emacs.c +++ b/src/emacs.c @@ -832,7 +832,7 @@ main (int argc, char **argv) (https://www.cygwin.com/ml/cygwin/2015-07/msg00096.html). */ struct rlimit rlim; if (getrlimit (RLIMIT_STACK, &rlim) == 0 - && rlim.rlim_cur <= LONG_MAX) + && 0 <= rlim.rlim_cur && rlim.rlim_cur <= LONG_MAX) { rlim_t lim = rlim.rlim_cur; @@ -866,7 +866,7 @@ main (int argc, char **argv) right thing anyway. */ long pagesize = getpagesize (); newlim += pagesize - 1; - if (rlim.rlim_max < newlim) + if (0 <= rlim.rlim_max && rlim.rlim_max < newlim) newlim = rlim.rlim_max; newlim -= newlim % pagesize; ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: 27bb4de72b * Port cleanup attribute to Oracle Studio 12.5 2017-06-15 18:46 ` Paul Eggert @ 2017-06-15 19:20 ` Philipp Stephani 2017-06-15 20:33 ` Paul Eggert 2017-06-16 3:19 ` Richard Stallman 0 siblings, 2 replies; 9+ messages in thread From: Philipp Stephani @ 2017-06-15 19:20 UTC (permalink / raw) To: Paul Eggert, Emacs developers [-- Attachment #1: Type: text/plain, Size: 5514 bytes --] Paul Eggert <eggert@cs.ucla.edu> schrieb am Do., 15. Juni 2017 um 20:46 Uhr: > On 06/15/2017 05:54 AM, Philipp Stephani wrote: > > I've considered a couple of options. I think the simplest and most > > portable one would be to compile as C++, which has destructors built > > into the language. > > That would require some autoconf and makefile hacking, but it would > work. That is, if modules are enabled, and __attribute__ ((cleanup)) is > not available but a C++ compiler is available, then the build process > could use the C++ compiler. Admittedly this is low priority. > Alternatively, we might even compile emacs-modules.c as C++ unconditionally. I guess by now every system that has a C compiler and supports shared libraries also has a C++ compiler. > > > How about using #pragma clang diagnostic push/pop/ignore to ignore the > > warnings in the specific statements where they arise and we know that > > they are false positives? I'd much prefer that over disabling them > > globally in configure, because most of the time the warnings are useful. > > Although many warnings are useful, in my experience these particular > warnings are not useful for Emacs. That is, the hassle they cause by > false alarms costs more than the benefits of actual bugs that they fix. > It's OK to disable such warnings globally. > > > > > And I'm still puzzled as to why you're getting the Clang warnings > > but I > > am not. Are you using an older Clang? Are you passing it extra > warning > > options? > > > > > > I'm using the Apple fork on macOS. It's mostly identical to upstream > > Clang and compiles Emacs just fine, but it is a fork and not 100% > > identical. I also get some of the warnings only when building with -O3 > > (haven't checked other optimization levels). > > I just now applied the attached patch, configured with: > > ./configure --enable-gcc-warnings CC=clang CFLAGS='-O3' > > and had no problems on Fedora 25. If you have a problem with this patch, > can you please send relevant output from 'clang --version' and 'make > V=1'? On macOS 10.12.5, current master, and the rlim_t patch reverted: $ git rev-parse HEAD 1ac8c9bb9b6ba44585ed68b03ebbce659a777041 $ gcc -c -Demacs -I. -I. -I../lib -I../lib -I/usr/X11/include -I/usr/local/Cellar/dbus/1.10.18/include/dbus-1.0 -I/usr/local/Cellar/dbus/1.10.18/lib/dbus-1.0/include -MMD -MF deps/emacs.d -MP -I/usr/local/Cellar/gnutls/3.5.13/include -I/usr/local/Cellar/nettle/3.3/include -I/usr/local/Cellar/libtasn1/4.12/include -I/usr/local/Cellar/p11-kit/0.23.7/include/p11-kit-1 -Wno-switch -Wno-tautological-constant-out-of-range-compare -Wno-pointer-sign -Wno-string-plus-int -Wno-unknown-attributes -ggdb3 -O0 emacs.c *emacs.c:835:12: **warning: **comparison of 0 <= unsigned expression is always true* * [-Wtautological-compare]* && 0 <= rlim.rlim_cur && rlim.rlim_cur <= LONG_MAX) * ~ ^ ~~~~~~~~~~~~~* *emacs.c:869:10: **warning: **comparison of 0 <= unsigned expression is always true* * [-Wtautological-compare]* if (0 <= rlim.rlim_max && rlim.rlim_max < newlim) * ~ ^ ~~~~~~~~~~~~~* 2 warnings generated. $ ./config.status --config '--with-modules' '--without-xml2' '--without-pop' '--with-mailutils' '--enable-checking' '--enable-check-lisp-object-type' 'MAKEINFO=/usr/local/opt/texinfo/bin/makeinfo' 'CFLAGS=-ggdb3 -O0' (i.e. this doesn't even need --enable-gcc-warnings) The other case, same repo version, but the other commit reverted: $ make -C lib-src make-docfile V=1 gcc -Wno-switch -Wno-tautological-constant-out-of-range-compare -Wno-pointer-sign -Wno-string-plus-int -Wno-unknown-attributes -I. -I../src -I../lib -I. -I./../src -I./../lib -O3 -save-temps make-docfile.c ../lib/libgnu.a -o make-docfile *make-docfile.c:227:19: **warning: **equality comparison with extraneous parentheses* * [-Wparentheses-equality]* if (((*tmp) == '/')) * ~~~~~~~^~~~~~* *make-docfile.c:227:19: **note: *remove extraneous parentheses around the comparison to silence this warning if (((*tmp) == '/')) * ~ ^ ~* *make-docfile.c:227:19: **note: *use '=' to turn this equality comparison into an assignment if (((*tmp) == '/')) * ^~* = 1 warning generated. $ ./config.status --config '--with-modules' '--without-xml2' '--without-pop' '--with-mailutils' '--enable-checking' '--enable-check-lisp-object-type' 'MAKEINFO=/usr/local/opt/texinfo/bin/makeinfo' 'CFLAGS=-O3 -save-temps' Note that the -save-temps is crucial here, without it the compiler doesn't look at the preprocessed output. It's weird that -save-temps generates more warnings, but GCC has similar issues (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57201). -save-temps also generates tons of warnings for other files, even without --enable-gcc-warnings. Not sure what to do about the -save-temps problem. Maybe that's just something to document somewhere, because there seems to be little we can do about it. $ gcc --version Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk/usr/include/c++/4.2.1 Apple LLVM version 8.1.0 (clang-802.0.42) Target: x86_64-apple-darwin16.6.0 Thread model: posix InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin [-- Attachment #2: Type: text/html, Size: 12465 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 27bb4de72b * Port cleanup attribute to Oracle Studio 12.5 2017-06-15 19:20 ` Philipp Stephani @ 2017-06-15 20:33 ` Paul Eggert 2017-06-16 16:34 ` Philipp Stephani 2017-06-16 3:19 ` Richard Stallman 1 sibling, 1 reply; 9+ messages in thread From: Paul Eggert @ 2017-06-15 20:33 UTC (permalink / raw) To: Philipp Stephani, Emacs developers [-- Attachment #1: Type: text/plain, Size: 845 bytes --] On 06/15/2017 12:20 PM, Philipp Stephani wrote: > we might even compile emacs-modules.c as C++ unconditionally. I guess > by now every system that has a C compiler and supports shared > libraries also has a C++ compiler. Although C++ compilers may be available, they (or their libraries) are not necessarily installed properly, and this can be a problem at runtime as well as at compile-time. So it may be better to stick with C if it works. > (i.e. this doesn't even need --enable-gcc-warnings) > Ah, that's key. Although 'configure' suppresses that false alarm if --enable-gcc-warnings is specified, it also needs to suppress it when --enable-gcc-warnings is *not* specified. I installed the attached, which I hope fixes the warnings that were annoying you (assuming we are not using -save-temps, which is a reasonable assumption). [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Pacify-clang-without-munging-C-source.patch --] [-- Type: text/x-patch; name="0001-Pacify-clang-without-munging-C-source.patch", Size: 6513 bytes --] From a8ce87fa150b318134aed4a97d1b1a8c2757c805 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Thu, 15 Jun 2017 13:29:04 -0700 Subject: [PATCH] Pacify clang without munging C source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * configure.ac (WARN_CFLAGS): With Clang, use -Wno-tautological-compare regardless of --enable-gcc-warnings. (WERROR_CFLAGS): Simplify assignments, and guarantee it’s always set. * lib/strftime.c: Copy from gnulib, reverting Clang-specific change which I hope is no longer needed. * src/emacs.c (main): Revert rlim_t change, as rlim_t is signed on some older non-POSIX hosts. --- configure.ac | 38 +++++++++++++++++++++----------------- lib/strftime.c | 27 +++++++++++---------------- src/emacs.c | 4 ++-- 3 files changed, 34 insertions(+), 35 deletions(-) diff --git a/configure.ac b/configure.ac index 459e314..9069e5b 100644 --- a/configure.ac +++ b/configure.ac @@ -891,6 +891,7 @@ AC_DEFUN [emacs_cv_clang=yes], [emacs_cv_clang=no])]) +WERROR_CFLAGS= # When compiling with GCC, prefer -isystem to -I when including system # include files, to avoid generating useless diagnostics for the files. AS_IF([test $gl_gcc_warnings = no], @@ -900,7 +901,6 @@ AC_DEFUN [ # Turn off some warnings if supported. gl_WARN_ADD([-Wno-switch]) - gl_WARN_ADD([-Wno-tautological-constant-out-of-range-compare]) gl_WARN_ADD([-Wno-pointer-sign]) gl_WARN_ADD([-Wno-string-plus-int]) gl_WARN_ADD([-Wno-unknown-attributes]) @@ -918,8 +918,7 @@ AC_DEFUN ;; esac AS_IF([test $gl_gcc_warnings = yes], - [gl_WARN_ADD([-Werror], [WERROR_CFLAGS])]) - AC_SUBST([WERROR_CFLAGS]) + [WERROR_CFLAGS=-Werror]) nw="$nw -Wduplicated-branches" # Too many false alarms nw="$nw -Wformat-overflow=2" # False alarms due to GCC bug 80776 @@ -961,7 +960,7 @@ AC_DEFUN nw="$nw -Wtype-limits" nw="$nw -Wunused-parameter" - if test $emacs_cv_clang = yes; then + if test "$emacs_cv_clang" = yes; then nw="$nw -Wcast-align" nw="$nw -Wdouble-promotion" nw="$nw -Wmissing-braces" @@ -984,11 +983,9 @@ AC_DEFUN gl_WARN_ADD([-Wno-unused-parameter]) # Too many warnings for now gl_WARN_ADD([-Wno-format-nonliteral]) - # More things that clang is unduly picky about. - if test $emacs_cv_clang = yes; then + # clang is unduly picky about braces. + if test "$emacs_cv_clang" = yes; then gl_WARN_ADD([-Wno-missing-braces]) - gl_WARN_ADD([-Wno-tautological-compare]) - gl_WARN_ADD([-Wno-tautological-constant-out-of-range-compare]) fi # This causes too much noise in the MinGW build @@ -1006,15 +1003,22 @@ AC_DEFUN # define _FORTIFY_SOURCE 2 #endif ]) + ]) - # We use a slightly smaller set of warning options for lib/. - # Remove the following and save the result in GNULIB_WARN_CFLAGS. - nw= - nw="$nw -Wunused-macros" +# clang is unduly picky about these regardless of whether +# --enable-gcc-warnings is specified. +if test "$emacs_cv_clang" = yes; then + gl_WARN_ADD([-Wno-tautological-compare]) + gl_WARN_ADD([-Wno-tautological-constant-out-of-range-compare]) +fi - gl_MANYWARN_COMPLEMENT([GNULIB_WARN_CFLAGS], [$WARN_CFLAGS], [$nw]) - AC_SUBST([GNULIB_WARN_CFLAGS]) - ]) +# Use a slightly smaller set of warning options for lib/. +nw= +nw="$nw -Wunused-macros" +gl_MANYWARN_COMPLEMENT([GNULIB_WARN_CFLAGS], [$WARN_CFLAGS], [$nw]) + +AC_SUBST([WERROR_CFLAGS]) +AC_SUBST([GNULIB_WARN_CFLAGS]) edit_cflags=" s,///*,/,g @@ -1033,7 +1037,7 @@ AC_DEFUN recommended for typical use.])], if test "${enableval}" != "no"; then ac_lto_supported=no - if test $emacs_cv_clang = yes; then + if test "$emacs_cv_clang" = yes; then AC_MSG_CHECKING([whether link-time optimization is supported by clang]) GOLD_PLUGIN=`$CC -print-file-name=LLVMgold.so 2>/dev/null` if test -x "$GOLD_PLUGIN"; then @@ -1062,7 +1066,7 @@ AC_DEFUN AC_MSG_RESULT([$ac_lto_supported]) if test "$ac_lto_supported" = "yes"; then CFLAGS="$CFLAGS $LTO" - if test x$emacs_cv_clang = xyes; then + if test "$emacs_cv_clang" = yes; then AC_MSG_WARN([Please read INSTALL before using link-time optimization with clang]) # WARNING: 'ar --plugin ...' doesn't work without # command, so plugin name is appended to ARFLAGS. diff --git a/lib/strftime.c b/lib/strftime.c index 18c899d..99bee4e 100644 --- a/lib/strftime.c +++ b/lib/strftime.c @@ -1123,23 +1123,18 @@ __strftime_internal (STREAM_OR_CHAR_T *s, STRFTIME_ARG (size_t maxsize) if (modifier == L_('E')) goto bad_format; - { - /* Use a new variable here instead of reusing number_value - because Clang complains about the self-assignment - generated by DO_NUMBER. */ - ptrdiff_t n = ns; - if (width == -1) - width = 9; - else - { - /* Take an explicit width less than 9 as a precision. */ - int j; - for (j = width; j < 9; j++) - n /= 10; - } + number_value = ns; + if (width == -1) + width = 9; + else + { + /* Take an explicit width less than 9 as a precision. */ + int j; + for (j = width; j < 9; j++) + number_value /= 10; + } - DO_NUMBER (width, n); - } + DO_NUMBER (width, number_value); #endif case L_('n'): diff --git a/src/emacs.c b/src/emacs.c index 08430de..da8df1b 100644 --- a/src/emacs.c +++ b/src/emacs.c @@ -832,7 +832,7 @@ main (int argc, char **argv) (https://www.cygwin.com/ml/cygwin/2015-07/msg00096.html). */ struct rlimit rlim; if (getrlimit (RLIMIT_STACK, &rlim) == 0 - && rlim.rlim_cur <= LONG_MAX) + && 0 <= rlim.rlim_cur && rlim.rlim_cur <= LONG_MAX) { rlim_t lim = rlim.rlim_cur; @@ -866,7 +866,7 @@ main (int argc, char **argv) right thing anyway. */ long pagesize = getpagesize (); newlim += pagesize - 1; - if (rlim.rlim_max < newlim) + if (0 <= rlim.rlim_max && rlim.rlim_max < newlim) newlim = rlim.rlim_max; newlim -= newlim % pagesize; -- 2.9.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: 27bb4de72b * Port cleanup attribute to Oracle Studio 12.5 2017-06-15 20:33 ` Paul Eggert @ 2017-06-16 16:34 ` Philipp Stephani 0 siblings, 0 replies; 9+ messages in thread From: Philipp Stephani @ 2017-06-16 16:34 UTC (permalink / raw) To: Paul Eggert, Emacs developers [-- Attachment #1: Type: text/plain, Size: 1084 bytes --] Paul Eggert <eggert@cs.ucla.edu> schrieb am Do., 15. Juni 2017 um 22:33 Uhr: > On 06/15/2017 12:20 PM, Philipp Stephani wrote: > > we might even compile emacs-modules.c as C++ unconditionally. I guess > > by now every system that has a C compiler and supports shared > > libraries also has a C++ compiler. > > Although C++ compilers may be available, they (or their libraries) are > not necessarily installed properly, and this can be a problem at runtime > as well as at compile-time. So it may be better to stick with C if it > works. > OK, let's keep the code as is for now; once we find a platform that causes problems, we can reevaluate the options. > > > (i.e. this doesn't even need --enable-gcc-warnings) > > > > Ah, that's key. Although 'configure' suppresses that false alarm if > --enable-gcc-warnings is specified, it also needs to suppress it when > --enable-gcc-warnings is *not* specified. I installed the attached, > which I hope fixes the warnings that were annoying you (assuming we are > not using -save-temps, which is a reasonable assumption). > OK, thanks. [-- Attachment #2: Type: text/html, Size: 1588 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 27bb4de72b * Port cleanup attribute to Oracle Studio 12.5 2017-06-15 19:20 ` Philipp Stephani 2017-06-15 20:33 ` Paul Eggert @ 2017-06-16 3:19 ` Richard Stallman 2017-06-20 19:18 ` Paul Eggert 1 sibling, 1 reply; 9+ messages in thread From: Richard Stallman @ 2017-06-16 3:19 UTC (permalink / raw) To: Philipp Stephani; +Cc: eggert, emacs-devel [[[ To any NSA and FBI agents reading my email: please consider ]]] [[[ whether defending the US Constitution against all enemies, ]]] [[[ foreign or domestic, requires you to follow Snowden's example. ]]] I am very strongly against making Emacs require a C++ compiler. -- Dr Richard Stallman President, Free Software Foundation (gnu.org, fsf.org) Internet Hall-of-Famer (internethalloffame.org) Skype: No way! See stallman.org/skype.html. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 27bb4de72b * Port cleanup attribute to Oracle Studio 12.5 2017-06-16 3:19 ` Richard Stallman @ 2017-06-20 19:18 ` Paul Eggert 0 siblings, 0 replies; 9+ messages in thread From: Paul Eggert @ 2017-06-20 19:18 UTC (permalink / raw) To: rms, Philipp Stephani; +Cc: emacs-devel On 06/15/2017 08:19 PM, Richard Stallman wrote: > I am very strongly against making Emacs require a C++ compiler. Yes, we've ruled that out. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-06-20 19:18 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-14 10:29 27bb4de72b * Port cleanup attribute to Oracle Studio 12.5 Philipp Stephani 2017-06-14 18:25 ` Paul Eggert 2017-06-15 12:54 ` Philipp Stephani 2017-06-15 18:46 ` Paul Eggert 2017-06-15 19:20 ` Philipp Stephani 2017-06-15 20:33 ` Paul Eggert 2017-06-16 16:34 ` Philipp Stephani 2017-06-16 3:19 ` Richard Stallman 2017-06-20 19:18 ` Paul Eggert
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).