From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Philipp Stephani Newsgroups: gmane.emacs.devel Subject: Re: 27bb4de72b * Port cleanup attribute to Oracle Studio 12.5 Date: Thu, 15 Jun 2017 19:20:26 +0000 Message-ID: References: NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="001a11c14faa464f5705520490a6" X-Trace: blaine.gmane.org 1497554489 5226 195.159.176.226 (15 Jun 2017 19:21:29 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 15 Jun 2017 19:21:29 +0000 (UTC) To: Paul Eggert , Emacs developers Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Jun 15 21:21:25 2017 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dLaKZ-0000pv-Tf for ged-emacs-devel@m.gmane.org; Thu, 15 Jun 2017 21:21:24 +0200 Original-Received: from localhost ([::1]:55544 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dLaKd-0006dl-5L for ged-emacs-devel@m.gmane.org; Thu, 15 Jun 2017 15:21:27 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:36159) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dLaJs-0006df-ME for emacs-devel@gnu.org; Thu, 15 Jun 2017 15:20:42 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dLaJq-0004RB-It for emacs-devel@gnu.org; Thu, 15 Jun 2017 15:20:40 -0400 Original-Received: from mail-oi0-x232.google.com ([2607:f8b0:4003:c06::232]:36793) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dLaJq-0004Qz-AF for emacs-devel@gnu.org; Thu, 15 Jun 2017 15:20:38 -0400 Original-Received: by mail-oi0-x232.google.com with SMTP id k145so13121735oih.3 for ; Thu, 15 Jun 2017 12:20:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=zMi5G8EKLC7Jm4f/yhdcvkQfdXZoqPMYS5A7srW/jac=; b=JTewiY6forErnG9+CdI/9XgfZ9+Q14AJXHXhp4Ius9tHHkhmSsotLpK0jF/xYKRfGc ob6bU9qrJhidtZ1SPtQIDZZ1HZGOJE9xP8tszXcCcfIJTe4PF6ecM++yoV5wNbQJn5Pe ljh1fbikHNg0vIcp1h4J1rOi8OeP3hevRm+bH3IBiEvL9e/vSSw7AGAkUEHrEkkJXhv3 a3UWXdV+HZ77wzSRn43lj3ArzoStwwAs0oPEQF3b4CUs28nZSfSatUaf2311Z5wkwP10 pO32RvHgUiW55t5Ao9Wxhbfog3KkrZSPLxhsFxJx1/823toBekSaI8MFbZWBxCL0XZ1R efPA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=zMi5G8EKLC7Jm4f/yhdcvkQfdXZoqPMYS5A7srW/jac=; b=lpzQi1narde69Ks0VQ+Ee3buLKE9JdrmX8MvadOiREqt1c5f/fpjWQl9wILVztzwpT 39OJGXi7s+SOGo9kFqVFz0bzirT4uW1lZrgjBw4rhRubGVRD0CjRPIMk4HfZ7mC+ivj2 76mID5QuLc/xDrXPhyYdWdgu2UooPnbNEh6Gjfccu3eS4rwObIDWj6CJNDMHOXxbsu72 GC2h/uEMbIkAjgBTBh+Vz2nN8EDGy+6N3qRI9wxj+aLKiOuUJ8OvmOXHU49bGBOjjlet GQQBZQVEpOeoed/gIj27zIIW6TaC+6gfWCRQRJZ1Q8FarRprZSuewQtdbc8WkU5YLZmS zG5A== X-Gm-Message-State: AKS2vOyzldzB+NO9So4yBpaP+PuNZXfqeO1QubfpqQVHNBMO/KfjAYjC gpeLVjSf9QaU0F0QP5mnyf5qGW1MwA== X-Received: by 10.202.117.8 with SMTP id q8mr3329127oic.191.1497554437423; Thu, 15 Jun 2017 12:20:37 -0700 (PDT) In-Reply-To: X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4003:c06::232 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 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" Xref: news.gmane.org gmane.emacs.devel:215652 Archived-At: --001a11c14faa464f5705520490a6 Content-Type: text/plain; charset="UTF-8" Paul Eggert 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 --001a11c14faa464f5705520490a6 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


Paul E= ggert <eggert@cs.ucla.edu> = schrieb am Do., 15. Juni 2017 um 20:46=C2=A0Uhr:
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 a= nd supports shared libraries also has a C++ compiler.
=C2=A0

> How about using #pragma clang diagnostic push/pop/ignore to ignore the=
> warnings in the specific statements where they arise and we know that<= br> > they are false positives? I'd much prefer that over disabling them=
> globally in configure, because most of the time the warnings are usefu= l.

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.

>
>=C2=A0 =C2=A0 =C2=A0And I'm still puzzled as to why you're gett= ing the Clang warnings
>=C2=A0 =C2=A0 =C2=A0but I
>=C2=A0 =C2=A0 =C2=A0am not. Are you using an older Clang? Are you passi= ng it extra warning
>=C2=A0 =C2=A0 =C2=A0options?
>
>
> I'm using the Apple fork on macOS. It's mostly identical to up= stream
> 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=3Dclang CFLAGS=3D'-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=3D1'?

On macOS 10.12.5, current mast= er, and the rlim_t patch reverted:

$ git rev-parse = HEAD

1ac8c9bb9b6ba44585ed68b03ebbce659a777041

$ gc= c -c=C2=A0 -Demacs= =C2=A0 -I. -I. -I.= ./lib -I../lib =C2=A0 -I/usr/X11/include = =C2=A0 =C2=A0 =C2=A0 -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 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 -= MMD -MF deps/emacs.d -MP= =C2=A0 -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=C2=A0 =C2=A0 -Wno-switch -Wno-tautological-constant-o= ut-of-range-compare -Wno-pointer-sign -Wno-string-plus-int -Wno-unknown-att= ributes -ggdb3 -O0=C2=A0 = emacs.c

emacs.c:835:1= 2: warning: comparison of 0 <=3D unsigned expression is a= lways true

=C2=A0 =C2=A0 =C2=A0 [-Wtauto= logical-compare]

=C2=A0 =C2=A0 =C2=A0 && 0 = <=3D rlim.rlim_cur && rlim.rlim_cur <=3D LONG_MAX)

=C2=A0=C2=A0 =C2=A0 =C2=A0 =C2=A0 ~ ^=C2=A0 ~~~= ~~~~~~~~~~

emacs.c:869:1= 0: warning: comparison of 0 <=3D unsigned expression is a= lways true

=C2=A0 =C2=A0 =C2=A0 [-Wtauto= logical-compare]

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (0 <=3D rlim.rlim_max && rlim.rlim_max < newlim)

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 ~ ^= =C2=A0 ~~~~~~~~~~~~~

2 warnings gener= ated.

=

$= ./config.status --config

'= --with-modules' '--without-xml2' '--without-pop' '-= -with-mailutils' '--enable-checking' '--enable-check-lisp-o= bject-type' 'MAKEINFO=3D/usr/local/opt/texinfo/bin/makeinfo' &#= 39;CFLAGS=3D-ggdb3 -O0'


(i.e. this doesn't even need --enable-gcc-warnings= )


=


<= /span>

The ot= her case, same repo version, but the other commit reverted:


$ make -C lib-src make= -docfile V=3D1

gcc =C2=A0 -Wno-switch -Wno-tautological-constant-out-of-range-compare -Wno-pointer-= sign -Wno-string-plus-int -Wno-unknown-attributes=C2=A0 -I. -I../src -I../lib -I. -I./../src = -I./../lib=C2=A0 =C2=A0 <= /span>-O3 -save-temps make-docfile.c=C2=A0 ../lib/libgnu.a=C2=A0 -o make-docfile

make-docfile.c:227:19: warning: equality comparison with extraneous parentheses

=C2=A0 =C2=A0 =C2=A0 [-Wparen= theses-equality]

=C2=A0 =C2= =A0 =C2=A0 if (((*tmp) =3D=3D '/'))

=C2=A0=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ~~~~= ~~~^~~~~~

make-docfile.c:227:19: note: remove extraneous paren= theses around the comparison

= =C2=A0 =C2=A0 =C2=A0 to silence this warning

=C2=A0 =C2=A0 =C2=A0 if (((*tmp) =3D=3D '/&= #39;))

=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 ~ = =C2=A0 =C2=A0 =C2=A0 ^ =C2=A0 =C2=A0 ~

make-docfile.c:227:19: note: use = '=3D' to turn this equality comparison into an

=C2=A0 =C2=A0 =C2=A0 assignment

=

=C2=A0 =C2=A0 =C2=A0 if (((*tmp) = =3D=3D '/'))

=C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ^~

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =3D

1 war= ning generated.


$ ./config.status --config

<= span class=3D"inbox-inbox-s1">

'= --with-modules' '--without-xml2' '--without-pop' '-= -with-mailutils' '--enable-checking' '--enable-check-lisp-o= bject-type' 'MAKEINFO=3D/usr/local/opt/texinfo/bin/makeinfo' &#= 39;CFLAGS=3D-O3 -save-temps'


Note that the -save-temps is crucial here, withou= t it the compiler doesn't look at the preprocessed output.

It's weird tha= t -save-temps generates more warnings, but GCC has similar issues (https://gcc.gnu.o= rg/bugzilla/show_bug.cgi?id=3D57201). -save-temps also generates tons o= f warnings for other files, even without --enable-gcc-warnings.

<= p class=3D"inbox-inbox-p1">Not sure what to = do about the -save-temps problem. Maybe that's just something to docume= nt somewhere, because there seems to be little we can do about it.


$ gcc --version=

Confi= gured with: --prefix=3D/Applications/Xcode.app/Contents/Developer/usr --wit= h-gxx-include-dir=3D/Applications/Xcode.app/Contents/Developer/Platforms/Ma= cOSX.platform/Developer/SDKs/MacOSX10.12.sdk/usr/include/c++/4.2.1

Apple LLVM ver= sion 8.1.0 (clang-802.0.42)

Target: x86_64-apple-darwin16.6.0

Thread model: posix

Insta= lledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault= .xctoolchain/usr/bin

--001a11c14faa464f5705520490a6--